[igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test

[Shankar, Uma]

>-----Original Message-----
>From: Landwerlin, Lionel G
>Sent: Monday, March 25, 2019 4:39 PM
>To: Shankar, Uma <uma.shankar at intel.com>; igt-dev at lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala at intel.com>; Lankhorst, Maarten
><maarten.lankhorst at intel.com>
>Subject: Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
>
>On 25/03/2019 10:40, Shankar, Uma wrote:
>>
>>> -----Original Message-----
>>> From: Landwerlin, Lionel G
>>> Sent: Monday, March 25, 2019 3:46 PM
>>> To: Shankar, Uma <uma.shankar at intel.com>;
>>> igt-dev at lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala at intel.com>; Lankhorst, Maarten
>>> <maarten.lankhorst at intel.com>
>>> Subject: Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues
>>> with ctm test
>>>
>>> On 25/03/2019 06:36, Shankar, Uma wrote:
>>>>> -----Original Message-----
>>>>> From: Landwerlin, Lionel G
>>>>> Sent: Monday, March 25, 2019 7:11 AM
>>>>> To: Shankar, Uma <uma.shankar at intel.com>;
>>>>> igt-dev at lists.freedesktop.org
>>>>> Cc: Syrjala, Ville <ville.syrjala at intel.com>; Lankhorst, Maarten
>>>>> <maarten.lankhorst at intel.com>
>>>>> Subject: Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch
>>>>> issues with ctm test
>>>>>
>>>>> When degamma & gamma LUTs are disabled & CTM is enabled, i915
>>>>> programs a linear table in the degamma/gamma LUTs :
>>>>>
>>>>> https://cgit.freedesktop.org/drm/drm-
>>>>> intel/tree/drivers/gpu/drm/i915/intel_color.c#n463
>>>>> https://cgit.freedesktop.org/drm/drm-
>>>>> intel/tree/drivers/gpu/drm/i915/intel_color.c#n495
>>>>>
>>>>> All the values programmed are in u16 range which is also what
>>>>> userspace can program (this applies to BDW/SKL/KBL).
>>>>> What is it that the kernel is able to program but not userspace?
>>>> Hi Lionel,
>>>> Issue is with representing 1.0 which needs a 3.16 kind of
>>>> representation. So directly doing it in Kernel to program linear luts using below:
>>>> for (i = 0; i < lut_size; i++) {
>>>> 			u32 v = (i * (1 << 16)) / (lut_size - 1);
>>>> 			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
>>>> 		}
>>>> takes care of the corner case. The ABI is getting extended and we
>>>> will need higher precision luts, but for now due to lack of it,
>>>> putting the same to be
>>> done in driver.
>>>> Regards,
>>>> Uma Shankar
>>>
>>> Hi Uma,
>>>
>>> This is for GLK right?
>>> What about the failures on KBL/SKL?
>>>
>>> There the hardware doesn't have fixed point values.
>> On Gen9, it uses bdw_load_gamma_lut function. For all values its good,
>> but when it comes to programming GCMAX, ABI fells short:
>>
>> I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
>> I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
>> I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
>>
>> This expects input in U1.16 format. This may be the reason we are
>> seeing this issue on
>> Gen9 as well.
>>
>> Regards,
>> Uma Shankar
>
>
>Okay... I started looking into this because I'm pretty sure it use to work on gen9 big
>cores.
>
>Running the tests on my SKL nuc at this revision :
>
>
>commit 07118ed9b6ed7eb77382d4eb057b7faf4af74dec (drm-tip/drm-tip)
>Author: Jani Nikula <jani.nikula at intel.com>
>Date:   Fri Mar 8 13:36:01 2019 +0200
>
>     drm-tip: 2019y-03m-08d-11h-34m-53s UTC integration manifest
>
>
>All the kms_color tests are passing.
>
>
>Are you sure there isn't another issue?

I haven’t checked on Gen9 machine, testing on ICL currently. This also depends
on pipe precision and rounding, so not sure but definitely we are not able to write 1.0
to register from user provide luts.

Regards,
Uma Shankar

>
>
>-Lionel
>
>
>>
>>> Thanks,
>>>
>>> -Lionel
>>>
>>>
>>>>> -Lionel
>>>>>
>>>>> On 24/03/2019 17:55, Uma Shankar wrote:
>>>>>> Due to Gamma/Degamma limitation with precision (lack of exact 1.0
>>>>>> representation) due to ABI restriction, applying linear gamma
>>>>>> affects crc. This patch fixes the same by making ctm tests
>>>>>> independant of gamma/degamma.
>>>>>>
>>>>>> v2: Disable degamma/gamma programming for ctm max test as it leads
>>>>>> to crc mimsmatch. Limiting it to this test case alone as other
>>>>>> tests need it to be enabled, hence not touching those scenarios.
>>>>>>
>>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108147
>>>>>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>>>>>> ---
>>>>>>     tests/kms_color.c | 16 ++++++++++++++--
>>>>>>     1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/kms_color.c b/tests/kms_color.c index
>>>>>> decf3c2..93560d4 100644
>>>>>> --- a/tests/kms_color.c
>>>>>> +++ b/tests/kms_color.c
>>>>>> @@ -696,8 +696,15 @@ static bool test_pipe_ctm(data_t *data,
>>>>>>     		igt_assert(fb_modeset_id);
>>>>>>     		igt_plane_set_fb(primary, &fb_modeset);
>>>>>>
>>>>>> -		set_degamma(data, primary->pipe, degamma_linear);
>>>>>> -		set_gamma(data, primary->pipe, gamma_linear);
>>>>>> +		/*
>>>>>> +		 * Don't program LUT's for max CTM cases as limitation of
>>>>>> +		 * representing 1.0 due to ABI limits causes crc mismatch
>>>>>> +		 */
>>>>>> +		if (memcmp(before, after, sizeof(color_t)))
>>>>>> +			set_degamma(data, primary->pipe, degamma_linear);
>>>>>> +			set_gamma(data, primary->pipe, gamma_linear);
>>>>>> +		}
>>>>>> +
>>>>>>     		disable_ctm(primary->pipe);
>>>>>>     		igt_display_commit(&data->display);
>>>>>>
>>>>>> @@ -1005,6 +1012,11 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>>>>>>     		 * produce with an 8 bits per color framebuffer. */
>>>>>>     		igt_require(!IS_CHERRYVIEW(data->devid));
>>>>>>
>>>>>> +		/* Disable and let default luts be applied by kernel */
>>>>>> +		disable_degamma(primary->pipe);
>>>>>> +		disable_gamma(primary->pipe);
>>>>>> +		igt_display_commit(&data->display);
>>>>>> +
>>>>>>     		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
>>>>>>     					 full_rgb, ctm));
>>>>>>     	}
>