[Intel-gfx] [PATCH v18 18/18] drm/i915: Keep plane size mult of 4 for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Thursday, March 29, 2018 3:04 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Lankhorst, Maarten
> <maarten.lankhorst at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v18 18/18] drm/i915: Keep plane size mult of
> 4 for NV12
> 
> On Thu, Mar 29, 2018 at 09:29:06AM +0000, Srinivas, Vidya wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > Sent: Thursday, March 29, 2018 2:56 PM
> > > To: Srinivas, Vidya <vidya.srinivas at intel.com>
> > > Cc: intel-gfx at lists.freedesktop.org; Lankhorst, Maarten
> > > <maarten.lankhorst at intel.com>
> > > Subject: Re: [Intel-gfx] [PATCH v18 18/18] drm/i915: Keep plane size
> > > mult of
> > > 4 for NV12
> > >
> > > On Thu, Mar 29, 2018 at 01:36:02PM +0530, Vidya Srinivas wrote:
> > > > As per display WA 1106, to avoid corruption issues
> > > > NV12 plane height needs to be multiplier of 4 Hence we modify the
> > > > fb src and destination height and width to be multiples of 4.
> > > > Without this, pipe fifo underruns were seen on APL and KBL.
> > > >
> > > > Credits-to: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_drv.h    | 2 ++
> > > >  drivers/gpu/drm/i915/intel_sprite.c | 8 ++++++++
> > > >  2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 9c58da0..a1f718d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -159,6 +159,8 @@
> > > >  #define INTEL_I2C_BUS_DVO 1
> > > >  #define INTEL_I2C_BUS_SDVO 2
> > > >
> > > > +#define MULT4(x) ((x + 3) & ~0x03)
> > > > +
> > > >  /* these are outputs from the chip - integrated only
> > > >     external chips are via DVO or SDVO output */  enum
> > > > intel_output_type { diff --git
> > > > a/drivers/gpu/drm/i915/intel_sprite.c
> > > > b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index 538d938..9f466c6 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -261,6 +261,14 @@ skl_update_plane(struct intel_plane *plane,
> > > >  	crtc_w--;
> > > >  	crtc_h--;
> > > >
> > > > +	if (fb->format->format == DRM_FORMAT_NV12) {
> > > > +		src_w = MULT4(src_w);
> > > > +		src_h = MULT4(src_h);
> > > > +		crtc_w = MULT4(crtc_w);
> > > > +		crtc_h = MULT4(crtc_h);
> > >
> > > No macros like this pls. I want to know what it's doing. Also this is
> wrong.
> > > You can't increase src_w/h without potentially pushing the scale
> > > factor past the hardware limits.
> >
> >
> > Thank you. I am trying with not modifying the src w and h. Instead we
> > just Avoid the truncation (drm_rect_adjust_size and remaining things)
> > for NV12 in Intel_check_sprite_plane. I will keep only crtc_w and
> > crtc_h as a mult of 4 and See if no fifo underruns are seen.
> 
> The limitations are on the scaler source side, so I doubt that will do
> anything. So I don't even understand why we're playing around with the
> destination coordinates here.
> 
> Anywyay, I thought we already agreed to just return an error when things
> are misaligned?

The limitation for height is on scaler side. But for Gen9, GLK and GLV
There is a workaround 1106 which says:

Display corruption/color shift observed when using NV12 with 270 rotation or 90 rotation + horizontal flip.
WA: NV12 with 270 rotation or 90 rotation + horizontal flip requires the programmed plane height to be a multiple of 4.

Based on all the trials we have done, if we don't keep the dest width and height aligned to mult of 4, we see fifo underrun on APL
and KBL.

> 
> >
> > Regards
> > Vidya
> >
> > >
> > > > +		DRM_ERROR("%d %d %d %d\n", src_w, src_h, crtc_w,
> > > crtc_h);
> > >
> > > No user triggrable errors. Also this doesn't even explain what it's
> printing.
> > Sorry about this. This went in by mistake. Will remove it.
> > >
> > > > +	}
> > > > +
> > > >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > > >
> > > >  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC