On 03/01/2018 07:21 PM, nouveau-requ...@lists.freedesktop.org wrote:

Message: 1
Date: Thu, 1 Mar 2018 08:15:55 -0500
From: Ilia Mirkin <imir...@alum.mit.edu>
To: Mario Kleiner <mario.kleiner...@gmail.com>
Cc: nouveau <nouveau@lists.freedesktop.org>
Subject: Re: [Nouveau] [PATCH] Fix colormap handling at screen depth
Content-Type: text/plain; charset="UTF-8"

NVLoadPalette is pretty hard-coded to 256. I haven't looked at what
all xf86HandleColormaps does, but it seems pretty suspicious. Also

It's also pretty dead :). NVLoadPalette is not ever used, because nouveau hooks up the .gamma_set function in xf86CrtcFuncsRec, so xf86HandleColormaps ignores the NVLoadPalette pointer. Iow. dead code that can be removed. I'll send some follow up patch, once this one is in. We have similar dead code in intel-ddx and modesetting-ddx which only serves to confuse the reader.

note that the kernel currently only exposes a 256-sized LUT to
userspace, even for 10bpc modes.

Yes, but that doesn't matter. In xbgr2101010 mode, the gpu seems to properly interpolate between the 256 (or 257) hw lut slots, as far as my measurments go. The X-Server maintains separate color palettes, per-x-screen xf86vidmode gamma lut's and per-crtc RandR gamma lut's and munches them together to produce the final 256 slot hw lut for the kernel, up/downsampling if needed. So adapting the values for xf86HandleColorMaps() is about properly sizing those internal palette's and lut's to avoid out-of-bounds segfaults or loss of precision somewhere in the whole multi-step remapping procedure, because one of the server internal tables is a bottleneck with too little slots.

This variant is the one that avoids crashes and also visual artifacts that i at least observed on tesla gpu's at depth 30.

One weird thing i still observed though is that in depth 30 xbgr2101010 scanout mode nouveau used dithering when i tried to output a linear intensity ramp, despite me disabling dithering via the xrandr property. But that is an unrelated problem.


On Thu, Mar 1, 2018 at 1:31 AM, Mario Kleiner
<mario.kleiner...@gmail.com> 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.

Tested at depths 16, 24 and 30 and tested at depths
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.

Signed-off-by: Mario Kleiner <mario.kleiner...@gmail.com>
  src/nv_driver.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nv_driver.c b/src/nv_driver.c
index 32062eb..4fcd4c1 100644
--- a/src/nv_driver.c
+++ b/src/nv_driver.c
@@ -1568,8 +1568,8 @@ NVScreenInit(SCREEN_INIT_ARGS_DECL)
          * Must follow initialization of the default colormap
         if (xf86_config->num_crtc &&
-           !xf86HandleColormaps(pScreen, 256, 8, NVLoadPalette,
-                                NULL, CMAP_PALETTED_TRUECOLOR))
+           !xf86HandleColormaps(pScreen, 1 << pScrn->rgbBits, pScrn->rgbBits,
+                                NVLoadPalette, NULL, CMAP_PALETTED_TRUECOLOR))
                 return FALSE;

         /* Report any unused options (only for the first generation) */
Nouveau mailing list

Reply via email to