Re: [PATCH i-g-t] tests/kms_color: Enable ctm-limited-range subtest

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Ville,

On 14-03-2025 06:38 pm, Ville Syrjälä wrote:
> On Fri, Mar 14, 2025 at 06:04:07PM +0530, Sharma, Swati2 wrote:
>> Hi Ville,
>>
>> On 15-02-2025 02:34 am, Ville Syrjälä wrote:
>>> On Fri, Feb 14, 2025 at 11:56:27PM +0530, Sharma, Swati2 wrote:
>>>> Hi Ville,
>>>>
>>>> On 14-02-2025 11:00 pm, Ville Syrjälä wrote:
>>>>> On Fri, Feb 14, 2025 at 09:40:11PM +0530, Swati Sharma wrote:
>>>>>> This tests is currently disabled since CRC computed on Intel
>>>>>> hardware seems to include data on the lower bits, this is
>>>>>> preventing us to CRC checks.
>>>>>>
>>>>>> Let's try to enable it back and check behavior on newer Intel
>>>>>> platforms.
>>>>>>
>>>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>>>>> ---
>>>>>>     tests/kms_color.c | 166 ++++++++++++++++++++++++++--------------------
>>>>>>     1 file changed, 93 insertions(+), 73 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/kms_color.c b/tests/kms_color.c
>>>>>> index 4b71d3dd3..c3b285b4e 100644
>>>>>> --- a/tests/kms_color.c
>>>>>> +++ b/tests/kms_color.c
>>>>>> @@ -58,6 +58,7 @@
>>>>>>      * @0-75:           for 0.75 transparency
>>>>>>      * @blue-to-red:    from blue to red
>>>>>>      * @green-to-red:   from green to red
>>>>>> + * @limited-range:  with identity matrix
>>>>>>      * @max:            for maximum transparency
>>>>>>      * @negative:       for negative transparency
>>>>>>      * @red-to-blue:    from red to blue
>>>>>> @@ -623,107 +624,97 @@ static bool test_pipe_ctm(data_t *data,
>>>>>>      * This test is currently disabled as the CRC computed on Intel hardware seems
>>>>>>      * to include data on the lower bits, this is preventing us to CRC checks.
>>>>>>      */
>>>>>> -#if 0
>>>>>> -static void test_pipe_limited_range_ctm(data_t *data,
>>>>>> +static bool test_pipe_limited_range_ctm(data_t *data,
>>>>>>     					igt_plane_t *primary)
>>>>>>     {
>>>>>>     	double limited_result = 235.0 / 255.0;
>>>>>> -	static const color_t red_green_blue_limited[] = {
>>>>>> +	color_t red_green_blue_limited[] = {
>>>>>>     		{ limited_result, 0.0, 0.0 },
>>>>>>     		{ 0.0, limited_result, 0.0 },
>>>>>> -		{ 0.0, 0.0, limited_result },
>>>>>> +		{ 0.0, 0.0, limited_result }
>>>>>>     	};
>>>>> This whole thing is fundementally broken. We can't generate
>>>>> limited range output without using the CSC post offsets,
>>>>> which are not exposed via the current CTM uapi.
>>>> We do have its equivalent test in kms_color_chamelium
>>>> and it seems its passing
>>>> https://gfx-ci.igk.intel.com/cibuglog-ng/results/all?query_key=af49bc8e4e7d1c69ce04f9a1196c167456e8344a
>>>> Is it wrong?
>>> Hmm. Looks like the test has nothing really to do with the
>>> CTM (despite the name), and instead if just uses identity CTM
>>> and puts the limited range equivalent data directly into the
>>> fb. So I guess technically it could sort of work.
>>>
>>> For the chamelium version the port will chop off the extra
>>> low bits so I guess that explains why it works.
>> I recently dumped port images of this test on chamelium. Since this test
>> is consistently passing on BMG; however failing on PTL.
>>
>> What i observed is
>>
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-A-DP-1-capture-ece4-ece4-ece4-ece4.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-A-DP-1-reference-0000-0000-0000-0000.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-B-DP-1-capture-ece4-ece4-ece4-ece4.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-B-DP-1-reference-0000-0000-0000-0000.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-C-DP-1-capture-ece4-ece4-ece4-ece4.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-C-DP-1-reference-0000-0000-0000-0000.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-D-DP-1-capture-ece4-ece4-ece4-ece4.png
>> /tmp/frame-kms_chamelium_color-ctm-limited-range-pipe-D-DP-1-reference-0000-0000-0000-0000.png
>>
>> CRC of reference and captured images are not same. Ref is all black. But
>> still test is passing :/
>> Is test broken even for chamelium?
> Looks like it:
> 1. creates a bunch of all black fbs
> 2. manually draw limited range equivalent content into one fb
> 3. set the output to full range
> 4. commit
> 5. redraw the same fb with full range contnet
> 6. set the output to limited range
> 7  *no* commit
> 6. capture the frame and compare against the all black fbref
>
> So looks completely broken to me, and I don't see how that could
> be passing on anything unless the chamelium frame comparison code
> itself is broken.
Tried fixing the test. Can you please review 
https://patchwork.freedesktop.org/series/146647/
With these frame dumps match however CRC differs as expected.
>
>>> The crc version is more tricky:
>>> - g4x presuambly won't work because the port color range
>>>     bit won't affect the pipe crc
>>> - ilk-ivb/vlv/chv won't work because TRANSCONF_COLOR_RANGE_SELECT
>>>     doesn't seem to affect the pipe crc either
>>> - icl+ uses the output csc for the limited range conversion
>>>     so the gamma LUT lsb chopping doesn't do anything, so we may
>>>     get some differences in the low bits
>>> - hsw-glk maybe could work if we do enable CTM+gamma since then
>>>     we'll end up doing the limited range adjustment on the gamma LUT
>>>     and thus it can chop off the low bits. If we didn't enable
>>>     both CTM and gamma then the limited range conversion would be
>>>     done on the pipe CSC and thus it would behave exactly like icl+.
>>>