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

Tommaso Merciai <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media,org.kernel.vger.stable
Message-ID <ak91BYMOgqJZoCSm@tom-desktop>
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 <[email protected]>
> > Cc: [email protected]
> > Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > Signed-off-by: Tommaso Merciai <[email protected]>
> > ---
> > 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/[email protected]/
[2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/[email protected]/

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 <[email protected]>
> 
> 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
> >
> >
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.