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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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