[PATCH v1 0/3] media: sun6i-csi: Improve capture and bridge

[PATCH v1 0/3] media: sun6i-csi: Improve capture and bridge

[Arash Golgol]

Hi,

This series improves sun6i-csi capture and bridge.

Patch 1 fixes sun6i_csi_capture_format_prepare() to align both width
and height to 16 for NV12_16L16, as its 16x16 tiles require.

Patch 2 adds vidioc_enum_framesizes() to sun6i-csi-capture, using the
same alignment.

Patch 3 removes duplicated mbus codes from sun6i-csi-bridge. It was
posted earlier and carries Paul's Reviewed-by.
Link: https://patchwork.linuxtv.org/project/linux-media/patch/20260527124552.33815-1-arash.golgol@gmail.com/

Tested on LicheePi Zero Dock (Allwinner V3s) with:
  ov5647 -> sun6i-mipi-csi2 -> sun6i-csi-bridge -> sun6i-csi-capture

As ov5647 only outputs Bayer, streaming HM12/NV12 is not possible on
this setup, so I did not test real streaming. I checked that
v4l2-ctl --try-fmt-video, --set-fmt-video, --get-fmt-video and
--list-framesizes report the expected sizes for HM12 (16 aligned) and
NV12 (2 aligned).

Arash Golgol (3):
  media: sun6i-csi: capture: Align size to 16 for NV12_16L16
  media: sun6i-csi: capture: Add enum_framesizes support
  media: sun6i-csi: bridge: Remove duplicate mbus format entries

 .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 12 -----
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 45 ++++++++++++++++---
 2 files changed, 39 insertions(+), 18 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] media: sun6i-csi: capture: Align size to 16 for NV12_16L16

[Arash Golgol]

The NV12_16L16 format is made of 16x16 tiles, so both the width and
the height must be a multiple of 16. The format preparation always
aligned dimensions to 2 pixels, whatever the pixel format, which
allowed sizes covering partial tiles.

Select the alignment from the pixel format, and do so after the
pixel format fallback so that the final format is the one used.

Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

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..f55edfd0b174 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -41,6 +41,12 @@ void sun6i_csi_capture_format(struct sun6i_csi_device *csi_dev,
 		*field = csi_dev->capture.format.fmt.pix.field;
 }
 
+static unsigned int sun6i_csi_capture_format_align(u32 pixelformat)
+{
+	/* Returns log2 of the required alignment. */
+	return pixelformat == V4L2_PIX_FMT_NV12_16L16 ? 4 : 1;
+}
+
 /* Format */
 
 static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = {
@@ -665,17 +671,19 @@ static void sun6i_csi_capture_format_prepare(struct v4l2_format *format)
 {
 	struct v4l2_pix_format *pix_format = &format->fmt.pix;
 	const struct v4l2_format_info *info;
-	unsigned int width, height;
-
-	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
-			      SUN6I_CSI_CAPTURE_WIDTH_MAX, 1,
-			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
-			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, 1, 0);
+	unsigned int width, height, align;
 
 	if (!sun6i_csi_capture_format_find(pix_format->pixelformat))
 		pix_format->pixelformat =
 			sun6i_csi_capture_formats[0].pixelformat;
 
+	align = sun6i_csi_capture_format_align(pix_format->pixelformat);
+
+	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
+			      SUN6I_CSI_CAPTURE_WIDTH_MAX, align,
+			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
+			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, align, 0);
+
 	width = pix_format->width;
 	height = pix_format->height;
 
-- 
2.34.1

[PATCH v1 2/3] media: sun6i-csi: capture: Add enum_framesizes support

[Arash Golgol]

Implement VIDIOC_ENUM_FRAMESIZES as a stepwise range from the minimum
and maximum sizes supported by the hardware. The step follows the
alignment enforced by try_fmt: 16 for NV12_16L16 and 2 otherwise.

Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

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 f55edfd0b174..3adb10b62857 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -746,6 +746,30 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
 	return 0;
 }
 
+static int sun6i_csi_capture_enum_framesizes(struct file *file, void *priv,
+					     struct v4l2_frmsizeenum *fsize)
+{
+	unsigned int step;
+
+	if (fsize->index != 0)
+		return -EINVAL;
+
+	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
+		return -EINVAL;
+
+	step = 1U << sun6i_csi_capture_format_align(fsize->pixel_format);
+
+	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.step_width = step;
+	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
+	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
+	fsize->stepwise.step_height = step;
+
+	return 0;
+}
+
 static int sun6i_csi_capture_g_fmt(struct file *file, void *priv,
 				   struct v4l2_format *format)
 {
@@ -813,6 +837,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_framesizes,
 	.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,
-- 
2.34.1

[PATCH v1 3/3] media: sun6i-csi: bridge: Remove duplicate mbus format entries

[Arash Golgol]

The driver reports MEDIA_BUS_FMT_UYVY8_2X8 and
MEDIA_BUS_FMT_UYVY8_1X16 twice when enumerating supported media bus
codes.

Remove the duplicated entries from sun6i_csi_bridge_formats[].

Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
---
 .../platform/sunxi/sun6i-csi/sun6i_csi_bridge.c      | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
index f9ee94fc1293..0331b55f489a 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
@@ -93,12 +93,6 @@ static const struct sun6i_csi_bridge_format sun6i_csi_bridge_formats[] = {
 		.input_yuv_seq		= SUN6I_CSI_INPUT_YUV_SEQ_YVYU,
 		.input_yuv_seq_invert	= SUN6I_CSI_INPUT_YUV_SEQ_YUYV,
 	},
-	{
-		.mbus_code		= MEDIA_BUS_FMT_UYVY8_2X8,
-		.input_format		= SUN6I_CSI_INPUT_FMT_YUV422,
-		.input_yuv_seq		= SUN6I_CSI_INPUT_YUV_SEQ_UYVY,
-		.input_yuv_seq_invert	= SUN6I_CSI_INPUT_YUV_SEQ_VYUY,
-	},
 	{
 		.mbus_code		= MEDIA_BUS_FMT_VYUY8_2X8,
 		.input_format		= SUN6I_CSI_INPUT_FMT_YUV422,
@@ -123,12 +117,6 @@ static const struct sun6i_csi_bridge_format sun6i_csi_bridge_formats[] = {
 		.input_yuv_seq		= SUN6I_CSI_INPUT_YUV_SEQ_YVYU,
 		.input_yuv_seq_invert	= SUN6I_CSI_INPUT_YUV_SEQ_YUYV,
 	},
-	{
-		.mbus_code		= MEDIA_BUS_FMT_UYVY8_1X16,
-		.input_format		= SUN6I_CSI_INPUT_FMT_YUV422,
-		.input_yuv_seq		= SUN6I_CSI_INPUT_YUV_SEQ_UYVY,
-		.input_yuv_seq_invert	= SUN6I_CSI_INPUT_YUV_SEQ_VYUY,
-	},
 	{
 		.mbus_code		= MEDIA_BUS_FMT_VYUY8_1X16,
 		.input_format		= SUN6I_CSI_INPUT_FMT_YUV422,
-- 
2.34.1

Re: [PATCH v1 1/3] media: sun6i-csi: capture: Align size to 16 for NV12_16L16

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: sun6i-csi: incorrect bytesperline for NV12_16L16 format
--

commit 476de016f26cd243aac004fdf752c97c70ae693b
Author: Arash Golgol <arash.golgol@gmail.com>

    media: sun6i-csi: capture: Align size to 16 for NV12_16L16

This commit introduces alignment handling specific to pixel formats in the
sun6i-csi capture driver. It sets the required alignment based on the format
and applies it during format preparation.

> 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..f55edfd0b174a 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c

[ ... ]

> @@ -665,17 +671,19 @@ static void sun6i_csi_capture_format_prepare(struct v4l2_format *format)
>  {
>  	struct v4l2_pix_format *pix_format = &format->fmt.pix;
>  	const struct v4l2_format_info *info;
> -	unsigned int width, height;
> -
> -	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
> -			      SUN6I_CSI_CAPTURE_WIDTH_MAX, 1,
> -			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
> -			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, 1, 0);
> +	unsigned int width, height, align;
>  
>  	if (!sun6i_csi_capture_format_find(pix_format->pixelformat))
>  		pix_format->pixelformat =
>  			sun6i_csi_capture_formats[0].pixelformat;
>  
> +	align = sun6i_csi_capture_format_align(pix_format->pixelformat);
> +
> +	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
> +			      SUN6I_CSI_CAPTURE_WIDTH_MAX, align,
> +			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
> +			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, align, 0);
> +
>  	width = pix_format->width;
>  	height = pix_format->height;
>
> 	info = v4l2_format_info(pix_format->pixelformat);
>
> 	switch (pix_format->pixelformat) {
> 	case V4L2_PIX_FMT_NV12_16L16:
> 		pix_format->bytesperline = width * 12 / 8;

[Severity: Medium]
Does this code violate the V4L2 API contract by incorrectly hardcoding
bytesperline to 1.5x width as a shortcut to calculate the correct sizeimage?

The V4L2 specification requires bytesperline to reflect the stride of the
largest plane, which for NV12 is just width. By exposing a stride of
1.5 * width, userspace applications may compute out-of-bounds plane offsets,
potentially leading to memory corruption or crashes within the userspace
client.

This is supported by sun6i_csi_setup_format(), which configures the
hardware using the correct luma and chroma strides of width internally:

drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c:sun6i_csi_setup_format() {
	...
	switch (pixelformat) {
	...
	case V4L2_PIX_FMT_NV12_16L16:
		luma_line = width;
		chroma_line = width;
		break;
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016113846.8602-1-arash.golgol@gmail.com?part=1