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

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

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?
>
> 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+.
>