> -----Original Message----- > From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > Sent: Wednesday, April 4, 2018 2:02 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: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. :) :) okay. Thank you. I will update the patch. Regards Vidya > > ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
