<div dir="ltr"><div dir="ltr">On Mon, Oct 15, 2018 at 6:21 PM Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Jun 12, 2018 at 06:20:35PM +0200, Mario Kleiner wrote:<br>
> The various clut handling functions like a setup<br>
> consistent with the x-screen color depth. Otherwise<br>
> we observe improper sampling in the gamma tables<br>
> at depth 30.<br>
> <br>
> Therefore replace hard-coded bitsPerRGB = 8 by actual<br>
> bits per channel scrn->rgbBits. Also use this for call<br>
> to xf86HandleColormaps().<br>
> <br>
> Tested for uxa and sna at depths 8, 16, 24 and 30 on<br>
> IvyBridge, and tested at depth 24 and 30 that xgamma<br>
> and gamma table animations work, and with measurement<br>
> equipment to make sure identity gamma ramps actually<br>
> are identity mappings at the output.<br>
> <br>
> v2: Also deal with X-Server 1.19 and earlier, which as of<br>
>     v1.19.6 lack a fix to color palette handling and can<br>
>     not deal with depths/bpc > 24/8 bpc. On < 1.20 we skip<br>
>     xf86HandleColormaps() setup at > 8 bpc. This disables<br>
>     color palette handling on such servers at > 8 bpc, but<br>
>     still keeps RandR gamma table handling intact.<br>
> <br>
>     Tested on 1.19.6 and 1.20.0 to do the right thing.<br>
> <br>
> Signed-off-by: Mario Kleiner <<a href="mailto:mario.kleiner.de@gmail.com" target="_blank">mario.kleiner.de@gmail.com</a>><br>
<br>
Forgot this didn't get applied. It did make sense to me at the<br>
time when I was looking at the explosions with depth 30.<br>
Still seems to do the trick on 1.19, and redshit still works<br>
so<br>
<br>
Reviewed-by: Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>><br>
<br></blockquote><div><br></div><div>Thanks Ville!</div><div><br></div><div>Now it just needs to get merged, please. Chris?<br></div><div><br></div><div>One last missing piece is support for 1024 slot gamma tables in i965-kms, or gamma table bypass for such high bit depth framebuffers to make them actually useful. Ville, i think you mentioned working on that around spring last year?<br></div><div><br></div><div>Thanks,</div><div>-mario</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ---<br>
>  src/sna/sna_driver.c   | 9 ++++++---<br>
>  src/uxa/intel_driver.c | 6 +++++-<br>
>  2 files changed, 11 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c<br>
> index 2007e354..8c79d43b 100644<br>
> --- a/src/sna/sna_driver.c<br>
> +++ b/src/sna/sna_driver.c<br>
> @@ -1152,7 +1152,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL)<br>
>       if (!miInitVisuals(&visuals, &depths, &nvisuals, &ndepths, &rootdepth,<br>
>                          &defaultVisual,<br>
>                          ((unsigned long)1 << (scrn->bitsPerPixel - 1)),<br>
> -                        8, -1))<br>
> +                        scrn->rgbBits, -1))<br>
>               return FALSE;<br>
>  <br>
>       if (!miScreenInit(screen, NULL,<br>
> @@ -1223,8 +1223,11 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL)<br>
>       if (!miCreateDefColormap(screen))<br>
>               return FALSE;<br>
>  <br>
> -     if (sna->mode.num_real_crtc &&<br>
> -         !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL,<br>
> +     /* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */<br>
> +     if (sna->mode.num_real_crtc && (scrn->rgbBits <= 8 ||<br>
> +         XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,20,0,0,0)) &&<br>
> +         !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,<br>
> +                              sna_load_palette, NULL,<br>
>                                CMAP_RELOAD_ON_MODE_SWITCH |<br>
>                                CMAP_PALETTED_TRUECOLOR))<br>
>               return FALSE;<br>
> diff --git a/src/uxa/intel_driver.c b/src/uxa/intel_driver.c<br>
> index 3703c412..77c0dc00 100644<br>
> --- a/src/uxa/intel_driver.c<br>
> +++ b/src/uxa/intel_driver.c<br>
> @@ -991,7 +991,11 @@ I830ScreenInit(SCREEN_INIT_ARGS_DECL)<br>
>       if (!miCreateDefColormap(screen))<br>
>               return FALSE;<br>
>  <br>
> -     if (!xf86HandleColormaps(screen, 256, 8, I830LoadPalette, NULL,<br>
> +     /* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */<br>
> +     if ((scrn->rgbBits <= 8 ||<br>
> +         XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,20,0,0,0)) &&<br>
> +         !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits,<br>
> +                              I830LoadPalette, NULL,<br>
>                                CMAP_RELOAD_ON_MODE_SWITCH |<br>
>                                CMAP_PALETTED_TRUECOLOR)) {<br>
>               return FALSE;<br>
> -- <br>
> 2.17.1<br>
<br>
-- <br>
Ville Syrjälä<br>
Intel<br>
</blockquote></div></div>