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

[Mario Kleiner]

On Mon, Oct 15, 2018 at 6:21 PM Ville Syrjälä <ville.syrjala at linux.intel.com>
wrote:

> 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 at 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 at linux.intel.com>
>
>
Thanks Ville!

Now it just needs to get merged, please. Chris?

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?

Thanks,
-mario

> ---
> >  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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190120/5e18fe1e/attachment.html>