Re: [PATCH 1/3] rcar-vin: align format width with hardware limits

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

Hi Hans,

Thanks for your feedback.

On 2018-09-14 10:47:41 +0200, Hans Verkuil wrote:
> On 09/14/2018 04:13 AM, Niklas Söderlund wrote:
> > The Gen3 datasheets lists specific alignment restrictions compared to
> > Gen2. This was overlooked when adding Gen3 support as no problematic
> > configuration was encountered. However when adding support for Gen3 Up
> > Down Scaler (UDS) strange issues could be observed for odd widths
> > without taking this limit into consideration.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index dc77682b47857c97..2fc2a05eaeacb134 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -673,6 +673,21 @@ static void rvin_mc_try_format(struct rvin_dev *vin,
> >  	pix->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, pix->colorspace,
> >  							  pix->ycbcr_enc);
> >  
> > +	switch (vin->format.pixelformat) {
> > +	case V4L2_PIX_FMT_NV16:
> > +		pix->width = ALIGN(pix->width, 0x80);
> > +		break;
> > +	case V4L2_PIX_FMT_YUYV:
> > +	case V4L2_PIX_FMT_UYVY:
> > +	case V4L2_PIX_FMT_RGB565:
> > +	case V4L2_PIX_FMT_XRGB555:
> > +		pix->width = ALIGN(pix->width, 0x40);
> > +		break;
> > +	default:
> > +		pix->width = ALIGN(pix->width, 0x20);
> > +		break;
> > +	}
> > +
> >  	rvin_format_align(vin, pix);
> >  }
> >  
> > 
> 
> This looks weird. So for NV16 the width must be a multiple of 128,
> do I read that correctly?
> 
> Are you sure you don't mean bytesperline?
> 
> And if it really is the width, doesn't this place very big constraints
> on the vin driver? If you don't want/need the UDS, then I can imagine
> that you don't want these alignments.

Thanks for brining this up, after more carefully reading the datasheet 
(which is a bit ambiguous in this section) I figured out that this is to 
strict. Will rewrite this patch for v2.

-- 
Regards,
Niklas Söderlund