Re: [PATCH] drm/i915/glk: Improve rounding caused by pre-CSC gamma tables

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

On Fri, Mar 10, 2017 at 12:18:34PM +0200, Ander Conselvan de Oliveira wrote:
> The 33rd entry in the pre-CSC gamma table in Geminilake can represent a
> value of 1.0 as 17 bits fixed point with one integer bit. However, the
> table was generated such that the value of 1.0 would be 0.ffff with
> all the intervals scaled accordingly. For instance, 0.5 mapped to
> 0.7fff instead of 0.8000.

Actually all the gamma tables except the legacy 8bpc gamma have
this 1.0 thing. And yet we program them all to <1.0. But I guess
in most cases that doesn't cause any issues. But I think this is
definitely something someone should look at.

One problem is that the uapi we got in the end uses 0.16 representation
instead of the 8.24 that I thought we were going with. So the conversion
isn't going to as nice as it could be I suppose.

But yeah, this patch seems trouble free since we don't yet expose
the pre-csc gamma to userspace.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> For a reason that is not clear to the author, the rounding seems to be
> different when a cursor plane is used, leading to some seemingly random
> failures of the kms_cursor_crc igt tests. The differences weren't
> perceptible at 8bpc with images captured by a Chamelium device, but did
> cause CRC mismatches.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index b9e5266d..306c6b0 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -465,14 +465,14 @@ static void glk_load_degamma_lut(struct drm_crtc_state *state)
>  	 *  different values per channel, so this just loads a linear table.
>  	 */
>  	for (i = 0; i < lut_size; i++) {
> -		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> +		uint32_t v = (i * (1 << 16)) / (lut_size - 1);
>  
>  		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
>  	}
>  
>  	/* Clamp values > 1.0. */
>  	while (i++ < 35)
> -		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
>  }
>  
>  static void glk_load_luts(struct drm_crtc_state *state)
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx