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. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
