| Message ID | 1528820435-2732-2-git-send-email-mario.kleiner.de@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Mon, Oct 15, 2018 at 6:21 PM Ville Syrjälä <ville.syrjala@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@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> > > 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 > <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>
Quoting Mario Kleiner (2019-01-20 19:45:18) > On Mon, Oct 15, 2018 at 6:21 PM Ville Syrjälä <ville.syrjala@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@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> > > > > Thanks Ville! > > Now it just needs to get merged, please. Chris? Pushed, thanks for the reminder. -Chris
On Sun, Jan 20, 2019 at 08:45:18PM +0100, Mario Kleiner wrote: > On Mon, Oct 15, 2018 at 6:21 PM Ville Syrjälä <ville.syrjala@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@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> > > > > > 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? Kernel bits for gamma table bypass are on the list: https://patchwork.freedesktop.org/series/55081/ Apart from that I've not had any real time to work on it.
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;
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> --- src/sna/sna_driver.c | 9 ++++++--- src/uxa/intel_driver.c | 6 +++++- 2 files changed, 11 insertions(+), 4 deletions(-)