Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of unknown single plane modes

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: juhapekka.heikkila@gmail.com, igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of unknown single plane modes
Date: Mon, 5 Nov 2018 13:42:29 +0100	[thread overview]
Message-ID: <23c96754-f75c-5c8e-222b-b0dcb9632953@linux.intel.com> (raw)
In-Reply-To: <7ca65f30-2b78-fb69-065e-da47ae9fc4b6@gmail.com>

Op 05-11-18 om 12:24 schreef Juha-Pekka Heikkila:
> On 05.11.2018 11:54, Maarten Lankhorst wrote:
>> Op 03-11-18 om 12:28 schreef Juha-Pekka Heikkila:
>>> Fix creation of gem buffer so that used bpp will always have value other
>>> than 0.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>> ---
>>>   tests/kms_available_modes_crc.c | 10 +++++-----
>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
>>> index 45a4775..006c2db 100644
>>> --- a/tests/kms_available_modes_crc.c
>>> +++ b/tests/kms_available_modes_crc.c
>>> @@ -164,7 +164,7 @@ static const struct {
>>>   #ifdef DRM_FORMAT_P016
>>>       { DRM_FORMAT_P016, 0, P010, 0x8000eb00},
>>>   #endif
>>> -    { 0, 0, 0, 0 }
>>> +    { 0, 0, SKIP4, 0 }
>> Remove this entry, and make the loop
>> for (i = 0; i < ARRAY_SIZE(fillers); i++) { }
>>
>> if (i == ARRAY_SIZE(fillers))
>> return false;
>>
>> ?
>>
>> This should prevent it from ever being a problem in the first place.
>>
>
> I was thinking this at first too but that 'skip4' flag is used to skip CRC checking at later time. This test runs in two levels, at first try to enable plane format and then do CRC. This last element is for the case where plane format was not hardcoded here.
>
> I did have another look at this test and made another patch which also relax CRC checking so this test will pass on our modern HW, I got idea from Ville's usage of gamma on kms_plane. I'll put it on the list in a minute. 
We shouldn't test unknown modes at least. SKIP4 was specifically added for Alpha RGB formats on gen9-10, because in some cases fully transparant and fully opaque framebuffers would have rounding errors resulting in a small mismatch picked up by the CRC tests.

This is exactly why it might not be a good idea to randomly apply gamma tables hiding those rounding errors. In the future we want to prevent future occurences. If it's just the YUV formats that need a small correction to be fully white, we need to document it explicitly and not paper over it.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev