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

[Niklas Söderlund]

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@xxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxx>
> > [Niklas: Use max(min(..)) instead of if statement]
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> >   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.

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