Current tinderbox regressions (video drivers)

[Michel Dänzer]

On Thu, 2009-02-26 at 10:36 +1000, Dave Airlie wrote:
> > > On Feb 24, 09 19:46:24 +0100, Michel Dänzer wrote:
> > > > On Tue, 2009-02-24 at 13:18 -0500, Chris Ball wrote:
> > > > >    > EXA: Handle separate alpha maps properly in Composite fallback
> > > > >    http://cgit.freedesktop.org/xorg/xserver/commit/?id=170cf1270dff38d3cce7f5ba5b940d1c0d70eff5
> > > > > Driver maintainers:  from this commit onwards, EXA requires you to pass
> > > > > in -DEXA_DRIVER_KNOWN_MAJOR=3, else it'll fail to build.  If you use
> > > > > Prepare/FinishAccess, UploadToScratch or ExaOffscreenSwap*, you'll need
> > > > > to make other changes too.
> 
> What was wrong with defining a new flag that the driver passes back into
> EXA to say it supports the new bits?

Nothing wrong, I just couldn't see how it can be done, and nobody made
any other suggestions when I sent out the change for review.

In hindsight, I agree the below should work. How does it look?


>From e4ae97126943dcbea92252bd9785742a7c68caa4 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Michel=20D=C3=A4nzer?= <daenzer at vmware.com>
Date: Thu, 26 Feb 2009 11:18:07 +0100
Subject: [PATCH] EXA: Handle separate alpha maps properly in Composite fallback, take two.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Preserve the EXA ABI by introducing a new driver flag EXA_SUPPORTS_PREPARE_AUX.
If the driver doesn't set this flag, we have to assume any Prepare/FinishAccess
driver hooks can't handle the EXA_PREPARE_AUX* indices, so we move out such
pixmaps at PrepareAccess time.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=18710 .

Signed-off-by: Michel Dänzer <daenzer at vmware.com>
---
 exa/exa.c         |   13 +++++++++++++
 exa/exa.h         |   14 ++++++++++++++
 exa/exa_unaccel.c |   25 ++++++++++++++++++++++++-
 3 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/exa/exa.c b/exa/exa.c
index a647699..5425f90 100644
--- a/exa/exa.c
+++ b/exa/exa.c
@@ -538,6 +538,12 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
     if (pExaScr->info->PrepareAccess == NULL)
 	return;
 
+    if (index >= EXA_PREPARE_AUX0 &&
+	!(pExaScr->info->flags & EXA_SUPPORTS_PREPARE_AUX)) {
+	exaMoveOutPixmap (pPixmap);
+	return;
+    }
+
     if (!(*pExaScr->info->PrepareAccess) (pPixmap, index)) {
 	ExaPixmapPriv (pPixmap);
 	if (pExaPixmap->score == EXA_PIXMAP_SCORE_PINNED)
@@ -597,6 +603,13 @@ exaFinishAccess(DrawablePtr pDrawable, int index)
     if (!exaPixmapIsOffscreen (pPixmap))
 	return;
 
+    if (index >= EXA_PREPARE_AUX0 &&
+	!(pExaScr->info->flags & EXA_SUPPORTS_PREPARE_AUX)) {
+	ErrorF("EXA bug: Trying to call driver FinishAccess hook with "
+	       "unsupported index EXA_PREPARE_AUX*\n");
+	return;
+    }
+
     (*pExaScr->info->FinishAccess) (pPixmap, index);
 }
 
diff --git a/exa/exa.h b/exa/exa.h
index 21a0f1a..12959e1 100644
--- a/exa/exa.h
+++ b/exa/exa.h
@@ -672,6 +672,13 @@ typedef struct _ExaDriver {
 	 * from.
 	 */
 	#define EXA_PREPARE_MASK	2
+	/**
+	 * EXA_PREPARE_AUX* are additional indices for other purposes, e.g.
+	 * separate alpha maps with Composite operations.
+	 */
+	#define EXA_PREPARE_AUX0	3
+	#define EXA_PREPARE_AUX1	4
+	#define EXA_PREPARE_AUX2	5
 	/** @} */
 
     /**
@@ -742,6 +749,13 @@ typedef struct _ExaDriver {
  */
 #define EXA_HANDLES_PIXMAPS             (1 << 3)
 
+/**
+ * EXA_SUPPORTS_PREPARE_AUX indicates to EXA that the driver can handle the
+ * EXA_PREPARE_AUX* indices in the Prepare/FinishAccess hooks. If there are no
+ * such hooks, this flag has no effect.
+ */
+#define EXA_SUPPORTS_PREPARE_AUX        (1 << 4)
+
 /** @} */
 
 /* in exa.c */
diff --git a/exa/exa_unaccel.c b/exa/exa_unaccel.c
index c821f0d..9a0b0e5 100644
--- a/exa/exa_unaccel.c
+++ b/exa/exa_unaccel.c
@@ -392,6 +392,15 @@ ExaCheckComposite (CARD8      op,
 
     REGION_NULL(pScreen, &region);
 
+    /* We need to prepare access to any separate alpha maps first, in case the
+     * driver doesn't support EXA_PREPARE_AUX*, in which case EXA_PREPARE_SRC
+     * may be used for moving them out.
+     */
+    if (pSrc->alphaMap && pSrc->alphaMap->pDrawable)
+	exaPrepareAccess(pSrc->alphaMap->pDrawable, EXA_PREPARE_AUX2);
+    if (pMask && pMask->alphaMap && pMask->alphaMap->pDrawable)
+	exaPrepareAccess(pMask->alphaMap->pDrawable, EXA_PREPARE_AUX1);
+
     if (!exaOpReadsDestination(op)) {
 	if (!miComputeCompositeRegion (&region, pSrc, pMask, pDst,
 				       xSrc, ySrc, xMask, yMask, xDst, yDst,
@@ -404,9 +413,17 @@ ExaCheckComposite (CARD8      op,
 
 	REGION_TRANSLATE(pScreen, &region, xoff, yoff);
 
+	if (pDst->alphaMap && pDst->alphaMap->pDrawable)
+	    exaPrepareAccessReg(pDst->alphaMap->pDrawable, EXA_PREPARE_AUX0,
+				&region);
+
 	exaPrepareAccessReg (pDst->pDrawable, EXA_PREPARE_DEST, &region);
-    } else
+    } else {
+	if (pDst->alphaMap && pDst->alphaMap->pDrawable)
+	    exaPrepareAccess(pDst->alphaMap->pDrawable, EXA_PREPARE_AUX0);
+
 	exaPrepareAccess (pDst->pDrawable, EXA_PREPARE_DEST);
+    }
 
     EXA_FALLBACK(("from picts %p/%p to pict %p\n",
 		 pSrc, pMask, pDst));
@@ -433,9 +450,15 @@ ExaCheckComposite (CARD8      op,
 #endif /* RENDER */
     if (pMask && pMask->pDrawable != NULL)
 	exaFinishAccess (pMask->pDrawable, EXA_PREPARE_MASK);
+    if (pMask && pMask->alphaMap && pMask->alphaMap->pDrawable)
+	exaFinishAccess(pMask->alphaMap->pDrawable, EXA_PREPARE_AUX1);
     if (pSrc->pDrawable != NULL)
 	exaFinishAccess (pSrc->pDrawable, EXA_PREPARE_SRC);
+    if (pSrc->alphaMap && pSrc->alphaMap->pDrawable)
+	exaFinishAccess(pSrc->alphaMap->pDrawable, EXA_PREPARE_AUX2);
     exaFinishAccess (pDst->pDrawable, EXA_PREPARE_DEST);
+    if (pDst->alphaMap && pDst->alphaMap->pDrawable)
+	exaFinishAccess(pDst->alphaMap->pDrawable, EXA_PREPARE_AUX0);
 
     REGION_UNINIT(pScreen, &region);
 }
-- 
1.6.2.rc1



-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer