Hi Niklas,
On Thursday, 4 October 2018 23:29:51 EEST Niklas Söderlund wrote:
> On 2018-10-04 23:11:50 +0300, Laurent Pinchart wrote:
> > On Thursday, 4 October 2018 23:04:00 EEST Niklas Söderlund wrote:
> >> Instead of aligning the image width to match the image stride at stream
> >> start time do so when configuring the format. This allows the format
> >> width to strictly match the image stride which is useful when enabling
> >> scaling on Gen3.
> >
> > But is this required ? Aren't there use cases where an image with a width
> > not aligned with the stride requirements should be captured ? As long as
> > the stride itself matches the hardware requirements, I don't see a reason
> > to disallow that.
>
> Yes there is a use-case for that. And the rcar-vin driver is starting to
> reaching a point where the whole format handling for buffers, source
> format, croping and scaling needs to be rewritten to enable more valid
> use-cases.
>
> This fix is however in my view required at this time with the current
> driver design. If we keep aligning the width at stream on and enable the
> UDS it becomes apparent that when the alignment is needed the values for
> the stride register conflicts which how the scaling coefficients are
> calculated and the captured frame is distorted.
>
> My hope is to add upstream to support for the UDS, support for
> sequential field captures and some more output pixel formats. And once
> the driver feature complete on Gen3 come back and simplify and if
> possible align the Gen2 and Gen3 format handling which in part adds to
> the somewhat messy current state.
I'd argue that it would be better to do it the other way around, but I won't
block this patch series. You might however get angry e-mails from VIN users
who all of a sudden realize that their use case stopped functioning.
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > > ---
> > >
> > > drivers/media/platform/rcar-vin/rcar-dma.c | 5 +----
> > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 9 +++++++++
> > > 2 files changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > b/drivers/media/platform/rcar-vin/rcar-dma.c index
> > > 92323310f7352147..e752bc86e40153b1 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > @@ -597,10 +597,7 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
> > >
> > > if (vin->info->model != RCAR_GEN3)
> > >
> > > rvin_crop_scale_comp_gen2(vin);
> > >
> > > - if (vin->format.pixelformat == V4L2_PIX_FMT_NV16)
> > > - rvin_write(vin, ALIGN(vin->format.width, 0x20), VNIS_REG);
> > > - else
> > > - rvin_write(vin, ALIGN(vin->format.width, 0x10), VNIS_REG);
> > > + rvin_write(vin, vin->format.width, VNIS_REG);
> > >
> > > }
> > >
> > > /*
> > >
> > > ------------------------------------------------------------------------
> > > ---
> > > -- diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> > > dc77682b47857c97..94bc559a0cb1e47a 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > > @@ -96,6 +96,15 @@ static void rvin_format_align(struct rvin_dev *vin,
> > > struct v4l2_pix_format *pix) pix->pixelformat == V4L2_PIX_FMT_XBGR32))
> > >
> > > pix->pixelformat = RVIN_DEFAULT_FORMAT;
> > >
> > > + switch (pix->pixelformat) {
> > > + case V4L2_PIX_FMT_NV16:
> > > + pix->width = ALIGN(pix->width, 0x20);
> > > + break;
> > > + default:
> > > + pix->width = ALIGN(pix->width, 0x10);
> > > + break;
> > > + }
> > > +
> > >
> > > switch (pix->field) {
> > > case V4L2_FIELD_TOP:
> > > case V4L2_FIELD_BOTTOM:
--
Regards,
Laurent Pinchart
