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

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 <[email protected]>
> > ---
> > 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 <[email protected]>
> 
> >  	.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
> >