All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: tomm.merciai@gmail.com, linux-renesas-soc@vger.kernel.org,
	biju.das.jz@bp.renesas.com,
	"Lad Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"Jacopo Mondi" <jacopo.mondi@ideasonboard.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Hans Verkuil" <hverkuil+cisco@kernel.org>,
	"Nicolas Dufresne" <nicolas.dufresne@collabora.com>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Mehdi Djait" <mehdi.djait@linux.intel.com>,
	"Sven Püschel" <s.pueschel@pengutronix.de>,
	"Nas Chung" <nas.chung@chipsnmedia.com>,
	"Isaac Scott" <isaac.scott@ideasonboard.com>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"Daniel Scally" <dan.scally+renesas@ideasonboard.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 4/4] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement
Date: Tue, 28 Jul 2026 16:52:49 +0200	[thread overview]
Message-ID: <amjCQbuolBw9M_U3@tom-desktop> (raw)
In-Reply-To: <amipz7WkKUsjxot_@kekkonen.localdomain>

Hi Sakari,
Thanks for your review.

On Tue, Jul 28, 2026 at 04:08:31PM +0300, Sakari Ailus wrote:
> Hi Tommaso,
> 
> On Wed, Jul 01, 2026 at 06:50:01PM +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.
> > 
> > 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 <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: stable@vger.kernel.org
> > Fixes: ace92ccef0c9 ("media: platform: rzg2l-cru: Use v4l2_fill_pixfmt()")
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v1->v2:
> >  - Collected tag
> >  - Add missing Cc stable
> 
> The problem with this is that it requires v4l2_fill_pixfmt_aligned() that's
> not Cc'd to stable. Can this be fixed first without introducing the use of
> v4l2_fill_pixfmt_aligned() (cc'd to stable, i.e. in this patch), followed
> by another one switching to v4l2_fill_pixfmt_aligned()?

Ok.

I will send add to v5 series before [1] a patch that fixes this
without introducing the use of v4l2_fill_pixfmt_aligned() this I think
can be done using:

        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;
+       }
+

Then I rework this patch into:

media: rzg2l-cru: Use v4l2_fill_pixfmt_aligned() for stride alignment

That just use the newly added v4l2_fill_pixfmt_aligned() as you suggest.


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


Kind Regards,
Tommaso

> 
> >  - 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);
> >  
> > -	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);
> >  
> >  	dev_dbg(cru->dev, "Format %ux%u bpl: %u size: %u\n",
> >  		pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> 
> -- 
> Regards,
> 
> Sakari Ailus

      reply	other threads:[~2026-07-28 14:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 16:49 [PATCH v2 0/4] media: rzg2l-cru: Fix DMA stride alignment Tommaso Merciai
2026-07-01 16:49 ` [PATCH v2 1/4] media: v4l2-common: Convert v4l2_fill_pixfmt_mp() to static inline wrapper Tommaso Merciai
2026-07-01 16:49 ` [PATCH v2 2/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned() Tommaso Merciai
2026-07-03 12:14   ` Sven Püschel
2026-07-03 14:16     ` Tommaso Merciai
2026-07-01 16:50 ` [PATCH v2 3/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper Tommaso Merciai
2026-07-01 16:50 ` [PATCH v2 4/4] media: rzg2l-cru: Align bytesperline to hardware DMA stride requirement Tommaso Merciai
2026-07-28 13:08   ` Sakari Ailus
2026-07-28 14:52     ` Tommaso Merciai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amjCQbuolBw9M_U3@tom-desktop \
    --to=tommaso.merciai.xr@bp.renesas.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=dan.scally+renesas@ideasonboard.com \
    --cc=hverkuil+cisco@kernel.org \
    --cc=isaac.scott@ideasonboard.com \
    --cc=jacopo.mondi@ideasonboard.com \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mehdi.djait@linux.intel.com \
    --cc=nas.chung@chipsnmedia.com \
    --cc=nicolas.dufresne@collabora.com \
    --cc=paul@crapouillou.net \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=s.pueschel@pengutronix.de \
    --cc=sakari.ailus@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=tomm.merciai@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.