[PATCH v5 1/5] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

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

Round bytesperline up to RZG2L_CRU_STRIDE_ALIGN and recompute
sizeimage when info->has_stride is set. RZ/G2L has no AMnIS register
and keeps the values from v4l2_fill_pixfmt() unchanged.

Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
Cc: stable@vger.kernel.org
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
v4->v5:
 - New patch.

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

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 5185a547461d..91eda5034248 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -851,6 +851,11 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
 
 	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;
+	}
+
 	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 v5 1/5] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Jacopo Mondi]

Hi Tommaso

On Wed, Jul 29, 2026 at 12:55:46PM +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
> 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.
>
> Round bytesperline up to RZG2L_CRU_STRIDE_ALIGN and recompute
> sizeimage when info->has_stride is set. RZ/G2L has no AMnIS register
> and keeps the values from v4l2_fill_pixfmt() unchanged.
>
> Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> v4->v5:
>  - New patch.
>
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 5185a547461d..91eda5034248 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -851,6 +851,11 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
>
>  	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;
> +	}
> +

Ack! I think this does what is expected for the single planar formats
the CRU supports!

I think after this cycle you could send a patch to replace the above
back-portable change with the usage of v4l2_fill_pixfmt_aligned().

When you do so, could you consider, replacing "has_stride" with a per
struct rzg2l_cru_info .stride field ?

I think it would look nicer as you could here unconditionally align
(assuming the new .stride will be set to 1 for G2L which currently has
.has_stride = 0).

Also, RZG2L_CRU_STRIDE_ALIGN is a bit of a mis-nomer, as only G3E and
V2H has a AMnIS register and alignments constraints as far as I see.

The only other user of 'has_stride' I see is:

 rzg2l_cru_initialize_axi()

	if (info->has_stride) {
		u32 stride = cru->format.bytesperline;
		u32 amnis;

		stride /= RZG2L_CRU_STRIDE_ALIGN;
		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
	}

the G2L has no register AMnIS, so we can't write it unconditionally
but rather replace the check with:

        if (info->stride > 1)

As a bonus, if any other variant will get a different stride, it will
be automatically supported.

What do you think ?

In the meantime for this patch
Reviewed-by: Jacopo Mondi <jacopo.mondi@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 v5 1/5] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Tommaso Merciai]

Hi Jacopo,
Thanks for your review.

On Thu, Jul 30, 2026 at 09:50:35PM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Wed, Jul 29, 2026 at 12:55:46PM +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
> > 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.
> >
> > Round bytesperline up to RZG2L_CRU_STRIDE_ALIGN and recompute
> > sizeimage when info->has_stride is set. RZ/G2L has no AMnIS register
> > and keeps the values from v4l2_fill_pixfmt() unchanged.
> >
> > Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v4->v5:
> >  - New patch.
> >
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 5185a547461d..91eda5034248 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -851,6 +851,11 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> >
> >  	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;
> > +	}
> > +
> 
> Ack! I think this does what is expected for the single planar formats
> the CRU supports!
> 
> I think after this cycle you could send a patch to replace the above
> back-portable change with the usage of v4l2_fill_pixfmt_aligned().

Ack but this is already done on PATCH 5/5 of this series ([1])
Please correct me if I'm wrong.

> 
> When you do so, could you consider, replacing "has_stride" with a per
> struct rzg2l_cru_info .stride field ?
> 
> I think it would look nicer as you could here unconditionally align
> (assuming the new .stride will be set to 1 for G2L which currently has
> .has_stride = 0).
> 
> Also, RZG2L_CRU_STRIDE_ALIGN is a bit of a mis-nomer, as only G3E and
> V2H has a AMnIS register and alignments constraints as far as I see.
> 
> The only other user of 'has_stride' I see is:
> 
>  rzg2l_cru_initialize_axi()
> 
> 	if (info->has_stride) {
> 		u32 stride = cru->format.bytesperline;
> 		u32 amnis;
> 
> 		stride /= RZG2L_CRU_STRIDE_ALIGN;
> 		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> 		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> 	}
> 
> the G2L has no register AMnIS, so we can't write it unconditionally
> but rather replace the check with:
> 
>         if (info->stride > 1)
> 
> As a bonus, if any other variant will get a different stride, it will
> be automatically supported.
> 
> What do you think ?

Fully agree!
I will post patches for this after this cycle, thanks.

> 
> In the meantime for this patch
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20260729105603.1160966-6-tommaso.merciai.xr@bp.renesas.com/

Kind Regards,
Tommaso

> 
> 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 v5 1/5] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement

[Jacopo Mondi]

Hi Tommaso,

On Fri, Jul 31, 2026 at 04:55:57PM +0200, Tommaso Merciai wrote:
> Hi Jacopo,
> Thanks for your review.
>
> On Thu, Jul 30, 2026 at 09:50:35PM +0200, Jacopo Mondi wrote:
> > Hi Tommaso
> >
> > On Wed, Jul 29, 2026 at 12:55:46PM +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
> > > 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.
> > >
> > > Round bytesperline up to RZG2L_CRU_STRIDE_ALIGN and recompute
> > > sizeimage when info->has_stride is set. RZ/G2L has no AMnIS register
> > > and keeps the values from v4l2_fill_pixfmt() unchanged.
> > >
> > > Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > ---
> > > v4->v5:
> > >  - New patch.
> > >
> > >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > index 5185a547461d..91eda5034248 100644
> > > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > > @@ -851,6 +851,11 @@ static void rzg2l_cru_format_align(struct rzg2l_cru_dev *cru,
> > >
> > >  	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;
> > > +	}
> > > +
> >
> > Ack! I think this does what is expected for the single planar formats
> > the CRU supports!
> >
> > I think after this cycle you could send a patch to replace the above
> > back-portable change with the usage of v4l2_fill_pixfmt_aligned().
>
> Ack but this is already done on PATCH 5/5 of this series ([1])
> Please correct me if I'm wrong.
>

Ah sorry, I completely missed that.

I'll review that one.

> >
> > When you do so, could you consider, replacing "has_stride" with a per
> > struct rzg2l_cru_info .stride field ?
> >
> > I think it would look nicer as you could here unconditionally align
> > (assuming the new .stride will be set to 1 for G2L which currently has
> > .has_stride = 0).
> >
> > Also, RZG2L_CRU_STRIDE_ALIGN is a bit of a mis-nomer, as only G3E and
> > V2H has a AMnIS register and alignments constraints as far as I see.
> >
> > The only other user of 'has_stride' I see is:
> >
> >  rzg2l_cru_initialize_axi()
> >
> > 	if (info->has_stride) {
> > 		u32 stride = cru->format.bytesperline;
> > 		u32 amnis;
> >
> > 		stride /= RZG2L_CRU_STRIDE_ALIGN;
> > 		amnis = rzg2l_cru_read(cru, AMnIS) & ~AMnIS_IS_MASK;
> > 		rzg2l_cru_write(cru, AMnIS, amnis | AMnIS_IS(stride));
> > 	}
> >
> > the G2L has no register AMnIS, so we can't write it unconditionally
> > but rather replace the check with:
> >
> >         if (info->stride > 1)
> >
> > As a bonus, if any other variant will get a different stride, it will
> > be automatically supported.
> >
> > What do you think ?
>
> Fully agree!
> I will post patches for this after this cycle, thanks.

Thank you!

>
> >
> > In the meantime for this patch
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20260729105603.1160966-6-tommaso.merciai.xr@bp.renesas.com/
>
> Kind Regards,
> Tommaso
>
> >
> > 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
> > >
> > >