> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
> Sent: Wednesday, April 4, 2018 2:18 PM
> To: Srinivas, Vidya <vidya.srini...@intel.com>; intel-
> g...@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankho...@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
>
> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 1:28 PM
> >> To: Srinivas, Vidya <vidya.srini...@intel.com>; intel-
> >> g...@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankho...@intel.com>
> >> Subject: Re: [Intel-gfx] [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.lankho...@linux.intel.com>
> >>> Signed-off-by: Vidya Srinivas <vidya.srini...@intel.com>
> >>> ---
> >>> 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.
> What you see as 1920 is 1921, which should be underrunning even more and
> is out of FB bounds.
Sorry, I gave a wrong example here. It doesn’t happen when we scale it to full
resolution.
It happened during other testing when the scaled dest width or height was not
multiple of 4.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
