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

[Alex Deucher]

On Mon, Sep 19, 2016 at 11:31 AM, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Sat, Sep 17, 2016 at 6:00 AM, Hans de Goede <hdegoede at redhat.com> 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")
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>
> Not too familiar with the code in question, but it looks sane to me.

For the series:

> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>
>> ---
>>  hw/xfree86/modes/xf86RandR12.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
>> index 36767b3..7b3b97b 100644
>> --- a/hw/xfree86/modes/xf86RandR12.c
>> +++ b/hw/xfree86/modes/xf86RandR12.c
>> @@ -1916,6 +1916,34 @@ xf86RandR12LoadPalette(ScrnInfoPtr pScrn, int numColors, int *indices,
>>      }
>>  }
>>
>> +/*
>> + * Compatibility with XF86VidMode's gamma changer.  This necessarily clobbers
>> + * any per-crtc setup.  You asked for it...
>> + */
>> +
>> +static int
>> +xf86RandR12ChangeGamma(ScrnInfoPtr pScrn, Gamma gamma)
>> +{
>> +    RRCrtcPtr randr_crtc = xf86CompatRRCrtc(pScrn);
>> +    int size;
>> +
>> +    if (!randr_crtc)
>> +        return Success;
>> +
>> +    size = max(0, randr_crtc->gammaSize);
>> +    if (!size)
>> +        return Success;
>> +
>> +    init_one_component(randr_crtc->gammaRed, size, gamma.red);
>> +    init_one_component(randr_crtc->gammaGreen, size, gamma.green);
>> +    init_one_component(randr_crtc->gammaBlue, size, gamma.blue);
>> +    xf86RandR12CrtcSetGamma(xf86ScrnToScreen(pScrn), randr_crtc);
>> +
>> +    pScrn->gamma = gamma;
>> +
>> +    return Success;
>> +}
>> +
>>  static Bool
>>  xf86RandR12EnterVT(ScrnInfoPtr pScrn)
>>  {
>> @@ -2115,6 +2143,7 @@ xf86RandR12Init12(ScreenPtr pScreen)
>>      rp->rrProviderDestroy = xf86RandR14ProviderDestroy;
>>
>>      pScrn->PointerMoved = xf86RandR12PointerMoved;
>> +    pScrn->ChangeGamma = xf86RandR12ChangeGamma;
>>
>>      randrp->orig_EnterVT = pScrn->EnterVT;
>>      pScrn->EnterVT = xf86RandR12EnterVT;
>> --
>> 2.9.3
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: https://lists.x.org/mailman/listinfo/xorg-devel