Re: [PATCH xf86-video-intel] sna/uxa: Fix colormap handling at screen depth 30. (v2)

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

On Tue, Jun 12, 2018 at 06:20:35PM +0200, 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.
> 
> v2: Also deal with X-Server 1.19 and earlier, which as of
>     v1.19.6 lack a fix to color palette handling and can
>     not deal with depths/bpc > 24/8 bpc. On < 1.20 we skip
>     xf86HandleColormaps() setup at > 8 bpc. This disables
>     color palette handling on such servers at > 8 bpc, but
>     still keeps RandR gamma table handling intact.
> 
>     Tested on 1.19.6 and 1.20.0 to do the right thing.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Forgot this didn't get applied. It did make sense to me at the
time when I was looking at the explosions with depth 30.
Still seems to do the trick on 1.19, and redshit still works
so

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

> ---
>  src/sna/sna_driver.c   | 9 ++++++---
>  src/uxa/intel_driver.c | 6 +++++-
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c
> index 2007e354..8c79d43b 100644
> --- a/src/sna/sna_driver.c
> +++ b/src/sna/sna_driver.c
> @@ -1152,7 +1152,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,
> @@ -1223,8 +1223,11 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL)
>  	if (!miCreateDefColormap(screen))
>  		return FALSE;
>  
> -	if (sna->mode.num_real_crtc &&
> -	    !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL,
> +	/* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
> +	if (sna->mode.num_real_crtc && (scrn->rgbBits <= 8 ||
> +	    XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,20,0,0,0)) &&
> +	    !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,
> +				 sna_load_palette, NULL,
>  				 CMAP_RELOAD_ON_MODE_SWITCH |
>  				 CMAP_PALETTED_TRUECOLOR))
>  		return FALSE;
> diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c
> index 3703c412..77c0dc00 100644
> --- a/src/uxa/intel_driver.c
> +++ b/src/uxa/intel_driver.c
> @@ -991,7 +991,11 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL)
>  	if (!miCreateDefColormap(screen))
>  		return FALSE;
>  
> -	if (!xf86HandleColormaps(screen, 256, 8, I830LoadPalette, NULL,
> +	/* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
> +	if ((scrn->rgbBits <= 8 ||
> +	    XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,20,0,0,0)) &&
> +	    !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,
> +				 I830LoadPalette, NULL,
>  				 CMAP_RELOAD_ON_MODE_SWITCH |
>  				 CMAP_PALETTED_TRUECOLOR)) {
>  		return FALSE;
> -- 
> 2.17.1

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