[PATCH 1/4] modesetting: Implement 32->24 bpp conversion in shadow update

Alex Deucher alexdeucher at gmail.com
Wed Jul 22 13:25:31 PDT 2015


On Wed, Jul 22, 2015 at 12:14 PM, Adam Jackson <ajax at redhat.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> 24bpp front buffers tend to be the least well tested path for client
> rendering.  On the qemu cirrus emulation, and on some Matrox G200 server
> chips, the hardware can't do 32bpp at all.  It's better to just allocate
> a 32bpp shadow and downconvert in the upload hook than expose a funky
> pixmap format to clients.
>
> [ajax: Ported from RHEL and separate modesetting driver, lifted kbpp
> into the drmmode struct, cleaned up commit message, fixed 16bpp]
>
> Reviewed-by: Adam Jackson <ajax at redhat.com>
> Signed-off-by: Dave Airlied <airlied at redhat.com>

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

> ---
>  hw/xfree86/drivers/modesetting/Makefile.am       |   2 +
>  hw/xfree86/drivers/modesetting/driver.c          |  41 ++++---
>  hw/xfree86/drivers/modesetting/drmmode_display.c |  24 ++--
>  hw/xfree86/drivers/modesetting/drmmode_display.h |   2 +
>  hw/xfree86/drivers/modesetting/sh3224.c          | 140 +++++++++++++++++++++++
>  hw/xfree86/drivers/modesetting/sh3224.h          |   7 ++
>  6 files changed, 193 insertions(+), 23 deletions(-)
>  create mode 100644 hw/xfree86/drivers/modesetting/sh3224.c
>  create mode 100644 hw/xfree86/drivers/modesetting/sh3224.h
>
> diff --git a/hw/xfree86/drivers/modesetting/Makefile.am b/hw/xfree86/drivers/modesetting/Makefile.am
> index 82c4f2f..ca7e05a 100644
> --- a/hw/xfree86/drivers/modesetting/Makefile.am
> +++ b/hw/xfree86/drivers/modesetting/Makefile.am
> @@ -51,6 +51,8 @@ modesetting_drv_la_SOURCES = \
>          dumb_bo.c \
>          dumb_bo.h \
>          present.c \
> +        sh3224.c \
> +        sh3224.h \
>          vblank.c \
>          $(NULL)
>
> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
> index 324b8bd..68a865f 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -60,6 +60,7 @@
>  #endif
>
>  #include "driver.h"
> +#include "sh3224.h"
>
>  static void AdjustFrame(ScrnInfoPtr pScrn, int x, int y);
>  static Bool CloseScreen(ScreenPtr pScreen);
> @@ -770,10 +771,16 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>      }
>  #endif
>      drmmode_get_default_bpp(pScrn, &ms->drmmode, &defaultdepth, &defaultbpp);
> -    if (defaultdepth == 24 && defaultbpp == 24)
> -        bppflags = SupportConvert32to24 | Support24bppFb;
> -    else
> -        bppflags = PreferConvert24to32 | SupportConvert24to32 | Support32bppFb;
> +    if (defaultdepth == 24 && defaultbpp == 24) {
> +       ms->drmmode.force_24_32 = TRUE;
> +       ms->drmmode.kbpp = 24;
> +       xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> +                  "Using 24bpp hw front buffer with 32bpp shadow\n");
> +       defaultbpp = 32;
> +    } else {
> +       ms->drmmode.kbpp = defaultbpp;
> +    }
> +    bppflags = PreferConvert24to32 | SupportConvert24to32 | Support32bppFb;
>
>      if (!xf86SetDepthBpp
>          (pScrn, defaultdepth, defaultdepth, defaultbpp, bppflags))
> @@ -827,18 +834,24 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>      } else {
>          Bool prefer_shadow = TRUE;
>
> -        ret = drmGetCap(ms->fd, DRM_CAP_DUMB_PREFER_SHADOW, &value);
> -        if (!ret) {
> -            prefer_shadow = !!value;
> -        }
> +       if (ms->drmmode.force_24_32) {
> +           prefer_shadow = TRUE;
> +           ms->drmmode.shadow_enable = TRUE;
> +       } else {
> +           ret = drmGetCap(ms->fd, DRM_CAP_DUMB_PREFER_SHADOW, &value);
> +           if (!ret) {
> +               prefer_shadow = !!value;
> +           }
>
> -        ms->drmmode.shadow_enable = xf86ReturnOptValBool(ms->Options,
> -                                                         OPTION_SHADOW_FB,
> -                                                         prefer_shadow);
> +           ms->drmmode.shadow_enable = xf86ReturnOptValBool(ms->Options,
> +                                                            OPTION_SHADOW_FB,
> +                                                            prefer_shadow);
> +       }
>
>          xf86DrvMsg(pScrn->scrnIndex, X_INFO,
>                     "ShadowFB: preferred %s, enabled %s\n",
>                     prefer_shadow ? "YES" : "NO",
> +                  ms->drmmode.force_24_32 ? "FORCE" :
>                     ms->drmmode.shadow_enable ? "YES" : "NO");
>
>          ms->drmmode.pageflip = FALSE;
> @@ -894,7 +907,7 @@ msShadowWindow(ScreenPtr screen, CARD32 row, CARD32 offset, int mode,
>      modesettingPtr ms = modesettingPTR(pScrn);
>      int stride;
>
> -    stride = (pScrn->displayWidth * pScrn->bitsPerPixel) / 8;
> +    stride = (pScrn->displayWidth * ms->drmmode.kbpp) / 8;
>      *size = stride;
>
>      return ((uint8_t *) ms->drmmode.front_bo.dumb->ptr + row * stride + offset);
> @@ -915,6 +928,7 @@ CreateScreenResources(ScreenPtr pScreen)
>      Bool ret;
>      void *pixels = NULL;
>      int err;
> +    Bool use_ms_shadow = ms->drmmode.force_24_32 && pScrn->bitsPerPixel == 32;
>
>      pScreen->CreateScreenResources = ms->createScreenResources;
>      ret = pScreen->CreateScreenResources(pScreen);
> @@ -946,7 +960,8 @@ CreateScreenResources(ScreenPtr pScreen)
>          FatalError("Couldn't adjust screen pixmap\n");
>
>      if (ms->drmmode.shadow_enable) {
> -        if (!shadowAdd(pScreen, rootPixmap, msUpdatePacked,
> +        if (!shadowAdd(pScreen, rootPixmap,
> +                      use_ms_shadow ? ms_shadowUpdate32to24 : msUpdatePacked,
>                         msShadowWindow, 0, 0))
>              return FALSE;
>      }
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 6a13660..839f34f 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -320,7 +320,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>      if (drmmode->fb_id == 0) {
>          ret = drmModeAddFB(drmmode->fd,
>                             pScrn->virtualX, height,
> -                           pScrn->depth, pScrn->bitsPerPixel,
> +                           pScrn->depth, drmmode->kbpp,
>                             drmmode_bo_get_pitch(&drmmode->front_bo),
>                             drmmode_bo_get_handle(&drmmode->front_bo),
>                             &drmmode->fb_id);
> @@ -1589,6 +1589,7 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>      uint32_t old_fb_id;
>      int i, pitch, old_width, old_height, old_pitch;
>      int cpp = (scrn->bitsPerPixel + 7) / 8;
> +    int kcpp = (drmmode->kbpp + 7) / 8;
>      PixmapPtr ppix = screen->GetScreenPixmap(screen);
>      void *new_pixels = NULL;
>
> @@ -1610,17 +1611,17 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>      old_front = drmmode->front_bo;
>
>      if (!drmmode_create_bo(drmmode, &drmmode->front_bo,
> -                           width, height, scrn->bitsPerPixel))
> +                           width, height, drmmode->kbpp))
>          goto fail;
>
>      pitch = drmmode_bo_get_pitch(&drmmode->front_bo);
>
>      scrn->virtualX = width;
>      scrn->virtualY = height;
> -    scrn->displayWidth = pitch / cpp;
> +    scrn->displayWidth = pitch / kcpp;
>
>      ret = drmModeAddFB(drmmode->fd, width, height, scrn->depth,
> -                       scrn->bitsPerPixel, pitch,
> +                       drmmode->kbpp, pitch,
>                         drmmode_bo_get_handle(&drmmode->front_bo),
>                         &drmmode->fb_id);
>      if (ret)
> @@ -1633,8 +1634,7 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>      }
>
>      if (drmmode->shadow_enable) {
> -        uint32_t size = scrn->displayWidth * scrn->virtualY *
> -            ((scrn->bitsPerPixel + 7) >> 3);
> +        uint32_t size = scrn->displayWidth * scrn->virtualY * cpp;
>          new_pixels = calloc(1, size);
>          if (new_pixels == NULL)
>              goto fail;
> @@ -1642,7 +1642,8 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>          drmmode->shadow_fb = new_pixels;
>      }
>
> -    screen->ModifyPixmapHeader(ppix, width, height, -1, -1, pitch, new_pixels);
> +    screen->ModifyPixmapHeader(ppix, width, height, -1, -1,
> +                              scrn->displayWidth * cpp, new_pixels);
>
>      if (!drmmode_glamor_handle_new_screen_pixmap(drmmode))
>          goto fail;
> @@ -1669,7 +1670,7 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>      drmmode->front_bo = old_front;
>      scrn->virtualX = old_width;
>      scrn->virtualY = old_height;
> -    scrn->displayWidth = old_pitch / cpp;
> +    scrn->displayWidth = old_pitch / kcpp;
>      drmmode->fb_id = old_fb_id;
>
>      return FALSE;
> @@ -1698,7 +1699,10 @@ drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
>      xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
>
>      drmmode->scrn = pScrn;
> -    drmmode->cpp = cpp;
> +    if (drmmode->force_24_32 && cpp == 4)
> +       drmmode->cpp = 3;
> +    else
> +       drmmode->cpp = cpp;
>      mode_res = drmModeGetResources(drmmode->fd);
>      if (!mode_res)
>          return FALSE;
> @@ -2013,7 +2017,7 @@ drmmode_create_initial_bos(ScrnInfoPtr pScrn, drmmode_ptr drmmode)
>      xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
>      int width;
>      int height;
> -    int bpp = pScrn->bitsPerPixel;
> +    int bpp = ms->drmmode.kbpp;
>      int i;
>      int cpp = (bpp + 7) / 8;
>
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
> index fe363c5..fcb1efa 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.h
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
> @@ -48,6 +48,7 @@ typedef struct {
>      unsigned fb_id;
>      drmModeFBPtr mode_fb;
>      int cpp;
> +    int        kbpp;
>      ScrnInfoPtr scrn;
>
>      struct gbm_device *gbm;
> @@ -64,6 +65,7 @@ typedef struct {
>      Bool shadow_enable;
>      /** Is Option "PageFlip" enabled? */
>      Bool pageflip;
> +    Bool force_24_32;
>      void *shadow_fb;
>
>      /**
> diff --git a/hw/xfree86/drivers/modesetting/sh3224.c b/hw/xfree86/drivers/modesetting/sh3224.c
> new file mode 100644
> index 0000000..a64a103
> --- /dev/null
> +++ b/hw/xfree86/drivers/modesetting/sh3224.c
> @@ -0,0 +1,140 @@
> +/*
> + *
> + * Copyright © 2000 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that
> + * copyright notice and this permission notice appear in supporting
> + * documentation, and that the name of Keith Packard not be used in
> + * advertising or publicity pertaining to distribution of the software without
> + * specific, written prior permission.  Keith Packard makes no
> + * representations about the suitability of this software for any purpose.  It
> + * is provided "as is" without express or implied warranty.
> + *
> + * KEITH PACKARD DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL KEITH PACKARD BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> + * PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#ifdef HAVE_DIX_CONFIG_H
> +#include "dix-config.h"
> +#endif
> +
> +#include    "shadow.h"
> +#include    "fb.h"
> +
> +#include "sh3224.h"
> +#define Get8(a)        ((CARD32) READ(a))
> +
> +#if BITMAP_BIT_ORDER == MSBFirst
> +#define Get24(a)    ((Get8(a) << 16) | (Get8((a)+1) << 8) | Get8((a)+2))
> +#define Put24(a,p)  ((WRITE((a+0), (CARD8) ((p) >> 16))), \
> +                    (WRITE((a+1), (CARD8) ((p) >> 8))), \
> +                    (WRITE((a+2), (CARD8) (p))))
> +#else
> +#define Get24(a)    (Get8(a) | (Get8((a)+1) << 8) | (Get8((a)+2)<<16))
> +#define Put24(a,p)  ((WRITE((a+0), (CARD8) (p))), \
> +                    (WRITE((a+1), (CARD8) ((p) >> 8))), \
> +                    (WRITE((a+2), (CARD8) ((p) >> 16))))
> +#endif
> +
> +static void
> +sh24_32BltLine(CARD8 *srcLine,
> +               CARD8 *dstLine,
> +               int width)
> +{
> +    CARD32 *src;
> +    CARD8 *dst;
> +    int w;
> +    CARD32 pixel;
> +
> +    src = (CARD32 *) srcLine;
> +    dst = dstLine;
> +    w = width;
> +
> +    while (((long)dst & 3) && w) {
> +       w--;
> +       pixel = READ(src++);
> +       Put24(dst, pixel);
> +       dst += 3;
> +    }
> +    /* Do four aligned pixels at a time */
> +    while (w >= 4) {
> +       CARD32 s0, s1;
> +
> +       s0 = READ(src++);
> +       s1 = READ(src++);
> +#if BITMAP_BIT_ORDER == LSBFirst
> +       WRITE((CARD32 *) dst, (s0 & 0xffffff) | (s1 << 24));
> +#else
> +       WRITE((CARD32 *) dst, (s0 << 8) | ((s1 & 0xffffff) >> 16));
> +#endif
> +       s0 = READ(src++);
> +#if BITMAP_BIT_ORDER == LSBFirst
> +       WRITE((CARD32 *) (dst + 4),
> +             ((s1 & 0xffffff) >> 8) | (s0 << 16));
> +#else
> +       WRITE((CARD32 *) (dst + 4),
> +             (s1 << 16) | ((s0 & 0xffffff) >> 8));
> +#endif
> +       s1 = READ(src++);
> +#if BITMAP_BIT_ORDER == LSBFirst
> +       WRITE((CARD32 *) (dst + 8),
> +             ((s0 & 0xffffff) >> 16) | (s1 << 8));
> +#else
> +       WRITE((CARD32 *) (dst + 8), (s0 << 24) | (s1 & 0xffffff));
> +#endif
> +       dst += 12;
> +       w -= 4;
> +    }
> +    while (w--) {
> +       pixel = READ(src++);
> +       Put24(dst, pixel);
> +       dst += 3;
> +    }
> +}
> +
> +void
> +ms_shadowUpdate32to24(ScreenPtr pScreen, shadowBufPtr pBuf)
> +{
> +    RegionPtr damage = shadowDamage(pBuf);
> +    PixmapPtr pShadow = pBuf->pPixmap;
> +    int nbox = RegionNumRects(damage);
> +    BoxPtr pbox = RegionRects(damage);
> +    FbStride shaStride;
> +    int shaBpp;
> +    _X_UNUSED int shaXoff, shaYoff;
> +    int x, y, w, h;
> +    CARD32 winSize;
> +    FbBits *shaBase, *shaLine;
> +    CARD8 *winBase = NULL, *winLine;
> +
> +    fbGetDrawable(&pShadow->drawable, shaBase, shaStride, shaBpp, shaXoff,
> +                  shaYoff);
> +
> +    /* just get the initial window base + stride */
> +    winBase = (*pBuf->window)(pScreen, 0, 0, SHADOW_WINDOW_WRITE,
> +                             &winSize, pBuf->closure);
> +
> +    while (nbox--) {
> +        x = pbox->x1;
> +        y = pbox->y1;
> +        w = pbox->x2 - pbox->x1;
> +        h = pbox->y2 - pbox->y1;
> +
> +       winLine = winBase + y * winSize + (x * 3);
> +        shaLine = shaBase + y * shaStride + ((x * shaBpp) >> FB_SHIFT);
> +
> +        while (h--) {
> +           sh24_32BltLine((CARD8 *)shaLine, (CARD8 *)winLine, w);
> +           winLine += winSize;
> +            shaLine += shaStride;
> +        }
> +        pbox++;
> +    }
> +}
> diff --git a/hw/xfree86/drivers/modesetting/sh3224.h b/hw/xfree86/drivers/modesetting/sh3224.h
> new file mode 100644
> index 0000000..fc301f9
> --- /dev/null
> +++ b/hw/xfree86/drivers/modesetting/sh3224.h
> @@ -0,0 +1,7 @@
> +#ifndef SH3224_H
> +#define SH3224_H
> +
> +void
> +ms_shadowUpdate32to24(ScreenPtr pScreen, shadowBufPtr pBuf);
> +
> +#endif
> --
> 2.4.3
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel


More information about the xorg-devel mailing list