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

Hi, Julien:

On Thu, 2024-11-21 at 09:53 +0100, Julien Stephan wrote:

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]

+/* -----------------------------------------------------------------------------
+ * VB2 Queue Operations
+ */
+
+static int mtk_cam_vb2_queue_setup(struct vb2_queue *vq,
+                                  unsigned int *num_buffers,
+                                  unsigned int *num_planes,
+                                  unsigned int sizes[],
+                                  struct device *alloc_devs[])
+{
+       struct mtk_cam_video_device *vdev =
+               vb2_queue_to_mtk_cam_video_device(vq);
+       struct mtk_cam_dev *cam = vb2_get_drv_priv(vq);
+       const struct v4l2_pix_format_mplane *fmt = &vdev->format;
+       unsigned int size, default_num_planes, i;
+
+       size = fmt->plane_fmt[0].sizeimage;
+
+       default_num_planes = 1;

Make this a constant.

+
+       if (*num_planes == 0) {
+               *num_planes = default_num_planes;
+               for (i = 0; i < *num_planes; ++i)
+                       sizes[i] = size;

*num_planes = 1, so this for-loop could be replaced by:

size[0] = size;

+       } else if (*num_planes != default_num_planes || sizes[0] < size) {

The term 'default_num_planes' would let's think it could support other number of planes.
But it only support one plane. Maybe use 'supported_num_planes'.

Regards,
CK

+               return -EINVAL;
+       }
+
+       (*cam->hw_functions->mtk_cam_fbc_init)(cam, *num_buffers);
+
+       return 0;
+}
+