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

[Hans de Goede]

Hi,

On 09/20/2016 04:58 AM, 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.

Ah, good one. I'll take a look at this.


> 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.

Hmm, I understand what you're getting at. The problem is not if drivers
can or cannot call xf86HandleColormaps though. The problem is that there
likely will be drivers which do not (even though they would be perfectly
fine doing so).

Although I agree that your proposal is an improvement, I would prefer to
just move forward with this patch-set to provide a fall-back to ensure
that we do not have any regressions caused by pScrn->ChangeGamma being
NULL.

Since this is a fallback which ideally should never get used (all
drivers should call xf86HandleColormaps) I believe it is not worth
the time to implement your (admittedly better) fallback suggestion.

Regards,

Hans