[Nouveau] Nouveau Digest, Vol 131, Issue 3

[Mario Kleiner]

On 03/01/2018 07:21 PM, nouveau-request at lists.freedesktop.org wrote:
> 
> Message: 1
> Date: Thu, 1 Mar 2018 08:15:55 -0500
> From: Ilia Mirkin <imirkin at alum.mit.edu>
> To: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: nouveau <nouveau at lists.freedesktop.org>
> Subject: Re: [Nouveau] [PATCH] Fix colormap handling at screen depth
> 	30.
> Message-ID:
> 	<CAKb7UvhnUF6W41vXHpC+Q+jVSROgnHuxd8CzvfOGHmYSq_xTHQ at mail.gmail.com>
> 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.

-mario

> On Thu, Mar 1, 2018 at 1:31 AM, Mario Kleiner
> <mario.kleiner.de at 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.de at 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) */
>> --
>> 2.7.4