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