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

[Barnabás Pőcze]

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@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.

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.


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