From 268e227ba06c027f5c56b1aaee5dcc6a2034403f Mon Sep 17 00:00:00 2001 From: Michel Dänzer Date: Tue, 21 Jul 2009 14:34:13 +0200 Subject: 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. --- exa/exa.c | 97 ++++++++++++++++++++++++++++------------------------------ exa/exa.h | 17 +++++----- exa/exa_priv.h | 7 +++-- 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; -- cgit v1.2.3