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

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

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
>