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

[Hans de Goede]

Hi,

On 09/23/2016 11:13 AM, Michel Dänzer wrote:
> On 23/09/16 03:46 PM, Hans de Goede wrote:
>> On 09/20/2016 04:58 AM, Michel Dänzer wrote:
>>
>>> 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.
>
> Leaving it as NULL would probably flush out such drivers more quickly
> though. :)
>
>
>> 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.
>
> Fine with me, but please at least change the comment to be more accurate
> and clearer.

OK, the comment now reads:
/*
  * Compatibility pScrn->ChangeGamma provider for ddx drivers which do not call
  * xf86HandleColormaps(). Note such drivers really should be fixed to call
  * xf86HandleColormaps() as this clobbers any per-crtc setup.
  */

REgards,

Hans