Re: [PATCH] sna/uxa: Fix colormap handling at screen depth 30.

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] sna/uxa: Fix colormap handling at screen depth 30.
Date: Thu, 1 Mar 2018 13:12:53 +0200	[thread overview]
Message-ID: <20180301111253.GU5453@intel.com> (raw)
In-Reply-To: <20180301012048.17717-1-mario.kleiner.de@gmail.com>

On Thu, Mar 01, 2018 at 02:20:48AM +0100, Mario Kleiner wrote:
> The various clut handling functions like a setup
> consistent with the x-screen color depth. Otherwise
> we observe improper sampling in the gamma tables
> at depth 30.
> 
> Therefore replace hard-coded bitsPerRGB = 8 by actual
> bits per channel scrn->rgbBits. Also use this for call
> to xf86HandleColormaps().
> 
> Tested for uxa and sna at depths 8, 16, 24 and 30 on
> IvyBridge, and tested at depth 24 and 30 that xgamma
> and gamma table animations work, and with measurement
> equipment to make sure identity gamma ramps actually
> are identity mappings at the output.

You mean identity mapping at 8bpc? We don't support higher precision
gamma on pre-bdw atm, and the ddx doesn't use the higher precision
stuff even on bdw+. I'm working on fixing both, but it turned out to
be a bit more work than I anticipated so will take a while.

> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>  src/sna/sna_driver.c   | 5 +++--
>  src/uxa/intel_driver.c | 3 ++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c
> index 2643e6c..9c4bcd4 100644
> --- a/src/sna/sna_driver.c
> +++ b/src/sna/sna_driver.c
> @@ -1145,7 +1145,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL)
>  	if (!miInitVisuals(&visuals, &depths, &nvisuals, &ndepths, &rootdepth,
>  			   &defaultVisual,
>  			   ((unsigned long)1 << (scrn->bitsPerPixel - 1)),
> -			   8, -1))
> +			   scrn->rgbBits, -1))
>  		return FALSE;
>  
>  	if (!miScreenInit(screen, NULL,
> @@ -1217,7 +1217,8 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL)
>  		return FALSE;
>  
>  	if (sna->mode.num_real_crtc &&
> -	    !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL,
> +	    !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,
> +				 sna_load_palette, NULL,
>  				 CMAP_RELOAD_ON_MODE_SWITCH |
>  				 CMAP_PALETTED_TRUECOLOR))

I already forgot what this does prior to your randr fix. IIRC bumping
the 8 alone would cause the thing to segfault, but I guess bumping both
was fine?

Hmm. So the server always initializes crtc->gamma_size to 256
(which does match the normal hw LUT size), and so before your
fix we will always get gamma_slots==0 at >8bpc and so the hw LUT
is never actually updated?

>  		return FALSE;
> diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
> index 3703c41..88c749e 100644
> --- a/src/uxa/intel_driver.c
> +++ b/src/uxa/intel_driver.c
> @@ -991,7 +991,8 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL)
>  	if (!miCreateDefColormap(screen))
>  		return FALSE;
>  
> -	if (!xf86HandleColormaps(screen, 256, 8, I830LoadPalette, NULL,
> +	if (!xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,
> +				 I830LoadPalette, NULL,
>  				 CMAP_RELOAD_ON_MODE_SWITCH |
>  				 CMAP_PALETTED_TRUECOLOR)) {
>  		return FALSE;
> -- 
> 2.7.4
> 
> _______________________________________________
> 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