[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 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

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