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

[Lionel Landwerlin]

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?


-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));
>>>>>     	}