Quoting Ville Syrjälä (2018-03-15 16:02:42) > On Thu, Mar 15, 2018 at 03:28:18PM +0000, Chris Wilson wrote: > > 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@xxxxxxxxx> > > > > --- > > > > 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... > > IIRC there was some kind of agreement that Mario's approach here is > generally what we want to do. The server randr code will simply throw > away the LUT entries we can't use and driver will still get the > expected 256 entry LUT via the randr hooks. Obviously that means the > output may not be exactly what the user wanted, but for normal > gamma curve type of stuff it's pretty close. > > I'm mostly concerned what happens when the server doesn't have > Mario's gamma fixes. IIRC something will either crash, or simply > not update the gamma LUT ever. I guess we just make it conditional on xorg_version >= 1.20 to be on the safe side. > Once I manage to pimp up the i915 gamma LUT stuff I plan to tackle > the ddx side to actually make use of the higher precision gamma modes > in the hardware. Actually I already started on the ddx side a bit just > didn't get the kernel quite ready yet. Once it's all done we should > have 10bit gamma to match the screen depth on all platforms. On gen4 > it will be an interpolated curve though, so if the user programs an > arbitrary LUT we'll not be able to replicate it correctly. But again, > for normal gamma curve type of stuff it'll work just fine. For ilk+ > we will get a full 1024 entry LUT. Sounds good. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
