[PATCH v3 0/4] media: rzg2l-cru: Fix DMA stride alignment

[PATCH v3 0/4] media: rzg2l-cru: Fix DMA stride alignment

[Tommaso Merciai]

Dear All,

While testing ov5645 + RZ/G3E we found out that the UYVY8_2X8/2592x1944
is broken. The issue is that the CRU DMA engine requires the line stride
to be a multiple of 128 bytes (RZG2L_CRU_STRIDE_ALIGN). v4l2_fill_pixfmt()
sets bytesperline as width * bytes_per_pixel with no alignment, so for
widths whose natural stride is not 128-byte aligned the hardware silently
rounds up, causing a mismatch that produces visible horizontal banding in
the captured image.

Tested using:

media-ctl -d /dev/media0 --set-v4l2 '"ov5645 0-003c":0[fmt:UYVY8_2X8/2592x1944 field:none]'
media-ctl -d /dev/media0 --set-v4l2 '"csi-16000400.csi2":0[fmt:UYVY8_2X8/2592x1944]'
media-ctl -d /dev/media0 --set-v4l2 '"cru-ip-16000000.video":0[fmt:UYVY8_2X8/2592x1944]'

v4l2-ctl -d /dev/video0 --verbose --set-fmt-video=width=2592,height=1944,pixelformat=UYVY --stream-mmap --stream-count=100 --stream-to=./frame.raw
gst-launch-1.0 v4l2src device=/dev/video0 blocksize=76800 io-mode=dmabuf num-buffers=200 ! video/x-raw,format=UYVY,width=2592,height=1944 !  videoconvert ! queue ! waylandsink sync=false

This patch fix the issue.

Kind Regards,
Tommaso

v2->v3:
 - PATCH 2/4: No changes, just moved to from PATCH 3/4 to PATCH 2/4
 - PATCH 3/4: Moved to PATCH 3/4, from PATCH 2/4.
   Fixed documentation as suggested by Sven Püschel

v1->v2:
 - PATCH 3/4: Move v4l2_fill_pixfmt() into v4l2-common.h as inline wrapper
   Add v4l2_fill_pixfmt_aligned() helper documentation.
 - PATCH 4/4: Collected tag, add missing Cc stable, fix s/commit/Commit/
   into commit body.

Tommaso Merciai (4):
  media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline
    wrapper
  media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper
  media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()
  media: rzg2l-cru: Align bytesperline to hardware DMA stride
    requirement

 .../platform/renesas/rzg2l-cru/rzg2l-video.c  |  3 +-
 drivers/media/v4l2-core/v4l2-common.c         | 20 ++---
 include/media/v4l2-common.h                   | 75 +++++++++++++++++--
 3 files changed, 79 insertions(+), 19 deletions(-)

-- 
2.54.0

[PATCH v3 1/4] media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline wrapper

[Tommaso Merciai]

Convert v4l2_fill_pixfmt_mp() to static inline wrapper: drop the exported
v4l2_fill_pixfmt_mp() function from v4l2-common.c and replace it with
an equivalent static inline in the header that delegates to
v4l2_fill_pixfmt_mp_aligned() with stride_alignment=1.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v2->v3:
 - No changes

v1->v2:
 - New patch

 drivers/media/v4l2-core/v4l2-common.c | 8 --------
 include/media/v4l2-common.h           | 9 +++++++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 65db7340ad38..54995ba8c20d 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -537,14 +537,6 @@ int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
 }
 EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
 
-int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
-			u32 pixelformat, u32 width, u32 height)
-{
-	return v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat,
-					   width, height, 1);
-}
-EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
-
 int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 		     u32 width, u32 height)
 {
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index edd416178c33..749fe38c134e 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -556,13 +556,18 @@ void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
 				    const struct v4l2_frmsize_stepwise *frmsize);
 int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 		     u32 width, u32 height);
-int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
-			u32 width, u32 height);
+
 /* @stride_alignment is a power of 2 value in bytes */
 int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
 				u32 pixelformat, u32 width, u32 height,
 				u8 stride_alignment);
 
+static inline int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
+				      u32 pixelformat, u32 width, u32 height)
+{
+	return v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat, width, height, 1);
+}
+
 /**
  * v4l2_get_link_freq - Get link rate from transmitter
  *
-- 
2.54.0

[PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Tommaso Merciai]

Add v4l2_fill_pixfmt_aligned(), a variant of v4l2_fill_pixfmt()
that accepts a stride_alignment parameter, mirroring the existing
v4l2_fill_pixfmt_mp() / v4l2_fill_pixfmt_mp_aligned() pair.

v4l2_fill_pixfmt() is refactored to call v4l2_fill_pixfmt_aligned()
with stride_alignment=1, preserving its existing behaviour.

The new helper is needed by drivers whose DMA engine requires the
line stride to be a multiple of a specific value, such as the
Renesas RZ/G3E CRU which requires 128-byte alignment.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v2->v3:
 - No changes, just moved to from PATCH 3/4 to PATCH 2/4

v1->v2:
 - Move v4l2_fill_pixfmt() into v4l2-common.h as inline wrapper
 - Add v4l2_fill_pixfmt_aligned() helper documentation.

 drivers/media/v4l2-core/v4l2-common.c | 12 +++++----
 include/media/v4l2-common.h           | 38 +++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 54995ba8c20d..2ce4f1c20fbc 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -537,8 +537,8 @@ int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
 }
 EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
 
-int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
-		     u32 width, u32 height)
+int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
+			     u32 width, u32 height, u8 stride_alignment)
 {
 	const struct v4l2_format_info *info;
 	int i;
@@ -554,15 +554,17 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
 	pixfmt->width = width;
 	pixfmt->height = height;
 	pixfmt->pixelformat = pixelformat;
-	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width, 1);
+	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width,
+							stride_alignment);
 	pixfmt->sizeimage = 0;
 
 	for (i = 0; i < info->comp_planes; i++)
 		pixfmt->sizeimage +=
-			v4l2_format_plane_size(info, i, width, height, 1);
+			v4l2_format_plane_size(info, i, width, height,
+					       stride_alignment);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
+EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_aligned);
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 static s64 v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler,
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 749fe38c134e..be4dd9762196 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -554,8 +554,42 @@ static inline bool v4l2_is_format_bayer(const struct v4l2_format_info *f)
 const struct v4l2_format_info *v4l2_format_info(u32 format);
 void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
 				    const struct v4l2_frmsize_stepwise *frmsize);
-int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
-		     u32 width, u32 height);
+
+/**
+ * v4l2_fill_pixfmt_aligned - Fill in a &struct v4l2_pix_format with stride
+ *	alignment requirements.
+ *
+ * @pixfmt: pointer to the &struct v4l2_pix_format to be filled
+ * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
+ * @width: image width in pixels
+ * @height: image height in pixels
+ * @stride_alignment: stride alignment in bytes, must be a power of 2
+ *
+ * Fills all fields of @pixfmt for the given pixel format, dimensions, and
+ * stride alignment. Only formats stored in a single memory plane are
+ * supported; returns -EINVAL for multi-memory-plane formats.
+ *
+ * @pixfmt->bytesperline is set to the stride of the primary (plane 0) plane,
+ * rounded up to a multiple of @stride_alignment. For formats that store
+ * multiple component planes in a single memory buffer (e.g. NV12), the
+ * alignment applied to each component plane's stride is scaled relative to
+ * @stride_alignment so that the chroma stride remains consistently derivable
+ * from the luma stride. @pixfmt->bytesperline therefore reflects only the
+ * primary plane stride.
+ *
+ * @pixfmt->sizeimage is set to the total size in bytes of all component planes.
+ *
+ * Return: 0 on success, -EINVAL if @pixelformat is unknown or uses multiple
+ *	memory planes.
+ */
+int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
+			     u32 width, u32 height, u8 stride_alignment);
+
+static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
+				   u32 pixelformat, u32 width, u32 height)
+{
+	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
+}
 
 /* @stride_alignment is a power of 2 value in bytes */
 int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
-- 
2.54.0

[PATCH v3 3/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()

[Tommaso Merciai]

Replace the bare placeholder comment with a full kernel-doc block
documenting all parameters, the function behaviour for both single
memory plane (mem_planes == 1) and multiple memory plane (mem_planes > 1)
formats, and the return value.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v2->v3:
 - Moved to PATCH 3/4
 - Fixed documentation as suggested by Sven Püschel

v1->v2:
 - New patch

 include/media/v4l2-common.h | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index be4dd9762196..f2b0c336ac81 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -591,7 +591,33 @@ static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
 	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
 }
 
-/* @stride_alignment is a power of 2 value in bytes */
+/**
+ * v4l2_fill_pixfmt_mp_aligned - Fill in a &struct v4l2_pix_format_mplane with
+ *	stride alignment requirements.
+ *
+ * @pixfmt: pointer to the &struct v4l2_pix_format_mplane to be filled
+ * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
+ * @width: image width in pixels
+ * @height: image height in pixels
+ * @stride_alignment: stride alignment in bytes; must be a power of 2
+ *
+ * Fills all fields of @pixfmt for the given pixel format, dimensions, and
+ * stride alignment.
+ *
+ * For formats stored in a single memory plane (mem_planes == 1), the
+ * behaviour matches v4l2_fill_pixfmt_aligned(): plane_fmt[0].bytesperline
+ * is set to the primary plane stride. The strides of all components are
+ * aligned to the @stride_alignment. To keep the chroma strides consistently
+ * derivable from the luma stride, strides may be aligned to a multiple of
+ * the @stride_alignment instead. plane_fmt[0].sizeimage covers all
+ * component planes.
+ *
+ * For formats with multiple memory planes (mem_planes > 1), each plane's
+ * bytesperline is independently rounded up to @stride_alignment, and
+ * sizeimage is set to bytesperline multiplied by the plane height.
+ *
+ * Return: 0 on success, -EINVAL if @pixelformat is unknown.
+ */
 int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
 				u32 pixelformat, u32 width, u32 height,
 				u8 stride_alignment);
-- 
2.54.0

[PATCH v3 4/4] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Tommaso Merciai]

The RZ/G3E CRU programs the line stride via the AMnIS register, whose
IS field encodes the value in units of 128 bytes. If bytesperline is
not a multiple of 128, the division truncates and the hardware uses a
wrong stride, causing horizontal banding.

Commit ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
replaced the open-coded aligned calculation with v4l2_fill_pixfmt(),
which sets no alignment, reintroducing the issue.

Switch to v4l2_fill_pixfmt_aligned() with RZG2L_CRU_STRIDE_ALIGN when
info->has_stride is set. RZ/G2L has no AMnIS register and keeps using
v4l2_fill_pixfmt() unchanged.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: stable@vger.kernel.org
Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v2->v3:
 - No changes.

v1->v2:
 - Collected tag
 - Add missing Cc stable
 - Fix s/commit/Commit/ into commit body

 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 69346a585f9f..478264f26466 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -860,7 +860,8 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
 			      &pix->height, 240, info->max_height, 0, 0);
 
-	v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
+	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
+				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
 
 	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
 		pix->width, pix->height, pix->bytesperline, pix->sizeimage);
-- 
2.54.0

Re: [PATCH v3 4/4] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Jacopo Mondi]

Hi Tommaso

On Wed, Jul 08, 2026 at 06:14:05PM +0200, Tommaso Merciai wrote:
> The RZ/G3E CRU programs the line stride via the AMnIS register, whose
> IS field encodes the value in units of 128 bytes. If bytesperline is

Unrelated, it seems for RGB888 the alignemtn requirement is 384 bytes,
something that doesn't seem handled at the moment ?

> not a multiple of 128, the division truncates and the hardware uses a
> wrong stride, causing horizontal banding.
>
> Commit ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> replaced the open-coded aligned calculation with v4l2_fill_pixfmt(),
> which sets no alignment, reintroducing the issue.
>
> Switch to v4l2_fill_pixfmt_aligned() with RZG2L_CRU_STRIDE_ALIGN when
> info->has_stride is set. RZ/G2L has no AMnIS register and keeps using
> v4l2_fill_pixfmt() unchanged.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: stable@vger.kernel.org
> Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v2->v3:
>  - No changes.
>
> v1->v2:
>  - Collected tag
>  - Add missing Cc stable
>  - Fix s/commit/Commit/ into commit body
>
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 69346a585f9f..478264f26466 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -860,7 +860,8 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
>  	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
>  			      &pix->height, 240, info->max_height, 0, 0);

This doesn't apply on media-committers/next which has

	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
			      &pix->height, 240, info->max_height, 2, 0);

in this line.

What have I missed ?

>
> -	v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
> +	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> +				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);

Rebasing apart, this seems correct

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Thanks
  j

>
>  	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
>  		pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> --
> 2.54.0
>
>

Re: [PATCH v3 1/4] media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline wrapper

[Jacopo Mondi]

Hi Tommaso

On Wed, Jul 08, 2026 at 06:14:02PM +0200, Tommaso Merciai wrote:
> Convert v4l2_fill_pixfmt_mp() to static inline wrapper: drop the exported
> v4l2_fill_pixfmt_mp() function from v4l2-common.c and replace it with
> an equivalent static inline in the header that delegates to
> v4l2_fill_pixfmt_mp_aligned() with stride_alignment=1.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v2->v3:
>  - No changes
>
> v1->v2:
>  - New patch
>
>  drivers/media/v4l2-core/v4l2-common.c | 8 --------
>  include/media/v4l2-common.h           | 9 +++++++--
>  2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 65db7340ad38..54995ba8c20d 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -537,14 +537,6 @@ int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
>
> -int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
> -			u32 pixelformat, u32 width, u32 height)
> -{
> -	return v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat,
> -					   width, height, 1);
> -}
> -EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
> -
>  int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  		     u32 width, u32 height)
>  {
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index edd416178c33..749fe38c134e 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -556,13 +556,18 @@ void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
>  				    const struct v4l2_frmsize_stepwise *frmsize);
>  int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  		     u32 width, u32 height);
> -int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
> -			u32 width, u32 height);
> +
>  /* @stride_alignment is a power of 2 value in bytes */

Well, 1 is certainly a power of 2 :)

>  int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
>  				u32 pixelformat, u32 width, u32 height,
>  				u8 stride_alignment);
>
> +static inline int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
> +				      u32 pixelformat, u32 width, u32 height)
> +{
> +	return v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat, width, height, 1);
> +}
> +

Makes sense
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

>  /**
>   * v4l2_get_link_freq - Get link rate from transmitter
>   *
> --
> 2.54.0
>
>

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Jacopo Mondi]

Hi Tommaso

On Wed, Jul 08, 2026 at 06:14:03PM +0200, Tommaso Merciai wrote:
> Add v4l2_fill_pixfmt_aligned(), a variant of v4l2_fill_pixfmt()
> that accepts a stride_alignment parameter, mirroring the existing
> v4l2_fill_pixfmt_mp() / v4l2_fill_pixfmt_mp_aligned() pair.
>
> v4l2_fill_pixfmt() is refactored to call v4l2_fill_pixfmt_aligned()
> with stride_alignment=1, preserving its existing behaviour.
>
> The new helper is needed by drivers whose DMA engine requires the
> line stride to be a multiple of a specific value, such as the
> Renesas RZ/G3E CRU which requires 128-byte alignment.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v2->v3:
>  - No changes, just moved to from PATCH 3/4 to PATCH 2/4
>
> v1->v2:
>  - Move v4l2_fill_pixfmt() into v4l2-common.h as inline wrapper
>  - Add v4l2_fill_pixfmt_aligned() helper documentation.
>
>  drivers/media/v4l2-core/v4l2-common.c | 12 +++++----
>  include/media/v4l2-common.h           | 38 +++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 54995ba8c20d..2ce4f1c20fbc 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -537,8 +537,8 @@ int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
>
> -int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> -		     u32 width, u32 height)
> +int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> +			     u32 width, u32 height, u8 stride_alignment)
>  {
>  	const struct v4l2_format_info *info;
>  	int i;
> @@ -554,15 +554,17 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  	pixfmt->width = width;
>  	pixfmt->height = height;
>  	pixfmt->pixelformat = pixelformat;
> -	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width, 1);
> +	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width,
> +							stride_alignment);
>  	pixfmt->sizeimage = 0;
>
>  	for (i = 0; i < info->comp_planes; i++)
>  		pixfmt->sizeimage +=
> -			v4l2_format_plane_size(info, i, width, height, 1);
> +			v4l2_format_plane_size(info, i, width, height,
> +					       stride_alignment);
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_aligned);
>
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  static s64 v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler,
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 749fe38c134e..be4dd9762196 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -554,8 +554,42 @@ static inline bool v4l2_is_format_bayer(const struct v4l2_format_info *f)
>  const struct v4l2_format_info *v4l2_format_info(u32 format);
>  void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
>  				    const struct v4l2_frmsize_stepwise *frmsize);
> -int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> -		     u32 width, u32 height);
> +
> +/**
> + * v4l2_fill_pixfmt_aligned - Fill in a &struct v4l2_pix_format with stride
> + *	alignment requirements.

nit:
I was about to suggest "No '.' at the end of the function's brief to
match the existing style" but I see the devm_v4l2_sensor_clk_get_legacy
has it. However the majority of the other functions don't, so maybe
consider dropping it.

> + *
> + * @pixfmt: pointer to the &struct v4l2_pix_format to be filled
> + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> + * @width: image width in pixels
> + * @height: image height in pixels
> + * @stride_alignment: stride alignment in bytes, must be a power of 2
> + *
> + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> + * stride alignment. Only formats stored in a single memory plane are
> + * supported; returns -EINVAL for multi-memory-plane formats.
> + *
> + * @pixfmt->bytesperline is set to the stride of the primary (plane 0) plane,
> + * rounded up to a multiple of @stride_alignment. For formats that store
> + * multiple component planes in a single memory buffer (e.g. NV12), the
> + * alignment applied to each component plane's stride is scaled relative to
> + * @stride_alignment so that the chroma stride remains consistently derivable

Does this rather mean that

"For formats that store multiple component planes in a single memory
buffer (e.g. NV12), the alignment applied to each component plane is
the first plane @stride_alignment scaled by the plane's sub-sampling
ratio" or have I mis-read this ?

> + * from the luma stride. @pixfmt->bytesperline therefore reflects only the
> + * primary plane stride.
> + *
> + * @pixfmt->sizeimage is set to the total size in bytes of all component planes.

maybe s/component // ?

> + *
> + * Return: 0 on success, -EINVAL if @pixelformat is unknown or uses multiple
> + *	memory planes.

Do you need this tab ?

> + */
> +int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> +			     u32 width, u32 height, u8 stride_alignment);
> +
> +static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
> +				   u32 pixelformat, u32 width, u32 height)
> +{
> +	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
> +}

All minors or nit-picking
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Thanks
  j

>
>  /* @stride_alignment is a power of 2 value in bytes */
>  int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> --
> 2.54.0
>
>

Re: [PATCH v3 3/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()

[Jacopo Mondi]

Hi Tommaso

On Wed, Jul 08, 2026 at 06:14:04PM +0200, Tommaso Merciai wrote:
> Replace the bare placeholder comment with a full kernel-doc block
> documenting all parameters, the function behaviour for both single
> memory plane (mem_planes == 1) and multiple memory plane (mem_planes > 1)
> formats, and the return value.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v2->v3:
>  - Moved to PATCH 3/4
>  - Fixed documentation as suggested by Sven Püschel
>
> v1->v2:
>  - New patch
>
>  include/media/v4l2-common.h | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index be4dd9762196..f2b0c336ac81 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -591,7 +591,33 @@ static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
>  	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
>  }
>
> -/* @stride_alignment is a power of 2 value in bytes */
> +/**
> + * v4l2_fill_pixfmt_mp_aligned - Fill in a &struct v4l2_pix_format_mplane with
> + *	stride alignment requirements.
> + *
> + * @pixfmt: pointer to the &struct v4l2_pix_format_mplane to be filled
> + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> + * @width: image width in pixels
> + * @height: image height in pixels
> + * @stride_alignment: stride alignment in bytes; must be a power of 2
> + *
> + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> + * stride alignment.
> + *
> + * For formats stored in a single memory plane (mem_planes == 1), the
> + * behaviour matches v4l2_fill_pixfmt_aligned(): plane_fmt[0].bytesperline
> + * is set to the primary plane stride. The strides of all components are
> + * aligned to the @stride_alignment. To keep the chroma strides consistently
> + * derivable from the luma stride, strides may be aligned to a multiple of
> + * the @stride_alignment instead. plane_fmt[0].sizeimage covers all


I guess this

"To keep the chroma strides consistently derivable from the luma
stride, strides may be aligned to a multiple of the @stride_alignment
instead."

comes from teh v4l2_format_plane_stride() implementation.

I admit is not 100% clear to me why the chroma strides is multiplied
and to which format this applies. But this is not on this patch...

> + * component planes.
> + *
> + * For formats with multiple memory planes (mem_planes > 1), each plane's
> + * bytesperline is independently rounded up to @stride_alignment, and

and each plane's sizeimage is ..

> + * sizeimage is set to bytesperline multiplied by the plane height.
> + *
> + * Return: 0 on success, -EINVAL if @pixelformat is unknown.
> + */

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Thanks
  j

>  int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
>  				u32 pixelformat, u32 width, u32 height,
>  				u8 stride_alignment);
> --
> 2.54.0
>
>

Re: [PATCH v3 4/4] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Tommaso Merciai]

Hi Jacopo,
Thanks for your review.

On Thu, Jul 09, 2026 at 11:19:17AM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Wed, Jul 08, 2026 at 06:14:05PM +0200, Tommaso Merciai wrote:
> > The RZ/G3E CRU programs the line stride via the AMnIS register, whose
> > IS field encodes the value in units of 128 bytes. If bytesperline is
> 
> Unrelated, it seems for RGB888 the alignemtn requirement is 384 bytes,
> something that doesn't seem handled at the moment ?

Yes, I had a similar discussion with Laurent at [1]

Currently neither RGB888 nor semi-planar YUV 4:2:0 are supported.
I will handle this once the support for those formats will be added
if for you is ok.

> 
> > not a multiple of 128, the division truncates and the hardware uses a
> > wrong stride, causing horizontal banding.
> >
> > Commit ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > replaced the open-coded aligned calculation with v4l2_fill_pixfmt(),
> > which sets no alignment, reintroducing the issue.
> >
> > Switch to v4l2_fill_pixfmt_aligned() with RZG2L_CRU_STRIDE_ALIGN when
> > info->has_stride is set. RZ/G2L has no AMnIS register and keeps using
> > v4l2_fill_pixfmt() unchanged.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: stable@vger.kernel.org
> > Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v2->v3:
> >  - No changes.
> >
> > v1->v2:
> >  - Collected tag
> >  - Add missing Cc stable
> >  - Fix s/commit/Commit/ into commit body
> >
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 69346a585f9f..478264f26466 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -860,7 +860,8 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> >  	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
> >  			      &pix->height, 240, info->max_height, 0, 0);
> 
> This doesn't apply on media-committers/next which has
> 
> 	v4l_bound_align_image(&pix->width, 320, info->max_width, 1,
> 			      &pix->height, 240, info->max_height, 2, 0);
> 
> in this line.
> 
> What have I missed ?

Mmmm my fault I errenously have [2]
on top of my local media-committers/next tree.

Will fix that in v4

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20260624104153.798953-3-tommaso.merciai.xr@bp.renesas.com/
[2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20260521131911.92845-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Kind Regards,
Tommaso

> 
> >
> > -	v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
> > +	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> > +				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> 
> Rebasing apart, this seems correct
> 
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> Thanks
>   j
> 
> >
> >  	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
> >  		pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> > --
> > 2.54.0
> >
> >

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Tommaso Merciai]

Hi Jacopo,
Thanks for your review.

On Thu, Jul 09, 2026 at 11:35:58AM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Wed, Jul 08, 2026 at 06:14:03PM +0200, Tommaso Merciai wrote:
> > Add v4l2_fill_pixfmt_aligned(), a variant of v4l2_fill_pixfmt()
> > that accepts a stride_alignment parameter, mirroring the existing
> > v4l2_fill_pixfmt_mp() / v4l2_fill_pixfmt_mp_aligned() pair.
> >
> > v4l2_fill_pixfmt() is refactored to call v4l2_fill_pixfmt_aligned()
> > with stride_alignment=1, preserving its existing behaviour.
> >
> > The new helper is needed by drivers whose DMA engine requires the
> > line stride to be a multiple of a specific value, such as the
> > Renesas RZ/G3E CRU which requires 128-byte alignment.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v2->v3:
> >  - No changes, just moved to from PATCH 3/4 to PATCH 2/4
> >
> > v1->v2:
> >  - Move v4l2_fill_pixfmt() into v4l2-common.h as inline wrapper
> >  - Add v4l2_fill_pixfmt_aligned() helper documentation.
> >
> >  drivers/media/v4l2-core/v4l2-common.c | 12 +++++----
> >  include/media/v4l2-common.h           | 38 +++++++++++++++++++++++++--
> >  2 files changed, 43 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 54995ba8c20d..2ce4f1c20fbc 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -537,8 +537,8 @@ int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
> >
> > -int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> > -		     u32 width, u32 height)
> > +int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> > +			     u32 width, u32 height, u8 stride_alignment)
> >  {
> >  	const struct v4l2_format_info *info;
> >  	int i;
> > @@ -554,15 +554,17 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> >  	pixfmt->width = width;
> >  	pixfmt->height = height;
> >  	pixfmt->pixelformat = pixelformat;
> > -	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width, 1);
> > +	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width,
> > +							stride_alignment);
> >  	pixfmt->sizeimage = 0;
> >
> >  	for (i = 0; i < info->comp_planes; i++)
> >  		pixfmt->sizeimage +=
> > -			v4l2_format_plane_size(info, i, width, height, 1);
> > +			v4l2_format_plane_size(info, i, width, height,
> > +					       stride_alignment);
> >  	return 0;
> >  }
> > -EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> > +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_aligned);
> >
> >  #ifdef CONFIG_MEDIA_CONTROLLER
> >  static s64 v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler,
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > index 749fe38c134e..be4dd9762196 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -554,8 +554,42 @@ static inline bool v4l2_is_format_bayer(const struct v4l2_format_info *f)
> >  const struct v4l2_format_info *v4l2_format_info(u32 format);
> >  void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
> >  				    const struct v4l2_frmsize_stepwise *frmsize);
> > -int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> > -		     u32 width, u32 height);
> > +
> > +/**
> > + * v4l2_fill_pixfmt_aligned - Fill in a &struct v4l2_pix_format with stride
> > + *	alignment requirements.
> 
> nit:
> I was about to suggest "No '.' at the end of the function's brief to
> match the existing style" but I see the devm_v4l2_sensor_clk_get_legacy
> has it. However the majority of the other functions don't, so maybe
> consider dropping it.

Ok, I will drop it in v4.

> 
> > + *
> > + * @pixfmt: pointer to the &struct v4l2_pix_format to be filled
> > + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> > + * @width: image width in pixels
> > + * @height: image height in pixels
> > + * @stride_alignment: stride alignment in bytes, must be a power of 2
> > + *
> > + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> > + * stride alignment. Only formats stored in a single memory plane are
> > + * supported; returns -EINVAL for multi-memory-plane formats.
> > + *
> > + * @pixfmt->bytesperline is set to the stride of the primary (plane 0) plane,
> > + * rounded up to a multiple of @stride_alignment. For formats that store
> > + * multiple component planes in a single memory buffer (e.g. NV12), the
> > + * alignment applied to each component plane's stride is scaled relative to
> > + * @stride_alignment so that the chroma stride remains consistently derivable
> 
> Does this rather mean that
> 
> "For formats that store multiple component planes in a single memory
> buffer (e.g. NV12), the alignment applied to each component plane is
> the first plane @stride_alignment scaled by the plane's sub-sampling
> ratio" or have I mis-read this ?

Yes thanks.
I will update this part with you suggestion.

> 
> > + * from the luma stride. @pixfmt->bytesperline therefore reflects only the
> > + * primary plane stride.
> > + *
> > + * @pixfmt->sizeimage is set to the total size in bytes of all component planes.
> 
> maybe s/component // ?

Ok, will drop this in v4.

> 
> > + *
> > + * Return: 0 on success, -EINVAL if @pixelformat is unknown or uses multiple
> > + *	memory planes.
> 
> Do you need this tab ?

Will drop this in v4.

> 
> > + */
> > +int v4l2_fill_pixfmt_aligned(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> > +			     u32 width, u32 height, u8 stride_alignment);
> > +
> > +static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
> > +				   u32 pixelformat, u32 width, u32 height)
> > +{
> > +	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
> > +}
> 
> All minors or nit-picking
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Kind Regards,
Tommaso

> 
> Thanks
>   j
> 
> >
> >  /* @stride_alignment is a power of 2 value in bytes */
> >  int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> > --
> > 2.54.0
> >
> >

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Sven Püschel]

Hi Jacopo,

On 7/9/26 11:35 AM, Jacopo Mondi wrote:
> Hi Tommaso
>
> On Wed, Jul 08, 2026 at 06:14:03PM +0200, Tommaso Merciai wrote:
>
>> + *
>> + * @pixfmt: pointer to the &struct v4l2_pix_format to be filled
>> + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
>> + * @width: image width in pixels
>> + * @height: image height in pixels
>> + * @stride_alignment: stride alignment in bytes, must be a power of 2
>> + *
>> + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
>> + * stride alignment. Only formats stored in a single memory plane are
>> + * supported; returns -EINVAL for multi-memory-plane formats.
>> + *
>> + * @pixfmt->bytesperline is set to the stride of the primary (plane 0) plane,
>> + * rounded up to a multiple of @stride_alignment. For formats that store
>> + * multiple component planes in a single memory buffer (e.g. NV12), the
>> + * alignment applied to each component plane's stride is scaled relative to
>> + * @stride_alignment so that the chroma stride remains consistently derivable
> Does this rather mean that
>
> "For formats that store multiple component planes in a single memory
> buffer (e.g. NV12), the alignment applied to each component plane is
> the first plane @stride_alignment scaled by the plane's sub-sampling
> ratio" or have I mis-read this ?

No, for the example of NV12, no stride will get scaled (although the 
sub-sampling of 4:2:0, resulting in a vdiv and hdiv of 2).

This is due to the fact, that while we have a hdiv of 2 we also 
interleave the cb and cr parts in a single plane, which results in the 
stride being the same number of bytes as for the y plane (and vdiv isn't 
relevant for the stride).

Therefore the stride scaling also respects the bits per plane (bpp) 
value to determine the scaling.

@Tommaso : While the sentence looks ok, the NV12 example is misguided. 
The intention is that for non-mp (not ending with M) formats we might do 
the scaling (e.g. YUV420 will have it's Y component stride alignment 
scaled to not break the u and v stride alignments, but YUV420M not)

Sincerely
     Sven

Re: [PATCH v3 3/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()

[Sven Püschel]

Hi Jacopo,

On 7/9/26 11:51 AM, Jacopo Mondi wrote:
> Hi Tommaso
>
> On Wed, Jul 08, 2026 at 06:14:04PM +0200, Tommaso Merciai wrote:
>> Replace the bare placeholder comment with a full kernel-doc block
>> documenting all parameters, the function behaviour for both single
>> memory plane (mem_planes == 1) and multiple memory plane (mem_planes > 1)
>> formats, and the return value.
>>
>> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>> ---
>> v2->v3:
>>   - Moved to PATCH 3/4
>>   - Fixed documentation as suggested by Sven Püschel
>>
>> v1->v2:
>>   - New patch
>>
>>   include/media/v4l2-common.h | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
>> index be4dd9762196..f2b0c336ac81 100644
>> --- a/include/media/v4l2-common.h
>> +++ b/include/media/v4l2-common.h
>> @@ -591,7 +591,33 @@ static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
>>   	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
>>   }
>>
>> -/* @stride_alignment is a power of 2 value in bytes */
>> +/**
>> + * v4l2_fill_pixfmt_mp_aligned - Fill in a &struct v4l2_pix_format_mplane with
>> + *	stride alignment requirements.
>> + *
>> + * @pixfmt: pointer to the &struct v4l2_pix_format_mplane to be filled
>> + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
>> + * @width: image width in pixels
>> + * @height: image height in pixels
>> + * @stride_alignment: stride alignment in bytes; must be a power of 2
>> + *
>> + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
>> + * stride alignment.
>> + *
>> + * For formats stored in a single memory plane (mem_planes == 1), the
>> + * behaviour matches v4l2_fill_pixfmt_aligned(): plane_fmt[0].bytesperline
>> + * is set to the primary plane stride. The strides of all components are
>> + * aligned to the @stride_alignment. To keep the chroma strides consistently
>> + * derivable from the luma stride, strides may be aligned to a multiple of
>> + * the @stride_alignment instead. plane_fmt[0].sizeimage covers all
>
> I guess this
>
> "To keep the chroma strides consistently derivable from the luma
> stride, strides may be aligned to a multiple of the @stride_alignment
> instead."
>
> comes from teh v4l2_format_plane_stride() implementation.
>
> I admit is not 100% clear to me why the chroma strides is multiplied
> and to which format this applies. But this is not on this patch...

When not using multi-planar formats, we only have the stride value for 
the Y component and the other stride values are derived from it. This is 
the cause of this whole scaling.

E.g. for YUV420 4x2px picture, we have 4 bytes stride in the y plane and 
2 byte in the cb and cr plane. If we align the stride to 4 bytes (in all 
planes), we want both values to be a multiple of 4. As the cb/cr stride 
is derived from the y stride, we have to set the y stride to 8 bytes to 
get the desired 4 bytes stride in the cb/cr planes.

The rare case for scaling the component stride is NV24/42 (at least this 
is the only one I currently know of), where we actually have 4:4:4 
sub-sampling and have the cb/cr parts interleaved. So for a 1x2px 
picture we have 1 bytes in the y plane and 2 bytes in the c plane. To 
align to 4 bytes we need to set the c plane stride to 8 to be able to 
set the y plane stride to 4.


For multi-planar formats we have a separate stride for each component, 
so we just align all component strides to the given alignment.


Sincerely
     Sven

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Jacopo Mondi]

Hi Sven

On Fri, Jul 10, 2026 at 10:36:53AM +0200, Sven Püschel wrote:
> Hi Jacopo,
>
> On 7/9/26 11:35 AM, Jacopo Mondi wrote:
> > Hi Tommaso
> >
> > On Wed, Jul 08, 2026 at 06:14:03PM +0200, Tommaso Merciai wrote:
> >
> > > + *
> > > + * @pixfmt: pointer to the &struct v4l2_pix_format to be filled
> > > + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> > > + * @width: image width in pixels
> > > + * @height: image height in pixels
> > > + * @stride_alignment: stride alignment in bytes, must be a power of 2
> > > + *
> > > + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> > > + * stride alignment. Only formats stored in a single memory plane are
> > > + * supported; returns -EINVAL for multi-memory-plane formats.
> > > + *
> > > + * @pixfmt->bytesperline is set to the stride of the primary (plane 0) plane,
> > > + * rounded up to a multiple of @stride_alignment. For formats that store
> > > + * multiple component planes in a single memory buffer (e.g. NV12), the
> > > + * alignment applied to each component plane's stride is scaled relative to
> > > + * @stride_alignment so that the chroma stride remains consistently derivable
> > Does this rather mean that
> >
> > "For formats that store multiple component planes in a single memory
> > buffer (e.g. NV12), the alignment applied to each component plane is
> > the first plane @stride_alignment scaled by the plane's sub-sampling
> > ratio" or have I mis-read this ?
>
> No, for the example of NV12, no stride will get scaled (although the
> sub-sampling of 4:2:0, resulting in a vdiv and hdiv of 2).

Ah, I had looked at v4l2_format_plane_stride() for the NV12 case where
byte_alignment gets adjusted for the second plane as:

byte_alignment *= DIV_ROUND_UP(info->bpp[1], info->hdiv * info->bpp[0]);

which for NV12 resolves at *= 1

and I got confused

Indeed this is not just

"the first plane @stride_alignment scaled by the plane's sub-sampling
ratio"

I proposed

>
> This is due to the fact, that while we have a hdiv of 2 we also interleave
> the cb and cr parts in a single plane, which results in the stride being the
> same number of bytes as for the y plane (and vdiv isn't relevant for the
> stride).
>
> Therefore the stride scaling also respects the bits per plane (bpp) value to
> determine the scaling.
>
> @Tommaso : While the sentence looks ok, the NV12 example is misguided. The

I guess the usage of NV12 was as example of a "formats that store
multiple component planes in a single memory"

NV24/42 works the same, but being 444 it needs the chroma plane stride to
be a multiple of the fist plane stride and might prove as a better
example ?

> intention is that for non-mp (not ending with M) formats we might do the
> scaling (e.g. YUV420 will have it's Y component stride alignment scaled to
> not break the u and v stride alignments, but YUV420M not)
>
> Sincerely
>     Sven
>
>

Re: [PATCH v3 3/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()

[Jacopo Mondi]

Hi Sven

On Fri, Jul 10, 2026 at 10:57:41AM +0200, Sven Püschel wrote:
> Hi Jacopo,
>
> On 7/9/26 11:51 AM, Jacopo Mondi wrote:
> > Hi Tommaso
> >
> > On Wed, Jul 08, 2026 at 06:14:04PM +0200, Tommaso Merciai wrote:
> > > Replace the bare placeholder comment with a full kernel-doc block
> > > documenting all parameters, the function behaviour for both single
> > > memory plane (mem_planes == 1) and multiple memory plane (mem_planes > 1)
> > > formats, and the return value.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > v2->v3:
> > >   - Moved to PATCH 3/4
> > >   - Fixed documentation as suggested by Sven Püschel
> > >
> > > v1->v2:
> > >   - New patch
> > >
> > >   include/media/v4l2-common.h | 28 +++++++++++++++++++++++++++-
> > >   1 file changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > > index be4dd9762196..f2b0c336ac81 100644
> > > --- a/include/media/v4l2-common.h
> > > +++ b/include/media/v4l2-common.h
> > > @@ -591,7 +591,33 @@ static inline int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt,
> > >   	return v4l2_fill_pixfmt_aligned(pixfmt, pixelformat, width, height, 1);
> > >   }
> > >
> > > -/* @stride_alignment is a power of 2 value in bytes */
> > > +/**
> > > + * v4l2_fill_pixfmt_mp_aligned - Fill in a &struct v4l2_pix_format_mplane with
> > > + *	stride alignment requirements.
> > > + *
> > > + * @pixfmt: pointer to the &struct v4l2_pix_format_mplane to be filled
> > > + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> > > + * @width: image width in pixels
> > > + * @height: image height in pixels
> > > + * @stride_alignment: stride alignment in bytes; must be a power of 2
> > > + *
> > > + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> > > + * stride alignment.
> > > + *
> > > + * For formats stored in a single memory plane (mem_planes == 1), the
> > > + * behaviour matches v4l2_fill_pixfmt_aligned(): plane_fmt[0].bytesperline
> > > + * is set to the primary plane stride. The strides of all components are
> > > + * aligned to the @stride_alignment. To keep the chroma strides consistently
> > > + * derivable from the luma stride, strides may be aligned to a multiple of
> > > + * the @stride_alignment instead. plane_fmt[0].sizeimage covers all
> >
> > I guess this
> >
> > "To keep the chroma strides consistently derivable from the luma
> > stride, strides may be aligned to a multiple of the @stride_alignment
> > instead."
> >
> > comes from teh v4l2_format_plane_stride() implementation.
> >
> > I admit is not 100% clear to me why the chroma strides is multiplied
> > and to which format this applies. But this is not on this patch...
>
> When not using multi-planar formats, we only have the stride value for the Y
> component and the other stride values are derived from it. This is the cause
> of this whole scaling.
>
> E.g. for YUV420 4x2px picture, we have 4 bytes stride in the y plane and 2
> byte in the cb and cr plane. If we align the stride to 4 bytes (in all
> planes), we want both values to be a multiple of 4. As the cb/cr stride is
> derived from the y stride, we have to set the y stride to 8 bytes to get the
> desired 4 bytes stride in the cb/cr planes.
>
> The rare case for scaling the component stride is NV24/42 (at least this is
> the only one I currently know of), where we actually have 4:4:4 sub-sampling
> and have the cb/cr parts interleaved. So for a 1x2px picture we have 1 bytes
> in the y plane and 2 bytes in the c plane. To align to 4 bytes we need to
> set the c plane stride to 8 to be able to set the y plane stride to 4.

I see, I was probably confusing strides and strides -alignments-.

Thanks for the explanation.


>
>
> For multi-planar formats we have a separate stride for each component, so we
> just align all component strides to the given alignment.
>
>
> Sincerely
>     Sven
>
>

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Sven Püschel]

Hi Jacopo,

On 7/10/26 11:38 AM, Jacopo Mondi wrote:
>> This is due to the fact, that while we have a hdiv of 2 we also interleave
>> the cb and cr parts in a single plane, which results in the stride being the
>> same number of bytes as for the y plane (and vdiv isn't relevant for the
>> stride).
>>
>> Therefore the stride scaling also respects the bits per plane (bpp) value to
>> determine the scaling.
>>
>> @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
> I guess the usage of NV12 was as example of a "formats that store
> multiple component planes in a single memory"
>
> NV24/42 works the same, but being 444 it needs the chroma plane stride to
> be a multiple of the fist plane stride and might prove as a better
> example ?
>
My potential concern is that NV as an example misguides the reader into 
one of the following:

- It's only for formats which interleave cb/cr into one plane (whereas 
YUV420 also gets scaled)
- NV24 in the example being though of including the NV24M variant 
(whereas latter won't be affected)

Maybe smth. like YUV420 but not YUV420M is a better example (could also 
be NV24 but not NV24M)?

Sincerely
     Sven

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Jacopo Mondi]

Hi Sven

On Fri, Jul 10, 2026 at 01:54:06PM +0200, Sven Püschel wrote:
> Hi Jacopo,
>
> On 7/10/26 11:38 AM, Jacopo Mondi wrote:
> > > This is due to the fact, that while we have a hdiv of 2 we also interleave
> > > the cb and cr parts in a single plane, which results in the stride being the
> > > same number of bytes as for the y plane (and vdiv isn't relevant for the
> > > stride).
> > >
> > > Therefore the stride scaling also respects the bits per plane (bpp) value to
> > > determine the scaling.
> > >
> > > @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
> > I guess the usage of NV12 was as example of a "formats that store
> > multiple component planes in a single memory"
> >
> > NV24/42 works the same, but being 444 it needs the chroma plane stride to
> > be a multiple of the fist plane stride and might prove as a better
> > example ?
> >
> My potential concern is that NV as an example misguides the reader into one
> of the following:
>
> - It's only for formats which interleave cb/cr into one plane (whereas
> YUV420 also gets scaled)
> - NV24 in the example being though of including the NV24M variant (whereas
> latter won't be affected)

>

M variants are not supported by the single-planar APIs
https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html

Some planar formats allow planes to be placed in independent memory
locations. They are identified by an ‘M’ suffix in their name (such as
in V4L2_PIX_FMT_NV12M). Those formats are intended to be used only in
drivers and applications that support the multi-planar API,

And here we're dealing with single-planar API only if I'm not mistaken


> Maybe smth. like YUV420 but not YUV420M is a better example (could also be
> NV24 but not NV24M)?
>
> Sincerely
>     Sven
>

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Sven Püschel]

Hi Jacopo,

On 7/10/26 2:15 PM, Jacopo Mondi wrote:
> Hi Sven
>
> On Fri, Jul 10, 2026 at 01:54:06PM +0200, Sven Püschel wrote:
>> Hi Jacopo,
>>
>> On 7/10/26 11:38 AM, Jacopo Mondi wrote:
>>>> This is due to the fact, that while we have a hdiv of 2 we also interleave
>>>> the cb and cr parts in a single plane, which results in the stride being the
>>>> same number of bytes as for the y plane (and vdiv isn't relevant for the
>>>> stride).
>>>>
>>>> Therefore the stride scaling also respects the bits per plane (bpp) value to
>>>> determine the scaling.
>>>>
>>>> @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
>>> I guess the usage of NV12 was as example of a "formats that store
>>> multiple component planes in a single memory"
>>>
>>> NV24/42 works the same, but being 444 it needs the chroma plane stride to
>>> be a multiple of the fist plane stride and might prove as a better
>>> example ?
>>>
>> My potential concern is that NV as an example misguides the reader into one
>> of the following:
>>
>> - It's only for formats which interleave cb/cr into one plane (whereas
>> YUV420 also gets scaled)
>> - NV24 in the example being though of including the NV24M variant (whereas
>> latter won't be affected)
> M variants are not supported by the single-planar APIs
> https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html
>
> Some planar formats allow planes to be placed in independent memory
> locations. They are identified by an ‘M’ suffix in their name (such as
> in V4L2_PIX_FMT_NV12M). Those formats are intended to be used only in
> drivers and applications that support the multi-planar API,
>
> And here we're dealing with single-planar API only if I'm not mistaken

Oh, sorry. Assumed that the added description of both functions would be 
similar/identical, which isn't the case.

Given this, I'm fine with the wording and agree to just change the 
example to smth. else than NV12.

Sincerely
     Sven

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Tommaso Merciai]

Hi Jacopo, Sven,
Thanks for your comments:

On Fri, Jul 10, 2026 at 02:26:23PM +0200, Sven Püschel wrote:
> Hi Jacopo,
> 
> On 7/10/26 2:15 PM, Jacopo Mondi wrote:
> > Hi Sven
> > 
> > On Fri, Jul 10, 2026 at 01:54:06PM +0200, Sven Püschel wrote:
> > > Hi Jacopo,
> > > 
> > > On 7/10/26 11:38 AM, Jacopo Mondi wrote:
> > > > > This is due to the fact, that while we have a hdiv of 2 we also interleave
> > > > > the cb and cr parts in a single plane, which results in the stride being the
> > > > > same number of bytes as for the y plane (and vdiv isn't relevant for the
> > > > > stride).
> > > > > 
> > > > > Therefore the stride scaling also respects the bits per plane (bpp) value to
> > > > > determine the scaling.
> > > > > 
> > > > > @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
> > > > I guess the usage of NV12 was as example of a "formats that store
> > > > multiple component planes in a single memory"
> > > > 
> > > > NV24/42 works the same, but being 444 it needs the chroma plane stride to
> > > > be a multiple of the fist plane stride and might prove as a better
> > > > example ?
> > > > 
> > > My potential concern is that NV as an example misguides the reader into one
> > > of the following:
> > > 
> > > - It's only for formats which interleave cb/cr into one plane (whereas
> > > YUV420 also gets scaled)
> > > - NV24 in the example being though of including the NV24M variant (whereas
> > > latter won't be affected)
> > M variants are not supported by the single-planar APIs
> > https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html
> > 
> > Some planar formats allow planes to be placed in independent memory
> > locations. They are identified by an ‘M’ suffix in their name (such as
> > in V4L2_PIX_FMT_NV12M). Those formats are intended to be used only in
> > drivers and applications that support the multi-planar API,
> > 
> > And here we're dealing with single-planar API only if I'm not mistaken
> 
> Oh, sorry. Assumed that the added description of both functions would be
> similar/identical, which isn't the case.
> 
> Given this, I'm fine with the wording and agree to just change the example
> to smth. else than NV12.

So if I'm not wrong we can then use YUV420 instead of NV12.
NV12 is an unlucky example where the alignment scaling factor is 1,
whereas YUV420 has a scaling factor of 2:

# NV12
(Y - luma)      bpp[0] = 1
(CbCr - chroma) bpp[1] = 2
                hdiv = 2

# YUV420
(Y - luma)    bpp[0] = 1
(Cb - chroma) bpp[1] = 1
              hdiv = 2

For plane = 0 (single memory-plane formats):

factor = DIV_ROUND_UP(hdiv * bpp[0], bpp[1])

	NV12:    factor = DIV_ROUND_UP(2 * 1, 2) = 1
	YUV420:  factor = DIV_ROUND_UP(2 * 1, 1) = 2

Then I will leave the wording as is and changing only parenthesis part
like Sven suggested:

	(e.g NV12) --> (e.g. YUV420)

Please correct me if I'm wrong.
If for you is ok I will fix this in v4.

Kind Regards,
Tommaso


> 
> Sincerely
>     Sven
> 
> 

Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Sven Püschel]

Hi Tommaso,

On 7/10/26 3:45 PM, Tommaso Merciai wrote:
> Hi Jacopo, Sven,
> Thanks for your comments:
>
> On Fri, Jul 10, 2026 at 02:26:23PM +0200, Sven Püschel wrote:
>> Hi Jacopo,
>>
>> On 7/10/26 2:15 PM, Jacopo Mondi wrote:
>>> Hi Sven
>>>
>>> On Fri, Jul 10, 2026 at 01:54:06PM +0200, Sven Püschel wrote:
>>>> Hi Jacopo,
>>>>
>>>> On 7/10/26 11:38 AM, Jacopo Mondi wrote:
>>>>>> This is due to the fact, that while we have a hdiv of 2 we also interleave
>>>>>> the cb and cr parts in a single plane, which results in the stride being the
>>>>>> same number of bytes as for the y plane (and vdiv isn't relevant for the
>>>>>> stride).
>>>>>>
>>>>>> Therefore the stride scaling also respects the bits per plane (bpp) value to
>>>>>> determine the scaling.
>>>>>>
>>>>>> @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
>>>>> I guess the usage of NV12 was as example of a "formats that store
>>>>> multiple component planes in a single memory"
>>>>>
>>>>> NV24/42 works the same, but being 444 it needs the chroma plane stride to
>>>>> be a multiple of the fist plane stride and might prove as a better
>>>>> example ?
>>>>>
>>>> My potential concern is that NV as an example misguides the reader into one
>>>> of the following:
>>>>
>>>> - It's only for formats which interleave cb/cr into one plane (whereas
>>>> YUV420 also gets scaled)
>>>> - NV24 in the example being though of including the NV24M variant (whereas
>>>> latter won't be affected)
>>> M variants are not supported by the single-planar APIs
>>> https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html
>>>
>>> Some planar formats allow planes to be placed in independent memory
>>> locations. They are identified by an ‘M’ suffix in their name (such as
>>> in V4L2_PIX_FMT_NV12M). Those formats are intended to be used only in
>>> drivers and applications that support the multi-planar API,
>>>
>>> And here we're dealing with single-planar API only if I'm not mistaken
>> Oh, sorry. Assumed that the added description of both functions would be
>> similar/identical, which isn't the case.
>>
>> Given this, I'm fine with the wording and agree to just change the example
>> to smth. else than NV12.
> So if I'm not wrong we can then use YUV420 instead of NV12.
> NV12 is an unlucky example where the alignment scaling factor is 1,
> whereas YUV420 has a scaling factor of 2:
>
> # NV12
> (Y - luma)      bpp[0] = 1
> (CbCr - chroma) bpp[1] = 2
>                  hdiv = 2
>
> # YUV420
> (Y - luma)    bpp[0] = 1
> (Cb - chroma) bpp[1] = 1
>                hdiv = 2
>
> For plane = 0 (single memory-plane formats):
>
> factor = DIV_ROUND_UP(hdiv * bpp[0], bpp[1])
>
> 	NV12:    factor = DIV_ROUND_UP(2 * 1, 2) = 1
> 	YUV420:  factor = DIV_ROUND_UP(2 * 1, 1) = 2
>
> Then I will leave the wording as is and changing only parenthesis part
> like Sven suggested:
>
> 	(e.g NV12) --> (e.g. YUV420)
>
> Please correct me if I'm wrong.
> If for you is ok I will fix this in v4.

lgtm

Sincerely
     Sven