[igt-dev] [PATCH i-g-t] [i-g-t] tests/kms_plane_scaling: fb height to be multiplier of 4 for YUV 420 planar

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst at linux.intel.com]
> Sent: Wednesday, March 21, 2018 8:34 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>; Ville Syrjälä
> <ville.syrjala at linux.intel.com>
> Cc: igt-dev at lists.freedesktop.org; Kamath, Sunil <sunil.kamath at intel.com>;
> Syrjala, Ville <ville.syrjala at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] [i-g-t] tests/kms_plane_scaling: fb
> height to be multiplier of 4 for YUV 420 planar
> 
> Op 20-03-18 om 11:02 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> >> Sent: Monday, March 19, 2018 7:35 PM
> >> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> >> Cc: igt-dev at lists.freedesktop.org; Kamath, Sunil
> >> <sunil.kamath at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>
> >> Subject: Re: [igt-dev] [PATCH i-g-t] [i-g-t] tests/kms_plane_scaling:
> >> fb height to be multiplier of 4 for YUV 420 planar
> >>
> >> On Sat, Mar 17, 2018 at 09:15:48AM +0000, Srinivas, Vidya wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> >>>> Sent: Friday, March 16, 2018 8:01 PM
> >>>> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> >>>> Cc: igt-dev at lists.freedesktop.org; Kamath, Sunil
> >>>> <sunil.kamath at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>
> >>>> Subject: Re: [igt-dev] [PATCH i-g-t] [i-g-t]
> >>>> tests/kms_plane_scaling: fb height to be multiplier of 4 for YUV
> >>>> 420 planar
> >>>>
> >>>> On Fri, Mar 16, 2018 at 06:50:27PM +0530, Vidya Srinivas wrote:
> >>>>> For Gen9, GLK, CNL, GLV: Display WA 1106:
> >>>>> Display corruption/color shift observed when using NV12 with
> >>>>> 270 rotation or 90 rotation + horizontal flip.
> >>>>> WA: NV12 with 270 rotation or 90 rotation + horizontal flip
> >>>>> requires the programmed plane height to be a multiple of 4.
> >>>>> Patch changes the NV12 buffer to 20x20 to maintain both fb > min
> >>>>> fb and also multiplier of 4
> >>>>>
> >>>>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> >>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> >>>>> ---
> >>>>>  tests/kms_plane_scaling.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> >>>>> index 36fcfc0..b94d785 100644
> >>>>> --- a/tests/kms_plane_scaling.c
> >>>>> +++ b/tests/kms_plane_scaling.c
> >>>>> @@ -132,7 +132,7 @@ static void
> >>>>> check_scaling_pipe_plane_rot(data_t *d,
> >>>> igt_plane_t *plane,
> >>>>>  	/* create buffer in the range of  min and max source side limit.*/
> >>>>>  	width = height = 9;
> >>>>>  	if (pixel_format == DRM_FORMAT_NV12)
> >>>>> -		width = height = 17;
> >>>>> +		width = height = 20;
> >>>> Why 20 and not 16? I thought 16 was the limit?
> >>> Yes, but when the NV12 17x17 buffer is flipped on the sprite, it
> >>> gets adjusted to 16x16 in Intel_check_sprite_plane and further
> >>> PLANE_SIZE gets
> >> written with 15x15.
> >>
> >> I don't see what the -1 bias of the register values has to do with this.
> >>
> >>> So, I thought 20x20 would be better. However when I run the
> >>> kms_plane_scaling with rotation About 100 times, I still see rare
> >>> fifo underruns. This may be due to the WA1106 Which says the plane
> >>> height for
> >> NV12 in case of rotation needs to be multiplier of 4.
> >>> Even with 20x20 buffer, after the adjustment it changes to 18x19 and
> >>> this further reduces to 17x18 in skl_update_plane This causes 18 to
> >>> be
> >> programmed as plane height which is not a multiplier of 4.
> >>
> >> The bias has nothing to do with this. But the reduction is source
> >> size to make it an exact multiple of the hscale/vscale can certainly
> >> be a problem. That thing needs some rethinking for sure. Sadly the
> >> spec doesn't document the scaling algorithm so it's hard to say what is
> the best apporach.
> >>
> >> But this does appear to highlight a problem in the nv12 patches. They
> >> should be checking the final source coordinates against the hardware
> >> minimum size, and reject the operation if we're trying to do
> >> something bad. Perhaps we already have that same problem with non-
> nv12?
> > Thank you. Will try implementing this in KMD.
> >
> > The problem is, the results are now showing consistent.
> > As in, not always the NV12 src height  multiplier of 4 for height results in
> non-underruns.
> > When I used a 29x29 buffer, even though the final source width height
> > was 28x28 It still showed fifo underrun.
> We should probably look at more data, what about checking minimums?
> 
> Does width need to be a multiple of 4, or 8? What about height?

As per Bspec work around section only height needs to be multiple of 4 that too
For 90/270 rotation case. But that’s only a WA. Main restriction is only about the height being more than
16 scanlines.

WA 1106: Display corruption/color shift observed when using NV12 with 270 rotation or 90 rotation + horizontal flip.
WA: NV12 with 270 rotation or 90 rotation + horizontal flip requires the programmed plane height to be a multiple of 4.

That way, there is one more WA 1173 which mentions:
Make sure that the plane size programming for frame n+1 meets the following constraint
0 Rotation: PLANE_OFFSET-> X position is a multiple of 32
180 Rotation: PLANE_OFFSET->X position + PLANE_SIZE->Width+1 is a multiple of Alignment Factor defined below.
Use Alignment Factor of 16 for RGB32, 32 for YUY2 and 64 for NV12.

Regards
Vidya