Re: [PATCH v7 4/5] media: platform: mediatek: isp: add mediatek ISP3.0 camsv

Le lun. 25 nov. 2024 à 09:14, CK Hu (胡俊光) [off-list ref] a écrit :

Hi, Julien:

On Thu, 2024-11-21 at 09:53 +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

+static void mtk_camsv30_update_buffers_add(struct mtk_cam_dev *cam_dev,
+                                          struct mtk_cam_dev_buffer *buf)
+{
+       mtk_camsv30_img0_write(cam_dev, CAMSV_IMGO_SV_BASE_ADDR, buf->daddr);
+}
+

[snip]

quoted

+static void mtk_camsv30_fbc_inc(struct mtk_cam_dev *cam_dev)
+{
+       unsigned int fbc_val;
+
+       if (pm_runtime_resume_and_get(cam_dev->dev) < 0) {

I think this pm_runtime_resume_and_get() is not necessary.
mtk_camsv30_fbc_inc() is called only in mtk_cam_vb2_buf_queue().
But when buf_list is empty, mtk_camsv30_update_buffers_add() is called before this function.
But mtk_camsv30_update_buffers_add() does not call pm_runtime_resume_and_get() and it works normally.
So this function is not necessary to call pm_runtime_resume_and_get().

Hi CK,

This one is actually needed because .buf_queue can be called before
.start_streaming in case a user requests to prepare buffers before
streaming.
But you are right, if a user requests to alloc buffer before streaming
mtk_camsv_update_buffers_address will be called without pm. Streaming
still works because in start streaming we call
mtk_camsv_update_buffers_address again.
So maybe I should put the pm stuff in mtk_cam_vb2_buf_queue ?

Cheers
Julien

In other register setting function, please also check this pm function is necessary or not.

Regards,
CK

quoted

+               dev_err(cam_dev->dev, "failed to get pm_runtime\n");
+               return;
+       }
+
+       fbc_val = mtk_camsv30_read(cam_dev, CAMSV_IMGO_FBC);
+       fbc_val |= CAMSV_IMGO_FBC_RCNT_INC;
+       mtk_camsv30_write(cam_dev, CAMSV_IMGO_FBC, fbc_val);
+       fbc_val &= ~CAMSV_IMGO_FBC_RCNT_INC;
+       mtk_camsv30_write(cam_dev, CAMSV_IMGO_FBC, fbc_val);
+
+       pm_runtime_put_autosuspend(cam_dev->dev);
+}
+

quoted

************* 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!