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

[Maarten Lankhorst]

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?