[PATCH v3 xserver 3/3] xfree86: Hook up colormaps and RandR 1.2 gamma code

Alex Deucher alexdeucher at gmail.com
Wed Jun 22 16:19:57 UTC 2016


On Tue, Jun 21, 2016 at 11:12 PM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> Instead of breaking the former when the driver supports the latter,
> hook them up so that the hardware LUTs reflect the combination of the
> current colourmap and gamma states. I.e. combine the colourmap, the
> global gamma value/ramp and the RandR 1.2 per-CRTC gamma ramps into one
> combined LUT per CRTC.
>
> Fixes e.g. gamma sliders not working in games.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27222
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>
> v3: Free randrp->palette in xf86RandR12CloseScreen, fixes memory leak.

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

>
>  hw/xfree86/common/xf86Helper.c |   7 +-
>  hw/xfree86/common/xf86cmap.c   |  55 ++-----------
>  hw/xfree86/modes/xf86RandR12.c | 175 +++++++++++++++++++++++++----------------
>  hw/xfree86/modes/xf86RandR12.h |   5 ++
>  4 files changed, 119 insertions(+), 123 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
> index bceb017..6f3a608 100644
> --- a/hw/xfree86/common/xf86Helper.c
> +++ b/hw/xfree86/common/xf86Helper.c
> @@ -54,7 +54,6 @@
>  #include "xf86Xinput.h"
>  #include "xf86InPriv.h"
>  #include "mivalidate.h"
> -#include "xf86Crtc.h"
>
>  /* For xf86GetClocks */
>  #if defined(CSRG_BASED) || defined(__GNU__)
> @@ -908,11 +907,7 @@ xf86SetGamma(ScrnInfoPtr scrp, Gamma gamma)
>          scrp->gamma.green = 1.0;
>          scrp->gamma.blue = 1.0;
>      }
> -    /* Pretend we succeeded if we support better a gamma system.
> -     * This avoids a confusing message.
> -     */
> -    if (xf86_crtc_supports_gamma(scrp))
> -        return TRUE;
> +
>      xf86DrvMsg(scrp->scrnIndex, from,
>                 "Using gamma correction (%.1f, %.1f, %.1f)\n",
>                 scrp->gamma.red, scrp->gamma.green, scrp->gamma.blue);
> diff --git a/hw/xfree86/common/xf86cmap.c b/hw/xfree86/common/xf86cmap.c
> index ba0b277..1f21a0b 100644
> --- a/hw/xfree86/common/xf86cmap.c
> +++ b/hw/xfree86/common/xf86cmap.c
> @@ -49,6 +49,7 @@
>  #include "xf86_OSproc.h"
>  #include "xf86str.h"
>  #include "micmap.h"
> +#include "xf86RandR12.h"
>  #include "xf86Crtc.h"
>
>  #ifdef XFreeXDGA
> @@ -132,9 +133,6 @@ static void CMapUnwrapScreen(ScreenPtr pScreen);
>  Bool
>  xf86ColormapAllocatePrivates(ScrnInfoPtr pScrn)
>  {
> -    /* If we support a better colormap system, then pretend we succeeded. */
> -    if (xf86_crtc_supports_gamma(pScrn))
> -        return TRUE;
>      if (!dixRegisterPrivateKey(&CMapScreenKeyRec, PRIVATE_SCREEN, 0))
>          return FALSE;
>
> @@ -157,10 +155,6 @@ xf86HandleColormaps(ScreenPtr pScreen,
>      int *indices;
>      int elements;
>
> -    /* If we support a better colormap system, then pretend we succeeded. */
> -    if (xf86_crtc_supports_gamma(pScrn))
> -        return TRUE;
> -
>      if (!maxColors || !sigRGBbits || !loadPalette)
>          return FALSE;
>
> @@ -193,7 +187,12 @@ xf86HandleColormaps(ScreenPtr pScreen,
>      pScreen->InstallColormap = CMapInstallColormap;
>      pScreen->StoreColors = CMapStoreColors;
>
> -    pScrn->LoadPalette = loadPalette;
> +    if (xf86_crtc_supports_gamma(pScrn)) {
> +        pScrn->LoadPalette = xf86RandR12LoadPalette;
> +        if (!xf86RandR12InitGamma(pScrn, elements))
> +            return FALSE;
> +    } else
> +        pScrn->LoadPalette = loadPalette;
>      pScrn->SetOverscan = setOverscan;
>      pScreenPriv->maxColors = maxColors;
>      pScreenPriv->sigRGBbits = sigRGBbits;
> @@ -1005,19 +1004,6 @@ xf86ChangeGammaRamp(ScreenPtr pScreen,
>      CMapScreenPtr pScreenPriv;
>      CMapLinkPtr pLink;
>
> -    if (xf86_crtc_supports_gamma(pScrn)) {
> -        RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
> -
> -        if (crtc) {
> -            if (crtc->gammaSize != size)
> -                return BadValue;
> -
> -            RRCrtcGammaSet(crtc, red, green, blue);
> -
> -            return Success;
> -        }
> -    }
> -
>      if (!CMapScreenKeyRegistered)
>          return BadImplementation;
>
> @@ -1077,16 +1063,8 @@ xf86ChangeGammaRamp(ScreenPtr pScreen,
>  int
>  xf86GetGammaRampSize(ScreenPtr pScreen)
>  {
> -    ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
>      CMapScreenPtr pScreenPriv;
>
> -    if (xf86_crtc_supports_gamma(pScrn)) {
> -        RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
> -
> -        if (crtc)
> -            return crtc->gammaSize;
> -    }
> -
>      if (!CMapScreenKeyRegistered)
>          return 0;
>
> @@ -1104,29 +1082,10 @@ xf86GetGammaRamp(ScreenPtr pScreen,
>                   unsigned short *red,
>                   unsigned short *green, unsigned short *blue)
>  {
> -    ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
>      CMapScreenPtr pScreenPriv;
>      LOCO *entry;
>      int shift, sigbits;
>
> -    if (xf86_crtc_supports_gamma(pScrn)) {
> -        RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
> -
> -        if (crtc) {
> -            if (crtc->gammaSize < size)
> -                return BadValue;
> -
> -            if (!RRCrtcGammaGet(crtc))
> -                return BadImplementation;
> -
> -            memcpy(red, crtc->gammaRed, size * sizeof(*red));
> -            memcpy(green, crtc->gammaGreen, size * sizeof(*green));
> -            memcpy(blue, crtc->gammaBlue, size * sizeof(*blue));
> -
> -            return Success;
> -        }
> -    }
> -
>      if (!CMapScreenKeyRegistered)
>          return BadImplementation;
>
> diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
> index e8a62e8..bcf4176 100644
> --- a/hw/xfree86/modes/xf86RandR12.c
> +++ b/hw/xfree86/modes/xf86RandR12.c
> @@ -40,6 +40,7 @@
>  #include <randrstr.h>
>  #include <X11/extensions/render.h>
>
> +#include "xf86cmap.h"
>  #include "xf86Crtc.h"
>  #include "xf86RandR12.h"
>
> @@ -55,6 +56,13 @@ typedef struct _xf86RandR12Info {
>      Rotation rotation;          /* current mode */
>      Rotation supported_rotations;       /* driver supported */
>
> +    /* Compatibility with colormaps and XF86VidMode's gamma */
> +    int palette_red_size;
> +    int palette_green_size;
> +    int palette_blue_size;
> +    int palette_size;
> +    LOCO *palette;
> +
>      /* Used to wrap EnterVT so we can re-probe the outputs when a laptop unsuspends
>       * (actually, any time that we switch back into our VT).
>       *
> @@ -882,6 +890,9 @@ xf86RandR12Init(ScreenPtr pScreen)
>
>      randrp->maxX = randrp->maxY = 0;
>
> +    randrp->palette_size = 0;
> +    randrp->palette = NULL;
> +
>      dixSetPrivate(&pScreen->devPrivates, xf86RandR12Key, randrp);
>
>  #if RANDR_12_INTERFACE
> @@ -905,6 +916,7 @@ xf86RandR12CloseScreen(ScreenPtr pScreen)
>      pScreen->ConstrainCursorHarder = randrp->orig_ConstrainCursorHarder;
>  #endif
>
> +    free(randrp->palette);
>      free(randrp);
>  }
>
> @@ -1235,37 +1247,49 @@ xf86RandR12CrtcSet(ScreenPtr pScreen,
>      return xf86RandR12CrtcNotify(randr_crtc);
>  }
>
> -static Bool
> -xf86RandR12CrtcSetGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
> +static void
> +xf86RandR12CrtcComputeGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
>  {
> +    XF86RandRInfoPtr randrp = XF86RANDRINFO(pScreen);
>      xf86CrtcPtr crtc = randr_crtc->devPrivate;
> +    int gamma_slots;
> +    int i, j;
>
> -    if (crtc->funcs->gamma_set == NULL)
> -        return FALSE;
> +    gamma_slots = crtc->gamma_size / randrp->palette_red_size;
> +    for (i = 0; i < randrp->palette_red_size; i++) {
> +        j = i * gamma_slots;
> +        crtc->gamma_red[j] = randr_crtc->gammaRed[randrp->palette[i].red];
>
> -    if (!crtc->scrn->vtSema)
> -        return TRUE;
> +        while (++j < (i + 1) * gamma_slots)
> +            crtc->gamma_red[j] = crtc->gamma_red[j - 1];
> +    }
>
> -    /* Realloc local gamma if needed. */
> -    if (randr_crtc->gammaSize != crtc->gamma_size) {
> -        CARD16 *tmp_ptr;
> +    gamma_slots = crtc->gamma_size / randrp->palette_green_size;
> +    for (i = 0; i < randrp->palette_green_size; i++) {
> +        j = i * gamma_slots;
> +        crtc->gamma_green[j] = randr_crtc->gammaGreen[randrp->palette[i].green];
>
> -        tmp_ptr = reallocarray(crtc->gamma_red,
> -                               randr_crtc->gammaSize, 3 * sizeof(CARD16));
> -        if (!tmp_ptr)
> -            return FALSE;
> -        crtc->gamma_red = tmp_ptr;
> -        crtc->gamma_green = crtc->gamma_red + randr_crtc->gammaSize;
> -        crtc->gamma_blue = crtc->gamma_green + randr_crtc->gammaSize;
> +        while (++j < (i + 1) * gamma_slots)
> +            crtc->gamma_green[j] = crtc->gamma_green[j - 1];
>      }
>
> -    crtc->gamma_size = randr_crtc->gammaSize;
> -    memcpy(crtc->gamma_red, randr_crtc->gammaRed,
> -           crtc->gamma_size * sizeof(CARD16));
> -    memcpy(crtc->gamma_green, randr_crtc->gammaGreen,
> -           crtc->gamma_size * sizeof(CARD16));
> -    memcpy(crtc->gamma_blue, randr_crtc->gammaBlue,
> -           crtc->gamma_size * sizeof(CARD16));
> +    gamma_slots = crtc->gamma_size / randrp->palette_blue_size;
> +    for (i = 0; i < randrp->palette_blue_size; i++) {
> +        j = i * gamma_slots;
> +        crtc->gamma_blue[j] = randr_crtc->gammaBlue[randrp->palette[i].blue];
> +
> +        while (++j < (i + 1) * gamma_slots)
> +            crtc->gamma_blue[j] = crtc->gamma_blue[j - 1];
> +    }
> +}
> +
> +static void
> +xf86RandR12CrtcReloadGamma(RRCrtcPtr randr_crtc)
> +{
> +    xf86CrtcPtr crtc = randr_crtc->devPrivate;
> +
> +    if (!crtc->scrn->vtSema || !crtc->funcs->gamma_set)
> +        return;
>
>      /* Only set it when the crtc is actually running.
>       * Otherwise it will be set when it's activated.
> @@ -1273,6 +1297,21 @@ xf86RandR12CrtcSetGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
>      if (crtc->active)
>          crtc->funcs->gamma_set(crtc, crtc->gamma_red, crtc->gamma_green,
>                                 crtc->gamma_blue, crtc->gamma_size);
> +}
> +
> +static Bool
> +xf86RandR12CrtcSetGamma(ScreenPtr pScreen, RRCrtcPtr randr_crtc)
> +{
> +    XF86RandRInfoPtr randrp = XF86RANDRINFO(pScreen);
> +    xf86CrtcPtr crtc = randr_crtc->devPrivate;
> +
> +    if (crtc->funcs->gamma_set == NULL)
> +        return FALSE;
> +
> +    if (randrp->palette_size) {
> +        xf86RandR12CrtcComputeGamma(pScreen, randr_crtc);
> +        xf86RandR12CrtcReloadGamma(randr_crtc);
> +    }
>
>      return TRUE;
>  }
> @@ -1352,7 +1391,7 @@ xf86RandR12OutputInitGamma(xf86OutputPtr output)
>      return TRUE;
>  }
>
> -static Bool
> +Bool
>  xf86RandR12InitGamma(ScrnInfoPtr pScrn, unsigned gammaSize) {
>      xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
>      int o, c;
> @@ -1809,57 +1848,56 @@ xf86RandR13SetPanning(ScreenPtr pScreen,
>  }
>
>  /*
> - * Compatibility with XF86VidMode's gamma changer.  This necessarily clobbers
> - * any per-crtc setup.  You asked for it...
> + * Compatibility with colormaps and XF86VidMode's gamma
>   */
> -
> -static void
> -gamma_to_ramp(float gamma, CARD16 *ramp, int size)
> +void
> +xf86RandR12LoadPalette(ScrnInfoPtr pScrn, int numColors, int *indices,
> +                       LOCO *colors, VisualPtr pVisual)
>  {
> -    int i;
> +    ScreenPtr pScreen = pScrn->pScreen;
> +    XF86RandRInfoPtr randrp = XF86RANDRINFO(pScreen);
> +    xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
> +    int reds, greens, blues, index, palette_size;
> +    int c, i;
>
> -    for (i = 0; i < size; i++) {
> -        if (gamma == 1.0)
> -            ramp[i] = i | i << 8;
> -        else
> -            ramp[i] =
> -                (CARD16) (pow((double) i / (double) (size - 1), 1. / gamma)
> -                          * (double) (size - 1) * 257);
> +    if (pVisual->class == TrueColor || pVisual->class == DirectColor) {
> +        reds = (pVisual->redMask >> pVisual->offsetRed) + 1;
> +        greens = (pVisual->greenMask >> pVisual->offsetGreen) + 1;
> +        blues = (pVisual->blueMask >> pVisual->offsetBlue) + 1;
> +    } else {
> +        reds = greens = blues = pVisual->ColormapEntries;
>      }
> -}
> -
> -static int
> -xf86RandR12ChangeGamma(ScrnInfoPtr pScrn, Gamma gamma)
> -{
> -    CARD16 *points, *red, *green, *blue;
> -    RRCrtcPtr crtc = xf86CompatRRCrtc(pScrn);
> -    int size;
> -
> -    if (!crtc)
> -        return Success;
> -
> -    size = max(0, crtc->gammaSize);
> -    if (!size)
> -        return Success;
>
> -    points = calloc(size, 3 * sizeof(CARD16));
> -    if (!points)
> -        return BadAlloc;
> +    palette_size = max(reds, max(greens, blues));
>
> -    red = points;
> -    green = points + size;
> -    blue = points + 2 * size;
> -
> -    gamma_to_ramp(gamma.red, red, size);
> -    gamma_to_ramp(gamma.green, green, size);
> -    gamma_to_ramp(gamma.blue, blue, size);
> -    RRCrtcGammaSet(crtc, red, green, blue);
> -
> -    free(points);
> +    for (c = 0; c < config->num_crtc; c++) {
> +        xf86CrtcPtr crtc = config->crtc[c];
> +        RRCrtcPtr randr_crtc = crtc->randr_crtc;
>
> -    pScrn->gamma = gamma;
> +        if (randrp->palette_size != palette_size) {
> +            randrp->palette =
> +                reallocarray(randrp->palette, palette_size,
> +                             sizeof(colors[0]));
> +            randrp->palette_size = palette_size;
> +        }
> +        randrp->palette_red_size = reds;
> +        randrp->palette_green_size = greens;
> +        randrp->palette_blue_size = blues;
> +
> +        for (i = 0; i < numColors; i++) {
> +            index = indices[i];
> +
> +            if (index < reds)
> +                randrp->palette[index].red = colors[index].red;
> +            if (index < greens)
> +                randrp->palette[index].green = colors[index].green;
> +            if (index < blues)
> +                randrp->palette[index].blue = colors[index].blue;
> +        }
>
> -    return Success;
> +        xf86RandR12CrtcComputeGamma(pScreen, randr_crtc);
> +        xf86RandR12CrtcReloadGamma(randr_crtc);
> +    }
>  }
>
>  static Bool
> @@ -1882,7 +1920,7 @@ xf86RandR12EnterVT(ScrnInfoPtr pScrn)
>
>      /* reload gamma */
>      for (i = 0; i < rp->numCrtcs; i++)
> -        xf86RandR12CrtcSetGamma(pScreen, rp->crtcs[i]);
> +        xf86RandR12CrtcReloadGamma(rp->crtcs[i]);
>
>      return RRGetInfo(pScreen, TRUE);    /* force a re-probe of outputs and notify clients about changes */
>  }
> @@ -2061,7 +2099,6 @@ xf86RandR12Init12(ScreenPtr pScreen)
>      rp->rrProviderDestroy = xf86RandR14ProviderDestroy;
>
>      pScrn->PointerMoved = xf86RandR12PointerMoved;
> -    pScrn->ChangeGamma = xf86RandR12ChangeGamma;
>
>      randrp->orig_EnterVT = pScrn->EnterVT;
>      pScrn->EnterVT = xf86RandR12EnterVT;
> diff --git a/hw/xfree86/modes/xf86RandR12.h b/hw/xfree86/modes/xf86RandR12.h
> index e603799..31aaaaf 100644
> --- a/hw/xfree86/modes/xf86RandR12.h
> +++ b/hw/xfree86/modes/xf86RandR12.h
> @@ -40,4 +40,9 @@ extern _X_EXPORT void xf86RandR12GetOriginalVirtualSize(ScrnInfoPtr pScrn,
>  extern _X_EXPORT Bool xf86RandR12PreInit(ScrnInfoPtr pScrn);
>  extern _X_EXPORT void xf86RandR12TellChanged(ScreenPtr pScreen);
>
> +extern void xf86RandR12LoadPalette(ScrnInfoPtr pScrn, int numColors,
> +                                   int *indices, LOCO *colors,
> +                                   VisualPtr pVisual);
> +extern Bool xf86RandR12InitGamma(ScrnInfoPtr pScrn, unsigned gammaSize);
> +
>  #endif                          /* _XF86_RANDR_H_ */
> --
> 2.8.1
>
> _______________________________________________
> 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


More information about the xorg-devel mailing list