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