[PATCH] modesetting: 24bpp are too confusing, shadow our way out.

Dave Airlie airlied at gmail.com
Sun Jan 19 22:33:12 PST 2014


From: Dave Airlie <airlied at redhat.com>

So we have 2 places where gpus with 24bpp frontbuffers are still
in use, cirrus (in qemu) and some early variants of the mgag200
server chips.

Currently we seem to get a lot of broken rendering in qt, mesa, gnome
if we expose the frontbuffer as 24bpp, nobody seems to test this
anymore upstream so client side apps are constantly broken, so lets
just make -modesetting expose a 24/32 shadow frontbuffer, and use
the shadow update hook to convert down to the actual 24bpp front,
this might be slower, but its correct and really anyone that wants
this already has enough problems.

Signed-off-by: Dave Airlie <airlied at redhat.com>
---
 src/Makefile.am       |   3 +-
 src/driver.c          |  37 ++++++++-----
 src/drmmode_display.c |  34 ++++++------
 src/drmmode_display.h |   1 +
 src/sh3224.c          | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/sh3224.h          |   7 +++
 6 files changed, 193 insertions(+), 29 deletions(-)
 create mode 100644 src/sh3224.c
 create mode 100644 src/sh3224.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 3cc4624..6d31fba 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -36,4 +36,5 @@ modesetting_drv_la_SOURCES = \
 	 driver.c \
 	 driver.h \
 	 drmmode_display.c \
-	 drmmode_display.h
+	 drmmode_display.h \
+	 sh3224.c
diff --git a/src/driver.c b/src/driver.c
index b84624e..210bf27 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -62,6 +62,7 @@
 #include "compat-api.h"
 #include "driver.h"
 
+#include "sh3224.h"
 static void AdjustFrame(ADJUST_FRAME_ARGS_DECL);
 static Bool CloseScreen(CLOSE_SCREEN_ARGS_DECL);
 static Bool EnterVT(VT_FUNC_ARGS_DECL);
@@ -663,9 +664,13 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 #endif
 #endif
     drmmode_get_default_bpp(pScrn, &ms->drmmode, &defaultdepth, &defaultbpp);
-    if (defaultdepth == 24 && defaultbpp == 24)
-	    bppflags = SupportConvert32to24 | Support24bppFb;
-    else
+    if (defaultdepth == 24 && defaultbpp == 24) {
+	ms->drmmode.force_24_32 = TRUE;
+	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+		   "No native 24bpp using shadow to workaround\n");
+	defaultbpp = 32;
+	bppflags = PreferConvert24to32 | SupportConvert24to32 | Support32bppFb;
+    } else
 	    bppflags = PreferConvert24to32 | SupportConvert24to32 | Support32bppFb;
     
     if (!xf86SetDepthBpp
@@ -701,14 +706,19 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 	ms->drmmode.sw_cursor = 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.shadow_enable ? "YES" : "NO");
+    xf86DrvMsg(pScrn->scrnIndex, X_INFO, "ShadowFB: preferred %s, enabled %s\n", prefer_shadow ? "YES" : "NO", ms->drmmode.shadow_enable ? (ms->drmmode.force_24_32 ? "FORCE" : "YES") : "NO");
     if (drmmode_pre_init(pScrn, &ms->drmmode, pScrn->bitsPerPixel / 8) == FALSE) {
 	xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "KMS setup failed\n");
 	goto fail;
@@ -758,8 +768,9 @@ msShadowWindow(ScreenPtr screen, CARD32 row, CARD32 offset, int mode,
     ScrnInfoPtr pScrn = xf86ScreenToScrn(screen);
     modesettingPtr ms = modesettingPTR(pScrn);
     int stride;
+    int kernbpp = (ms->drmmode.force_24_32 && pScrn->bitsPerPixel == 32) ? 24 : 32;
 
-    stride = (pScrn->displayWidth * pScrn->bitsPerPixel) / 8;
+    stride = (pScrn->displayWidth * kernbpp) / 8;
     *size = stride;
 
     return ((uint8_t *)ms->drmmode.front_bo->ptr + row * stride + offset);
@@ -773,10 +784,11 @@ CreateScreenResources(ScreenPtr pScreen)
     PixmapPtr rootPixmap;
     Bool ret;
     void *pixels;
+    Bool use_ms_shadow = ms->drmmode.force_24_32 && pScrn->bitsPerPixel == 32;
     pScreen->CreateScreenResources = ms->createScreenResources;
     ret = pScreen->CreateScreenResources(pScreen);
     pScreen->CreateScreenResources = CreateScreenResources;
-
+    
     if (!drmmode_set_desired_modes(pScrn, &ms->drmmode))
       return FALSE;
 
@@ -797,7 +809,7 @@ CreateScreenResources(ScreenPtr pScreen)
 	FatalError("Couldn't adjust screen pixmap\n");
 
     if (ms->drmmode.shadow_enable) {
-	if (!shadowAdd(pScreen, rootPixmap, shadowUpdatePackedWeak(),
+	if (!shadowAdd(pScreen, rootPixmap, use_ms_shadow ? ms_shadowUpdate32to24 : shadowUpdatePackedWeak(),
 		       msShadowWindow, 0, 0))
 	    return FALSE;
     }
@@ -852,7 +864,6 @@ ScreenInit(SCREEN_INIT_ARGS_DECL)
     modesettingPtr ms = modesettingPTR(pScrn);
     VisualPtr visual;
     int ret;
-
     pScrn->pScreen = pScreen;
 
     ret = drmSetMaster(ms->fd);
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 25641ce..324b527 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -314,13 +314,13 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	uint32_t fb_id;
 	drmModeModeInfo kmode;
 	int height;
-
+	int bpp = (drmmode->force_24_32 && pScrn->bitsPerPixel == 32) ? 24 : 32;
 	height = pScrn->virtualY;
 
 	if (drmmode->fb_id == 0) {
 		ret = drmModeAddFB(drmmode->fd,
 				   pScrn->virtualX, height,
-                                   pScrn->depth, pScrn->bitsPerPixel,
+                                   pScrn->depth, bpp,
 				   drmmode->front_bo->pitch,
 				   drmmode->front_bo->handle,
                                    &drmmode->fb_id);
@@ -1142,7 +1142,8 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	int cpp = (scrn->bitsPerPixel + 7) / 8;
 	PixmapPtr ppix = screen->GetScreenPixmap(screen);
 	void *new_pixels;
-
+	int kernbpp = (drmmode->force_24_32 && scrn->bitsPerPixel == 32) ? 24 : 32;
+	int kerncpp = (kernbpp + 7) / 8;
 	if (scrn->virtualX == width && scrn->virtualY == height)
 		return TRUE;
 
@@ -1156,7 +1157,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	old_fb_id = drmmode->fb_id;
 	old_front = drmmode->front_bo;
 
-	drmmode->front_bo = dumb_bo_create(drmmode->fd, width, height, scrn->bitsPerPixel);
+	drmmode->front_bo = dumb_bo_create(drmmode->fd, width, height, kernbpp);
 	if (!drmmode->front_bo)
 		goto fail;
 
@@ -1164,10 +1165,10 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 
 	scrn->virtualX = width;
 	scrn->virtualY = height;
-	scrn->displayWidth = pitch / cpp;
+	scrn->displayWidth = pitch / kerncpp;
 
 	ret = drmModeAddFB(drmmode->fd, width, height, scrn->depth,
-			   scrn->bitsPerPixel, pitch,
+			   kernbpp, pitch,
 			   drmmode->front_bo->handle,
 			   &drmmode->fb_id);
 	if (ret)
@@ -1190,7 +1191,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 		free(drmmode->shadow_fb);
 		drmmode->shadow_fb = new_shadow;
 		screen->ModifyPixmapHeader(ppix, width, height, -1, -1,
-					   pitch, drmmode->shadow_fb);
+					   scrn->displayWidth * cpp, drmmode->shadow_fb);
 	}
 
 #if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,9,99,1,0)
@@ -1220,7 +1221,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 / kerncpp;
 	drmmode->fb_id = old_fb_id;
 
 	return FALSE;
@@ -1246,7 +1247,10 @@ Bool 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;
 	drmmode->mode_res = drmModeGetResources(drmmode->fd);
 	if (!drmmode->mode_res)
 		return FALSE;
@@ -1488,24 +1492,24 @@ Bool 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 kernbpp = (drmmode->force_24_32 && pScrn->bitsPerPixel == 32) ? 24 : 32;
 	int i;
-	int cpp = (bpp + 7) / 8;
+	int kerncpp = (kernbpp + 7) / 8;
 
 	width = pScrn->virtualX;
 	height = pScrn->virtualY;
 
-	drmmode->front_bo = dumb_bo_create(drmmode->fd, width, height, bpp);
+	drmmode->front_bo = dumb_bo_create(drmmode->fd, width, height, kernbpp);
 	if (!drmmode->front_bo)
 		return FALSE;
-	pScrn->displayWidth = drmmode->front_bo->pitch / cpp;
+	pScrn->displayWidth = drmmode->front_bo->pitch / kerncpp;
 
 	width = height = 64;
-	bpp = 32;
+	kernbpp = 32;
 	for (i = 0; i < xf86_config->num_crtc; i++) {
 		xf86CrtcPtr crtc = xf86_config->crtc[i];
 		drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
-		drmmode_crtc->cursor_bo = dumb_bo_create(drmmode->fd, width, height, bpp);
+		drmmode_crtc->cursor_bo = dumb_bo_create(drmmode->fd, width, height, kernbpp);
 	}
 	return TRUE;
 }
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 745c484..fe05e90 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -67,6 +67,7 @@ typedef struct {
     Bool shadow_enable;
     void *shadow_fb;
 
+    Bool force_24_32;
 #ifdef HAVE_SCREEN_SPECIFIC_PRIVATE_KEYS
     DevPrivateKeyRec pixmapPrivateKeyRec;
 #endif
diff --git a/src/sh3224.c b/src/sh3224.c
new file mode 100644
index 0000000..cb8accd
--- /dev/null
+++ b/src/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_CONFIG_H
+#include "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/src/sh3224.h b/src/sh3224.h
new file mode 100644
index 0000000..fc301f9
--- /dev/null
+++ b/src/sh3224.h
@@ -0,0 +1,7 @@
+#ifndef SH3224_H
+#define SH3224_H
+
+void
+ms_shadowUpdate32to24(ScreenPtr pScreen, shadowBufPtr pBuf);
+
+#endif
-- 
1.8.3.1



More information about the xorg-devel mailing list