Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

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

 




> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx]
> Sent: Wednesday, April 4, 2018 1:28 PM
> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Lankhorst, Maarten <maarten.lankhorst@xxxxxxxxx>
> Subject: Re:  [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> > As per display WA 1106, to avoid corruption issues
> > NV12 plane height needs to be multiplier of 4 We expect the src
> > dimensions to be multiplier of 4 We fail the case where src width or
> > height is not multiple of 4 for NV12.
> > We also set the scaler destination height and width to be multiple of
> > 4. Without this, pipe fifo underruns were seen on APL and KBL. We also
> > skip src trunction/adjustments for NV12 case and handle the sizes
> > directly in skl_update_plane
> >
> > Credits-to: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > 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 d5dad44..b2292dd 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >  	unsigned long irqflags;
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12 &&
> > +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> > +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> > +		return;
> > +	}
> > +
> You can't do this check in skl_update_plane, it's way too late. It should be
> rejected in the plane check with -EINVAL, or perhaps in skl_update_scaler.
Have done it right from intel_framebuffer_init onwards, have done it in skl_update_scaler also
In fact it will get rejected in framebuffer init and all these are duplicate checks, can remove them.
> >  	/* Sizes are 0 based */
> >  	src_w--;
> >  	src_h--;
> > @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> scaler->mode);
> >  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> << 16) | crtc_y);
> > -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> > -
> > +		if (fb->format->format == DRM_FORMAT_NV12)
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      (MULT4(crtc_w) << 16) |
> MULT4(crtc_h));
> See the comment above, sizes are 0 based. This will add 1 to the size, so the
> size is always 1 more than requested.
> I don't think it would pass plane CRC tests..

When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920 back.
If we don’t do this, I see fifo underruns. The destination width and height
If not mult of 4, I am seeing underruns.

> > +		else
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
> >  	} else {
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> | crtc_x);
> > @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  		return -EINVAL;
> >  	}
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12)
> > +		goto check_plane_surface;
> > +
> >  	/* setup can_scale, min_scale, max_scale */
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		if (state->base.fb)
> > @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  	dst->y1 = crtc_y;
> >  	dst->y2 = crtc_y + crtc_h;
> >
> > +check_plane_surface:
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		ret = skl_check_plane_surface(crtc_state, state);
> >  		if (ret)
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux