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

[Michel Dänzer]

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.


If there are other drivers which can't call xf86HandleColormaps for some
reason, a better solution would be to combine the per-screen gamma set
via pScrn->ChangeGamma with the per-CRTC gamma set via
xf86RandR12CrtcSetGamma.


> +/*
> + * Compatibility with XF86VidMode's gamma changer.  This necessarily clobbers
> + * any per-crtc setup.  You asked for it...
> + */

At the very least though, this comment should be changed to something
along the lines of:

/* Fallback pScrn->ChangeGamma implementation for drivers which can't
 * call xf86HandleColormaps. This will clobber the per-CRTC gamma ramp
 * of the CRTC assigned to the RandR compatibility output.
 */


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