[xserver, 2/2] xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error

[Michel Dänzer]

On 23/03/17 07:59 PM, Hans de Goede wrote:
> On 20-09-16 03:58, Michel Dänzer wrote:
>> On 17/09/16 07:00 PM, Hans De Goede wrote:
>>> Commit b4e46c0444bb ("xfree86: Hook up colormaps and RandR 1.2 gamma
>>> code")
>>> dropped the providing of a pScrn->ChangeGamma callback from the
>>> xf86RandR12
>>> code. Leaving pScrn->ChangeGamma NULL in most cases.
>>>
>>> This triggers the BadImplementation error in xf86ChangeGamma() :
>>>
>>>     if (pScrn->ChangeGamma)
>>>         return (*pScrn->ChangeGamma) (pScrn, gamma);
>>>
>>>     return BadImplementation;
>>>
>>> Which causes X-apps using XF86VidModeSetGamma to crash with a
>>> X protocol error.
>>>
>>> This commit fixes this by re-introducing the xf86RandR12ChangeGamma
>>> helper removed by the commit and adjusting it to work with the new
>>> combined palette / gamma code.
>>>
>>> Fixes: b4e46c0444bb ("xfree86: Hook up colormaps and RandR 1.2 gamma
>>> code")
>>
>> I suspect you really want to fix the modesetting driver to call
>> xf86HandleColormaps, so it can actually benefit from that change and
>> e.g. gamma sliders in games start working.
> 
> Ok, so going through my TODO list I started looking at this,
> amdgpu does:
> 
>         if (xf86_config->num_crtc) {
>                 xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO,
> AMDGPU_LOGLEVEL_DEBUG,
>                                "Initializing kms color map\n");
>                 if (!miCreateDefColormap(pScreen))
>                         return FALSE;
>                 /* all amdgpus support 10 bit CLUTs */
>                 if (!xf86HandleColormaps(pScreen, 256, 10,
>                                          NULL, NULL,
>                                          CMAP_PALETTED_TRUECOLOR
>                                          | CMAP_RELOAD_ON_MODE_SWITCH))
>                         return FALSE;
>         }
> 
> Which seems like it should mostly work for modesetting to,
> except for the all "amdgpus support 10 bit CLUTs" part,
> any ideas what to put there for modesetting, or how to
> query this from the kernel driver ?

I'm not sure either, but hardcoding 10 might be fine for modesetting as
well. The table for mapping from the post-colormap values to the final
values will be larger than necessary for hardware which only really
supports 8 bits or even less, but other than that I think the behaviour
should be mostly the same.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer