Op 04-04-18 om 10:29 schreef Srinivas, Vidya: > >> -----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. > If we don’t keep dest width and height as mult of 4, fifo underruns are seen. > Shall we reject those also? Then it will be safe. As per my observation, > We have only two options. Either change it to mult of 4 or reject it :( > Please suggest. Reject non multiples of 4. Userspace can adapt. :) ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
