| Message ID | 20240709160656.31146-11-quic_depengs@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: qcom: camss: Add sm8550 support | expand |
On 09/07/2024 18:06, Depeng Shao wrote: > Add support for VFE found on SM8550 (Titan 780). This implementation is > based on the titan 480 implementation. It supports the normal and lite > VFE. > > Co-developed-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> > --- ... > + > +static const struct camss_video_ops vfe_video_ops_780 = { > + .queue_buffer = vfe_queue_buffer, > + .flush_buffers = vfe_flush_buffers, > +}; > + > +static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe) > +{ > + vfe->video_ops = vfe_video_ops_780; > +} > + > +const struct vfe_hw_ops vfe_ops_780 = { I guess another warning... > + .global_reset = NULL, > + .hw_version = vfe_hw_version, > + .isr = vfe_isr, > + .pm_domain_off = vfe_pm_domain_off, > + .pm_domain_on = vfe_pm_domain_on, > + .subdev_init = vfe_subdev_init, > + .vfe_disable = vfe_disable, > + .vfe_enable = vfe_enable, > + .vfe_halt = vfe_halt, > + .vfe_wm_stop = vfe_wm_stop, > +}; Best regards, Krzysztof
On 09/07/2024 17:06, Depeng Shao wrote: > Add support for VFE found on SM8550 (Titan 780). This implementation is > based on the titan 480 implementation. It supports the normal and lite > VFE. > > Co-developed-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> > --- > drivers/media/platform/qcom/camss/Makefile | 1 + > .../media/platform/qcom/camss/camss-vfe-780.c | 404 ++++++++++++++++++ > 2 files changed, 405 insertions(+) > create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c > > diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile > index c336e4c1a399..a83b7a8dcef7 100644 > --- a/drivers/media/platform/qcom/camss/Makefile > +++ b/drivers/media/platform/qcom/camss/Makefile > @@ -17,6 +17,7 @@ qcom-camss-objs += \ > camss-vfe-4-8.o \ > camss-vfe-17x.o \ > camss-vfe-480.o \ > + camss-vfe-780.o \ > camss-vfe-gen1.o \ > camss-vfe.o \ > camss-video.o \ > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c > new file mode 100644 > index 000000000000..abef2d5b9c2e > --- /dev/null > +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c > @@ -0,0 +1,404 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * camss-vfe-780.c > + * > + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v780 (SM8550) > + * > + * Copyright (c) 2024 Qualcomm Technologies, Inc. > + */ > + > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > + > +#include "camss.h" > +#include "camss-vfe.h" > + > +#define VFE_HW_VERSION (vfe_is_lite(vfe) ? 0x1000 : 0x0) > + > +#define VFE_IRQ_CMD (vfe_is_lite(vfe) ? 0x1038 : 0x30) > +#define IRQ_CMD_GLOBAL_CLEAR BIT(0) > + > +#define VFE_IRQ_MASK(n) ((vfe_is_lite(vfe) ? 0x1024 : 0x34) + (n) * 4) > +#define IRQ_MASK_0_BUS_TOP_IRQ (vfe_is_lite(vfe) ? BIT(0) | BIT(1) | BIT(2) : \ > + BIT(0) | BIT(4) | BIT(18)) > +#define IRQ_MASK_1_BUS_TOP_IRQ(n) (vfe_is_lite(vfe) ? BIT(2 * n + 2) | BIT(2 * n + 3) : \ > + BIT(2 * n + 8) | BIT(2 * n + 9)) > +#define VFE_IRQ_CLEAR(n) ((vfe_is_lite(vfe) ? 0x102C : 0x3C) + (n) * 4) > +#define VFE_IRQ_STATUS(n) ((vfe_is_lite(vfe) ? 0x101C : 0x44) + (n) * 4) > + > +#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x1200 : 0xC00) > + > +#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08) > +#define WM_CGC_OVERRIDE_ALL (0x7FFFFFF) > + > +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xDC) > + > +#define VFE_BUS_WM_CFG(n) (BUS_REG_BASE + 0x200 + (n) * 0x100) > +#define WM_CFG_EN (0) > +#define WM_VIR_FRM_EN (1) > +#define WM_CFG_MODE (16) > +#define MODE_QCOM_PLAIN (0) > +#define MODE_MIPI_RAW (1) > +#define VFE_BUS_WM_IMAGE_ADDR(n) (BUS_REG_BASE + 0x204 + (n) * 0x100) > +#define VFE_BUS_WM_FRAME_INCR(n) (BUS_REG_BASE + 0x208 + (n) * 0x100) > +#define VFE_BUS_WM_IMAGE_CFG_0(n) (BUS_REG_BASE + 0x20c + (n) * 0x100) > +#define WM_IMAGE_CFG_0_DEFAULT_WIDTH (0xFFFF) > +#define VFE_BUS_WM_IMAGE_CFG_1(n) (BUS_REG_BASE + 0x210 + (n) * 0x100) > +#define VFE_BUS_WM_IMAGE_CFG_2(n) (BUS_REG_BASE + 0x214 + (n) * 0x100) > +#define WM_IMAGE_CFG_2_DEFAULT_STRIDE (0xFFFF) > +#define VFE_BUS_WM_PACKER_CFG(n) (BUS_REG_BASE + 0x218 + (n) * 0x100) > +#define VFE_BUS_WM_HEADER_ADDR(n) (BUS_REG_BASE + 0x220 + (n) * 0x100) > +#define VFE_BUS_WM_HEADER_INCR(n) (BUS_REG_BASE + 0x224 + (n) * 0x100) > +#define VFE_BUS_WM_HEADER_CFG(n) (BUS_REG_BASE + 0x228 + (n) * 0x100) > + > +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n) (BUS_REG_BASE + 0x230 + (n) * 0x100) > +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n) (BUS_REG_BASE + 0x234 + (n) * 0x100) > +#define VFE_BUS_WM_FRAMEDROP_PERIOD(n) (BUS_REG_BASE + 0x238 + (n) * 0x100) > +#define VFE_BUS_WM_FRAMEDROP_PATTERN(n) (BUS_REG_BASE + 0x23c + (n) * 0x100) > + > +#define VFE_BUS_WM_MMU_PREFETCH_CFG(n) (BUS_REG_BASE + 0x260 + (n) * 0x100) > +#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n) (BUS_REG_BASE + 0x264 + (n) * 0x100) > +#define VFE_BUS_WM_SYSTEM_CACHE_CFG(n) (BUS_REG_BASE + 0x268 + (n) * 0x100) > + > +/* for titan 780, each bus client is hardcoded to a specific path */ > +#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0x0 : 0x17) + (n)) > + > +#define MAX_VFE_OUTPUT_LINES 4 > +#define MAX_VFE_ACT_BUF 1 > + > +static u32 vfe_hw_version(struct vfe_device *vfe) > +{ > + u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION); > + > + u32 gen = (hw_version >> 28) & 0xF; > + u32 rev = (hw_version >> 16) & 0xFFF; > + u32 step = hw_version & 0xFFFF; > + > + dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step); > + > + return hw_version; > +} This could be functionally decomposed into vfe_hw_version_v2() or similar and exported by camss-vfe.c > + > +static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line) > +{ > + struct v4l2_pix_format_mplane *pix = > + &line->video_out.active_fmt.fmt.pix_mp; > + > + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ > + > + /* no clock gating at bus input */ > + writel_relaxed(0, vfe->base + VFE_BUS_WM_CGC_OVERRIDE); > + > + writel_relaxed(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL); > + > + writel_relaxed(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8, > + vfe->base + VFE_BUS_WM_FRAME_INCR(wm)); > + writel_relaxed((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF), > + vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm)); > + writel_relaxed(WM_IMAGE_CFG_2_DEFAULT_STRIDE, > + vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm)); > + writel_relaxed(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm)); > + > + /* no dropped frames, one irq per frame */ > + writel_relaxed(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm)); > + writel_relaxed(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm)); > + writel_relaxed(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm)); > + writel_relaxed(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm)); > + > + writel_relaxed(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm)); > + writel_relaxed(0xFFFFFFFF, vfe->base + VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm)); > + > + writel_relaxed(1 << WM_CFG_EN | MODE_MIPI_RAW << WM_CFG_MODE, > + vfe->base + VFE_BUS_WM_CFG(wm)); > +} > + > +static void vfe_wm_stop(struct vfe_device *vfe, u8 wm) > +{ > + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ > + writel_relaxed(0, vfe->base + VFE_BUS_WM_CFG(wm)); > +} > + > +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr, > + struct vfe_line *line) > +{ > + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ > + writel_relaxed((addr >> 8) & 0xFFFFFFFF, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm)); > + > + dev_dbg(vfe->camss->dev, "%s wm:%d, image buf addr:0x%llx\n", > + __func__, wm, addr); > +} > + > +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id) > +{ > + /* TODO: Add register update support */ > +} > + > +static inline void vfe_reg_update_clear(struct vfe_device *vfe, > + enum vfe_line_id line_id) > +{ > + /* TODO: Add register update clear support */ > +} > + > +/* > + * vfe_isr - VFE module interrupt handler > + * @irq: Interrupt line > + * @dev: VFE device > + * > + * Return IRQ_HANDLED on success > + */ > +static irqreturn_t vfe_isr(int irq, void *dev) > +{ > + /* Buf Done has beem moved to CSID in Titan 780. > + * Disable VFE related IRQ. > + * Clear the contents of this function. > + * Return IRQ_HANDLED. > + */ > + return IRQ_HANDLED; > +} What's the point of this ISR at all if it never fires and just returns done ? Since it takes no action - it can't do anything useful and therefore if it ever did fire, would fire ad infinitum. Please drop. > + > +/* > + * vfe_halt - Trigger halt on VFE module and wait to complete > + * @vfe: VFE device > + * > + * Return 0 on success or a negative error code otherwise > + */ > +static int vfe_halt(struct vfe_device *vfe) > +{ > + /* rely on vfe_disable_output() to stop the VFE */ > + return 0; > +} > + > +static int vfe_get_output(struct vfe_line *line) > +{ > + struct vfe_device *vfe = to_vfe(line); > + struct vfe_output *output; > + unsigned long flags; > + > + spin_lock_irqsave(&vfe->output_lock, flags); > + > + output = &line->output; > + if (output->state > VFE_OUTPUT_RESERVED) { > + dev_err(vfe->camss->dev, "Output is running\n"); > + goto error; > + } > + > + output->wm_num = 1; > + > + /* Correspondence between VFE line number and WM number. > + * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3 > + * Note this 1:1 mapping will not work for PIX streams. > + */ > + output->wm_idx[0] = line->id; > + vfe->wm_output_map[line->id] = line->id; > + > + output->drop_update_idx = 0; > + > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + > + return 0; > + > +error: > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + output->state = VFE_OUTPUT_OFF; > + > + return -EINVAL; > +} This is copy/paste from vfe480 and should be functionally decomposed into a common function in camss-vfe.c > + > +static int vfe_enable_output(struct vfe_line *line) > +{ > + struct vfe_device *vfe = to_vfe(line); > + struct vfe_output *output = &line->output; > + unsigned long flags; > + unsigned int i; > + > + spin_lock_irqsave(&vfe->output_lock, flags); > + > + vfe_reg_update_clear(vfe, line->id); > + > + if (output->state > VFE_OUTPUT_RESERVED) { > + dev_err(vfe->camss->dev, "Output is not in reserved state %d\n", > + output->state); > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + return -EINVAL; > + } > + > + WARN_ON(output->gen2.active_num); > + > + output->state = VFE_OUTPUT_ON; > + > + output->sequence = 0; > + > + vfe_wm_start(vfe, output->wm_idx[0], line); > + > + for (i = 0; i < MAX_VFE_ACT_BUF; i++) { > + output->buf[i] = vfe_buf_get_pending(output); > + if (!output->buf[i]) > + break; > + output->gen2.active_num++; > + vfe_wm_update(vfe, output->wm_idx[0], output->buf[i]->addr[0], line); > + > + vfe_reg_update(vfe, line->id); I see this differs from vfe480 in that vfe_reg_update(vfe, line-id); is done on each iteration of this loop whereas in 480 it is done directly after the loop, seems to me this would be a valid fix for 480 too leading to my next comment > + } > + > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + > + return 0; > +} This function is so similar across different SoCs with very minor differences that instead of copy/pasting and very slightly tweaking, we should be functionally decomposing and using a flag of some kind to differentaite between wait_reg_update logic in 480 and not in 780. Again I think we should functionally decompose into camss-vfe.c and use a flag to branch the logic for the very slight logical difference between the two vfe-480.c output->sequence = 0; output->wait_reg_update = 0; reinit_completion(&output->reg_update); As a result your fix for line->id would be useful across SoCs instead of isolated to vfe 780. > + > +/* > + * vfe_enable - Enable streaming on VFE line > + * @line: VFE line > + * > + * Return 0 on success or a negative error code otherwise > + */ > +static int vfe_enable(struct vfe_line *line) > +{ > + struct vfe_device *vfe = to_vfe(line); > + int ret; > + > + mutex_lock(&vfe->stream_lock); > + > + vfe->stream_count++; > + > + mutex_unlock(&vfe->stream_lock); > + > + ret = vfe_get_output(line); > + if (ret < 0) > + goto error_get_output; > + > + ret = vfe_enable_output(line); > + if (ret < 0) > + goto error_enable_output; > + > + vfe->was_streaming = 1; > + > + return 0; > + > +error_enable_output: > + vfe_put_output(line); > + > +error_get_output: > + mutex_lock(&vfe->stream_lock); > + > + vfe->stream_count--; > + > + mutex_unlock(&vfe->stream_lock); > + > + return ret; > +} Same thesis on functional decomposition - this should be moved to camss-vfe.c and made common - its only a minor difference betwen the required logic on different SoCs so there's not a compelling reason to have largely identical functions living in difference .c files in the same driver. > + > +/* > + * vfe_buf_done - Process write master done interrupt > + * @vfe: VFE Device > + * @wm: Write master id > + */ > +static void vfe_buf_done(struct vfe_device *vfe, int wm) > +{ > + struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]]; > + struct camss_buffer *ready_buf; > + struct vfe_output *output; > + unsigned long flags; > + u32 index; > + u64 ts = ktime_get_ns(); > + > + spin_lock_irqsave(&vfe->output_lock, flags); > + > + if (vfe->wm_output_map[wm] == VFE_LINE_NONE) { > + dev_err_ratelimited(vfe->camss->dev, > + "Received wm done for unmapped index\n"); > + goto out_unlock; > + } > + output = &vfe->line[vfe->wm_output_map[wm]].output; > + > + ready_buf = output->buf[0]; > + if (!ready_buf) { > + dev_err_ratelimited(vfe->camss->dev, > + "Missing ready buf %d!\n", output->state); > + goto out_unlock; > + } > + > + ready_buf->vb.vb2_buf.timestamp = ts; > + ready_buf->vb.sequence = output->sequence++; > + > + index = 0; > + output->buf[0] = output->buf[1]; > + if (output->buf[0]) > + index = 1; > + > + output->buf[index] = vfe_buf_get_pending(output); > + > + if (output->buf[index]) { > + vfe_wm_update(vfe, output->wm_idx[0], output->buf[index]->addr[0], line); > + vfe_reg_update(vfe, line->id); > + } else > + output->gen2.active_num--; > + > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + > + vb2_buffer_done(&ready_buf->vb.vb2_buf, VB2_BUF_STATE_DONE); > + > + return; > + > +out_unlock: > + spin_unlock_irqrestore(&vfe->output_lock, flags); > +} > + > +/* > + * vfe_queue_buffer - Add empty buffer > + * @vid: Video device structure > + * @buf: Buffer to be enqueued > + * > + * Add an empty buffer - depending on the current number of buffers it will be > + * put in pending buffer queue or directly given to the hardware to be filled. > + * > + * Return 0 on success or a negative error code otherwise > + */ > +static int vfe_queue_buffer(struct camss_video *vid, > + struct camss_buffer *buf) > +{ > + struct vfe_line *line = container_of(vid, struct vfe_line, video_out); > + struct vfe_device *vfe = to_vfe(line); > + struct vfe_output *output; > + unsigned long flags; > + > + output = &line->output; > + > + spin_lock_irqsave(&vfe->output_lock, flags); > + > + if (output->state == VFE_OUTPUT_ON && > + output->gen2.active_num < MAX_VFE_ACT_BUF) { > + output->buf[output->gen2.active_num++] = buf; > + vfe_wm_update(vfe, output->wm_idx[0], buf->addr[0], line); > + vfe_reg_update(vfe, line->id); > + } else { > + vfe_buf_add_pending(output, buf); > + } > + > + spin_unlock_irqrestore(&vfe->output_lock, flags); > + > + return 0; > +} > + > +static const struct camss_video_ops vfe_video_ops_780 = { > + .queue_buffer = vfe_queue_buffer, > + .flush_buffers = vfe_flush_buffers, > +}; > + > +static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe) > +{ > + vfe->video_ops = vfe_video_ops_780; > +} > + > +const struct vfe_hw_ops vfe_ops_780 = { > + .global_reset = NULL, > + .hw_version = vfe_hw_version, > + .isr = vfe_isr, > + .pm_domain_off = vfe_pm_domain_off, > + .pm_domain_on = vfe_pm_domain_on, > + .subdev_init = vfe_subdev_init, > + .vfe_disable = vfe_disable, > + .vfe_enable = vfe_enable, > + .vfe_halt = vfe_halt, > + .vfe_wm_stop = vfe_wm_stop, > +};
Hi Bryan, On 7/10/2024 7:47 PM, Bryan O'Donoghue wrote: > On 09/07/2024 17:06, Depeng Shao wrote: >> Add support for VFE found on SM8550 (Titan 780). This implementation is >> based on the titan 480 implementation. It supports the normal and lite >> VFE. >> >> Co-developed-by: Yongsheng Li <quic_yon@quicinc.com> >> Signed-off-by: Yongsheng Li <quic_yon@quicinc.com> >> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> >> --- >> drivers/media/platform/qcom/camss/Makefile | 1 + >> .../media/platform/qcom/camss/camss-vfe-780.c | 404 ++++++++++++++++++ >> 2 files changed, 405 insertions(+) >> create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c >> >> diff --git a/drivers/media/platform/qcom/camss/Makefile >> b/drivers/media/platform/qcom/camss/Makefile >> index c336e4c1a399..a83b7a8dcef7 100644 >> --- a/drivers/media/platform/qcom/camss/Makefile >> +++ b/drivers/media/platform/qcom/camss/Makefile >> @@ -17,6 +17,7 @@ qcom-camss-objs += \ >> camss-vfe-4-8.o \ >> camss-vfe-17x.o \ >> camss-vfe-480.o \ >> + camss-vfe-780.o \ >> camss-vfe-gen1.o \ >> camss-vfe.o \ >> camss-video.o \ >> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c >> b/drivers/media/platform/qcom/camss/camss-vfe-780.c >> new file mode 100644 >> index 000000000000..abef2d5b9c2e >> --- /dev/null >> +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c >> @@ -0,0 +1,404 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * camss-vfe-780.c >> + >> +static u32 vfe_hw_version(struct vfe_device *vfe) >> +{ >> + u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION); >> + >> + u32 gen = (hw_version >> 28) & 0xF; >> + u32 rev = (hw_version >> 16) & 0xFFF; >> + u32 step = hw_version & 0xFFFF; >> + >> + dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen, >> rev, step); >> + >> + return hw_version; >> +} > > This could be functionally decomposed into vfe_hw_version_v2() or > similar and exported by camss-vfe.c > >> + Yes, same with below comments, I will try to figure out which functions can be moved to common files. >> + >> +/* >> + * vfe_isr - VFE module interrupt handler >> + * @irq: Interrupt line >> + * @dev: VFE device >> + * >> + * Return IRQ_HANDLED on success >> + */ >> +static irqreturn_t vfe_isr(int irq, void *dev) >> +{ >> + /* Buf Done has beem moved to CSID in Titan 780. >> + * Disable VFE related IRQ. >> + * Clear the contents of this function. >> + * Return IRQ_HANDLED. >> + */ >> + return IRQ_HANDLED; >> +} > > What's the point of this ISR at all if it never fires and just returns > done ? > > Since it takes no action - it can't do anything useful and therefore if > it ever did fire, would fire ad infinitum. > > Please drop Sure, will drop it. >> + >> +static int vfe_get_output(struct vfe_line *line) >> +{ >> + struct vfe_device *vfe = to_vfe(line); >> + struct vfe_output *output; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&vfe->output_lock, flags); >> + >> + output = &line->output; >> + if (output->state > VFE_OUTPUT_RESERVED) { >> + dev_err(vfe->camss->dev, "Output is running\n"); >> + goto error; >> + } >> + >> + output->wm_num = 1; >> + >> + /* Correspondence between VFE line number and WM number. >> + * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> >> PIX/RDI3 >> + * Note this 1:1 mapping will not work for PIX streams. >> + */ >> + output->wm_idx[0] = line->id; >> + vfe->wm_output_map[line->id] = line->id; >> + >> + output->drop_update_idx = 0; >> + >> + spin_unlock_irqrestore(&vfe->output_lock, flags); >> + >> + return 0; >> + >> +error: >> + spin_unlock_irqrestore(&vfe->output_lock, flags); >> + output->state = VFE_OUTPUT_OFF; >> + >> + return -EINVAL; >> +} > > This is copy/paste from vfe480 and should be functionally decomposed > into a common function in camss-vfe. Sure, the flow of some functions are same with other platform, and don't read/write registers, this can be moved to a common file and reused by all platform. I will think about this. >> + >> +static int vfe_enable_output(struct vfe_line *line) >> +{ >> + struct vfe_device *vfe = to_vfe(line); >> + struct vfe_output *output = &line->output; >> + unsigned long flags; >> + unsigned int i; >> + >> + spin_lock_irqsave(&vfe->output_lock, flags); >> + >> + vfe_reg_update_clear(vfe, line->id); >> + >> + if (output->state > VFE_OUTPUT_RESERVED) { >> + dev_err(vfe->camss->dev, "Output is not in reserved state %d\n", >> + output->state); >> + spin_unlock_irqrestore(&vfe->output_lock, flags); >> + return -EINVAL; >> + } >> + >> + WARN_ON(output->gen2.active_num); >> + >> + output->state = VFE_OUTPUT_ON; >> + >> + output->sequence = 0; >> + >> + vfe_wm_start(vfe, output->wm_idx[0], line); >> + >> + for (i = 0; i < MAX_VFE_ACT_BUF; i++) { >> + output->buf[i] = vfe_buf_get_pending(output); >> + if (!output->buf[i]) >> + break; >> + output->gen2.active_num++; >> + vfe_wm_update(vfe, output->wm_idx[0], >> output->buf[i]->addr[0], line); >> + >> + vfe_reg_update(vfe, line->id); > > I see this differs from vfe480 in that vfe_reg_update(vfe, line-id); is > done on each iteration of this loop whereas in 480 it is done directly > after the loop, seems to me this would be a valid fix for 480 too > leading to my next comment > Yes, vfe-480 also need this. >> + } >> + >> + spin_unlock_irqrestore(&vfe->output_lock, flags); >> + >> + return 0; >> +} > > This function is so similar across different SoCs with very minor > differences that instead of copy/pasting and very slightly tweaking, we > should be functionally decomposing and using a flag of some kind to > differentaite between wait_reg_update logic in 480 and not in 780. > > Again I think we should functionally decompose into camss-vfe.c and use > a flag to branch the logic for the very slight logical difference > between the two > > vfe-480.c > > output->sequence = 0; > output->wait_reg_update = 0; > reinit_completion(&output->reg_update); > > As a result your fix for line->id would be useful across SoCs instead of > isolated to vfe 780. > Yes, some functions are same code flow, and don't read/write register, this can be moved to a common file and reused by all platform. I will think about this. >> + >> +/* >> + * vfe_enable - Enable streaming on VFE line >> + * @line: VFE line >> + * >> + * Return 0 on success or a negative error code otherwise >> + */ >> +static int vfe_enable(struct vfe_line *line) >> +{ >> + struct vfe_device *vfe = to_vfe(line); >> + int ret; >> + >> + mutex_lock(&vfe->stream_lock); >> + >> + vfe->stream_count++; >> + >> + mutex_unlock(&vfe->stream_lock); >> + >> + ret = vfe_get_output(line); >> + if (ret < 0) >> + goto error_get_output; >> + >> + ret = vfe_enable_output(line); >> + if (ret < 0) >> + goto error_enable_output; >> + >> + vfe->was_streaming = 1; >> + >> + return 0; >> + >> +error_enable_output: >> + vfe_put_output(line); >> + >> +error_get_output: >> + mutex_lock(&vfe->stream_lock); >> + >> + vfe->stream_count--; >> + >> + mutex_unlock(&vfe->stream_lock); >> + >> + return ret; >> +} > > Same thesis on functional decomposition - this should be moved to > camss-vfe.c and made common - its only a minor difference betwen the > required logic on different SoCs so there's not a compelling reason to > have largely identical functions living in difference .c files in the > same driver. > Sure, I will check which functions can be moved to camss-vfe.c. Thanks, Depeng
diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/media/platform/qcom/camss/Makefile index c336e4c1a399..a83b7a8dcef7 100644 --- a/drivers/media/platform/qcom/camss/Makefile +++ b/drivers/media/platform/qcom/camss/Makefile @@ -17,6 +17,7 @@ qcom-camss-objs += \ camss-vfe-4-8.o \ camss-vfe-17x.o \ camss-vfe-480.o \ + camss-vfe-780.o \ camss-vfe-gen1.o \ camss-vfe.o \ camss-video.o \ diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c new file mode 100644 index 000000000000..abef2d5b9c2e --- /dev/null +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c @@ -0,0 +1,404 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * camss-vfe-780.c + * + * Qualcomm MSM Camera Subsystem - VFE (Video Front End) Module v780 (SM8550) + * + * Copyright (c) 2024 Qualcomm Technologies, Inc. + */ + +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> + +#include "camss.h" +#include "camss-vfe.h" + +#define VFE_HW_VERSION (vfe_is_lite(vfe) ? 0x1000 : 0x0) + +#define VFE_IRQ_CMD (vfe_is_lite(vfe) ? 0x1038 : 0x30) +#define IRQ_CMD_GLOBAL_CLEAR BIT(0) + +#define VFE_IRQ_MASK(n) ((vfe_is_lite(vfe) ? 0x1024 : 0x34) + (n) * 4) +#define IRQ_MASK_0_BUS_TOP_IRQ (vfe_is_lite(vfe) ? BIT(0) | BIT(1) | BIT(2) : \ + BIT(0) | BIT(4) | BIT(18)) +#define IRQ_MASK_1_BUS_TOP_IRQ(n) (vfe_is_lite(vfe) ? BIT(2 * n + 2) | BIT(2 * n + 3) : \ + BIT(2 * n + 8) | BIT(2 * n + 9)) +#define VFE_IRQ_CLEAR(n) ((vfe_is_lite(vfe) ? 0x102C : 0x3C) + (n) * 4) +#define VFE_IRQ_STATUS(n) ((vfe_is_lite(vfe) ? 0x101C : 0x44) + (n) * 4) + +#define BUS_REG_BASE (vfe_is_lite(vfe) ? 0x1200 : 0xC00) + +#define VFE_BUS_WM_CGC_OVERRIDE (BUS_REG_BASE + 0x08) +#define WM_CGC_OVERRIDE_ALL (0x7FFFFFF) + +#define VFE_BUS_WM_TEST_BUS_CTRL (BUS_REG_BASE + 0xDC) + +#define VFE_BUS_WM_CFG(n) (BUS_REG_BASE + 0x200 + (n) * 0x100) +#define WM_CFG_EN (0) +#define WM_VIR_FRM_EN (1) +#define WM_CFG_MODE (16) +#define MODE_QCOM_PLAIN (0) +#define MODE_MIPI_RAW (1) +#define VFE_BUS_WM_IMAGE_ADDR(n) (BUS_REG_BASE + 0x204 + (n) * 0x100) +#define VFE_BUS_WM_FRAME_INCR(n) (BUS_REG_BASE + 0x208 + (n) * 0x100) +#define VFE_BUS_WM_IMAGE_CFG_0(n) (BUS_REG_BASE + 0x20c + (n) * 0x100) +#define WM_IMAGE_CFG_0_DEFAULT_WIDTH (0xFFFF) +#define VFE_BUS_WM_IMAGE_CFG_1(n) (BUS_REG_BASE + 0x210 + (n) * 0x100) +#define VFE_BUS_WM_IMAGE_CFG_2(n) (BUS_REG_BASE + 0x214 + (n) * 0x100) +#define WM_IMAGE_CFG_2_DEFAULT_STRIDE (0xFFFF) +#define VFE_BUS_WM_PACKER_CFG(n) (BUS_REG_BASE + 0x218 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_ADDR(n) (BUS_REG_BASE + 0x220 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_INCR(n) (BUS_REG_BASE + 0x224 + (n) * 0x100) +#define VFE_BUS_WM_HEADER_CFG(n) (BUS_REG_BASE + 0x228 + (n) * 0x100) + +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(n) (BUS_REG_BASE + 0x230 + (n) * 0x100) +#define VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(n) (BUS_REG_BASE + 0x234 + (n) * 0x100) +#define VFE_BUS_WM_FRAMEDROP_PERIOD(n) (BUS_REG_BASE + 0x238 + (n) * 0x100) +#define VFE_BUS_WM_FRAMEDROP_PATTERN(n) (BUS_REG_BASE + 0x23c + (n) * 0x100) + +#define VFE_BUS_WM_MMU_PREFETCH_CFG(n) (BUS_REG_BASE + 0x260 + (n) * 0x100) +#define VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(n) (BUS_REG_BASE + 0x264 + (n) * 0x100) +#define VFE_BUS_WM_SYSTEM_CACHE_CFG(n) (BUS_REG_BASE + 0x268 + (n) * 0x100) + +/* for titan 780, each bus client is hardcoded to a specific path */ +#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0x0 : 0x17) + (n)) + +#define MAX_VFE_OUTPUT_LINES 4 +#define MAX_VFE_ACT_BUF 1 + +static u32 vfe_hw_version(struct vfe_device *vfe) +{ + u32 hw_version = readl_relaxed(vfe->base + VFE_HW_VERSION); + + u32 gen = (hw_version >> 28) & 0xF; + u32 rev = (hw_version >> 16) & 0xFFF; + u32 step = hw_version & 0xFFFF; + + dev_info(vfe->camss->dev, "VFE HW Version = %u.%u.%u\n", gen, rev, step); + + return hw_version; +} + +static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line) +{ + struct v4l2_pix_format_mplane *pix = + &line->video_out.active_fmt.fmt.pix_mp; + + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ + + /* no clock gating at bus input */ + writel_relaxed(0, vfe->base + VFE_BUS_WM_CGC_OVERRIDE); + + writel_relaxed(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL); + + writel_relaxed(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8, + vfe->base + VFE_BUS_WM_FRAME_INCR(wm)); + writel_relaxed((WM_IMAGE_CFG_0_DEFAULT_WIDTH & 0xFFFF), + vfe->base + VFE_BUS_WM_IMAGE_CFG_0(wm)); + writel_relaxed(WM_IMAGE_CFG_2_DEFAULT_STRIDE, + vfe->base + VFE_BUS_WM_IMAGE_CFG_2(wm)); + writel_relaxed(0, vfe->base + VFE_BUS_WM_PACKER_CFG(wm)); + + /* no dropped frames, one irq per frame */ + writel_relaxed(0, vfe->base + VFE_BUS_WM_FRAMEDROP_PERIOD(wm)); + writel_relaxed(1, vfe->base + VFE_BUS_WM_FRAMEDROP_PATTERN(wm)); + writel_relaxed(0, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PERIOD(wm)); + writel_relaxed(1, vfe->base + VFE_BUS_WM_IRQ_SUBSAMPLE_PATTERN(wm)); + + writel_relaxed(1, vfe->base + VFE_BUS_WM_MMU_PREFETCH_CFG(wm)); + writel_relaxed(0xFFFFFFFF, vfe->base + VFE_BUS_WM_MMU_PREFETCH_MAX_OFFSET(wm)); + + writel_relaxed(1 << WM_CFG_EN | MODE_MIPI_RAW << WM_CFG_MODE, + vfe->base + VFE_BUS_WM_CFG(wm)); +} + +static void vfe_wm_stop(struct vfe_device *vfe, u8 wm) +{ + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ + writel_relaxed(0, vfe->base + VFE_BUS_WM_CFG(wm)); +} + +static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u64 addr, + struct vfe_line *line) +{ + wm = RDI_WM(wm); /* map to actual WM used (from wm=RDI index) */ + writel_relaxed((addr >> 8) & 0xFFFFFFFF, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm)); + + dev_dbg(vfe->camss->dev, "%s wm:%d, image buf addr:0x%llx\n", + __func__, wm, addr); +} + +static void vfe_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id) +{ + /* TODO: Add register update support */ +} + +static inline void vfe_reg_update_clear(struct vfe_device *vfe, + enum vfe_line_id line_id) +{ + /* TODO: Add register update clear support */ +} + +/* + * vfe_isr - VFE module interrupt handler + * @irq: Interrupt line + * @dev: VFE device + * + * Return IRQ_HANDLED on success + */ +static irqreturn_t vfe_isr(int irq, void *dev) +{ + /* Buf Done has beem moved to CSID in Titan 780. + * Disable VFE related IRQ. + * Clear the contents of this function. + * Return IRQ_HANDLED. + */ + return IRQ_HANDLED; +} + +/* + * vfe_halt - Trigger halt on VFE module and wait to complete + * @vfe: VFE device + * + * Return 0 on success or a negative error code otherwise + */ +static int vfe_halt(struct vfe_device *vfe) +{ + /* rely on vfe_disable_output() to stop the VFE */ + return 0; +} + +static int vfe_get_output(struct vfe_line *line) +{ + struct vfe_device *vfe = to_vfe(line); + struct vfe_output *output; + unsigned long flags; + + spin_lock_irqsave(&vfe->output_lock, flags); + + output = &line->output; + if (output->state > VFE_OUTPUT_RESERVED) { + dev_err(vfe->camss->dev, "Output is running\n"); + goto error; + } + + output->wm_num = 1; + + /* Correspondence between VFE line number and WM number. + * line 0 -> RDI 0, line 1 -> RDI1, line 2 -> RDI2, line 3 -> PIX/RDI3 + * Note this 1:1 mapping will not work for PIX streams. + */ + output->wm_idx[0] = line->id; + vfe->wm_output_map[line->id] = line->id; + + output->drop_update_idx = 0; + + spin_unlock_irqrestore(&vfe->output_lock, flags); + + return 0; + +error: + spin_unlock_irqrestore(&vfe->output_lock, flags); + output->state = VFE_OUTPUT_OFF; + + return -EINVAL; +} + +static int vfe_enable_output(struct vfe_line *line) +{ + struct vfe_device *vfe = to_vfe(line); + struct vfe_output *output = &line->output; + unsigned long flags; + unsigned int i; + + spin_lock_irqsave(&vfe->output_lock, flags); + + vfe_reg_update_clear(vfe, line->id); + + if (output->state > VFE_OUTPUT_RESERVED) { + dev_err(vfe->camss->dev, "Output is not in reserved state %d\n", + output->state); + spin_unlock_irqrestore(&vfe->output_lock, flags); + return -EINVAL; + } + + WARN_ON(output->gen2.active_num); + + output->state = VFE_OUTPUT_ON; + + output->sequence = 0; + + vfe_wm_start(vfe, output->wm_idx[0], line); + + for (i = 0; i < MAX_VFE_ACT_BUF; i++) { + output->buf[i] = vfe_buf_get_pending(output); + if (!output->buf[i]) + break; + output->gen2.active_num++; + vfe_wm_update(vfe, output->wm_idx[0], output->buf[i]->addr[0], line); + + vfe_reg_update(vfe, line->id); + } + + spin_unlock_irqrestore(&vfe->output_lock, flags); + + return 0; +} + +/* + * vfe_enable - Enable streaming on VFE line + * @line: VFE line + * + * Return 0 on success or a negative error code otherwise + */ +static int vfe_enable(struct vfe_line *line) +{ + struct vfe_device *vfe = to_vfe(line); + int ret; + + mutex_lock(&vfe->stream_lock); + + vfe->stream_count++; + + mutex_unlock(&vfe->stream_lock); + + ret = vfe_get_output(line); + if (ret < 0) + goto error_get_output; + + ret = vfe_enable_output(line); + if (ret < 0) + goto error_enable_output; + + vfe->was_streaming = 1; + + return 0; + +error_enable_output: + vfe_put_output(line); + +error_get_output: + mutex_lock(&vfe->stream_lock); + + vfe->stream_count--; + + mutex_unlock(&vfe->stream_lock); + + return ret; +} + +/* + * vfe_buf_done - Process write master done interrupt + * @vfe: VFE Device + * @wm: Write master id + */ +static void vfe_buf_done(struct vfe_device *vfe, int wm) +{ + struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]]; + struct camss_buffer *ready_buf; + struct vfe_output *output; + unsigned long flags; + u32 index; + u64 ts = ktime_get_ns(); + + spin_lock_irqsave(&vfe->output_lock, flags); + + if (vfe->wm_output_map[wm] == VFE_LINE_NONE) { + dev_err_ratelimited(vfe->camss->dev, + "Received wm done for unmapped index\n"); + goto out_unlock; + } + output = &vfe->line[vfe->wm_output_map[wm]].output; + + ready_buf = output->buf[0]; + if (!ready_buf) { + dev_err_ratelimited(vfe->camss->dev, + "Missing ready buf %d!\n", output->state); + goto out_unlock; + } + + ready_buf->vb.vb2_buf.timestamp = ts; + ready_buf->vb.sequence = output->sequence++; + + index = 0; + output->buf[0] = output->buf[1]; + if (output->buf[0]) + index = 1; + + output->buf[index] = vfe_buf_get_pending(output); + + if (output->buf[index]) { + vfe_wm_update(vfe, output->wm_idx[0], output->buf[index]->addr[0], line); + vfe_reg_update(vfe, line->id); + } else + output->gen2.active_num--; + + spin_unlock_irqrestore(&vfe->output_lock, flags); + + vb2_buffer_done(&ready_buf->vb.vb2_buf, VB2_BUF_STATE_DONE); + + return; + +out_unlock: + spin_unlock_irqrestore(&vfe->output_lock, flags); +} + +/* + * vfe_queue_buffer - Add empty buffer + * @vid: Video device structure + * @buf: Buffer to be enqueued + * + * Add an empty buffer - depending on the current number of buffers it will be + * put in pending buffer queue or directly given to the hardware to be filled. + * + * Return 0 on success or a negative error code otherwise + */ +static int vfe_queue_buffer(struct camss_video *vid, + struct camss_buffer *buf) +{ + struct vfe_line *line = container_of(vid, struct vfe_line, video_out); + struct vfe_device *vfe = to_vfe(line); + struct vfe_output *output; + unsigned long flags; + + output = &line->output; + + spin_lock_irqsave(&vfe->output_lock, flags); + + if (output->state == VFE_OUTPUT_ON && + output->gen2.active_num < MAX_VFE_ACT_BUF) { + output->buf[output->gen2.active_num++] = buf; + vfe_wm_update(vfe, output->wm_idx[0], buf->addr[0], line); + vfe_reg_update(vfe, line->id); + } else { + vfe_buf_add_pending(output, buf); + } + + spin_unlock_irqrestore(&vfe->output_lock, flags); + + return 0; +} + +static const struct camss_video_ops vfe_video_ops_780 = { + .queue_buffer = vfe_queue_buffer, + .flush_buffers = vfe_flush_buffers, +}; + +static void vfe_subdev_init(struct device *dev, struct vfe_device *vfe) +{ + vfe->video_ops = vfe_video_ops_780; +} + +const struct vfe_hw_ops vfe_ops_780 = { + .global_reset = NULL, + .hw_version = vfe_hw_version, + .isr = vfe_isr, + .pm_domain_off = vfe_pm_domain_off, + .pm_domain_on = vfe_pm_domain_on, + .subdev_init = vfe_subdev_init, + .vfe_disable = vfe_disable, + .vfe_enable = vfe_enable, + .vfe_halt = vfe_halt, + .vfe_wm_stop = vfe_wm_stop, +};