Le jeu. 20 mars 2025 à 03:18, CK Hu (胡俊光) [off-list ref] a écrit :

On Wed, 2025-01-22 at 14:59 +0100, Julien Stephan wrote:

quoted

External email : Please do not click links or open attachments until you have verified the sender or the content.


From: Phi-bang Nguyen <redacted>

This driver provides a path to bypass the SoC ISP so that image data
coming from the SENINF can go directly into memory without any image
processing. This allows the use of an external ISP.

Signed-off-by: Phi-bang Nguyen <redacted>
Signed-off-by: Florian Sylvestre <redacted>
[Paul Elder fix irq locking]
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Co-developed-by: Julien Stephan <jstephan@baylibre.com>
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---

[snip]

quoted

+
+void mtk_camsv_setup(struct mtk_cam_dev *cam_dev, unsigned int count,
+                    u32 w, u32 h, u32 bpl, u32 mbus_fmt)
+{
+       const struct mtk_cam_conf *conf = cam_dev->conf;
+       const struct mtk_cam_format_info *fmtinfo;
+       struct mtk_cam_dev_buffer *buf;
+       unsigned long flags;
+       unsigned int i;
+       u32 tmp;
+
+       for (i = 0; i < count; i++)
+               mtk_camsv_fbc_inc(cam_dev);

I think you should call mtk_camsv_fbc_inc() only after mtk_camsv_update_buffers_add().

quoted

+
+       fmtinfo = mtk_cam_format_info_by_code(mbus_fmt);
+
+       mtk_camsv_tg_write(cam_dev, CAMSV_TG_SEN_MODE, conf->tg_sen_mode);
+
+       mtk_camsv_tg_write(cam_dev, CAMSV_TG_SEN_GRAB_PXL,
+                            (w * fmtinfo->w_factor) << 16U);
+
+       mtk_camsv_tg_write(cam_dev, CAMSV_TG_SEN_GRAB_LIN, h << 16U);
+
+       /* YUV_U2S_DIS: disable YUV sensor unsigned to signed */
+       mtk_camsv_tg_write(cam_dev, CAMSV_TG_PATH_CFG, 0x1000U);
+
+       /* Reset cam */
+       mtk_camsv_write(cam_dev, CAMSV_SW_CTL, CAMSV_SW_RST);
+       mtk_camsv_write(cam_dev, CAMSV_SW_CTL, 0x0U);
+       mtk_camsv_write(cam_dev, CAMSV_SW_CTL, CAMSV_IMGO_RST_TRIG);
+
+       readl_poll_timeout_atomic(cam_dev->regs + CAMSV_SW_CTL, tmp,
+                                 (tmp == (CAMSV_IMGO_RST_TRIG |
+                                          CAMSV_IMGO_RST_ST)), 10, 200);
+
+       mtk_camsv_write(cam_dev, CAMSV_SW_CTL, 0x0U);
+
+       mtk_camsv_write(cam_dev, CAMSV_INT_EN, INT_ST_MASK_CAMSV);
+
+       mtk_camsv_write(cam_dev, CAMSV_MODULE_EN,
+                         conf->module_en | fmtinfo->module_en_pak);
+       mtk_camsv_write(cam_dev, CAMSV_FMT_SEL, fmtinfo->fmt_sel);
+       mtk_camsv_write(cam_dev, CAMSV_PAK, fmtinfo->pak);
+
+       mtk_camsv_img0_write(cam_dev, CAMSV_IMGO_SV_XSIZE, bpl - 1U);
+       mtk_camsv_img0_write(cam_dev, CAMSV_IMGO_SV_YSIZE, h - 1U);
+
+       mtk_camsv_img0_write(cam_dev, CAMSV_IMGO_SV_STRIDE,
+                              fmtinfo->imgo_stride | bpl);
+
+       mtk_camsv_img0_write(cam_dev, CAMSV_IMGO_SV_CON, conf->imgo_con);
+       mtk_camsv_img0_write(cam_dev, CAMSV_IMGO_SV_CON2, conf->imgo_con2);
+
+       /* Set buf addr */
+       spin_lock_irqsave(&cam_dev->buf_list_lock, flags);
+       buf = list_first_entry_or_null(&cam_dev->buf_list,
+                                      struct mtk_cam_dev_buffer,
+                                      list);
+       if (buf)
+               mtk_camsv_update_buffers_add(cam_dev, buf);
+       spin_unlock_irqrestore(&cam_dev->buf_list_lock, flags);
+
+       /* CMOS_EN first */
+       mtk_camsv_tg_write(cam_dev, CAMSV_TG_SEN_MODE,
+                            mtk_camsv_tg_read(cam_dev, CAMSV_TG_SEN_MODE) |
+                            CAMSV_TG_SEN_MODE_CMOS_EN);
+
+       /* finally, CAMSV_MODULE_EN : IMGO_EN */
+       mtk_camsv_write(cam_dev, CAMSV_MODULE_EN,
+                         mtk_camsv_read(cam_dev, CAMSV_MODULE_EN) |
+                         CAMSV_MODULE_EN_IMGO_EN);
+}
+

[snip]

quoted

+
+static int mtk_camsv_runtime_resume(struct device *dev)
+{
+       struct mtk_cam_dev *cam_dev = dev_get_drvdata(dev);
+       struct vb2_queue *vbq = &cam_dev->vdev.vbq;
+       struct mtk_cam_dev_buffer *buf;
+       unsigned long flags = 0;
+       int ret;
+
+       ret = clk_bulk_prepare_enable(cam_dev->num_clks, cam_dev->clks);
+       if (ret) {
+               dev_err(dev, "failed to enable clock:%d\n", ret);
+               return ret;
+       }
+
+       if (vb2_is_streaming(vbq)) {
+
+               spin_lock_irqsave(&cam_dev->buf_list_lock, flags);
+               buf = list_first_entry_or_null(&cam_dev->buf_list,
+                                              struct mtk_cam_dev_buffer,
+                                              list);
+               if (buf)
+                       mtk_camsv_update_buffers_add(cam_dev, buf);

I don't know, when suspend, the register value would be kept or not.
If register is not kept, I think you should re-program all register in mtk_camsv_setup().
But it seems it's not necessary, so the register is kept and you don't need to set register again.
So this mtk_camsv_update_buffers_add() is redundant.

quoted

+
+               spin_unlock_irqrestore(&cam_dev->buf_list_lock, flags);
+
+               /* Stream on the sub-device */
+               ret = v4l2_subdev_enable_streams(&cam_dev->subdev,
+                                                cam_dev->subdev_pads[MTK_CAM_CIO_PAD_VIDEO].index,
+                                                BIT(0));
+
+               if (ret)
+                       goto fail_no_stream;
+       }
+
+       return 0;
+
+fail_no_stream:
+       mtk_cam_vb2_return_all_buffers(cam_dev, VB2_BUF_STATE_QUEUED);
+       return ret;
+}
+
+static const struct dev_pm_ops mtk_camsv_pm_ops = {
+       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+                               pm_runtime_force_resume)
+       SET_RUNTIME_PM_OPS(mtk_camsv_runtime_suspend,
+                          mtk_camsv_runtime_resume, NULL)
+};
+#endif
+

[snip]

quoted

+/* -----------------------------------------------------------------------------
+ * Format Information
+ */
+
+static const struct mtk_cam_format_info mtk_cam_format_info[] = {
+       {
+               .fourcc = V4L2_PIX_FMT_SBGGR8,
+               .code = MEDIA_BUS_FMT_SBGGR8_1X8,
+               .bpp = 8,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_8
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_8,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGBRG8,
+               .code = MEDIA_BUS_FMT_SGBRG8_1X8,
+               .bpp = 8,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_8
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_8,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGRBG8,
+               .code = MEDIA_BUS_FMT_SGRBG8_1X8,
+               .bpp = 8,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_8
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_8,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SRGGB8,
+               .code = MEDIA_BUS_FMT_SRGGB8_1X8,
+               .bpp = 8,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_8
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_8,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SBGGR10,

In stream_out_fmts[], it does not declare V4L2_PIX_FMT_SBGGR10.
So you may remove it in mtk_cam_format_info[], or add it in stream_out_fmts[].

quoted

+               .code = MEDIA_BUS_FMT_SBGGR10_1X10,
+               .bpp = 10,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_10
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_10,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGBRG10,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SGBRG10_1X10,
+               .bpp = 10,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_10
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_10,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGRBG10,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SGRBG10_1X10,
+               .bpp = 10,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_10
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_10,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SRGGB10,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SRGGB10_1X10,
+               .bpp = 10,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_10
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_10,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SBGGR12,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SBGGR12_1X12,
+               .bpp = 12,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_12
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_12,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGBRG12,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SGBRG12_1X12,
+               .bpp = 12,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_12
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_12,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SGRBG12,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SGRBG12_1X12,
+               .bpp = 12,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_12
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_12,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_SRGGB12,

Ditto.

quoted

+               .code = MEDIA_BUS_FMT_SRGGB12_1X12,
+               .bpp = 12,
+               .w_factor = 1,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_EN,
+               .pak = CAMSV_PAK_PAK_MODE_RAW_12
+                       | CAMSV_PAK_PAK_DBL_MODE_2_PIXELS,
+               .fmt_sel = CAMSV_FMT_SEL_TG1_FMT_RAW_12,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_32,
+       }, {
+               .fourcc = V4L2_PIX_FMT_UYVY,
+               .code = MEDIA_BUS_FMT_UYVY8_1X16,
+               .bpp = 16,
+               .w_factor = 2,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_SEL,
+               .pak = 0, /* ignored */
+               .fmt_sel = CAMSV_FMT_SEL_IMGO_BUS_SIZE_16
+                       | CAMSV_FMT_SEL_TG1_FMT_YUV422,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_16,
+       }, {
+               .fourcc = V4L2_PIX_FMT_VYUY,
+               .code = MEDIA_BUS_FMT_VYUY8_1X16,
+               .bpp = 16,
+               .w_factor = 2,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_SEL,
+               .pak = 0, /* ignored */
+               .fmt_sel = CAMSV_FMT_SEL_IMGO_BUS_SIZE_16
+                       | CAMSV_FMT_SEL_TG1_FMT_YUV422,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_16,
+       }, {
+               .fourcc = V4L2_PIX_FMT_YUYV,
+               .code = MEDIA_BUS_FMT_YUYV8_1X16,
+               .bpp = 16,
+               .w_factor = 2,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_SEL,
+               .pak = 0, /* ignored */
+               .fmt_sel = CAMSV_FMT_SEL_IMGO_BUS_SIZE_16
+                       | CAMSV_FMT_SEL_TG1_FMT_YUV422,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_16,
+       }, {
+               .fourcc = V4L2_PIX_FMT_YVYU,
+               .code = MEDIA_BUS_FMT_YVYU8_1X16,
+               .bpp = 16,
+               .w_factor = 2,
+               .module_en_pak = CAMSV_MODULE_EN_PAK_SEL,
+               .pak = 0, /* ignored */
+               .fmt_sel = CAMSV_FMT_SEL_IMGO_BUS_SIZE_16
+                       | CAMSV_FMT_SEL_TG1_FMT_YUV422,
+               .imgo_stride = CAMSV_IMGO_SV_STRIDE_BUS_SIZE_EN
+                       | CAMSV_IMGO_SV_STRIDE_BUS_SIZE_16,
+       },
+};
+

[snip]

quoted

+
+static void mtk_cam_vb2_buf_queue(struct vb2_buffer *vb)
+{
+       struct mtk_cam_dev *cam = vb2_get_drv_priv(vb->vb2_queue);
+       struct mtk_cam_dev_buffer *buf = to_mtk_cam_dev_buffer(vb);
+       unsigned long flags;
+
+       /* Add the buffer into the tracking list */
+       spin_lock_irqsave(&cam->buf_list_lock, flags);
+       if (vb2_start_streaming_called(vb->vb2_queue) && list_empty(&cam->buf_list))
+               mtk_camsv_update_buffers_add(cam, buf);
+
+       list_add_tail(&buf->list, &cam->buf_list);
+       spin_unlock_irqrestore(&cam->buf_list_lock, flags);
+       if (vb2_start_streaming_called(vb->vb2_queue))
+               mtk_camsv_fbc_inc(cam);

I think you should call mtk_camsv_fbc_inc() just after mtk_camsv_update_buffers_add();

Hi CK,

Is there any particular reason? I moved it at the bottom, to reduce
the spinlock region as you requested in v7.. OR maybe I am missing
something ?

Cheers
Julien

quoted

+}
+

[snip]

quoted

+
+static int mtk_cam_vidioc_try_fmt(struct file *file, void *fh,
+                                 struct v4l2_format *f)
+{
+       struct mtk_cam_video_device *vdev = file_to_mtk_cam_video_device(file);
+       struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
+       const struct mtk_cam_format_info *fmtinfo;
+
+       /* Validate pixelformat */
+       if (!mtk_cam_dev_find_fmt(vdev->desc, pix_mp->pixelformat))
+               pix_mp->pixelformat = vdev->desc->fmts[0];

In [1], it says return EINVAL when driver not support this buffer format.

[1] https://docs.kernel.org/userspace-api/media/v4l/vidioc-g-fmt.html#description

Regards,
CK

quoted

+
+       pix_mp->width = clamp_val(pix_mp->width, IMG_MIN_WIDTH, IMG_MAX_WIDTH);
+       pix_mp->height = clamp_val(pix_mp->height, IMG_MIN_HEIGHT,
+                                  IMG_MAX_HEIGHT);
+
+       pix_mp->num_planes = 1;
+
+       fmtinfo = mtk_cam_format_info_by_fourcc(pix_mp->pixelformat);
+       calc_bpl_size_pix_mp(fmtinfo, pix_mp);
+
+       /* Constant format fields */
+       pix_mp->colorspace = V4L2_COLORSPACE_SRGB;
+       pix_mp->field = V4L2_FIELD_NONE;
+       pix_mp->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+       pix_mp->quantization = V4L2_QUANTIZATION_DEFAULT;
+       pix_mp->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+       return 0;
+}
+

************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!