EXA: Make Prepare/FinishAccess tracking resilient to repeated / nested calls.

Use reference counting and do nothing unless the reference count transitions
to/from 0.

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

As a bonus, this avoids calling the driver Prepare/FinishAccess hooks more than
once per pixmap and operation.

Also update the Doxygen documentation for the PrepareAccess driver hook to
better match current reality.

3 files changed, 60 insertions, 61 deletions

diff --git a/exa/exa.c b/exa/exa.c
index 8d488b3c2..daa4a7a03 100644
--- a/exa/exa.c
+++ b/exa/exa.c
@@ -554,6 +554,7 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
     PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable);
     ExaPixmapPriv(pPixmap);
     Bool offscreen;
+    int i;
 
     if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
 	return FALSE;
@@ -561,19 +562,25 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
     if (pExaPixmap == NULL)
 	EXA_FatalErrorDebugWithRet(("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n"), FALSE);
 
-    /* Check if we're dealing SRC == DST or similar.
-     * In that case the first PrepareAccess has already set pPixmap->devPrivate.ptr.
-     */
-    if (pPixmap->devPrivate.ptr != NULL) {
-	int i;
-	for (i = 0; i < 6; i++)
-	    if (pExaScr->prepare_access[i] == pPixmap)
+    /* Handle repeated / nested calls. */
+    for (i = 0; i < EXA_NUM_PREPARE_INDICES; i++) {
+	if (pExaScr->access[i].pixmap == pPixmap) {
+	    pExaScr->access[i].count++;
+	    return TRUE;
+	}
+    }
+
+    /* If slot for this index is taken, find an empty slot */
+    if (pExaScr->access[index].pixmap) {
+	for (index = EXA_NUM_PREPARE_INDICES - 1; index >= 0; index--)
+	    if (!pExaScr->access[index].pixmap)
 		break;
+    }
 
-	/* No known PrepareAccess or double prepare on the same index. */
-	if (i == 6 || i == index)
-	    EXA_FatalErrorDebug(("EXA bug: pPixmap->devPrivate.ptr was %p, but should have been NULL.\n",
-		pPixmap->devPrivate.ptr));
+    /* Access to this pixmap hasn't been prepared yet, so data pointer should be NULL. */
+    if (pPixmap->devPrivate.ptr != NULL) {
+	EXA_FatalErrorDebug(("EXA bug: pPixmap->devPrivate.ptr was %p, but should have been NULL.\n",
+			     pPixmap->devPrivate.ptr));
     }
 
     offscreen = exaPixmapIsOffscreen(pPixmap);
@@ -583,8 +590,9 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
     else
 	pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr;
 
-    /* Store so we can check SRC and DEST being the same. */
-    pExaScr->prepare_access[index] = pPixmap;
+    /* Store so we can handle repeated / nested calls. */
+    pExaScr->access[index].pixmap = pPixmap;
+    pExaScr->access[index].count = 1;
 
     if (!offscreen)
 	return FALSE;
@@ -662,6 +670,7 @@ exaFinishAccess(DrawablePtr pDrawable, int index)
     ExaScreenPriv  (pScreen);
     PixmapPtr	    pPixmap = exaGetDrawablePixmap (pDrawable);
     ExaPixmapPriv  (pPixmap);
+    int i;
 
     if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
 	return;
@@ -669,11 +678,22 @@ exaFinishAccess(DrawablePtr pDrawable, int index)
     if (pExaPixmap == NULL)
 	EXA_FatalErrorDebugWithRet(("EXA bug: exaFinishAccesss was called on a non-exa pixmap.\n"),);
 
-    /* Avoid mismatching indices. */
-    if (pExaScr->prepare_access[index] != pPixmap)
-	EXA_FatalErrorDebug(("EXA bug: Calling FinishAccess on pixmap %p with index %d while "
-			"it should have been %p.\n", pPixmap, index, pExaScr->prepare_access[index]));
-    pExaScr->prepare_access[index] = NULL;
+    /* Handle repeated / nested calls. */
+    for (i = 0; i < EXA_NUM_PREPARE_INDICES; i++) {
+	if (pExaScr->access[i].pixmap == pPixmap) {
+	    if (--pExaScr->access[i].count > 0)
+		return;
+	    index = i;
+	    break;
+	}
+    }
+
+    /* Catch unbalanced Prepare/FinishAccess calls. */
+    if (i == EXA_NUM_PREPARE_INDICES)
+	EXA_FatalErrorDebug(("EXA bug: FinishAccess called without PrepareAccess for pixmap 0x%p.\n",
+			     pPixmap));
+
+    pExaScr->access[index].pixmap = NULL;
 
     /* We always hide the devPrivate.ptr. */
     pPixmap->devPrivate.ptr = NULL;
@@ -768,15 +788,7 @@ exaCreatePixmapWithPrepare(ScreenPtr pScreen, int w, int h, int depth,
      * For EXA_HANDLES_PIXMAPS the driver will handle whatever is needed.
      * We want to signal that the pixmaps will be used as destination.
      */
-    if (pExaScr->prepare_access[EXA_PREPARE_DEST] == NULL) {
-	ExaDoPrepareAccess(&pPixmap->drawable, EXA_PREPARE_DEST);
-	pExaScr->prepare_access[EXA_PREPARE_DEST] = pPixmap;
-    } else if (pExaScr->prepare_access[EXA_PREPARE_AUX_DEST] == NULL) {
-	ExaDoPrepareAccess(&pPixmap->drawable, EXA_PREPARE_AUX_DEST);
-	pExaScr->prepare_access[EXA_PREPARE_AUX_DEST] = pPixmap;
-    } else {
-	FatalError("exaCreatePixmapWithPrepare can only accomodate two pixmaps, we're at three.\n");
-    }
+    ExaDoPrepareAccess(&pPixmap->drawable, EXA_PREPARE_AUX_DEST);
 
     return pPixmap;
 }
@@ -786,12 +798,9 @@ exaDestroyPixmapWithFinish(PixmapPtr pPixmap)
 {
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
     ExaScreenPriv(pScreen);
-    int i;
     Bool ret;
 
-    for (i = 0; i < 6; i++)
-	if (pExaScr->prepare_access[i] == pPixmap)
-	    exaFinishAccess(&pPixmap->drawable, i);
+    exaFinishAccess(&pPixmap->drawable, EXA_PREPARE_AUX_DEST);
 
     /* This swaps between this function and the real upper layer function.
      * Normally this would swap to the fb layer pointer, this is a very special case.
@@ -853,8 +862,8 @@ exaValidateGC(GCPtr pGC,
 
     (*pGC->funcs->ValidateGC)(pGC, changes, pDrawable);
 
-    if (pExaScr->prepare_access[EXA_PREPARE_SRC]) /* tile */
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_SRC]->drawable, EXA_PREPARE_SRC);
+    if (pTile)
+	exaFinishAccess(&pTile->drawable, EXA_PREPARE_SRC);
     if (pGC->stipple)
         exaFinishAccess(&pGC->stipple->drawable, EXA_PREPARE_MASK);
 
@@ -868,13 +877,6 @@ exaValidateGC(GCPtr pGC,
     /* restore copy of fb layer pointer. */
     pExaScr->SavedDestroyPixmap = old_ptr2;
 
-    if (pExaScr->prepare_access[EXA_PREPARE_DEST])
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_DEST]->drawable,
-		EXA_PREPARE_DEST);
-    if (pExaScr->prepare_access[EXA_PREPARE_AUX_DEST])
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_AUX_DEST]->drawable,
-		EXA_PREPARE_AUX_DEST);
-
     EXA_GC_EPILOGUE(pGC);
 }
 
@@ -984,10 +986,10 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
     ret = pScreen->ChangeWindowAttributes(pWin, mask);
     swap(pExaScr, pScreen, ChangeWindowAttributes);
 
-    if (pExaScr->prepare_access[EXA_PREPARE_SRC]) /* background */
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_SRC]->drawable, EXA_PREPARE_SRC);
-    if (pExaScr->prepare_access[EXA_PREPARE_MASK]) /* border */
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_MASK]->drawable, EXA_PREPARE_MASK);
+    if ((mask & CWBackPixmap) && pWin->backgroundState == BackgroundPixmap) 
+	exaFinishAccess(&pWin->background.pixmap->drawable, EXA_PREPARE_SRC);
+    if ((mask & CWBorderPixmap) && pWin->borderIsPixel == FALSE)
+	exaFinishAccess(&pWin->border.pixmap->drawable, EXA_PREPARE_MASK);
 
     /* switch back to the normal upper layer. */
     unwrap(pExaScr, pScreen, CreatePixmap);
@@ -999,13 +1001,6 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
     /* restore copy of fb layer pointer. */
     pExaScr->SavedDestroyPixmap = old_ptr2;
 
-    if (pExaScr->prepare_access[EXA_PREPARE_DEST])
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_DEST]->drawable,
-		EXA_PREPARE_DEST);
-    if (pExaScr->prepare_access[EXA_PREPARE_AUX_DEST])
-	exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_AUX_DEST]->drawable,
-		EXA_PREPARE_AUX_DEST);
-
     return ret;
 }
 
diff --git a/exa/exa.h b/exa/exa.h
index 0701ec98f..1d2c6a9e6 100644
--- a/exa/exa.h
+++ b/exa/exa.h
@@ -586,14 +586,14 @@ typedef struct _ExaDriver {
      * untiling, or to adjust the pixmap's devPrivate.ptr for the purpose of
      * making CPU access use a different aperture.
      *
-     * The index is one of #EXA_PREPARE_DEST, #EXA_PREPARE_SRC, or
-     * #EXA_PREPARE_MASK, indicating which pixmap is in question.  Since only up
-     * to three pixmaps will have PrepareAccess() called on them per operation,
-     * drivers can have a small, statically-allocated space to maintain state
-     * for PrepareAccess() and FinishAccess() in.  Note that the same pixmap may
-     * have PrepareAccess() called on it more than once, for example when doing
-     * a copy within the same pixmap (so it gets PrepareAccess as()
-     * #EXA_PREPARE_DEST and then as #EXA_PREPARE_SRC).
+     * The index is one of #EXA_PREPARE_DEST, #EXA_PREPARE_SRC,
+     * #EXA_PREPARE_MASK, #EXA_PREPARE_AUX_DEST, #EXA_PREPARE_AUX_SRC, or
+     * #EXA_PREPARE_AUX_MASK. Since only up to #EXA_NUM_PREPARE_INDICES pixmaps
+     * will have PrepareAccess() called on them per operation, drivers can have
+     * a small, statically-allocated space to maintain state for PrepareAccess()
+     * and FinishAccess() in.  Note that PrepareAccess() is only called once per
+     * pixmap and operation, regardless of whether the pixmap is used as a
+     * destination and/or source, and the index may not reflect the usage.
      *
      * PrepareAccess() may fail.  An example might be the case of hardware that
      * can set up 1 or 2 surfaces for CPU access, but not 3.  If PrepareAccess()
@@ -663,6 +663,7 @@ typedef struct _ExaDriver {
 	#define EXA_PREPARE_AUX_DEST	3
 	#define EXA_PREPARE_AUX_SRC	4
 	#define EXA_PREPARE_AUX_MASK	5
+	#define EXA_NUM_PREPARE_INDICES	6
 	/** @} */
 
     /**
diff --git a/exa/exa_priv.h b/exa/exa_priv.h
index b3df1a586..f67a9cbd8 100644
--- a/exa/exa_priv.h
+++ b/exa/exa_priv.h
@@ -176,8 +176,11 @@ typedef struct {
     CARD32			 lastDefragment;
     CARD32			 nextDefragment;
 
-    /* Store all accessed pixmaps, so we can check for duplicates. */
-    PixmapPtr prepare_access[6];
+    /* Reference counting for accessed pixmaps */
+    struct {
+	PixmapPtr pixmap;
+	int count;
+    } access[EXA_NUM_PREPARE_INDICES];
 
     /* Holds information on fallbacks that cannot be relayed otherwise. */
     unsigned int fallback_flags;