Re: [PATCH v2 1/3] rcar-vin: align width before stream start

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

Thanks for your feedback.

On 2018-10-04 23:11:50 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> 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.

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

-- 
Regards,
Niklas Söderlund



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux