[PATCH v6 0/5] media: rzg2l-cru: Fix DMA stride alignment

[PATCH v6 0/5] 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

v5->v6:
 - Rebased on top of media-committers/next
 - Add new patch to replace has_stride with stride_align.
 - Collected tag in PATCH 4/5

v4->v5:
 - Rebased on top of media-committers/next
 - Split the fix so it no longer depends on v4l2_fill_pixfmt_aligned()
   and moved it first in the series, so it can be backported to stable
   on its own (This the old v4 PATCH 4/4).

v3->v4:
 - Rebased on top of media-committers/next
 - PATCH 1/4: Collected tag.
 - PATCH 2/4: Collected Jacopo tag. Removed "." at the end of the function's
   brief description. Removed "component" from @pixfmt->sizeimage line
   Removed wrong tab.
   Fixed example (e.g NV12) -> (e.g. YUV420) into function description
 - PATCH 3/4: Collected Jacopo tag. Fixed into "and each plane's sizeimage is"
 - PATCH 4/4: Collected tag.

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 (5):
  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: Use v4l2_fill_pixfmt_aligned() for stride alignment
  media: rzg2l-cru: Replace has_stride with stride_align field

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

-- 
2.54.0

[PATCH v6 1/5] 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.

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v5->v6:
 - No changes.

v4->v5:
 - No changes.

v3->v4:
 - Collected tag.
 - Removed "." at the end of the function's brief description
 - Removed "component" from @pixfmt->sizeimage line
 - Removed wrong tab
 - Fixed example (e.g NV12) -> (e.g. YUV420) into function description

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 | 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 v6 2/5] 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.

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v5->v6:
 - No changes.

v4->v5:
 - No changes.

v3->v4:
 - Collected tag.
 - Removed "." at the end of the function's brief description
 - Removed "component" from @pixfmt->sizeimage line
 - Removed wrong tab
 - Fixed example (e.g NV12) -> (e.g. YUV420) into function description

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..4e5c5ffaf651 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. YUV420), 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 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 v6 3/5] 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.

Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v5->v6:
 - No changes.

v4->v5:
 - No changes.

v3->v4:
 - Collected tag.
 - Fixed function documentation.

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 4e5c5ffaf651..33f5713734cb 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 each
+ * plane's 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 v6 4/5] media: rzg2l-cru: Use v4l2_fill_pixfmt_aligned() for stride alignment

[Tommaso Merciai]

Replace the open-coded bytesperline/sizeimage rounding with the newly
added v4l2_fill_pixfmt_aligned().

No functional change intended.

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v5->v6:
 - Collected tag.

v4->v5:
 - Split the fix so it no longer depends on v4l2_fill_pixfmt_aligned()
   and moved it first in the series, so it can be backported to stable
   on its own (This the old v4 PATCH 4/4).

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

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 91eda5034248..27a35ef2a6df 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -849,12 +849,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, 2, 0);
 
-	v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
-
-	if (info->has_stride) {
-		pix->bytesperline = ALIGN(pix->bytesperline, RZG2L_CRU_STRIDE_ALIGN);
-		pix->sizeimage = pix->bytesperline * 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

[PATCH v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Tommaso Merciai]

RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
need, as only they have an AMnIS register.

Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
RZ/G2L, and update the code accordingly.

No functional change intended.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v5->v6:
 - New patch.

 drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
index 3c5fbd857371..a2b833e2bf9a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
@@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
 	.max_width = 4095,
 	.max_height = 4095,
 	.image_conv = ICnIPMC_C0,
-	.has_stride = true,
+	.stride_align = 128,
 	.regs = rzg3e_cru_regs,
 	.irq_handler = rzg3e_cru_irq,
 	.enable_interrupts = rzg3e_cru_enable_interrupts,
@@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
 	.max_width = 2800,
 	.max_height = 4095,
 	.image_conv = ICnMC,
+	.stride_align = 1,
 	.regs = rzg2l_cru_regs,
 	.irq_handler = rzg2l_cru_irq,
 	.enable_interrupts = rzg2l_cru_enable_interrupts,
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
index b426bc7898bf..2c192d370dcb 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
@@ -75,7 +75,7 @@ struct rzg2l_cru_info {
 	unsigned int max_height;
 	u16 image_conv;
 	const u16 *regs;
-	bool has_stride;
+	u8 stride_align;
 	irqreturn_t (*irq_handler)(int irq, void *data);
 	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
 	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 27a35ef2a6df..a7b6dce66570 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -32,7 +32,6 @@
 #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
 
 #define RZG2L_CRU_STRIDE_MAX		32640
-#define RZG2L_CRU_STRIDE_ALIGN		128
 
 struct rzg2l_cru_buffer {
 	struct vb2_v4l2_buffer vb;
@@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
 		rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
 	}
 
-	if (info->has_stride) {
+	if (info->stride_align > 1) {
 		u32 stride = cru->format.bytesperline;
 		u32 amnis;
 
-		stride /= RZG2L_CRU_STRIDE_ALIGN;
+		stride /= info->stride_align;
 		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
 		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
 	}
@@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 			      &pix->height, 240, info->max_height, 2, 0);
 
 	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
-				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
+				 info->stride_align);
 
 	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 v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Jacopo Mondi]

Hi Tommaso,
   thanks for the update

On Wed, Aug 19, 2026 at 12:28:09PM +0200, Tommaso Merciai wrote:
> RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
> need, as only they have an AMnIS register.
>
> Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
> RZ/G2L, and update the code accordingly.
>
> No functional change intended.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>  - New patch.
>
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> index 3c5fbd857371..a2b833e2bf9a 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> @@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
>  	.max_width = 4095,
>  	.max_height = 4095,
>  	.image_conv = ICnIPMC_C0,
> -	.has_stride = true,
> +	.stride_align = 128,

We could use a #define here

Apart from that, the patch looks good, thank you!
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  	.regs = rzg3e_cru_regs,
>  	.irq_handler = rzg3e_cru_irq,
>  	.enable_interrupts = rzg3e_cru_enable_interrupts,
> @@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
>  	.max_width = 2800,
>  	.max_height = 4095,
>  	.image_conv = ICnMC,
> +	.stride_align = 1,
>  	.regs = rzg2l_cru_regs,
>  	.irq_handler = rzg2l_cru_irq,
>  	.enable_interrupts = rzg2l_cru_enable_interrupts,
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> index b426bc7898bf..2c192d370dcb 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> @@ -75,7 +75,7 @@ struct rzg2l_cru_info {
>  	unsigned int max_height;
>  	u16 image_conv;
>  	const u16 *regs;
> -	bool has_stride;
> +	u8 stride_align;
>  	irqreturn_t (*irq_handler)(int irq, void *data);
>  	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
>  	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 27a35ef2a6df..a7b6dce66570 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -32,7 +32,6 @@
>  #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
>
>  #define RZG2L_CRU_STRIDE_MAX		32640
> -#define RZG2L_CRU_STRIDE_ALIGN		128
>
>  struct rzg2l_cru_buffer {
>  	struct vb2_v4l2_buffer vb;
> @@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
>  		rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
>  	}
>
> -	if (info->has_stride) {
> +	if (info->stride_align > 1) {
>  		u32 stride = cru->format.bytesperline;
>  		u32 amnis;
>
> -		stride /= RZG2L_CRU_STRIDE_ALIGN;
> +		stride /= info->stride_align;
>  		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
>  		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
>  	}
> @@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
>  			      &pix->height, 240, info->max_height, 2, 0);
>
>  	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> -				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> +				 info->stride_align);
>
>  	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 v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Tommaso Merciai]

Hi Jacopo,
Thanks for your review.

On Wed, Aug 19, 2026 at 04:24:14PM +0200, Jacopo Mondi wrote:
> Hi Tommaso,
>    thanks for the update
> 
> On Wed, Aug 19, 2026 at 12:28:09PM +0200, Tommaso Merciai wrote:
> > RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
> > need, as only they have an AMnIS register.
> >
> > Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
> > RZ/G2L, and update the code accordingly.
> >
> > No functional change intended.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v5->v6:
> >  - New patch.
> >
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > index 3c5fbd857371..a2b833e2bf9a 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > @@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
> >  	.max_width = 4095,
> >  	.max_height = 4095,
> >  	.image_conv = ICnIPMC_C0,
> > -	.has_stride = true,
> > +	.stride_align = 128,
> 
> We could use a #define here


Maybe we can use something like:

#define AMnIS_IS_UNIT		128

?

Not sure. Just to share, I see other drivers using raw values for
stride_alignment. e.g. rockchip/rga stores this info in it's
rga_hw struct and initializes it with plain numbers [1][2].


What do you think?

[1] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga-hw.c#L604
[2] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga3-hw.c#L502

Thanks, Tommaso


> 
> Apart from that, the patch looks good, thank you!
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> >  	.regs = rzg3e_cru_regs,
> >  	.irq_handler = rzg3e_cru_irq,
> >  	.enable_interrupts = rzg3e_cru_enable_interrupts,
> > @@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
> >  	.max_width = 2800,
> >  	.max_height = 4095,
> >  	.image_conv = ICnMC,
> > +	.stride_align = 1,
> >  	.regs = rzg2l_cru_regs,
> >  	.irq_handler = rzg2l_cru_irq,
> >  	.enable_interrupts = rzg2l_cru_enable_interrupts,
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > index b426bc7898bf..2c192d370dcb 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > @@ -75,7 +75,7 @@ struct rzg2l_cru_info {
> >  	unsigned int max_height;
> >  	u16 image_conv;
> >  	const u16 *regs;
> > -	bool has_stride;
> > +	u8 stride_align;
> >  	irqreturn_t (*irq_handler)(int irq, void *data);
> >  	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
> >  	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 27a35ef2a6df..a7b6dce66570 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -32,7 +32,6 @@
> >  #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
> >
> >  #define RZG2L_CRU_STRIDE_MAX		32640
> > -#define RZG2L_CRU_STRIDE_ALIGN		128
> >
> >  struct rzg2l_cru_buffer {
> >  	struct vb2_v4l2_buffer vb;
> > @@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> >  		rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
> >  	}
> >
> > -	if (info->has_stride) {
> > +	if (info->stride_align > 1) {
> >  		u32 stride = cru->format.bytesperline;
> >  		u32 amnis;
> >
> > -		stride /= RZG2L_CRU_STRIDE_ALIGN;
> > +		stride /= info->stride_align;
> >  		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> >  		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> >  	}
> > @@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> >  			      &pix->height, 240, info->max_height, 2, 0);
> >
> >  	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> > -				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> > +				 info->stride_align);
> >
> >  	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 v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Jacopo Mondi]

Hi Tommaso

On Wed, Aug 19, 2026 at 04:59:35PM +0200, Tommaso Merciai wrote:
> Hi Jacopo,
> Thanks for your review.
>
> On Wed, Aug 19, 2026 at 04:24:14PM +0200, Jacopo Mondi wrote:
> > Hi Tommaso,
> >    thanks for the update
> >
> > On Wed, Aug 19, 2026 at 12:28:09PM +0200, Tommaso Merciai wrote:
> > > RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
> > > need, as only they have an AMnIS register.
> > >
> > > Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
> > > RZ/G2L, and update the code accordingly.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > v5->v6:
> > >  - New patch.
> > >
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
> > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > index 3c5fbd857371..a2b833e2bf9a 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > @@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
> > >  	.max_width = 4095,
> > >  	.max_height = 4095,
> > >  	.image_conv = ICnIPMC_C0,
> > > -	.has_stride = true,
> > > +	.stride_align = 128,
> >
> > We could use a #define here
>
>
> Maybe we can use something like:
>
> #define AMnIS_IS_UNIT		128
>
> ?
>
> Not sure. Just to share, I see other drivers using raw values for
> stride_alignment. e.g. rockchip/rga stores this info in it's
> rga_hw struct and initializes it with plain numbers [1][2].
>
>
> What do you think?

I think it's fine, it was just a suggestion ;)

>
> [1] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga-hw.c#L604
> [2] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga3-hw.c#L502
>
> Thanks, Tommaso
>
>
> >
> > Apart from that, the patch looks good, thank you!
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > >  	.regs = rzg3e_cru_regs,
> > >  	.irq_handler = rzg3e_cru_irq,
> > >  	.enable_interrupts = rzg3e_cru_enable_interrupts,
> > > @@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
> > >  	.max_width = 2800,
> > >  	.max_height = 4095,
> > >  	.image_conv = ICnMC,
> > > +	.stride_align = 1,
> > >  	.regs = rzg2l_cru_regs,
> > >  	.irq_handler = rzg2l_cru_irq,
> > >  	.enable_interrupts = rzg2l_cru_enable_interrupts,
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > index b426bc7898bf..2c192d370dcb 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > @@ -75,7 +75,7 @@ struct rzg2l_cru_info {
> > >  	unsigned int max_height;
> > >  	u16 image_conv;
> > >  	const u16 *regs;
> > > -	bool has_stride;
> > > +	u8 stride_align;
> > >  	irqreturn_t (*irq_handler)(int irq, void *data);
> > >  	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
> > >  	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > index 27a35ef2a6df..a7b6dce66570 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > @@ -32,7 +32,6 @@
> > >  #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
> > >
> > >  #define RZG2L_CRU_STRIDE_MAX		32640
> > > -#define RZG2L_CRU_STRIDE_ALIGN		128
> > >
> > >  struct rzg2l_cru_buffer {
> > >  	struct vb2_v4l2_buffer vb;
> > > @@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> > >  		rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
> > >  	}
> > >
> > > -	if (info->has_stride) {
> > > +	if (info->stride_align > 1) {
> > >  		u32 stride = cru->format.bytesperline;
> > >  		u32 amnis;
> > >
> > > -		stride /= RZG2L_CRU_STRIDE_ALIGN;
> > > +		stride /= info->stride_align;
> > >  		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> > >  		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> > >  	}
> > > @@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> > >  			      &pix->height, 240, info->max_height, 2, 0);
> > >
> > >  	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> > > -				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> > > +				 info->stride_align);
> > >
> > >  	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 v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Tommaso Merciai]

On Wed, Aug 19, 2026 at 05:20:12PM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Wed, Aug 19, 2026 at 04:59:35PM +0200, Tommaso Merciai wrote:
> > Hi Jacopo,
> > Thanks for your review.
> >
> > On Wed, Aug 19, 2026 at 04:24:14PM +0200, Jacopo Mondi wrote:
> > > Hi Tommaso,
> > >    thanks for the update
> > >
> > > On Wed, Aug 19, 2026 at 12:28:09PM +0200, Tommaso Merciai wrote:
> > > > RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
> > > > need, as only they have an AMnIS register.
> > > >
> > > > Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
> > > > RZ/G2L, and update the code accordingly.
> > > >
> > > > No functional change intended.
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > > ---
> > > > v5->v6:
> > > >  - New patch.
> > > >
> > > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
> > > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
> > > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
> > > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > index 3c5fbd857371..a2b833e2bf9a 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > > > @@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
> > > >  	.max_width = 4095,
> > > >  	.max_height = 4095,
> > > >  	.image_conv = ICnIPMC_C0,
> > > > -	.has_stride = true,
> > > > +	.stride_align = 128,
> > >
> > > We could use a #define here
> >
> >
> > Maybe we can use something like:
> >
> > #define AMnIS_IS_UNIT		128
> >
> > ?
> >
> > Not sure. Just to share, I see other drivers using raw values for
> > stride_alignment. e.g. rockchip/rga stores this info in it's
> > rga_hw struct and initializes it with plain numbers [1][2].
> >
> >
> > What do you think?
> 
> I think it's fine, it was just a suggestion ;)

Thanks! :)
I'll keep the raw value then.

Kind regards,
Tommaso

> 
> >
> > [1] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga-hw.c#L604
> > [2] https://elixir.bootlin.com/linux/v7.2/source/drivers/media/platform/rockchip/rga/rga3-hw.c#L502
> >
> > Thanks, Tommaso
> >
> >
> > >
> > > Apart from that, the patch looks good, thank you!
> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > >
> > > >  	.regs = rzg3e_cru_regs,
> > > >  	.irq_handler = rzg3e_cru_irq,
> > > >  	.enable_interrupts = rzg3e_cru_enable_interrupts,
> > > > @@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
> > > >  	.max_width = 2800,
> > > >  	.max_height = 4095,
> > > >  	.image_conv = ICnMC,
> > > > +	.stride_align = 1,
> > > >  	.regs = rzg2l_cru_regs,
> > > >  	.irq_handler = rzg2l_cru_irq,
> > > >  	.enable_interrupts = rzg2l_cru_enable_interrupts,
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > index b426bc7898bf..2c192d370dcb 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > > > @@ -75,7 +75,7 @@ struct rzg2l_cru_info {
> > > >  	unsigned int max_height;
> > > >  	u16 image_conv;
> > > >  	const u16 *regs;
> > > > -	bool has_stride;
> > > > +	u8 stride_align;
> > > >  	irqreturn_t (*irq_handler)(int irq, void *data);
> > > >  	void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
> > > >  	void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> > > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > index 27a35ef2a6df..a7b6dce66570 100644
> > > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > > @@ -32,7 +32,6 @@
> > > >  #define RZG2L_CRU_DEFAULT_COLORSPACE	V4L2_COLORSPACE_SRGB
> > > >
> > > >  #define RZG2L_CRU_STRIDE_MAX		32640
> > > > -#define RZG2L_CRU_STRIDE_ALIGN		128
> > > >
> > > >  struct rzg2l_cru_buffer {
> > > >  	struct vb2_v4l2_buffer vb;
> > > > @@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> > > >  		rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
> > > >  	}
> > > >
> > > > -	if (info->has_stride) {
> > > > +	if (info->stride_align > 1) {
> > > >  		u32 stride = cru->format.bytesperline;
> > > >  		u32 amnis;
> > > >
> > > > -		stride /= RZG2L_CRU_STRIDE_ALIGN;
> > > > +		stride /= info->stride_align;
> > > >  		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> > > >  		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> > > >  	}
> > > > @@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> > > >  			      &pix->height, 240, info->max_height, 2, 0);
> > > >
> > > >  	v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> > > > -				 info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> > > > +				 info->stride_align);
> > > >
> > > >  	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 v6 1/5] media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline wrapper

[Lad, Prabhakar]

On Wed, Aug 19, 2026 at 11:31 AM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>  - No changes.
>
> v4->v5:
>  - No changes.
>
> v3->v4:
>  - Collected tag.
>  - Removed "." at the end of the function's brief description
>  - Removed "component" from @pixfmt->sizeimage line
>  - Removed wrong tab
>  - Fixed example (e.g NV12) -> (e.g. YUV420) into function description
>
> 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 | 8 --------
>  include/media/v4l2-common.h           | 9 +++++++--
>  2 files changed, 7 insertions(+), 10 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

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

[Lad, Prabhakar]

On Wed, Aug 19, 2026 at 11:29 AM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>  - No changes.
>
> v4->v5:
>  - No changes.
>
> v3->v4:
>  - Collected tag.
>  - Removed "." at the end of the function's brief description
>  - Removed "component" from @pixfmt->sizeimage line
>  - Removed wrong tab
>  - Fixed example (e.g NV12) -> (e.g. YUV420) into function description
>
> 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(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> 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..4e5c5ffaf651 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. YUV420), 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 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
>
>

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

[Lad, Prabhakar]

On Wed, Aug 19, 2026 at 11:32 AM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>  - No changes.
>
> v4->v5:
>  - No changes.
>
> v3->v4:
>  - Collected tag.
>  - Fixed function documentation.
>
> 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(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 4e5c5ffaf651..33f5713734cb 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 each
> + * plane's 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
>
>

Re: [PATCH v6 4/5] media: rzg2l-cru: Use v4l2_fill_pixfmt_aligned() for stride alignment

[Lad, Prabhakar]

On Wed, Aug 19, 2026 at 11:32 AM Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
>
> Replace the open-coded bytesperline/sizeimage rounding with the newly
> added v4l2_fill_pixfmt_aligned().
>
> No functional change intended.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>  - Collected tag.
>
> v4->v5:
>  - Split the fix so it no longer depends on v4l2_fill_pixfmt_aligned()
>    and moved it first in the series, so it can be backported to stable
>    on its own (This the old v4 PATCH 4/4).
>
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 91eda5034248..27a35ef2a6df 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -849,12 +849,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, 2, 0);
>
> -       v4l2_fill_pixfmt(pix, pix->pixelformat, pix->width, pix->height);
> -
> -       if (info->has_stride) {
> -               pix->bytesperline = ALIGN(pix->bytesperline, RZG2L_CRU_STRIDE_ALIGN);
> -               pix->sizeimage = pix->bytesperline * 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 v6 5/5] media: rzg2l-cru: Replace has_stride with stride_align field

[Lad, Prabhakar]

On Wed, Aug 19, 2026 at 3:24 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Tommaso,
>    thanks for the update
>
> On Wed, Aug 19, 2026 at 12:28:09PM +0200, Tommaso Merciai wrote:
> > RZG2L_CRU_STRIDE_ALIGN hardcodes an alignment only RZ/G3E and RZ/V2H
> > need, as only they have an AMnIS register.
> >
> > Store the alignment into rzg2l_cru_info instead: 128 on RZ/G3E, 1 on
> > RZ/G2L, and update the code accordingly.
> >
> > No functional change intended.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v5->v6:
> >  - New patch.
> >
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c  | 3 ++-
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h   | 2 +-
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 7 +++----
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > index 3c5fbd857371..a2b833e2bf9a 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > @@ -361,7 +361,7 @@ static const struct rzg2l_cru_info rzg3e_cru_info = {
> >       .max_width = 4095,
> >       .max_height = 4095,
> >       .image_conv = ICnIPMC_C0,
> > -     .has_stride = true,
> > +     .stride_align = 128,
>
> We could use a #define here
>
I think to be consistent with most of the members of rzg3e_cru_info
raw value should be OK?

Cheers,
Prabhakar

> Apart from that, the patch looks good, thank you!
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> >       .regs = rzg3e_cru_regs,
> >       .irq_handler = rzg3e_cru_irq,
> >       .enable_interrupts = rzg3e_cru_enable_interrupts,
> > @@ -406,6 +406,7 @@ static const struct rzg2l_cru_info rzg2l_cru_info = {
> >       .max_width = 2800,
> >       .max_height = 4095,
> >       .image_conv = ICnMC,
> > +     .stride_align = 1,
> >       .regs = rzg2l_cru_regs,
> >       .irq_handler = rzg2l_cru_irq,
> >       .enable_interrupts = rzg2l_cru_enable_interrupts,
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > index b426bc7898bf..2c192d370dcb 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > @@ -75,7 +75,7 @@ struct rzg2l_cru_info {
> >       unsigned int max_height;
> >       u16 image_conv;
> >       const u16 *regs;
> > -     bool has_stride;
> > +     u8 stride_align;
> >       irqreturn_t (*irq_handler)(int irq, void *data);
> >       void (*enable_interrupts)(struct rzg2l_cru_dev *cru);
> >       void (*disable_interrupts)(struct rzg2l_cru_dev *cru);
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 27a35ef2a6df..a7b6dce66570 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -32,7 +32,6 @@
> >  #define RZG2L_CRU_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB
> >
> >  #define RZG2L_CRU_STRIDE_MAX         32640
> > -#define RZG2L_CRU_STRIDE_ALIGN               128
> >
> >  struct rzg2l_cru_buffer {
> >       struct vb2_v4l2_buffer vb;
> > @@ -277,11 +276,11 @@ static void rzg2l_cru_initialize_axi(struct rzg2l_cru_dev *cru)
> >               rzg2l_cru_fill_hw_slot(cru, cru->num_buf - 1);
> >       }
> >
> > -     if (info->has_stride) {
> > +     if (info->stride_align > 1) {
> >               u32 stride = cru->format.bytesperline;
> >               u32 amnis;
> >
> > -             stride /= RZG2L_CRU_STRIDE_ALIGN;
> > +             stride /= info->stride_align;
> >               amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> >               rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> >       }
> > @@ -850,7 +849,7 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> >                             &pix->height, 240, info->max_height, 2, 0);
> >
> >       v4l2_fill_pixfmt_aligned(pix, pix->pixelformat, pix->width, pix->height,
> > -                              info->has_stride ? RZG2L_CRU_STRIDE_ALIGN : 1);
> > +                              info->stride_align);
> >
> >       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 v6 1/5] media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline wrapper

[Sven Püschel]

Hi Tommaso,

On 8/19/26 12:28, 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>   - No changes.
>
> v4->v5:
>   - No changes.
>
> v3->v4:
>   - Collected tag.
>   - Removed "." at the end of the function's brief description
>   - Removed "component" from @pixfmt->sizeimage line
>   - Removed wrong tab
>   - Fixed example (e.g NV12) -> (e.g. YUV420) into function description
>
> 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 | 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
>    *
Reviewed-by: Sven Püschel <s.pueschel@pengutronix.de>

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

[Sven Püschel]

Hi Tommaso,

On 8/19/26 12:28, 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>   - No changes.
>
> v4->v5:
>   - No changes.
>
> v3->v4:
>   - Collected tag.
>   - Fixed function documentation.
>
> 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 4e5c5ffaf651..33f5713734cb 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.

Given that the behavior matches the non mp function, it feels a bit 
redundant to explain the stride scaling again. But I don't mind keeping 
it to make the stride scaling behavior more obvious, so feel free to add

Reviewed-by: Sven Püschel <s.pueschel@pengutronix.de>

> + *
> + * For formats with multiple memory planes (mem_planes > 1), each plane's
> + * bytesperline is independently rounded up to @stride_alignment, and each
> + * plane's 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);

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

[Sven Püschel]

Hi Tommaso,

On 8/19/26 12:28, 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.
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v5->v6:
>   - No changes.
>
> v4->v5:
>   - No changes.
>
> v3->v4:
>   - Collected tag.
>   - Removed "." at the end of the function's brief description
>   - Removed "component" from @pixfmt->sizeimage line
>   - Removed wrong tab
>   - Fixed example (e.g NV12) -> (e.g. YUV420) into function description
>
> 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..4e5c5ffaf651 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. YUV420), 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 planes.

Out of interest: Why is this description different from the _mp 
description for a single memory plane (e.g. not equal with the exception 
of pixfmt->bytesperline vs plane_fmt[0].bytesperline)? Just had to look, 
if both describe the same behavior. Personally I also prefer the _mp 
description, as it's a bit shorter and better conveys the intention of 
only using a multiple of the stride_alignment to fulfill the luma stride 
derivation.

Also on my first reading I've thought of "rounded up to a multiple of 
@stride_alignment" just meaning "rounded up to @stride_alignment", as 
rounding up means setting the bytesperline to the next multiple of 
stride_alignment.

But given that both descriptions describe the same behavior (and I think 
I've also partially wrote the mp description, therefore being biased 
towards it), feel free to keep it as is and add my

Reviewed-by: Sven Püschel <s.pueschel@pengutronix.de>

> + *
> + * 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,