Re: [PATCH xserver 2/2] xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error
On Mon, Sep 19, 2016 at 11:31 AM, Alex Deucher wrote:
> On Sat, Sep 17, 2016 at 6:00 AM, 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")
>> Signed-off-by: Hans de Goede
>
> Not too familiar with the code in question, but it looks sane to me.
For the series:
> Reviewed-by: Alex Deucher
>
>> ---
>> 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@lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/2] xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error
On Sat, Sep 17, 2016 at 6:00 AM, 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")
> Signed-off-by: Hans de Goede
Not too familiar with the code in question, but it looks sane to me.
Reviewed-by: Alex Deucher
> ---
> 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@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
