All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
@ 2019-03-24 17:55 Uma Shankar
  2019-03-24 17:56 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_color: Fix CRC mismatch issues with ctm test (rev2) Patchwork
  2019-03-25  1:40 ` [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Lionel Landwerlin
  0 siblings, 2 replies; 10+ messages in thread
From: Uma Shankar @ 2019-03-24 17:55 UTC (permalink / raw)
  To: igt-dev; +Cc: ville.syrjala, maarten.lankhorst

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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_color: Fix CRC mismatch issues with ctm test (rev2)
  2019-03-24 17:55 [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Uma Shankar
@ 2019-03-24 17:56 ` Patchwork
  2019-03-25  1:40 ` [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Lionel Landwerlin
  1 sibling, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-03-24 17:56 UTC (permalink / raw)
  To: Uma Shankar; +Cc: igt-dev

== Series Details ==

Series: tests/kms_color: Fix CRC mismatch issues with ctm test (rev2)
URL   : https://patchwork.freedesktop.org/series/58412/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
09796413394c5490c4adfc5cded5d4344af6af17 tests/kms_cursor_legacy: Fix assertion failure due to vblank mismatch

../tests/kms_color.c:711:40: error: ‘fb’ undeclared (first use in this function)
   paint_rectangles(data, mode, after, &fb);
                                        ^~
../tests/kms_color.c:716:45: error: ‘crc_software’ undeclared (first use in this function)
   igt_pipe_crc_collect_crc(data->pipe_crc, &crc_software);
                                             ^~~~~~~~~~~~
../tests/kms_color.c:724:45: error: ‘crc_hardware’ undeclared (first use in this function); did you mean ‘crc_software’?
   igt_pipe_crc_collect_crc(data->pipe_crc, &crc_hardware);
                                             ^~~~~~~~~~~~
                                             crc_software
../tests/kms_color.c:667:7: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  bool ret = true;
       ^~~
../tests/kms_color.c: At top level:
../tests/kms_color.c:735:2: warning: data definition has no type or storage class
  free(degamma_linear);
  ^~~~
../tests/kms_color.c:735:2: error: type defaults to ‘int’ in declaration of ‘free’ [-Werror=implicit-int]
../tests/kms_color.c:735:2: warning: parameter names (without types) in function declaration
../tests/kms_color.c:735:2: error: conflicting types for ‘free’
In file included from ../lib/igt_core.h:37:0,
                 from ../lib/drmtest.h:39,
                 from ../tests/kms_color.c:29:
/usr/include/stdlib.h:563:13: note: previous declaration of ‘free’ was here
 extern void free (void *__ptr) __THROW;
             ^~~~
../tests/kms_color.c:736:2: warning: data definition has no type or storage class
  free(gamma_linear);
  ^~~~
../tests/kms_color.c:736:2: error: type defaults to ‘int’ in declaration of ‘free’ [-Werror=implicit-int]
../tests/kms_color.c:736:2: warning: parameter names (without types) in function declaration
../tests/kms_color.c:736:2: error: conflicting types for ‘free’
In file included from ../lib/igt_core.h:37:0,
                 from ../lib/drmtest.h:39,
                 from ../tests/kms_color.c:29:
/usr/include/stdlib.h:563:13: note: previous declaration of ‘free’ was here
 extern void free (void *__ptr) __THROW;
             ^~~~
../tests/kms_color.c:738:2: error: expected identifier or ‘(’ before ‘return’
  return ret;
  ^~~~~~
../tests/kms_color.c:739:1: error: expected identifier or ‘(’ before ‘}’ token
 }
 ^
../tests/kms_color.c: In function ‘test_pipe_ctm’:
../tests/kms_color.c:733:2: error: control reaches end of non-void function [-Werror=return-type]
  }
  ^
cc1: some warnings being treated as errors
ninja: build stopped: subcommand failed.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-24 17:55 [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Uma Shankar
  2019-03-24 17:56 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_color: Fix CRC mismatch issues with ctm test (rev2) Patchwork
@ 2019-03-25  1:40 ` Lionel Landwerlin
  2019-03-25  6:36   ` Shankar, Uma
  1 sibling, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2019-03-25  1:40 UTC (permalink / raw)
  To: Uma Shankar, igt-dev; +Cc: ville.syrjala, maarten.lankhorst

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?

-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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25  1:40 ` [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Lionel Landwerlin
@ 2019-03-25  6:36   ` Shankar, Uma
  2019-03-25 10:15     ` Lionel Landwerlin
  0 siblings, 1 reply; 10+ messages in thread
From: Shankar, Uma @ 2019-03-25  6:36 UTC (permalink / raw)
  To: Landwerlin, Lionel G, igt-dev; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Landwerlin, Lionel G
>Sent: Monday, March 25, 2019 7:11 AM
>To: Shankar, Uma <uma.shankar@intel.com>; igt-dev@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@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

>-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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25  6:36   ` Shankar, Uma
@ 2019-03-25 10:15     ` Lionel Landwerlin
  2019-03-25 10:40       ` Shankar, Uma
  0 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2019-03-25 10:15 UTC (permalink / raw)
  To: Shankar, Uma, igt-dev; +Cc: Syrjala, Ville, Lankhorst, Maarten

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@intel.com>; igt-dev@lists.freedesktop.org
>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> <maarten.lankhorst@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.

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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25 10:15     ` Lionel Landwerlin
@ 2019-03-25 10:40       ` Shankar, Uma
  2019-03-25 10:51         ` Ville Syrjälä
  2019-03-25 11:08         ` Lionel Landwerlin
  0 siblings, 2 replies; 10+ messages in thread
From: Shankar, Uma @ 2019-03-25 10:40 UTC (permalink / raw)
  To: Landwerlin, Lionel G, igt-dev; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Landwerlin, Lionel G
>Sent: Monday, March 25, 2019 3:46 PM
>To: Shankar, Uma <uma.shankar@intel.com>; igt-dev@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@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@intel.com>;
>>> igt-dev@lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>> <maarten.lankhorst@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25 10:40       ` Shankar, Uma
@ 2019-03-25 10:51         ` Ville Syrjälä
  2019-03-25 13:12           ` Shankar, Uma
  2019-03-25 11:08         ` Lionel Landwerlin
  1 sibling, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2019-03-25 10:51 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: igt-dev, Syrjala, Ville, Lankhorst, Maarten

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@intel.com>; igt-dev@lists.freedesktop.org
> >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> ><maarten.lankhorst@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@intel.com>;
> >>> igt-dev@lists.freedesktop.org
> >>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> >>> <maarten.lankhorst@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.

> 
> 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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25 10:40       ` Shankar, Uma
  2019-03-25 10:51         ` Ville Syrjälä
@ 2019-03-25 11:08         ` Lionel Landwerlin
  2019-03-25 11:32           ` Shankar, Uma
  1 sibling, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2019-03-25 11:08 UTC (permalink / raw)
  To: Shankar, Uma, igt-dev; +Cc: Syrjala, Ville, Lankhorst, Maarten

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@intel.com>; igt-dev@lists.freedesktop.org
>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> <maarten.lankhorst@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@intel.com>;
>>>> igt-dev@lists.freedesktop.org
>>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>>> <maarten.lankhorst@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@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25 11:08         ` Lionel Landwerlin
@ 2019-03-25 11:32           ` Shankar, Uma
  0 siblings, 0 replies; 10+ messages in thread
From: Shankar, Uma @ 2019-03-25 11:32 UTC (permalink / raw)
  To: Landwerlin, Lionel G, igt-dev; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Landwerlin, Lionel G
>Sent: Monday, March 25, 2019 4:39 PM
>To: Shankar, Uma <uma.shankar@intel.com>; igt-dev@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@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@intel.com>;
>>> igt-dev@lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>> <maarten.lankhorst@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@intel.com>;
>>>>> igt-dev@lists.freedesktop.org
>>>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>>>>> <maarten.lankhorst@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@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test
  2019-03-25 10:51         ` Ville Syrjälä
@ 2019-03-25 13:12           ` Shankar, Uma
  0 siblings, 0 replies; 10+ messages in thread
From: Shankar, Uma @ 2019-03-25 13:12 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Monday, March 25, 2019 4:21 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: Landwerlin, Lionel G <lionel.g.landwerlin@intel.com>; igt-
>dev@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst,
>Maarten <maarten.lankhorst@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@intel.com>;
>> >igt-dev@lists.freedesktop.org
>> >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> ><maarten.lankhorst@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@intel.com>;
>> >>> igt-dev@lists.freedesktop.org
>> >>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
>> >>> <maarten.lankhorst@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@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-03-25 13:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-24 17:55 [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Uma Shankar
2019-03-24 17:56 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_color: Fix CRC mismatch issues with ctm test (rev2) Patchwork
2019-03-25  1:40 ` [igt-dev] [v2] tests/kms_color: Fix CRC mismatch issues with ctm test Lionel Landwerlin
2019-03-25  6:36   ` Shankar, Uma
2019-03-25 10:15     ` Lionel Landwerlin
2019-03-25 10:40       ` Shankar, Uma
2019-03-25 10:51         ` Ville Syrjälä
2019-03-25 13:12           ` Shankar, Uma
2019-03-25 11:08         ` Lionel Landwerlin
2019-03-25 11:32           ` Shankar, Uma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.