[Intel-gfx] [PATCH] sna/uxa: Fix colormap handling at screen depth 30.

[Chris Wilson]

Quoting Ville Syrjälä (2018-03-01 11:12:53)
> 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 at 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?

Was there any conclusion to this? Aiui, these bits should be set based
on the underlying HW gamma table depth which is not the same as the
screen colour depth. It stuck at 256 for ages as that is all anyone
ever expected...
-Chris