diff mbox series

[v4,2/2] media: sun6i-csi: capture: Implement vidioc_enum_framesizes

Message ID 20260717160547.44460-3-arash.golgol@gmail.com (mailing list archive)
State New
Headers
Series media: sun6i-csi: Convert to active state and improve MC support |

Commit Message

Arash Golgol July 17, 2026, 4:05 p.m. UTC 
Report the stepwise frame size range supported by the CSI capture
hardware for the pixel formats exposed by the driver.

The hardware does not perform scaling and accepts any even width and
height within the reported limits.

Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

sashiko-bot@kernel.org July 17, 2026, 5:17 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `vidioc_enum_framesizes` incorrectly advertises a 2-pixel step size for `V4L2_PIX_FMT_NV12_16L16`, violating the format's strict 16-pixel alignment requirement.
--

--- Patch [2]: [PATCH v4 2/2] media: sun6i-csi: capture: Implement vidioc_enum_framesizes ---
commit c1eb6dcf0f09ace0fcc277bb3e50e12e234dce7b
Author: Arash Golgol <arash.golgol@gmail.com>

media: sun6i-csi: capture: Implement vidioc_enum_framesizes

This commit implements vidioc_enum_framesizes to report the stepwise
frame size range supported by the CSI capture hardware. It sets the
supported dimensions with a uniform step width and height of 2.

> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index d90abba213098..f788b42346739 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
[ ... ]
> @@ -738,6 +738,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
>  	return 0;
>  }
>  
> +static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
> +					    struct v4l2_frmsizeenum *fsize)
> +{
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	/* Only accept format in map table. */
> +	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
> +	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
> +	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
> +	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
> +	fsize->stepwise.step_width = 2;
> +	fsize->stepwise.step_height = 2;

[Severity: High]
Does this hardcoded step size properly handle V4L2_PIX_FMT_NV12_16L16?

The V4L2 specification requires both the line stride and image height for
V4L2_PIX_FMT_NV12_16L16 to be a multiple of 16, as it is a 16x16 tiled
format. By setting step_width and step_height to 2 for all formats, this
advertises that arbitrary even dimensions are valid for the tiled format.

If userspace requests a non-16-aligned dimension, such as 18x18, the driver
may calculate an undersized buffer. Because the hardware operates on
complete 16x16 macroblocks, it will attempt to DMA at least a 32x32 block to
cover the 18x18 image, resulting in an out-of-bounds DMA memory write.

Should the step sizes be dynamically adjusted based on fsize->pixel_format
to enforce 16-pixel block alignment when necessary?

> +
> +	return 0;
> +}
> +
diff mbox series

Patch

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
index d90abba21309..f788b4234673 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -738,6 +738,27 @@  static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
 	return 0;
 }
 
+static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
+					    struct v4l2_frmsizeenum *fsize)
+{
+	if (fsize->index)
+		return -EINVAL;
+
+	/* Only accept format in map table. */
+	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
+	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
+	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
+	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
+	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
+	fsize->stepwise.step_width = 2;
+	fsize->stepwise.step_height = 2;
+
+	return 0;
+}
+
 static int sun6i_csi_capture_g_fmt(struct file *file, void *priv,
 				   struct v4l2_format *format)
 {
@@ -805,6 +826,7 @@  static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
 	.vidioc_querycap		= sun6i_csi_capture_querycap,
 
 	.vidioc_enum_fmt_vid_cap	= sun6i_csi_capture_enum_fmt,
+	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesize,
 	.vidioc_g_fmt_vid_cap		= sun6i_csi_capture_g_fmt,
 	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
 	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,