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

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Monday, March 25, 2019 4:21 PM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: Landwerlin, Lionel G <lionel.g.landwerlin at intel.com>; igt-
>dev at lists.freedesktop.org; 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 Mon, Mar 25, 2019 at 10:40:18AM +0000, 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);
>
>IIRC that's also the wrong set of register we're writing here.

Hmm yeah, it seems these need to be PREC_PAL_EXT_GC_MAX.
Somehow writing into PAL_GC_MAX is also working, but yes this needs
to be updated.

>>
>> 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));
>> >>>>    	}
>> >
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>--
>Ville Syrjälä
>Intel