> -----Original Message----- > From: Srinivas, Vidya > Sent: Tuesday, April 17, 2018 1:32 PM > To: 'Maarten Lankhorst' <maarten.lankhorst@xxxxxxxxxxxxxxx>; Saarinen, > Jani <jani.saarinen@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Kamath, Sunil <sunil.kamath@xxxxxxxxx> > Subject: RE: [PATCH v2 6/6] drm/i915: Do not do fb src adjustments for > NV12 > > > > > -----Original Message----- > > From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > > Sent: Tuesday, April 17, 2018 1:21 PM > > To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; Saarinen, Jani > > <jani.saarinen@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Kamath, Sunil <sunil.kamath@xxxxxxxxx> > > Subject: Re: [PATCH v2 6/6] drm/i915: Do not do fb src adjustments for > > NV12 > > > > Op 17-04-18 om 09:38 schreef Srinivas, Vidya: > > > > > >> -----Original Message----- > > >> From: Maarten Lankhorst [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > > >> Sent: Tuesday, April 17, 2018 12:55 PM > > >> To: Saarinen, Jani <jani.saarinen@xxxxxxxxx>; Srinivas, Vidya > > >> <vidya.srinivas@xxxxxxxxx>; intel-gfx-trybot@xxxxxxxxxxxxxxxxxxxxx; > > >> intel- gfx@xxxxxxxxxxxxxxxxxxxxx > > >> Cc: Kamath, Sunil <sunil.kamath@xxxxxxxxx> > > >> Subject: Re: [PATCH v2 6/6] drm/i915: Do not do fb src adjustments > > >> for > > >> NV12 > > >> > > >> Op 17-04-18 om 08:03 schreef Saarinen, Jani: > > >>> Lets move this to intel-gfx to get more eyes on it. > > >>> > > >>>> -----Original Message----- > > >>>> From: Srinivas, Vidya > > >>>> Sent: tiistai 17. huhtikuuta 2018 5.47 > > >>>> To: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>; > > >>>> intel-gfx- trybot@xxxxxxxxxxxxxxxxxxxxx > > >>>> Cc: Kamath, Sunil <sunil.kamath@xxxxxxxxx>; Saarinen, Jani > > >>>> <jani.saarinen@xxxxxxxxx> > > >>>> Subject: RE: [PATCH v2 6/6] drm/i915: Do not do fb src > > >>>> adjustments for NV12 > > >>>> > > >>>> > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: Maarten Lankhorst > > >>>>> [mailto:maarten.lankhorst@xxxxxxxxxxxxxxx] > > >>>>> Sent: Monday, April 16, 2018 8:00 PM > > >>>>> To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel-gfx- > > >>>>> trybot@xxxxxxxxxxxxxxxxxxxxx > > >>>>> Cc: Kamath, Sunil <sunil.kamath@xxxxxxxxx>; Saarinen, Jani > > >>>>> <jani.saarinen@xxxxxxxxx> > > >>>>> Subject: Re: [PATCH v2 6/6] drm/i915: Do not do fb src > > >>>>> adjustments for > > >>>>> NV12 > > >>>>> > > >>>>> Hmm, more thought about src adjustments... > > >>>>> > > >>>>> Op 13-04-18 om 07:22 schreef Vidya Srinivas: > > >>>>>> We skip src trunction/adjustments for > > >>>>>> NV12 case and handle the sizes directly. > > >>>>>> Without this, pipe fifo underruns are seen on APL/KBL. > > >>>>>> > > >>>>>> v2: For NV12, making the src coordinates multiplier of 4 > > >>>>>> > > >>>>>> Credits-to: Maarten Lankhorst > > >>>>>> <maarten.lankhorst@xxxxxxxxxxxxxxx> > > >>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx> > > >>>>>> --- > > >>>>>> drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++ > > >>>>>> drivers/gpu/drm/i915/intel_sprite.c | 15 +++++++++++++++ > > >>>>>> 2 files changed, 26 insertions(+) > > >>>>>> > > >>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c > > >>>>>> b/drivers/gpu/drm/i915/intel_display.c > > >>>>>> index bc83f10..f64708f 100644 > > >>>>>> --- a/drivers/gpu/drm/i915/intel_display.c > > >>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c > > >>>>>> @@ -12978,6 +12978,10 @@ intel_check_primary_plane(struct > > >>>>> intel_plane *plane, > > >>>>>> bool can_position = false; > > >>>>>> int ret; > > >>>>>> uint32_t pixel_format = 0; > > >>>>>> + struct drm_plane_state *plane_state = &state->base; > > >>>>>> + struct drm_rect *src = &plane_state->src; > > >>>>>> + > > >>>>>> + *src = drm_plane_state_src(plane_state); > > >>>>>> > > >>>>>> if (INTEL_GEN(dev_priv) >= 9) { > > >>>>>> /* use scaler when colorkey is not required */ @@ - > > 13001,6 > > >>>>>> +13005,13 @@ intel_check_primary_plane(struct intel_plane > > *plane, > > >>>>>> if (!state->base.fb) > > >>>>>> return 0; > > >>>>>> > > >>>>>> + if (pixel_format == DRM_FORMAT_NV12) { > > >>>>>> + src->x1 = (((src->x1 >> 16)/4)*4) << 16; > > >>>>>> + src->x2 = (((src->x2 >> 16)/4)*4) << 16; > > >>>>>> + src->y1 = (((src->y1 >> 16)/4)*4) << 16; > > >>>>>> + src->y2 = (((src->y2 >> 16)/4)*4) << 16; > > >>>>>> + } > > >>>>> Lets reject non multiple of 4 coordinates for plane_state's > > >>>>> src_x and src_y, and before adjustment also reject non multiple > > >>>>> of 4 src_w/src_h.fter that we can also reject clipping to the > > >>>>> right/bottom edge of the screen, if the pipe_src_w/h is not a > > >>>>> multiple of 4. That way an application trying to go beyond the > screen. > > >>>> kms_plane_scaling and some other tests during execution generate > > >>>> lots of non mult of 4 buffer width/height. All these tests will > > >>>> show fail in the IGT BAT. > > >>>> In some cases, even thought the width and height will be multiple > > >>>> of > > >>>> 4 before the Adjustment (say our 16x16 buffer), this after > > >>>> adjustment becomes a 15 in intel_check_sprite_plane. > > >>>> This again would cause underrun. If we reject non multiple of 4s > > >>>> in our skl_update_scaler also, then even simple tests with 16x16 > > >>>> like rotation will fail due to it getting adjusted. > > >> Correct. This is why we fix up to a multiple of 4 after > > >> adjustments, and reject user passed coordinates before adjustment.. > > >> plane_state->src_x/y/w/h is the property set by the user, while the > > >> plane_state->src rect is set and fixed up in the kernel. > > >> > > >> Rejecting clipping with positive coordinates if pipe_src w/h is not > > >> a multiple of 4 will fix a border case that shouldn't affect most > > >> modes, where the plane wouldn't extend to the crtc borders, which > > >> is > > what userspace wants there. > > >> > > > Thank you. Oh okay. Sorry. I thought we just reject and not do the > > > fixing > > (mult of 4 later). > > > Sure, I will change the patch to also reject the coordinates before > > > adjustment (if its not Mult of 4). > > > > > > > > This can be done in skl_check_plane_surface, > > plane_state->src_{x,y,w,h} contains the user requested values. > > Apologies for asking again. The adjustments are done in check_plane (both > primary and sprite) And towards the end of check_plane we call the > skl_check_plane_surface. > I got your point that we will reject the user requested values if they are not > multiple of 4. > But before we reach skl_check_plane_surface, they would’ve been > adjusted/clipped etc They wont remain user requested as is when they enter > skl_check_plane_surface. > Please correct me if I am wrong or I am missing in understanding something > here :( > > I can add the add conversion to multiple of 4 in skl_check_plane_surface but > rejection should be done before the clipping/adjustment Or else, I move the > entire clipping/adjustment code to skl_check_plane_surface? > Sorry, I got it. You said, we don’t do adjustments in case of NV12 in sprite and move The handling to skl_check_plane_surface. I will do the change and submit. Sorry. Thank you. > Regards > Vidya > > > > ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
