> -----Original Message----- > From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > Sent: Wednesday, April 4, 2018 1:40 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 04-04-18 om 10:06 schreef Srinivas, Vidya: > > > >> -----Original Message----- > >> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > >> Sent: Wednesday, April 4, 2018 1:33 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; > >>> + } > >> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or > >> disable_plane calls. It can take an arbitrary amount of time, > >> resulting in an atomic update failure. > > Sure, thank you. These are anyway duplicate checks. I will remove them. > > The main one is done in intel_framebuffer_init as suggested by you. > They're not duplicate, they're done for related reasons: > 1. It doesn't make sense to allow creation of a framebuffer with an invalid > width/height if the full width/height cannot be used. > 2. The checks in skl_update_scaler are the ones that will reject even showing > a subset with invalid width, which can cause the fifo underruns. Sorry, yeah that’s correct. I will remove it only from skl_update_plane. Regarding the crtc_w and crtc_h, I see that when its not even, fifo underruns are seen. Kindly suggest on that. Thank you. > >>> + > >>> /* 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)); > >>> + 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
