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.
This commit is contained in:
Michel Dänzer 2009-07-21 14:34:13 +02:00
parent de7a14ca92
commit 268e227ba0
3 changed files with 61 additions and 62 deletions

View File

@ -554,6 +554,7 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable); PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable);
ExaPixmapPriv(pPixmap); ExaPixmapPriv(pPixmap);
Bool offscreen; Bool offscreen;
int i;
if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS)) if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
return FALSE; return FALSE;
@ -561,19 +562,25 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
if (pExaPixmap == NULL) if (pExaPixmap == NULL)
EXA_FatalErrorDebugWithRet(("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n"), FALSE); EXA_FatalErrorDebugWithRet(("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n"), FALSE);
/* Check if we're dealing SRC == DST or similar. /* Handle repeated / nested calls. */
* In that case the first PrepareAccess has already set pPixmap->devPrivate.ptr. for (i = 0; i < EXA_NUM_PREPARE_INDICES; i++) {
*/ if (pExaScr->access[i].pixmap == pPixmap) {
if (pPixmap->devPrivate.ptr != NULL) { pExaScr->access[i].count++;
int i; return TRUE;
for (i = 0; i < 6; i++) }
if (pExaScr->prepare_access[i] == pPixmap) }
break;
/* No known PrepareAccess or double prepare on the same index. */ /* If slot for this index is taken, find an empty slot */
if (i == 6 || i == index) if (pExaScr->access[index].pixmap) {
EXA_FatalErrorDebug(("EXA bug: pPixmap->devPrivate.ptr was %p, but should have been NULL.\n", for (index = EXA_NUM_PREPARE_INDICES - 1; index >= 0; index--)
pPixmap->devPrivate.ptr)); if (!pExaScr->access[index].pixmap)
break;
}
/* 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); offscreen = exaPixmapIsOffscreen(pPixmap);
@ -583,8 +590,9 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index)
else else
pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr; pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr;
/* Store so we can check SRC and DEST being the same. */ /* Store so we can handle repeated / nested calls. */
pExaScr->prepare_access[index] = pPixmap; pExaScr->access[index].pixmap = pPixmap;
pExaScr->access[index].count = 1;
if (!offscreen) if (!offscreen)
return FALSE; return FALSE;
@ -662,6 +670,7 @@ exaFinishAccess(DrawablePtr pDrawable, int index)
ExaScreenPriv (pScreen); ExaScreenPriv (pScreen);
PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable); PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable);
ExaPixmapPriv (pPixmap); ExaPixmapPriv (pPixmap);
int i;
if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS)) if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
return; return;
@ -669,11 +678,22 @@ exaFinishAccess(DrawablePtr pDrawable, int index)
if (pExaPixmap == NULL) if (pExaPixmap == NULL)
EXA_FatalErrorDebugWithRet(("EXA bug: exaFinishAccesss was called on a non-exa pixmap.\n"),); EXA_FatalErrorDebugWithRet(("EXA bug: exaFinishAccesss was called on a non-exa pixmap.\n"),);
/* Avoid mismatching indices. */ /* Handle repeated / nested calls. */
if (pExaScr->prepare_access[index] != pPixmap) for (i = 0; i < EXA_NUM_PREPARE_INDICES; i++) {
EXA_FatalErrorDebug(("EXA bug: Calling FinishAccess on pixmap %p with index %d while " if (pExaScr->access[i].pixmap == pPixmap) {
"it should have been %p.\n", pPixmap, index, pExaScr->prepare_access[index])); if (--pExaScr->access[i].count > 0)
pExaScr->prepare_access[index] = NULL; 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. */ /* We always hide the devPrivate.ptr. */
pPixmap->devPrivate.ptr = NULL; 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. * For EXA_HANDLES_PIXMAPS the driver will handle whatever is needed.
* We want to signal that the pixmaps will be used as destination. * 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_AUX_DEST);
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");
}
return pPixmap; return pPixmap;
} }
@ -786,12 +798,9 @@ exaDestroyPixmapWithFinish(PixmapPtr pPixmap)
{ {
ScreenPtr pScreen = pPixmap->drawable.pScreen; ScreenPtr pScreen = pPixmap->drawable.pScreen;
ExaScreenPriv(pScreen); ExaScreenPriv(pScreen);
int i;
Bool ret; Bool ret;
for (i = 0; i < 6; i++) exaFinishAccess(&pPixmap->drawable, EXA_PREPARE_AUX_DEST);
if (pExaScr->prepare_access[i] == pPixmap)
exaFinishAccess(&pPixmap->drawable, i);
/* This swaps between this function and the real upper layer function. /* 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. * 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); (*pGC->funcs->ValidateGC)(pGC, changes, pDrawable);
if (pExaScr->prepare_access[EXA_PREPARE_SRC]) /* tile */ if (pTile)
exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_SRC]->drawable, EXA_PREPARE_SRC); exaFinishAccess(&pTile->drawable, EXA_PREPARE_SRC);
if (pGC->stipple) if (pGC->stipple)
exaFinishAccess(&pGC->stipple->drawable, EXA_PREPARE_MASK); exaFinishAccess(&pGC->stipple->drawable, EXA_PREPARE_MASK);
@ -868,13 +877,6 @@ exaValidateGC(GCPtr pGC,
/* restore copy of fb layer pointer. */ /* restore copy of fb layer pointer. */
pExaScr->SavedDestroyPixmap = old_ptr2; 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); EXA_GC_EPILOGUE(pGC);
} }
@ -984,10 +986,10 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
ret = pScreen->ChangeWindowAttributes(pWin, mask); ret = pScreen->ChangeWindowAttributes(pWin, mask);
swap(pExaScr, pScreen, ChangeWindowAttributes); swap(pExaScr, pScreen, ChangeWindowAttributes);
if (pExaScr->prepare_access[EXA_PREPARE_SRC]) /* background */ if ((mask & CWBackPixmap) && pWin->backgroundState == BackgroundPixmap)
exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_SRC]->drawable, EXA_PREPARE_SRC); exaFinishAccess(&pWin->background.pixmap->drawable, EXA_PREPARE_SRC);
if (pExaScr->prepare_access[EXA_PREPARE_MASK]) /* border */ if ((mask & CWBorderPixmap) && pWin->borderIsPixel == FALSE)
exaFinishAccess(&pExaScr->prepare_access[EXA_PREPARE_MASK]->drawable, EXA_PREPARE_MASK); exaFinishAccess(&pWin->border.pixmap->drawable, EXA_PREPARE_MASK);
/* switch back to the normal upper layer. */ /* switch back to the normal upper layer. */
unwrap(pExaScr, pScreen, CreatePixmap); unwrap(pExaScr, pScreen, CreatePixmap);
@ -999,13 +1001,6 @@ exaChangeWindowAttributes(WindowPtr pWin, unsigned long mask)
/* restore copy of fb layer pointer. */ /* restore copy of fb layer pointer. */
pExaScr->SavedDestroyPixmap = old_ptr2; 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; return ret;
} }

View File

@ -586,14 +586,14 @@ typedef struct _ExaDriver {
* untiling, or to adjust the pixmap's devPrivate.ptr for the purpose of * untiling, or to adjust the pixmap's devPrivate.ptr for the purpose of
* making CPU access use a different aperture. * making CPU access use a different aperture.
* *
* The index is one of #EXA_PREPARE_DEST, #EXA_PREPARE_SRC, or * The index is one of #EXA_PREPARE_DEST, #EXA_PREPARE_SRC,
* #EXA_PREPARE_MASK, indicating which pixmap is in question. Since only up * #EXA_PREPARE_MASK, #EXA_PREPARE_AUX_DEST, #EXA_PREPARE_AUX_SRC, or
* to three pixmaps will have PrepareAccess() called on them per operation, * #EXA_PREPARE_AUX_MASK. Since only up to #EXA_NUM_PREPARE_INDICES pixmaps
* drivers can have a small, statically-allocated space to maintain state * will have PrepareAccess() called on them per operation, drivers can have
* for PrepareAccess() and FinishAccess() in. Note that the same pixmap may * a small, statically-allocated space to maintain state for PrepareAccess()
* have PrepareAccess() called on it more than once, for example when doing * and FinishAccess() in. Note that PrepareAccess() is only called once per
* a copy within the same pixmap (so it gets PrepareAccess as() * pixmap and operation, regardless of whether the pixmap is used as a
* #EXA_PREPARE_DEST and then as #EXA_PREPARE_SRC). * destination and/or source, and the index may not reflect the usage.
* *
* PrepareAccess() may fail. An example might be the case of hardware that * 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() * 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_DEST 3
#define EXA_PREPARE_AUX_SRC 4 #define EXA_PREPARE_AUX_SRC 4
#define EXA_PREPARE_AUX_MASK 5 #define EXA_PREPARE_AUX_MASK 5
#define EXA_NUM_PREPARE_INDICES 6
/** @} */ /** @} */
/** /**

View File

@ -176,8 +176,11 @@ typedef struct {
CARD32 lastDefragment; CARD32 lastDefragment;
CARD32 nextDefragment; CARD32 nextDefragment;
/* Store all accessed pixmaps, so we can check for duplicates. */ /* Reference counting for accessed pixmaps */
PixmapPtr prepare_access[6]; struct {
PixmapPtr pixmap;
int count;
} access[EXA_NUM_PREPARE_INDICES];
/* Holds information on fallbacks that cannot be relayed otherwise. */ /* Holds information on fallbacks that cannot be relayed otherwise. */
unsigned int fallback_flags; unsigned int fallback_flags;