Re: [PATCH 1/4] media: rcar-isp: ispcore: Honour requested bytesperline on RAW input

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

On 2026-10-20 17:25:52 +0200, Barnabás Pőcze wrote:
> 2026. 10. 20. 15:44 keltezéssel, Niklas Söderlund írta:
> > Hi Barnabás,
> > 
> > Thanks for your review.
> > 
> > On 2026-10-20 11:36:04 +0200, Barnabás Pőcze wrote:
> > > Hi
> > > 
> > > 2026. 10. 19. 21:35 keltezéssel, Niklas Söderlund írta:
> > > > From: Yuya Hamamachi <yuya.hamamachi.sx@renesas.com>
> > > > 
> > > > The RAW input video device always overwrote the bytesperline requested
> > > > by userspace with width * bpp. The input buffers are however shared with
> > > > the VIN, which may use a larger line stride (for example 3296 instead of
> > > > 3280 bytes for a 1640 pixel wide 10 bit image). The dmabuf exported by
> > > > the ISP was then smaller than the VIN buffer size, and queueing it to
> > > > the VIN failed with -EINVAL:
> > > > 
> > > >     __prepare_dmabuf: invalid dmabuf length 4042752 for plane 0,
> > > >     minimum length 4060672
> > > > 
> > > > Use the requested bytesperline if it is larger than the minimum.
> > > > 
> > > > Signed-off-by: Yuya Hamamachi <yuya.hamamachi.sx@renesas.com>
> > > > [Niklas: Use max(min(..)) instead of if statement]
> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > ---
> > > >    drivers/media/platform/renesas/rcar-isp/core-io.c | 11 ++++++++++-
> > > >    1 file changed, 10 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/media/platform/renesas/rcar-isp/core-io.c b/drivers/media/platform/renesas/rcar-isp/core-io.c
> > > > index b60b91f43d42..badde72352c1 100644
> > > > --- a/drivers/media/platform/renesas/rcar-isp/core-io.c
> > > > +++ b/drivers/media/platform/renesas/rcar-isp/core-io.c
> > > > @@ -5,6 +5,8 @@
> > > >     * Copyright (C) 2026 Ragnatech AB
> > > >     */
> > > > +#include <linux/sizes.h>
> > > > +
> > > >    #include <media/v4l2-ctrls.h>
> > > >    #include <media/v4l2-event.h>
> > > >    #include <media/v4l2-ioctl.h>
> > > > @@ -335,6 +337,7 @@ static void risp_io_input_try_format(struct rcar_isp_core_io *io,
> > > >    				     struct v4l2_pix_format_mplane *pix)
> > > >    {
> > > >    	unsigned int bpp = 0;
> > > > +	unsigned int bpl;
> > > >    	v4l_bound_align_image(&pix->width, RISP_MIN_WIDTH, RISP_MAX_WIDTH, RISP_SIZE_ALIGNMENT,
> > > >    			      &pix->height, RISP_MIN_HEIGHT, RISP_MAX_HEIGHT, RISP_SIZE_ALIGNMENT,
> > > > @@ -352,11 +355,17 @@ static void risp_io_input_try_format(struct rcar_isp_core_io *io,
> > > >    		bpp = risp_io_input_formats[0].bpp;
> > > >    	}
> > > > +	/*
> > > > +	 * Honour the line stride requested by userspace, the buffers are
> > > > +	 * shared with the VIN which may use a larger stride than the minimum.
> > > > +	 */
> > > > +	bpl = max(pix->width * bpp, min(pix->plane_fmt[0].bytesperline, SZ_64K));
> > > 
> > >    clamp(pix->plane_fmt[0].bytesperline, pix->width * bpp, SZ_64K)
> > 
> > Good idea.
> > 
> > > 
> > > ?
> > > 
> > > And does the device really accept any value between these two limits?
> > 
> > The stride register where this is programmed is 16 bit wide.
> 
> Right, and no additional constraints in the documentation?
> (I don't think I have access to that.) In any case, I think
> it should be (SZ_64K - 1) then.

You are right, will fix together with switching to clamp().

> 
> 
> > 
> > > 
> > > 
> > > > +
> > > >    	pix->field = V4L2_FIELD_NONE;
> > > >    	pix->colorspace = V4L2_COLORSPACE_RAW;
> > > >    	pix->num_planes = 1;
> > > > -	pix->plane_fmt[0].bytesperline = pix->width * bpp;
> > > > +	pix->plane_fmt[0].bytesperline = bpl;
> > > >    	pix->plane_fmt[0].sizeimage = pix->plane_fmt[0].bytesperline * pix->height;
> > > >    }
> > > > 
> > > 
> > 
> 

-- 
Kind Regards,
Niklas Söderlund