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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

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

-- 
Ville Syrjälä
Intel