commit 86ca6baee221fc691c7828b078008314ac989864 Author: Kristian Høgsberg Date: Fri Apr 16 05:55:32 2010 -0400 glx: Track GLX 1.3 style GLX drawables under their X drawable ID as well This ensures that the DrawableGone callback gets called as necessary when the X drawable goes away. Otherwise, using a GLX drawable (say, glXSwapBuffers) in indirect mode after the X drawable has been destroyed will crash the server. Signed-off-by: Kristian Høgsberg Reviewed-by: Michel Dänzer Signed-off-by: Keith Packard (cherry picked from commit f0006aa58f6cf7552a239e169ff6e7e4fda532f4) diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 77afbf4..04c6d40 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -161,7 +161,11 @@ validGlxDrawable(ClientPtr client, XID id, int type, int access_mode, return FALSE; } + /* If the ID of the glx drawable we looked up doesn't match the id + * we looked for, it's because we looked it up under the X + * drawable ID (see DoCreateGLXDrawable). */ if (rc == BadValue || + (*drawable)->drawId != id || (type != GLX_DRAWABLE_ANY && type != (*drawable)->type)) { client->errorValue = id; switch (type) { @@ -1128,6 +1132,14 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf return BadAlloc; } + /* Add the glx drawable under the XID of the underlying X drawable + * too. That way we'll get a callback in DrawableGone and can + * clean up properly when the drawable is destroyed. */ + if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { + pGlxDraw->destroy (pGlxDraw); + return BadAlloc; + } + return Success; } diff --git a/glx/glxext.c b/glx/glxext.c index 59bcfbe..89e58b0 100644 --- a/glx/glxext.c +++ b/glx/glxext.c @@ -126,6 +126,17 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) { __GLXcontext *c; + /* If this drawable was created using glx 1.3 drawable + * constructors, we added it as a glx drawable resource under both + * its glx drawable ID and it X drawable ID. Remove the other + * resource now so we don't a callback for freed memory. */ + if (glxPriv->drawId != glxPriv->pDraw->id) { + if (xid == glxPriv->drawId) + FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE); + else + FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE); + } + for (c = glxAllContexts; c; c = c->next) { if (c->isCurrent && (c->drawPriv == glxPriv || c->readPriv == glxPriv)) { int i; commit 0460a76b9ae25fe26f683f0cbff1e4157287cf56 Author: Kristian Høgsberg Date: Fri Apr 16 05:55:33 2010 -0400 glx: Let the resource system destroy pixmaps GLX pbuffers are implemented using a pixmap allocated by the server. With the change to DRI2 to track DRI2 drawables as resources, we need to make sure that every drawable we create a DRI2 drawable for has an XID. By using the XID of the pbuffer, the resource system will automatically reclaim the hidden pixmap and the DRI2 drawable when the pbuffer is destroyed or the client exits. Signed-off-by: Kristian Høgsberg Signed-off-by: Keith Packard (cherry picked from commit 22da7aa9d743deee198aaf6df5d370a446db9763) diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 04c6d40..087d52e 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -1101,14 +1101,6 @@ __glXDrawableInit(__GLXdrawable *drawable, void __glXDrawableRelease(__GLXdrawable *drawable) { - ScreenPtr pScreen = drawable->pDraw->pScreen; - - switch (drawable->type) { - case GLX_DRAWABLE_PIXMAP: - case GLX_DRAWABLE_PBUFFER: - (*pScreen->DestroyPixmap)((PixmapPtr) drawable->pDraw); - break; - } } static int @@ -1117,8 +1109,6 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf { __GLXdrawable *pGlxDraw; - LEGAL_NEW_RESOURCE(glxDrawableId, client); - if (pGlxScreen->pScreen != pDraw->pScreen) return BadMatch; @@ -1135,7 +1125,8 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf /* Add the glx drawable under the XID of the underlying X drawable * too. That way we'll get a callback in DrawableGone and can * clean up properly when the drawable is destroyed. */ - if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { + if (pDraw->id != glxDrawableId && + !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { pGlxDraw->destroy (pGlxDraw); return BadAlloc; } @@ -1150,6 +1141,8 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config DrawablePtr pDraw; int err; + LEGAL_NEW_RESOURCE(glxDrawableId, client); + err = dixLookupDrawable(&pDraw, drawableId, client, 0, DixAddAccess); if (err != Success) { client->errorValue = drawableId; @@ -1163,9 +1156,6 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw, glxDrawableId, GLX_DRAWABLE_PIXMAP); - if (err == Success) - ((PixmapPtr) pDraw)->refcnt++; - return err; } @@ -1306,6 +1296,8 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId, PixmapPtr pPixmap; int err; + LEGAL_NEW_RESOURCE(glxDrawableId, client); + if (!validGlxScreen(client, screenNum, &pGlxScreen, &err)) return err; if (!validGlxFBConfig(client, pGlxScreen, fbconfigId, &config, &err)) @@ -1316,6 +1308,13 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId, width, height, config->rgbBits, 0); __glXleaveServer(GL_FALSE); + /* Assign the pixmap the same id as the pbuffer and add it as a + * resource so it and the DRI2 drawable will be reclaimed when the + * pbuffer is destroyed. */ + pPixmap->drawable.id = glxDrawableId; + if (!AddResource(pPixmap->drawable.id, RT_PIXMAP, pPixmap)) + return BadAlloc; + return DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable, glxDrawableId, GLX_DRAWABLE_PBUFFER); } @@ -1423,6 +1422,8 @@ int __glXDisp_CreateWindow(__GLXclientState *cl, GLbyte *pc) DrawablePtr pDraw; int err; + LEGAL_NEW_RESOURCE(req->glxwindow, client); + if (!validGlxScreen(client, req->screen, &pGlxScreen, &err)) return err; if (!validGlxFBConfig(client, pGlxScreen, req->fbconfig, &config, &err)) commit 0c499f2ee4ae2b7dc424009abb336fc81a8a2853 Author: Kristian Høgsberg Date: Fri Apr 16 05:55:34 2010 -0400 DRI2: Track DRI2 drawables as resources, not privates The main motivation here is to have the resource system clean up the DRI2 drawable automatically so glx doesn't have to. Right now, the glx drawable resource must be destroyed before the X drawable, so that calling DRI2DestroyDrawable doesn't crash. By making the DRI2 drawable a resource, GLX doesn't have to worry about that and the resource destruction order becomes irrelevant. https://bugs.freedesktop.org/show_bug.cgi?id=26394 [Ported to 1.8 branch by ajax@redhat.com] Signed-off-by: Kristian Høgsberg Signed-off-by: Keith Packard diff --git a/glx/glxdri2.c b/glx/glxdri2.c index edd29b0..bde519a 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -105,11 +105,6 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable) (*core->destroyDrawable)(private->driDrawable); - /* If the X window was destroyed, the dri DestroyWindow hook will - * aready have taken care of this, so only call if pDraw isn't NULL. */ - if (drawable->pDraw != NULL) - DRI2DestroyDrawable(drawable->pDraw); - __glXDrawableRelease(drawable); xfree(private); diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 48618e1..63bef28 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -45,15 +45,14 @@ #include "xf86.h" -static int dri2ScreenPrivateKeyIndex; +static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; -static int dri2WindowPrivateKeyIndex; -static DevPrivateKey dri2WindowPrivateKey = &dri2WindowPrivateKeyIndex; -static int dri2PixmapPrivateKeyIndex; -static DevPrivateKey dri2PixmapPrivateKey = &dri2PixmapPrivateKeyIndex; +static RESTYPE dri2DrawableRes; + +typedef struct _DRI2Screen *DRI2ScreenPtr; typedef struct _DRI2Drawable { - unsigned int refCount; + DRI2ScreenPtr dri2_screen; int width; int height; DRI2BufferPtr *buffers; @@ -67,9 +66,8 @@ typedef struct _DRI2Drawable { int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; -typedef struct _DRI2Screen *DRI2ScreenPtr; - typedef struct _DRI2Screen { + ScreenPtr screen; unsigned int numDrivers; const char **driverNames; const char *deviceName; @@ -95,43 +93,33 @@ DRI2GetScreen(ScreenPtr pScreen) static DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { - WindowPtr pWin; - PixmapPtr pPixmap; - - if (!pDraw) + DRI2DrawablePtr pPriv; + int rc; + + rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, + dri2DrawableRes, NULL, DixReadAccess); + if (rc != Success) return NULL; - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - return dixLookupPrivate(&pWin->devPrivates, dri2WindowPrivateKey); - } - else - { - pPixmap = (PixmapPtr) pDraw; - return dixLookupPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey); - } + return pPriv; } int DRI2CreateDrawable(DrawablePtr pDraw) { - WindowPtr pWin; - PixmapPtr pPixmap; DRI2DrawablePtr pPriv; + int rc; - pPriv = DRI2GetDrawable(pDraw); - if (pPriv != NULL) - { - pPriv->refCount++; - return Success; - } + rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, + dri2DrawableRes, NULL, DixReadAccess); + if (rc == Success || rc != BadValue) + return rc; pPriv = xalloc(sizeof *pPriv); if (pPriv == NULL) return BadAlloc; - pPriv->refCount = 1; + pPriv->dri2_screen = DRI2GetScreen(pDraw->pScreen); pPriv->width = pDraw->width; pPriv->height = pDraw->height; pPriv->buffers = NULL; @@ -144,43 +132,30 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->last_swap_target = -1; pPriv->swap_limit = 1; /* default to double buffering */ - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv); - } - else - { - pPixmap = (PixmapPtr) pDraw; - dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv); - } + if (!AddResource(pDraw->id, dri2DrawableRes, pPriv)) + return BadAlloc; return Success; } -static void -DRI2FreeDrawable(DrawablePtr pDraw) +static int DRI2DrawableGone(pointer p, XID id) { - DRI2DrawablePtr pPriv; - WindowPtr pWin; - PixmapPtr pPixmap; + DRI2DrawablePtr pPriv = p; + DRI2ScreenPtr ds = pPriv->dri2_screen; + DrawablePtr root; + int i; - pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) - return; + root = &WindowTable[ds->screen->myNum]->drawable; + if (pPriv->buffers != NULL) { + for (i = 0; i < pPriv->bufferCount; i++) + (*ds->DestroyBuffer)(root, pPriv->buffers[i]); + + xfree(pPriv->buffers); + } xfree(pPriv); - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL); - } - else - { - pPixmap = (PixmapPtr) pDraw; - dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL); - } + return Success; } static int @@ -534,13 +509,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } - if (pPriv->refCount == 0) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "[DRI2] %s: bad drawable refcount\n", __func__); - DRI2FreeDrawable(pDraw); - return; - } - ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); @@ -753,36 +721,6 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, return Success; } -void -DRI2DestroyDrawable(DrawablePtr pDraw) -{ - DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); - DRI2DrawablePtr pPriv; - - pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) - return; - - pPriv->refCount--; - if (pPriv->refCount > 0) - return; - - if (pPriv->buffers != NULL) { - int i; - - for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); - - xfree(pPriv->buffers); - } - - /* If the window is destroyed while we have a swap pending, don't - * actually free the priv yet. We'll need it in the DRI2SwapComplete() - * callback and we'll free it there once we're done. */ - if (!pPriv->swapsPending) - DRI2FreeDrawable(pDraw); -} - Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd, const char **driverName, const char **deviceName) @@ -834,6 +772,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) if (!ds) return FALSE; + ds->screen = pScreen; ds->fd = info->fd; ds->deviceName = info->deviceName; @@ -897,6 +836,8 @@ DRI2Setup(pointer module, pointer opts, int *errmaj, int *errmin) { static Bool setupDone = FALSE; + dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); + if (!setupDone) { setupDone = TRUE; diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index bd92fd3..1ac4a5f 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -51,7 +51,6 @@ #include "xf86Module.h" static ExtensionEntry *dri2Extension; -static RESTYPE dri2DrawableRes; static Bool validDrawable(ClientPtr client, XID drawable, Mask access_mode, @@ -172,11 +171,6 @@ ProcDRI2CreateDrawable(ClientPtr client) if (status != Success) return status; - if (!AddResource(stuff->drawable, dri2DrawableRes, pDrawable)) { - DRI2DestroyDrawable(pDrawable); - return BadAlloc; - } - return client->noClientException; } @@ -192,8 +186,6 @@ ProcDRI2DestroyDrawable(ClientPtr client) &pDrawable, &status)) return status; - FreeResourceByType(stuff->drawable, dri2DrawableRes, FALSE); - return client->noClientException; } @@ -620,25 +612,11 @@ SProcDRI2Dispatch (ClientPtr client) } } -static int DRI2DrawableGone(pointer p, XID id) -{ - DrawablePtr pDrawable = p; - - DRI2DestroyDrawable(pDrawable); - - return Success; -} - int DRI2EventBase; static void DRI2ExtensionInit(void) { - dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); - - if (!dri2DrawableRes) - return; - dri2Extension = AddExtension(DRI2_NAME, DRI2NumberEvents, DRI2NumberErrors, commit 6ab87eb04cf24619c17c1fa05daaeebb481a6a50 Author: Kristian Høgsberg Date: Fri Apr 16 05:55:35 2010 -0400 glx: Drop DestroyWindow hook Now that glx doesn't call DRI2DestroyDrawable anymore, we don't need to force a specific resource destruction order in the DestroyWindow hook. Signed-off-by: Kristian Høgsberg Reviewed-by: Michel Dänzer https://bugs.freedesktop.org/show_bug.cgi?id=26394 Signed-off-by: Keith Packard diff --git a/glx/glxscreens.c b/glx/glxscreens.c index 58d8ee0..b75aea6 100644 --- a/glx/glxscreens.c +++ b/glx/glxscreens.c @@ -215,7 +215,6 @@ glxCloseScreen (int index, ScreenPtr pScreen) __GLXscreen *pGlxScreen = glxGetScreen(pScreen); pScreen->CloseScreen = pGlxScreen->CloseScreen; - pScreen->DestroyWindow = pGlxScreen->DestroyWindow; pGlxScreen->destroy(pGlxScreen); @@ -347,31 +346,6 @@ pickFBConfig(__GLXscreen *pGlxScreen, VisualPtr visual) return best; } -static Bool -glxDestroyWindow(WindowPtr pWin) -{ - ScreenPtr pScreen = pWin->drawable.pScreen; - __GLXscreen *pGlxScreen = glxGetScreen(pScreen); - Bool retval = TRUE; - - FreeResource(pWin->drawable.id, FALSE); - - /* call lower wrapped functions */ - if (pGlxScreen->DestroyWindow) { - /* unwrap */ - pScreen->DestroyWindow = pGlxScreen->DestroyWindow; - - /* call lower layers */ - retval = (*pScreen->DestroyWindow)(pWin); - - /* rewrap */ - pGlxScreen->DestroyWindow = pScreen->DestroyWindow; - pScreen->DestroyWindow = glxDestroyWindow; - } - - return retval; -} - void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen) { __GLXconfig *m; @@ -394,8 +368,6 @@ void __glXScreenInit(__GLXscreen *pGlxScreen, ScreenPtr pScreen) pGlxScreen->CloseScreen = pScreen->CloseScreen; pScreen->CloseScreen = glxCloseScreen; - pGlxScreen->DestroyWindow = pScreen->DestroyWindow; - pScreen->DestroyWindow = glxDestroyWindow; i = 0; for (m = pGlxScreen->fbconfigs; m != NULL; m = m->next) { diff --git a/glx/glxscreens.h b/glx/glxscreens.h index bff4363..d52099f 100644 --- a/glx/glxscreens.h +++ b/glx/glxscreens.h @@ -173,7 +173,6 @@ struct __GLXscreen { /*@}*/ Bool (*CloseScreen)(int index, ScreenPtr pScreen); - Bool (*DestroyWindow)(WindowPtr pWindow); }; commit c394b17266301d363a9e234f58f8015f74e01307 Author: Peter Hutterer Date: Tue Apr 27 17:53:22 2010 +1000 Revert "DRI2: Track DRI2 drawables as resources, not privates" This change breaks GLX compositing managers. See Bug 27767 This reverts commit 0c499f2ee4ae2b7dc424009abb336fc81a8a2853. Signed-off-by: Peter Hutterer diff --git a/glx/glxdri2.c b/glx/glxdri2.c index bde519a..edd29b0 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -105,6 +105,11 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable) (*core->destroyDrawable)(private->driDrawable); + /* If the X window was destroyed, the dri DestroyWindow hook will + * aready have taken care of this, so only call if pDraw isn't NULL. */ + if (drawable->pDraw != NULL) + DRI2DestroyDrawable(drawable->pDraw); + __glXDrawableRelease(drawable); xfree(private); diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 63bef28..48618e1 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -45,14 +45,15 @@ #include "xf86.h" -static int dri2ScreenPrivateKeyIndex; +static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; -static RESTYPE dri2DrawableRes; - -typedef struct _DRI2Screen *DRI2ScreenPtr; +static int dri2WindowPrivateKeyIndex; +static DevPrivateKey dri2WindowPrivateKey = &dri2WindowPrivateKeyIndex; +static int dri2PixmapPrivateKeyIndex; +static DevPrivateKey dri2PixmapPrivateKey = &dri2PixmapPrivateKeyIndex; typedef struct _DRI2Drawable { - DRI2ScreenPtr dri2_screen; + unsigned int refCount; int width; int height; DRI2BufferPtr *buffers; @@ -66,8 +67,9 @@ typedef struct _DRI2Drawable { int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; +typedef struct _DRI2Screen *DRI2ScreenPtr; + typedef struct _DRI2Screen { - ScreenPtr screen; unsigned int numDrivers; const char **driverNames; const char *deviceName; @@ -93,33 +95,43 @@ DRI2GetScreen(ScreenPtr pScreen) static DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { - DRI2DrawablePtr pPriv; - int rc; - - rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, - dri2DrawableRes, NULL, DixReadAccess); - if (rc != Success) + WindowPtr pWin; + PixmapPtr pPixmap; + + if (!pDraw) return NULL; - return pPriv; + if (pDraw->type == DRAWABLE_WINDOW) + { + pWin = (WindowPtr) pDraw; + return dixLookupPrivate(&pWin->devPrivates, dri2WindowPrivateKey); + } + else + { + pPixmap = (PixmapPtr) pDraw; + return dixLookupPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey); + } } int DRI2CreateDrawable(DrawablePtr pDraw) { + WindowPtr pWin; + PixmapPtr pPixmap; DRI2DrawablePtr pPriv; - int rc; - rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, - dri2DrawableRes, NULL, DixReadAccess); - if (rc == Success || rc != BadValue) - return rc; + pPriv = DRI2GetDrawable(pDraw); + if (pPriv != NULL) + { + pPriv->refCount++; + return Success; + } pPriv = xalloc(sizeof *pPriv); if (pPriv == NULL) return BadAlloc; - pPriv->dri2_screen = DRI2GetScreen(pDraw->pScreen); + pPriv->refCount = 1; pPriv->width = pDraw->width; pPriv->height = pDraw->height; pPriv->buffers = NULL; @@ -132,30 +144,43 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->last_swap_target = -1; pPriv->swap_limit = 1; /* default to double buffering */ - if (!AddResource(pDraw->id, dri2DrawableRes, pPriv)) - return BadAlloc; + if (pDraw->type == DRAWABLE_WINDOW) + { + pWin = (WindowPtr) pDraw; + dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv); + } + else + { + pPixmap = (PixmapPtr) pDraw; + dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv); + } return Success; } -static int DRI2DrawableGone(pointer p, XID id) +static void +DRI2FreeDrawable(DrawablePtr pDraw) { - DRI2DrawablePtr pPriv = p; - DRI2ScreenPtr ds = pPriv->dri2_screen; - DrawablePtr root; - int i; - - root = &WindowTable[ds->screen->myNum]->drawable; - if (pPriv->buffers != NULL) { - for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(root, pPriv->buffers[i]); + DRI2DrawablePtr pPriv; + WindowPtr pWin; + PixmapPtr pPixmap; - xfree(pPriv->buffers); - } + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + return; xfree(pPriv); - return Success; + if (pDraw->type == DRAWABLE_WINDOW) + { + pWin = (WindowPtr) pDraw; + dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL); + } + else + { + pPixmap = (PixmapPtr) pDraw; + dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL); + } } static int @@ -509,6 +534,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } + if (pPriv->refCount == 0) { + xf86DrvMsg(pScreen->myNum, X_ERROR, + "[DRI2] %s: bad drawable refcount\n", __func__); + DRI2FreeDrawable(pDraw); + return; + } + ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); @@ -721,6 +753,36 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, return Success; } +void +DRI2DestroyDrawable(DrawablePtr pDraw) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); + DRI2DrawablePtr pPriv; + + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + return; + + pPriv->refCount--; + if (pPriv->refCount > 0) + return; + + if (pPriv->buffers != NULL) { + int i; + + for (i = 0; i < pPriv->bufferCount; i++) + (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); + + xfree(pPriv->buffers); + } + + /* If the window is destroyed while we have a swap pending, don't + * actually free the priv yet. We'll need it in the DRI2SwapComplete() + * callback and we'll free it there once we're done. */ + if (!pPriv->swapsPending) + DRI2FreeDrawable(pDraw); +} + Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd, const char **driverName, const char **deviceName) @@ -772,7 +834,6 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) if (!ds) return FALSE; - ds->screen = pScreen; ds->fd = info->fd; ds->deviceName = info->deviceName; @@ -836,8 +897,6 @@ DRI2Setup(pointer module, pointer opts, int *errmaj, int *errmin) { static Bool setupDone = FALSE; - dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); - if (!setupDone) { setupDone = TRUE; diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 1ac4a5f..bd92fd3 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -51,6 +51,7 @@ #include "xf86Module.h" static ExtensionEntry *dri2Extension; +static RESTYPE dri2DrawableRes; static Bool validDrawable(ClientPtr client, XID drawable, Mask access_mode, @@ -171,6 +172,11 @@ ProcDRI2CreateDrawable(ClientPtr client) if (status != Success) return status; + if (!AddResource(stuff->drawable, dri2DrawableRes, pDrawable)) { + DRI2DestroyDrawable(pDrawable); + return BadAlloc; + } + return client->noClientException; } @@ -186,6 +192,8 @@ ProcDRI2DestroyDrawable(ClientPtr client) &pDrawable, &status)) return status; + FreeResourceByType(stuff->drawable, dri2DrawableRes, FALSE); + return client->noClientException; } @@ -612,11 +620,25 @@ SProcDRI2Dispatch (ClientPtr client) } } +static int DRI2DrawableGone(pointer p, XID id) +{ + DrawablePtr pDrawable = p; + + DRI2DestroyDrawable(pDrawable); + + return Success; +} + int DRI2EventBase; static void DRI2ExtensionInit(void) { + dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); + + if (!dri2DrawableRes) + return; + dri2Extension = AddExtension(DRI2_NAME, DRI2NumberEvents, DRI2NumberErrors, commit 18dc5d1f15fe0df7e836b057d34c4ca596e31e8f Author: Jesse Barnes Date: Thu Mar 4 09:19:13 2010 -0800 DRI2: fixup handling of last_swap_target We need to initialize the swap target, which is passed to the driver to schedule events. Rather than using -1 to indicate that the field is uninitialized, just make sure we initialize it at drawable creation time. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit c4d54816f2ee4883d8f9bcf4595474fb58c95146) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 48618e1..d60bd5e 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -116,9 +116,11 @@ DRI2GetDrawable(DrawablePtr pDraw) int DRI2CreateDrawable(DrawablePtr pDraw) { + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); WindowPtr pWin; PixmapPtr pPixmap; DRI2DrawablePtr pPriv; + CARD64 ust; pPriv = DRI2GetDrawable(pDraw); if (pPriv != NULL) @@ -141,7 +143,10 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->swap_count = 0; pPriv->target_sbc = -1; pPriv->swap_interval = 1; - pPriv->last_swap_target = -1; + /* Initialize last swap target from DDX if possible */ + if (!ds->GetMSC || !(*ds->GetMSC)(pDraw, &ust, &pPriv->last_swap_target)) + pPriv->last_swap_target = 0; + pPriv->swap_limit = 1; /* default to double buffering */ if (pDraw->type == DRAWABLE_WINDOW) @@ -579,7 +584,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); DRI2DrawablePtr pPriv; DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; - CARD64 ust; int ret, i; pPriv = DRI2GetDrawable(pDraw); @@ -621,27 +625,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, } /* - * In the simple glXSwapBuffers case, all params will be 0, and we just - * need to schedule a swap for the last swap target + the swap interval. - * If the last swap target hasn't been set yet, call into the driver - * to get the current count. - */ - if (target_msc == 0 && divisor == 0 && remainder == 0 && - pPriv->last_swap_target < 0) { - ret = (*ds->GetMSC)(pDraw, &ust, &target_msc); - if (!ret) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "[DRI2] %s: driver failed to return current MSC\n", - __func__); - return BadDrawable; - } - } - - /* First swap needs to initialize last_swap_target */ - if (pPriv->last_swap_target < 0) - pPriv->last_swap_target = target_msc; - - /* * Swap target for this swap is last swap target + swap interval since * we have to account for the current swap count, interval, and the * number of pending swaps. commit 96a12314ae41a2edec67d388c1c4770bd099d9ec Author: Jesse Barnes Date: Thu Mar 4 09:54:15 2010 -0800 DRI2: make target_sbc signed We need to track invalid targets as well as 0 targets, so just make it signed so our comparisons work like they should. Reviewed-by: Mario Kleiner Reported-by: Kristian Høgsberg Signed-off-by: Jesse Barnes (cherry picked from commit 4c8ec49826a46eb3b36c69d2ad3f82320c179c38) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index d60bd5e..ec4f982 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -62,7 +62,7 @@ typedef struct _DRI2Drawable { ClientPtr blockedClient; int swap_interval; CARD64 swap_count; - CARD64 target_sbc; /* -1 means no SBC wait outstanding */ + int64_t target_sbc; /* -1 means no SBC wait outstanding */ CARD64 last_swap_target; /* most recently queued swap target */ int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; commit a9b70da1afe7361955a19d34bb5e9b94dc156889 Author: Mario Kleiner Date: Sun Feb 21 05:25:59 2010 +0100 DRI2: Fix glitches in DRI2SwapComplete() and DRI2WakeupClient() DRI2SwapComplete(): Increment pPriv->swap_count++; before calling into callback for INTEL_swap_events extension, so the swap event contains the current SBC after swap completion instead of the previous one. DRI2WakeupClient: Check for pPriv->target_sbc <= pPriv->swap_count, had wrong comparison pPriv->target_sbc >= pPriv->swap_count for unblocking of clients of DRI2WaitSBC(). Signed-off-by: Mario Kleiner (cherry picked from commit 0de4974b90b10fa6a447cdf980b4a114c6c9e5a8) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index ec4f982..cb227be 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -509,7 +509,7 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, * blocked due to GLX activity during a swap. */ if (pPriv->target_sbc != -1 && - pPriv->target_sbc >= pPriv->swap_count) { + pPriv->target_sbc <= pPriv->swap_count) { ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec, frame, pPriv->swap_count); pPriv->target_sbc = -1; @@ -546,13 +546,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } + pPriv->swapsPending--; + pPriv->swap_count++; + ust = ((CARD64)tv_sec * 1000000) + tv_usec; if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); - pPriv->swapsPending--; - pPriv->swap_count++; - DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); } commit a56178402ede82ed3ffd81439b2efc4a39f2779e Author: Mario Kleiner Date: Sun Feb 21 05:26:00 2010 +0100 DRI2WaitSbc(): Fixes for correct semantic of glXWaitForSbcOML() Added implementation for case target_sbc == 0. In that case, the function shall schedule a wait until all pending swaps for the drawable have completed. Fix for non-blocking case. Old implementation returned random, uninitialized values for (ust,msc,sbc) if it returned immediately without scheduling a wait due to sbc >= target_sbc. Now if function doesn't schedule a wait, but returns immediately, it returns the (ust,msc,sbc) of the most recently completed swap, i.e., the UST and MSC corresponding to the time when the returned current SBC was reached. Signed-off-by: Mario Kleiner (cherry picked from commit 751e8c09d34df4b41e8d8384a3ec1bf5cb8ca028) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index cb227be..ef838ff 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -64,6 +64,8 @@ typedef struct _DRI2Drawable { CARD64 swap_count; int64_t target_sbc; /* -1 means no SBC wait outstanding */ CARD64 last_swap_target; /* most recently queued swap target */ + CARD64 last_swap_msc; /* msc at completion of most recent swap */ + CARD64 last_swap_ust; /* ust at completion of most recent swap */ int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; @@ -148,6 +150,8 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->last_swap_target = 0; pPriv->swap_limit = 1; /* default to double buffering */ + pPriv->last_swap_msc = 0; + pPriv->last_swap_ust = 0; if (pDraw->type == DRAWABLE_WINDOW) { @@ -553,6 +557,9 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, if (swap_complete) swap_complete(client, swap_data, type, ust, frame, pPriv->swap_count); + pPriv->last_swap_msc = frame; + pPriv->last_swap_ust = ust; + DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); } @@ -727,8 +734,22 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, if (pPriv == NULL) return BadDrawable; - if (pPriv->swap_count >= target_sbc) - return Success; + /* target_sbc == 0 means to block until all pending swaps are + * finished. Recalculate target_sbc to get that behaviour. + */ + if (target_sbc == 0) + target_sbc = pPriv->swap_count + pPriv->swapsPending; + + /* If current swap count already >= target_sbc, + * return immediately with (ust, msc, sbc) triplet of + * most recent completed swap. + */ + if (pPriv->swap_count >= target_sbc) { + *sbc = pPriv->swap_count; + *msc = pPriv->last_swap_msc; + *ust = pPriv->last_swap_ust; + return Success; + } pPriv->target_sbc = target_sbc; DRI2BlockClient(client, pDraw); commit 23d78005b06b15f7b68fc4eed7e325ad29d0880a Author: Jesse Barnes Date: Thu Mar 4 10:31:59 2010 -0800 DRI2: fix swapbuffers handling of SBC and target MSC Returns expected SBC after completion of swap to caller, as required by OML_sync_control spec, instead of the last_swap_target value. Passes target_msc, divisor, remainder, correctly for glXSwapBuffersMscOML() call, while retaining old behaviour for simple glXSwapBuffers() call. An OML swap can have a 0 target_msc, which just means it needs to satisfy the divisor/remainder equation. Pass this down to the driver as needed so we can support it. Signed-off-by: Jesse Barnes Signed-off-by: Mario Kleiner (cherry picked from commit b180e43977710b56ccfd6780f204ddcc952987a1) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index ef838ff..354b724 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -632,11 +632,20 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, } /* - * Swap target for this swap is last swap target + swap interval since - * we have to account for the current swap count, interval, and the - * number of pending swaps. + * In the simple glXSwapBuffers case, all params will be 0, and we just + * need to schedule a swap for the last swap target + the swap interval. */ - *swap_target = pPriv->last_swap_target + pPriv->swap_interval; + if (target_msc == 0 && divisor == 0 && remainder == 0) { + /* + * Swap target for this swap is last swap target + swap interval since + * we have to account for the current swap count, interval, and the + * number of pending swaps. + */ + *swap_target = pPriv->last_swap_target + pPriv->swap_interval; + } else { + /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */ + *swap_target = target_msc; + } ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, swap_target, divisor, remainder, func, data); @@ -649,6 +658,11 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, pPriv->swapsPending++; pPriv->last_swap_target = *swap_target; + /* According to spec, return expected swapbuffers count SBC after this swap + * will complete. + */ + *swap_target = pPriv->swap_count + pPriv->swapsPending; + return Success; } commit 73fe4a0952b5c8aea90bce6f758546c13d0754b5 Author: Jesse Barnes Date: Fri Mar 5 09:15:24 2010 -0800 DRI2: drawable lifetime fixes Handle drawable destruction and lifetime correctly. Check whether the drawable priv is valid in DRI2SwapInterval(), DRI2WaitSBC() and DRI2WaitMSC(); it may have gone away, so be sure to check it before using it. If more than 1 outstanding swap is queued, we may complete several after an app has exited. If we free it after the first one completes and the refcount reaches 0, we'll crash the server on subsequent completions. So delay freeing until all swaps complete and remove the error message as this is a normal occurence. To do this properly, we must also avoid destroying drawables in DRI2DestroyDrawable() if a swap or wait event is pending. And finally, make sure we free drawables in DRI2WaitMSCComplete() if necessary (i.e. if the refcount has reached 0 and this MSC was the last pending event on the object). Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit 8476d99231cb725c090305d60f1c1c889d25c8dc) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 354b724..58f478f 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -492,6 +492,10 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; + + /* If there's still a swap pending, let DRI2SwapComplete free it */ + if (pPriv->refCount == 0 && pPriv->swapsPending == 0) + DRI2FreeDrawable(pDraw); } static void @@ -543,13 +547,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, return; } - if (pPriv->refCount == 0) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "[DRI2] %s: bad drawable refcount\n", __func__); - DRI2FreeDrawable(pDraw); - return; - } - pPriv->swapsPending--; pPriv->swap_count++; @@ -561,6 +558,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, pPriv->last_swap_ust = ust; DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); + + /* + * It's normal for the app to have exited with a swap outstanding, but + * don't free the drawable until they're all complete. + */ + if (pPriv->swapsPending == 0 && pPriv->refCount == 0) + DRI2FreeDrawable(pDraw); } Bool @@ -669,10 +673,16 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, void DRI2SwapInterval(DrawablePtr pDrawable, int interval) { + ScreenPtr pScreen = pDrawable->pScreen; DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable); - /* fixme: check against arbitrary max? */ + if (pPriv == NULL) { + xf86DrvMsg(pScreen->myNum, X_ERROR, + "[DRI2] %s: bad drawable\n", __func__); + return; + } + /* fixme: check against arbitrary max? */ pPriv->swap_interval = interval; } @@ -721,7 +731,7 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, Bool ret; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) + if (pPriv == NULL || pPriv->refCount == 0) return BadDrawable; /* Old DDX just completes immediately */ @@ -745,7 +755,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, DRI2DrawablePtr pPriv; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) + if (pPriv == NULL || pPriv->refCount == 0) return BadDrawable; /* target_sbc == 0 means to block until all pending swaps are @@ -794,10 +804,10 @@ DRI2DestroyDrawable(DrawablePtr pDraw) xfree(pPriv->buffers); } - /* If the window is destroyed while we have a swap pending, don't + /* If the window is destroyed while we have a swap or wait pending, don't * actually free the priv yet. We'll need it in the DRI2SwapComplete() * callback and we'll free it there once we're done. */ - if (!pPriv->swapsPending) + if (!pPriv->swapsPending && !pPriv->blockedClient) DRI2FreeDrawable(pDraw); } commit 20708781d43501f116b4161c1cc96b0eb39e96b1 Author: Jesse Barnes Date: Fri Mar 5 09:49:03 2010 -0800 DRI2: handle swap_interval of 0 correctly A 0 swap interval means that swaps shouldn't be sync'd to vblank, so just complete the swap immediately in that case. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit 87ca6320f26eb3129e3c19056e1d8fa5c1784723) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 58f478f..9825a55 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -616,8 +616,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, return BadDrawable; } - /* Old DDX, just blit */ - if (!ds->ScheduleSwap) { + /* Old DDX or no swap interval, just blit */ + if (!ds->ScheduleSwap || !pPriv->swap_interval) { BoxRec box; RegionRec region; commit c7e9e36fd3cdb889704d3b0e1333653d06ef9069 Author: Jesse Barnes Date: Mon Mar 8 12:38:37 2010 -0800 DRI2: advertise lowest supported DRI2 protocol version Update our supported DRI2 protocol version as each driver does DRI2ScreenInit, since depending on available kernel features, each DDX may support different callbacks and therefore protocol. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit db1c7cb604167baf49e61be4c09ccf7b592c4af3) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 9825a55..3fc7f4e 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -45,6 +45,9 @@ #include "xf86.h" +CARD8 dri2_major; /* version of DRI2 supported by DDX */ +CARD8 dri2_minor; + static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; static int dri2WindowPrivateKeyIndex; @@ -848,6 +851,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) "VDPAU", /* DRI2DriverVDPAU */ }; unsigned int i; + CARD8 cur_minor; if (info->version < 3) return FALSE; @@ -864,6 +868,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->fd = info->fd; ds->deviceName = info->deviceName; + dri2_major = 1; ds->CreateBuffer = info->CreateBuffer; ds->DestroyBuffer = info->DestroyBuffer; @@ -873,8 +878,15 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->ScheduleSwap = info->ScheduleSwap; ds->ScheduleWaitMSC = info->ScheduleWaitMSC; ds->GetMSC = info->GetMSC; + cur_minor = 2; + } else { + cur_minor = 1; } + /* Initialize minor if needed and set to minimum provied by DDX */ + if (!dri2_minor || dri2_minor > cur_minor) + dri2_minor = cur_minor; + if (info->version == 3 || info->numDrivers == 0) { /* Driver too old: use the old-style driverName field */ ds->numDrivers = 1; diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 1c8626b..066cc39 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -46,6 +46,9 @@ typedef struct { void *driverPrivate; } DRI2BufferRec, *DRI2BufferPtr; +extern CARD8 dri2_major; /* version of DRI2 supported by DDX */ +extern CARD8 dri2_minor; + typedef DRI2BufferRec DRI2Buffer2Rec, *DRI2Buffer2Ptr; typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type, CARD64 ust, CARD64 msc, CARD64 sbc); diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index bd92fd3..7a9f8ca 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -80,8 +80,8 @@ ProcDRI2QueryVersion(ClientPtr client) rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; - rep.majorVersion = SERVER_DRI2_MAJOR_VERSION; - rep.minorVersion = SERVER_DRI2_MINOR_VERSION; + rep.majorVersion = dri2_major; + rep.minorVersion = dri2_minor; if (client->swapped) { swaps(&rep.sequenceNumber, n); diff --git a/include/protocol-versions.h b/include/protocol-versions.h index c74b7fa..97ef5da 100644 --- a/include/protocol-versions.h +++ b/include/protocol-versions.h @@ -51,10 +51,6 @@ #define SERVER_DMX_MINOR_VERSION 2 #define SERVER_DMX_PATCH_VERSION 20040604 -/* DRI2 */ -#define SERVER_DRI2_MAJOR_VERSION 1 -#define SERVER_DRI2_MINOR_VERSION 2 - /* Generic event extension */ #define SERVER_GE_MAJOR_VERSION 1 #define SERVER_GE_MINOR_VERSION 0 commit fcd76ddfc59de6a1d37ca2aa6e6e91cb8d814744 Author: Jesse Barnes Date: Mon Mar 8 12:39:54 2010 -0800 DRI2: throttle swaps at submission time too We need to throttle swaps here in addition to when the context is made current to avoid causing problems with clients that just swap. Throttling here also ensures our swaps get ordered as long as we block the client occasionally. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit 0294ff2a5cadddc8fcc77ba9a851f979f0b91fc3) diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 7a9f8ca..094d54d 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -384,6 +384,13 @@ ProcDRI2SwapBuffers(ClientPtr client) DixReadAccess | DixWriteAccess, &pDrawable, &status)) return status; + /* + * Ensures an out of control client can't exhaust our swap queue, and + * also orders swaps. + */ + if (DRI2ThrottleClient(client, pDrawable)) + return client->noClientException; + target_msc = vals_to_card64(stuff->target_msc_lo, stuff->target_msc_hi); divisor = vals_to_card64(stuff->divisor_lo, stuff->divisor_hi); remainder = vals_to_card64(stuff->remainder_lo, stuff->remainder_hi); commit 5c54e7fa004532d595160a815177dda467b3cc9c Author: Jesse Barnes Date: Mon Mar 8 12:41:25 2010 -0800 DRI2: handle swapsPending better Avoid a potential swapsPending underflow by incrementing it before ScheduleSwap, which may complete it immediately. And be sure to decrement it again in case the schedule failed. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit b00d435ddf2e9817e33bfd5f7e9b905442dc23c7) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 3fc7f4e..8a67122 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -654,15 +654,16 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, *swap_target = target_msc; } + pPriv->swapsPending++; ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer, swap_target, divisor, remainder, func, data); if (!ret) { + pPriv->swapsPending--; /* didn't schedule */ xf86DrvMsg(pScreen->myNum, X_ERROR, "[DRI2] %s: driver failed to schedule swap\n", __func__); return BadDrawable; } - pPriv->swapsPending++; pPriv->last_swap_target = *swap_target; /* According to spec, return expected swapbuffers count SBC after this swap commit b24856626e1836657ebf35ad4c61984e0a407c5b Author: Jesse Barnes Date: Mon Mar 8 15:10:47 2010 -0800 DRI2: prevent swap wakes from waking MSC waiters If a few swaps were queued leading to a throttle related block on the client, and then the client submitted an MSC wait, one of the previous swap wakeups could have caused the MSC wait to complete early. Add a flag for this to prevent a swap wake from prematurely waking an MSC waiter. Reported-by: Mario Kleiner Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit 5933b0abc6a76aaea84aa534df89900cd795c888) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 8a67122..8b4c36f 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -63,6 +63,7 @@ typedef struct _DRI2Drawable { int bufferCount; unsigned int swapsPending; ClientPtr blockedClient; + Bool blockedOnMsc; int swap_interval; CARD64 swap_count; int64_t target_sbc; /* -1 means no SBC wait outstanding */ @@ -145,6 +146,7 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->bufferCount = 0; pPriv->swapsPending = 0; pPriv->blockedClient = NULL; + pPriv->blockedOnMsc = FALSE; pPriv->swap_count = 0; pPriv->target_sbc = -1; pPriv->swap_interval = 1; @@ -402,6 +404,15 @@ DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw) return FALSE; } +static void +__DRI2BlockClient(ClientPtr client, DRI2DrawablePtr pPriv) +{ + if (pPriv->blockedClient == NULL) { + IgnoreClient(client); + pPriv->blockedClient = client; + } +} + void DRI2BlockClient(ClientPtr client, DrawablePtr pDraw) { @@ -411,10 +422,8 @@ DRI2BlockClient(ClientPtr client, DrawablePtr pDraw) if (pPriv == NULL) return; - if (pPriv->blockedClient == NULL) { - IgnoreClient(client); - pPriv->blockedClient = client; - } + __DRI2BlockClient(client, pPriv); + pPriv->blockedOnMsc = TRUE; } int @@ -495,6 +504,7 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; + pPriv->blockedOnMsc = FALSE; /* If there's still a swap pending, let DRI2SwapComplete free it */ if (pPriv->refCount == 0 && pPriv->swapsPending == 0) @@ -516,8 +526,12 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, } /* - * Swap completed. Either wake up an SBC waiter or a client that was - * blocked due to GLX activity during a swap. + * Swap completed. + * Wake the client iff: + * - it was waiting on SBC + * - was blocked due to GLX make current + * - was blocked due to swap throttling + * - is not blocked due to an MSC wait */ if (pPriv->target_sbc != -1 && pPriv->target_sbc <= pPriv->swap_count) { @@ -527,10 +541,11 @@ DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame, AttendClient(pPriv->blockedClient); pPriv->blockedClient = NULL; - } else if (pPriv->target_sbc == -1) { - if (pPriv->blockedClient) + } else if (pPriv->target_sbc == -1 && !pPriv->blockedOnMsc) { + if (pPriv->blockedClient) { AttendClient(pPriv->blockedClient); - pPriv->blockedClient = NULL; + pPriv->blockedClient = NULL; + } } } @@ -582,7 +597,7 @@ DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable) pPriv->blockedClient == NULL) { ResetCurrentRequest(client); client->sequence--; - DRI2BlockClient(client, pDrawable); + __DRI2BlockClient(client, pPriv); return TRUE; } @@ -780,7 +795,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, } pPriv->target_sbc = target_sbc; - DRI2BlockClient(client, pDraw); + __DRI2BlockClient(client, pPriv); return Success; } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 066cc39..e1881ba 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -257,6 +257,7 @@ extern _X_EXPORT Bool DRI2CanFlip(DrawablePtr pDraw); extern _X_EXPORT Bool DRI2CanExchange(DrawablePtr pDraw); +/* Note: use *only* for MSC related waits */ extern _X_EXPORT void DRI2BlockClient(ClientPtr client, DrawablePtr pDraw); extern _X_EXPORT void DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, commit a8de0866130df62c2b1e4653e028a0fe365e0e3a Author: Jesse Barnes Date: Tue Mar 23 09:47:08 2010 -0700 GLX/DRI2: expose swap control extensions if DDX support is present Export DDX swap control status from the DRI2 module and check for it in GLX when initializing extensions. Reviewed-by: Mario Kleiner Signed-off-by: Jesse Barnes (cherry picked from commit 165a4a9c7de0fcc6ef6a6421736b412ccb35965e) diff --git a/glx/glxdri2.c b/glx/glxdri2.c index edd29b0..e791bf6 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -616,6 +616,7 @@ glxDRILeaveVT (int index, int flags) static void initializeExtensions(__GLXDRIscreen *screen) { + ScreenPtr pScreen = screen->base.pScreen; const __DRIextension **extensions; int i; @@ -625,10 +626,17 @@ initializeExtensions(__GLXDRIscreen *screen) "GLX_MESA_copy_sub_buffer"); LogMessage(X_INFO, "AIGLX: enabled GLX_MESA_copy_sub_buffer\n"); - /* FIXME: only if DDX supports it */ __glXEnableExtension(screen->glx_enable_bits, "GLX_INTEL_swap_event"); LogMessage(X_INFO, "AIGLX: enabled GLX_INTEL_swap_event\n"); + if (DRI2HasSwapControl(pScreen)) { + __glXEnableExtension(screen->glx_enable_bits, + "GLX_SGI_swap_control"); + __glXEnableExtension(screen->glx_enable_bits, + "GLX_MESA_swap_control"); + LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_swap_control and GLX_MESA_swap_control\n"); + } + for (i = 0; extensions[i]; i++) { #ifdef __DRI_READ_DRAWABLE if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) { @@ -639,19 +647,6 @@ initializeExtensions(__GLXDRIscreen *screen) } #endif -#ifdef __DRI_SWAP_CONTROL - if (strcmp(extensions[i]->name, __DRI_SWAP_CONTROL) == 0) { - screen->swapControl = - (const __DRIswapControlExtension *) extensions[i]; - __glXEnableExtension(screen->glx_enable_bits, - "GLX_SGI_swap_control"); - __glXEnableExtension(screen->glx_enable_bits, - "GLX_MESA_swap_control"); - - LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_swap_control and GLX_MESA_swap_control\n"); - } -#endif - #ifdef __DRI_TEX_BUFFER if (strcmp(extensions[i]->name, __DRI_TEX_BUFFER) == 0) { screen->texBuffer = diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 8b4c36f..2bdb733 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -831,6 +831,14 @@ DRI2DestroyDrawable(DrawablePtr pDraw) } Bool +DRI2HasSwapControl(ScreenPtr pScreen) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pScreen); + + return (ds->ScheduleSwap && ds->GetMSC); +} + +Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd, const char **driverName, const char **deviceName) { diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index e1881ba..ce8a5df 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -188,6 +188,8 @@ extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, extern _X_EXPORT void DRI2CloseScreen(ScreenPtr pScreen); +extern _X_EXPORT Bool DRI2HasSwapControl(ScreenPtr pScreen); + extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen, unsigned int driverType, int *fd, commit a847cc9bce744cab72637d62bc6bd45237bd3de5 Author: Kristian Høgsberg Date: Sat May 1 13:07:46 2010 -0400 dix: Update element count in FreeResource*() FreeResource() keeps clientTable[cid].elements up to date with the number of resources allocated to the client. The other free resource functions (FreeResourceByType(), FreeClientNeverRetainResources() and FreeClientResources()) don't maintain this invariant. Typically, the only consequence is that the element count is too high and we end up allocating the hash table bigger than necessary. However, FreeResource() also relies on the element count to restart the search if the list of resources has been changed during a resource destruction callback. Since FreeResourceByType() doesn't update the count, if we call that from a resource destruction callback from FreeResource(), the loop isn't restarted and we end up following an invalid next pointer. Furthermore, LookupClientResourceComplex() and FreeClientNeverRetainResources() don't use the element count to detect if a callback deleted a resource and may end up following an invalid next pointer if the resource system is called into recursively. Signed-off-by: Kristian Høgsberg Reviewed-by: Keith Packard (cherry picked from commit 6d7ba5e0fcb5d1bce6bb213dec009f3a0f802d26) diff --git a/dix/resource.c b/dix/resource.c index 91d0cfb..ab3762e 100644 --- a/dix/resource.c +++ b/dix/resource.c @@ -589,6 +589,7 @@ FreeResourceByType(XID id, RESTYPE type, Bool skipFree) res->value, TypeNameString(res->type)); #endif *prev = res->next; + clientTable[cid].elements--; CallResourceStateCallback(ResourceStateFreeing, res); @@ -734,12 +735,14 @@ FreeClientNeverRetainResources(ClientPtr client) ResourcePtr *resources; ResourcePtr this; ResourcePtr *prev; - int j; + int j, elements; + int *eltptr; if (!client) return; resources = clientTable[client->index].resources; + eltptr = &clientTable[client->index].elements; for (j=0; j < clientTable[client->index].buckets; j++) { prev = &resources[j]; @@ -753,11 +756,15 @@ FreeClientNeverRetainResources(ClientPtr client) this->value, TypeNameString(this->type)); #endif *prev = this->next; + clientTable[client->index].elements--; CallResourceStateCallback(ResourceStateFreeing, this); + elements = *eltptr; (*DeleteFuncs[rtype & TypeMask])(this->value, this->id); xfree(this); + if (*eltptr != elements) + prev = &resources[j]; /* prev may no longer be valid */ } else prev = &this->next; @@ -804,6 +811,7 @@ FreeClientResources(ClientPtr client) this->value, TypeNameString(this->type)); #endif *head = this->next; + clientTable[client->index].elements--; CallResourceStateCallback(ResourceStateFreeing, this); commit 44521bb9d03a0b7af10f89c98e51bd4109fec327 Author: Kristian Høgsberg Date: Sat May 1 13:13:54 2010 -0400 glxdri2: Hard-code the extension version we need If we use the #define'd version from dri_interface.h, the server will require at least that version of the extension. If we're compiling against a dri_interface.h with a newer version we don't really require, glxdri2 will require a too high version of the extension. The right approach is to just hard-code the version we need instead of using the #defines. Signed-off-by: Kristian Høgsberg Reviewed-by: Ian Romanick Reviewed-by: Adam Jackson (cherry picked from commit 4a8a615d01b9ed18c272414bd11dc2fc661727e5) diff --git a/glx/glxdri2.c b/glx/glxdri2.c index e791bf6..e84c28b 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -658,7 +658,7 @@ initializeExtensions(__GLXDRIscreen *screen) #ifdef __DRI2_FLUSH if (strcmp(extensions[i]->name, __DRI2_FLUSH) == 0 && - extensions[i]->version >= __DRI2_FLUSH_VERSION) { + extensions[i]->version >= 3) { screen->flush = (__DRI2flushExtension *) extensions[i]; } #endif @@ -718,11 +718,11 @@ __glXDRIscreenProbe(ScreenPtr pScreen) for (i = 0; extensions[i]; i++) { if (strcmp(extensions[i]->name, __DRI_CORE) == 0 && - extensions[i]->version >= __DRI_CORE_VERSION) { + extensions[i]->version >= 1) { screen->core = (const __DRIcoreExtension *) extensions[i]; } if (strcmp(extensions[i]->name, __DRI_DRI2) == 0 && - extensions[i]->version >= __DRI_DRI2_VERSION) { + extensions[i]->version >= 1) { screen->dri2 = (const __DRIdri2Extension *) extensions[i]; } } commit 95f56176255d5b689b06793d9cbeed36c6a46227 Author: Kristian Høgsberg Date: Sat May 1 13:15:00 2010 -0400 list.h: Add list_for_each_entry_safe() Signed-off-by: Kristian Høgsberg Reviewed-by: Adam Jackson (cherry picked from commit 32381363cd8f43aeb741bad70bcf96a287dac0c9) diff --git a/include/list.h b/include/list.h index a126a65..89dc29d 100644 --- a/include/list.h +++ b/include/list.h @@ -94,4 +94,10 @@ list_is_empty(struct list *head) &pos->member != (head); \ pos = __container_of(pos->member.next, pos, member)) +#define list_for_each_entry_safe(pos, next, head, member) \ + for (pos = __container_of((head)->next, pos, member), \ + next = __container_of(pos->member.next, pos, member); \ + &pos->member != (head); \ + pos = next, next = __container_of(next->member.next, next, member)) + #endif commit 7faff42deb4b5a71504377375eba95c3cac2e013 Author: Peter Hutterer Date: Wed Jun 2 11:11:21 2010 +1000 Revert "Revert "DRI2: Track DRI2 drawables as resources, not privates"" This reverts commit c394b17266301d363a9e234f58f8015f74e01307. Follow-up patch should fix the reason this was reverted in the first place. Conflicts: hw/xfree86/dri2/dri2.c diff --git a/glx/glxdri2.c b/glx/glxdri2.c index e84c28b..2975bb3 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -105,11 +105,6 @@ __glXDRIdrawableDestroy(__GLXdrawable *drawable) (*core->destroyDrawable)(private->driDrawable); - /* If the X window was destroyed, the dri DestroyWindow hook will - * aready have taken care of this, so only call if pDraw isn't NULL. */ - if (drawable->pDraw != NULL) - DRI2DestroyDrawable(drawable->pDraw); - __glXDrawableRelease(drawable); xfree(private); diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 2bdb733..e8985b0 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -48,15 +48,14 @@ CARD8 dri2_major; /* version of DRI2 supported by DDX */ CARD8 dri2_minor; -static int dri2ScreenPrivateKeyIndex; +static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; -static int dri2WindowPrivateKeyIndex; -static DevPrivateKey dri2WindowPrivateKey = &dri2WindowPrivateKeyIndex; -static int dri2PixmapPrivateKeyIndex; -static DevPrivateKey dri2PixmapPrivateKey = &dri2PixmapPrivateKeyIndex; +static RESTYPE dri2DrawableRes; + +typedef struct _DRI2Screen *DRI2ScreenPtr; typedef struct _DRI2Drawable { - unsigned int refCount; + DRI2ScreenPtr dri2_screen; int width; int height; DRI2BufferPtr *buffers; @@ -73,9 +72,8 @@ typedef struct _DRI2Drawable { int swap_limit; /* for N-buffering */ } DRI2DrawableRec, *DRI2DrawablePtr; -typedef struct _DRI2Screen *DRI2ScreenPtr; - typedef struct _DRI2Screen { + ScreenPtr screen; unsigned int numDrivers; const char **driverNames; const char *deviceName; @@ -101,45 +99,35 @@ DRI2GetScreen(ScreenPtr pScreen) static DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { - WindowPtr pWin; - PixmapPtr pPixmap; + DRI2DrawablePtr pPriv; + int rc; - if (!pDraw) + rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, + dri2DrawableRes, NULL, DixReadAccess); + if (rc != Success) return NULL; - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - return dixLookupPrivate(&pWin->devPrivates, dri2WindowPrivateKey); - } - else - { - pPixmap = (PixmapPtr) pDraw; - return dixLookupPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey); - } + return pPriv; } int DRI2CreateDrawable(DrawablePtr pDraw) { DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); - WindowPtr pWin; - PixmapPtr pPixmap; DRI2DrawablePtr pPriv; CARD64 ust; + int rc; - pPriv = DRI2GetDrawable(pDraw); - if (pPriv != NULL) - { - pPriv->refCount++; - return Success; - } + rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, + dri2DrawableRes, NULL, DixReadAccess); + if (rc == Success || rc != BadValue) + return rc; pPriv = xalloc(sizeof *pPriv); if (pPriv == NULL) return BadAlloc; - pPriv->refCount = 1; + pPriv->dri2_screen = DRI2GetScreen(pDraw->pScreen); pPriv->width = pDraw->width; pPriv->height = pDraw->height; pPriv->buffers = NULL; @@ -158,43 +146,30 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->last_swap_msc = 0; pPriv->last_swap_ust = 0; - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv); - } - else - { - pPixmap = (PixmapPtr) pDraw; - dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv); - } + if (!AddResource(pDraw->id, dri2DrawableRes, pPriv)) + return BadAlloc; return Success; } -static void -DRI2FreeDrawable(DrawablePtr pDraw) +static int DRI2DrawableGone(pointer p, XID id) { - DRI2DrawablePtr pPriv; - WindowPtr pWin; - PixmapPtr pPixmap; + DRI2DrawablePtr pPriv = p; + DRI2ScreenPtr ds = pPriv->dri2_screen; + DrawablePtr root; + int i; - pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) - return; + root = &WindowTable[ds->screen->myNum]->drawable; + if (pPriv->buffers != NULL) { + for (i = 0; i < pPriv->bufferCount; i++) + (*ds->DestroyBuffer)(root, pPriv->buffers[i]); + + xfree(pPriv->buffers); + } xfree(pPriv); - if (pDraw->type == DRAWABLE_WINDOW) - { - pWin = (WindowPtr) pDraw; - dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL); - } - else - { - pPixmap = (PixmapPtr) pDraw; - dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL); - } + return Success; } static int @@ -506,9 +481,6 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame, pPriv->blockedClient = NULL; pPriv->blockedOnMsc = FALSE; - /* If there's still a swap pending, let DRI2SwapComplete free it */ - if (pPriv->refCount == 0 && pPriv->swapsPending == 0) - DRI2FreeDrawable(pDraw); } static void @@ -576,13 +548,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame, pPriv->last_swap_ust = ust; DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec); - - /* - * It's normal for the app to have exited with a swap outstanding, but - * don't free the drawable until they're all complete. - */ - if (pPriv->swapsPending == 0 && pPriv->refCount == 0) - DRI2FreeDrawable(pDraw); } Bool @@ -750,7 +715,7 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, Bool ret; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL || pPriv->refCount == 0) + if (pPriv == NULL) return BadDrawable; /* Old DDX just completes immediately */ @@ -774,7 +739,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, DRI2DrawablePtr pPriv; pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL || pPriv->refCount == 0) + if (pPriv == NULL) return BadDrawable; /* target_sbc == 0 means to block until all pending swaps are @@ -800,36 +765,6 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, return Success; } -void -DRI2DestroyDrawable(DrawablePtr pDraw) -{ - DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); - DRI2DrawablePtr pPriv; - - pPriv = DRI2GetDrawable(pDraw); - if (pPriv == NULL) - return; - - pPriv->refCount--; - if (pPriv->refCount > 0) - return; - - if (pPriv->buffers != NULL) { - int i; - - for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); - - xfree(pPriv->buffers); - } - - /* If the window is destroyed while we have a swap or wait pending, don't - * actually free the priv yet. We'll need it in the DRI2SwapComplete() - * callback and we'll free it there once we're done. */ - if (!pPriv->swapsPending && !pPriv->blockedClient) - DRI2FreeDrawable(pDraw); -} - Bool DRI2HasSwapControl(ScreenPtr pScreen) { @@ -890,6 +825,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) if (!ds) return FALSE; + ds->screen = pScreen; ds->fd = info->fd; ds->deviceName = info->deviceName; dri2_major = 1; @@ -961,6 +897,8 @@ DRI2Setup(pointer module, pointer opts, int *errmaj, int *errmin) { static Bool setupDone = FALSE; + dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); + if (!setupDone) { setupDone = TRUE; diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 094d54d..17df130 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -51,7 +51,6 @@ #include "xf86Module.h" static ExtensionEntry *dri2Extension; -static RESTYPE dri2DrawableRes; static Bool validDrawable(ClientPtr client, XID drawable, Mask access_mode, @@ -172,11 +171,6 @@ ProcDRI2CreateDrawable(ClientPtr client) if (status != Success) return status; - if (!AddResource(stuff->drawable, dri2DrawableRes, pDrawable)) { - DRI2DestroyDrawable(pDrawable); - return BadAlloc; - } - return client->noClientException; } @@ -192,8 +186,6 @@ ProcDRI2DestroyDrawable(ClientPtr client) &pDrawable, &status)) return status; - FreeResourceByType(stuff->drawable, dri2DrawableRes, FALSE); - return client->noClientException; } @@ -627,25 +619,11 @@ SProcDRI2Dispatch (ClientPtr client) } } -static int DRI2DrawableGone(pointer p, XID id) -{ - DrawablePtr pDrawable = p; - - DRI2DestroyDrawable(pDrawable); - - return Success; -} - int DRI2EventBase; static void DRI2ExtensionInit(void) { - dri2DrawableRes = CreateNewResourceType(DRI2DrawableGone, "DRI2Drawable"); - - if (!dri2DrawableRes) - return; - dri2Extension = AddExtension(DRI2_NAME, DRI2NumberEvents, DRI2NumberErrors, commit 598603021f6c4fa64161366177e93de67511bc2e Author: Peter Hutterer Date: Wed Jun 2 11:13:17 2010 +1000 dri2: Take an XID for tracking the DRI2 drawable Some pixmaps (window pixmaps and scratch pixmaps) don't have the drawable->id set and thus DRI2 gets confused when using that field for looking up the DRI2 drawable. Go back to using privates for getting at the DRI2 drawable from a DrawablePtr. We need to keep the resource tracking in place so we can remove the DRI2 drawable when the X resource it was created for goes away. Additionally, we also now track the DRI2 drawable using a client XID so we can reclaim the DRI2 drawable even if the client goes before the drawable and doesn't destroy the DRI2 drawable. Tested-by: Owen W. Taylor Signed-off-by: Kristian Høgsberg (cherry picked from commit 9de0e31746d5f0d9d39d11c94ec3cbc04a9935fc) Conflicts: hw/xfree86/dri2/dri2.c diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 087d52e..ec3bbe6 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -512,8 +512,9 @@ __glXGetDrawable(__GLXcontext *glxc, GLXDrawable drawId, ClientPtr client, if (!validGlxFBConfigForWindow(client, glxc->config, pDraw, error)) return NULL; - pGlxDraw = glxc->pGlxScreen->createDrawable(glxc->pGlxScreen, - pDraw, GLX_DRAWABLE_WINDOW, + pGlxDraw = glxc->pGlxScreen->createDrawable(client, glxc->pGlxScreen, + pDraw, drawId, + GLX_DRAWABLE_WINDOW, drawId, glxc->config); /* since we are creating the drawablePrivate, drawId should be new */ @@ -1104,15 +1105,17 @@ __glXDrawableRelease(__GLXdrawable *drawable) } static int -DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config, - DrawablePtr pDraw, XID glxDrawableId, int type) +DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, + __GLXconfig *config, DrawablePtr pDraw, XID drawableId, + XID glxDrawableId, int type) { __GLXdrawable *pGlxDraw; if (pGlxScreen->pScreen != pDraw->pScreen) return BadMatch; - pGlxDraw = pGlxScreen->createDrawable(pGlxScreen, pDraw, type, + pGlxDraw = pGlxScreen->createDrawable(client, pGlxScreen, pDraw, + drawableId, type, glxDrawableId, config); if (pGlxDraw == NULL) return BadAlloc; @@ -1125,7 +1128,7 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *conf /* Add the glx drawable under the XID of the underlying X drawable * too. That way we'll get a callback in DrawableGone and can * clean up properly when the drawable is destroyed. */ - if (pDraw->id != glxDrawableId && + if (drawableId != glxDrawableId && !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { pGlxDraw->destroy (pGlxDraw); return BadAlloc; @@ -1153,7 +1156,7 @@ DoCreateGLXPixmap(ClientPtr client, __GLXscreen *pGlxScreen, __GLXconfig *config return BadPixmap; } - err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw, + err = DoCreateGLXDrawable(client, pGlxScreen, config, pDraw, drawableId, glxDrawableId, GLX_DRAWABLE_PIXMAP); return err; @@ -1316,7 +1319,8 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId, return BadAlloc; return DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable, - glxDrawableId, GLX_DRAWABLE_PBUFFER); + glxDrawableId, glxDrawableId, + GLX_DRAWABLE_PBUFFER); } int __glXDisp_CreatePbuffer(__GLXclientState *cl, GLbyte *pc) @@ -1439,7 +1443,8 @@ int __glXDisp_CreateWindow(__GLXclientState *cl, GLbyte *pc) return err; return DoCreateGLXDrawable(client, pGlxScreen, config, - pDraw, req->glxwindow, GLX_DRAWABLE_WINDOW); + pDraw, req->window, + req->glxwindow, GLX_DRAWABLE_WINDOW); } int __glXDisp_DestroyWindow(__GLXclientState *cl, GLbyte *pc) diff --git a/glx/glxdri.c b/glx/glxdri.c index 21e44d1..e4870c3 100644 --- a/glx/glxdri.c +++ b/glx/glxdri.c @@ -682,10 +682,12 @@ __glXDRIscreenCreateContext(__GLXscreen *baseScreen, } static __GLXdrawable * -__glXDRIscreenCreateDrawable(__GLXscreen *screen, +__glXDRIscreenCreateDrawable(ClientPtr client, + __GLXscreen *screen, DrawablePtr pDraw, - int type, XID drawId, + int type, + XID glxDrawId, __GLXconfig *glxConfig) { __GLXDRIscreen *driScreen = (__GLXDRIscreen *) screen; @@ -699,7 +701,7 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen, return NULL; if (!__glXDrawableInit(&private->base, screen, - pDraw, type, drawId, glxConfig)) { + pDraw, type, glxDrawId, glxConfig)) { xfree(private); return NULL; } diff --git a/glx/glxdri2.c b/glx/glxdri2.c index 2975bb3..a580df3 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -430,10 +430,12 @@ __glXDRIscreenCreateContext(__GLXscreen *baseScreen, } static __GLXdrawable * -__glXDRIscreenCreateDrawable(__GLXscreen *screen, +__glXDRIscreenCreateDrawable(ClientPtr client, + __GLXscreen *screen, DrawablePtr pDraw, - int type, XID drawId, + int type, + XID glxDrawId, __GLXconfig *glxConfig) { __GLXDRIscreen *driScreen = (__GLXDRIscreen *) screen; @@ -446,7 +448,7 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen, private->screen = driScreen; if (!__glXDrawableInit(&private->base, screen, - pDraw, type, drawId, glxConfig)) { + pDraw, type, glxDrawId, glxConfig)) { xfree(private); return NULL; } @@ -457,7 +459,7 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen, private->base.waitGL = __glXDRIdrawableWaitGL; private->base.waitX = __glXDRIdrawableWaitX; - if (DRI2CreateDrawable(pDraw)) { + if (DRI2CreateDrawable(client, pDraw, drawId)) { xfree(private); return NULL; } diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c index c647d83..6a34393 100644 --- a/glx/glxdriswrast.c +++ b/glx/glxdriswrast.c @@ -301,10 +301,12 @@ glxChangeGC(GCPtr gc, BITS32 mask, CARD32 val) } static __GLXdrawable * -__glXDRIscreenCreateDrawable(__GLXscreen *screen, +__glXDRIscreenCreateDrawable(ClientPtr client, + __GLXscreen *screen, DrawablePtr pDraw, - int type, XID drawId, + int type, + XID glxDrawId, __GLXconfig *glxConfig) { __GLXDRIscreen *driScreen = (__GLXDRIscreen *) screen; @@ -319,7 +321,7 @@ __glXDRIscreenCreateDrawable(__GLXscreen *screen, private->screen = driScreen; if (!__glXDrawableInit(&private->base, screen, - pDraw, type, drawId, glxConfig)) { + pDraw, type, glxDrawId, glxConfig)) { xfree(private); return NULL; } diff --git a/glx/glxscreens.h b/glx/glxscreens.h index d52099f..861e03c 100644 --- a/glx/glxscreens.h +++ b/glx/glxscreens.h @@ -134,10 +134,12 @@ struct __GLXscreen { __GLXconfig *modes, __GLXcontext *shareContext); - __GLXdrawable *(*createDrawable)(__GLXscreen *context, + __GLXdrawable *(*createDrawable)(ClientPtr client, + __GLXscreen *context, DrawablePtr pDraw, - int type, XID drawId, + int type, + XID glxDrawId, __GLXconfig *modes); int (*swapInterval) (__GLXdrawable *drawable, int interval); diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index e8985b0..dd9bb41 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -37,6 +37,7 @@ #include #include #include "xf86Module.h" +#include "list.h" #include "scrnintstr.h" #include "windowstr.h" #include "dixstruct.h" @@ -50,12 +51,18 @@ CARD8 dri2_minor; static int dri2ScreenPrivateKeyIndex; static DevPrivateKey dri2ScreenPrivateKey = &dri2ScreenPrivateKeyIndex; +static int dri2WindowPrivateKeyIndex; +static DevPrivateKey dri2WindowPrivateKey = &dri2WindowPrivateKeyIndex; +static int dri2PixmapPrivateKeyIndex; +static DevPrivateKey dri2PixmapPrivateKey = &dri2PixmapPrivateKeyIndex; static RESTYPE dri2DrawableRes; typedef struct _DRI2Screen *DRI2ScreenPtr; typedef struct _DRI2Drawable { DRI2ScreenPtr dri2_screen; + DrawablePtr drawable; + struct list reference_list; int width; int height; DRI2BufferPtr *buffers; @@ -74,6 +81,7 @@ typedef struct _DRI2Drawable { typedef struct _DRI2Screen { ScreenPtr screen; + int refcnt; unsigned int numDrivers; const char **driverNames; const char *deviceName; @@ -99,35 +107,33 @@ DRI2GetScreen(ScreenPtr pScreen) static DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw) { - DRI2DrawablePtr pPriv; - int rc; - - rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, - dri2DrawableRes, NULL, DixReadAccess); - if (rc != Success) - return NULL; + WindowPtr pWin; + PixmapPtr pPixmap; - return pPriv; + if (pDraw->type == DRAWABLE_WINDOW) { + pWin = (WindowPtr) pDraw; + return dixLookupPrivate(&pWin->devPrivates, dri2WindowPrivateKey); + } else { + pPixmap = (PixmapPtr) pDraw; + return dixLookupPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey); + } } -int -DRI2CreateDrawable(DrawablePtr pDraw) +static DRI2DrawablePtr +DRI2AllocateDrawable(DrawablePtr pDraw) { DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); DRI2DrawablePtr pPriv; CARD64 ust; - int rc; - - rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id, - dri2DrawableRes, NULL, DixReadAccess); - if (rc == Success || rc != BadValue) - return rc; + WindowPtr pWin; + PixmapPtr pPixmap; pPriv = xalloc(sizeof *pPriv); if (pPriv == NULL) - return BadAlloc; + return NULL; - pPriv->dri2_screen = DRI2GetScreen(pDraw->pScreen); + pPriv->dri2_screen = ds; + pPriv->drawable = pDraw; pPriv->width = pDraw->width; pPriv->height = pDraw->height; pPriv->buffers = NULL; @@ -145,9 +151,77 @@ DRI2CreateDrawable(DrawablePtr pDraw) pPriv->swap_limit = 1; /* default to double buffering */ pPriv->last_swap_msc = 0; pPriv->last_swap_ust = 0; + list_init(&pPriv->reference_list); - if (!AddResource(pDraw->id, dri2DrawableRes, pPriv)) + if (pDraw->type == DRAWABLE_WINDOW) { + pWin = (WindowPtr) pDraw; + dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv); + } else { + pPixmap = (PixmapPtr) pDraw; + dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv); + } + + return pPriv; +} + +typedef struct DRI2DrawableRefRec { + XID id; + XID dri2_id; + struct list link; +} DRI2DrawableRefRec, *DRI2DrawableRefPtr; + +static DRI2DrawableRefPtr +DRI2LookupDrawableRef(DRI2DrawablePtr pPriv, XID id) +{ + DRI2DrawableRefPtr ref; + + list_for_each_entry(ref, &pPriv->reference_list, link) { + if (ref->id == id) + return ref; + } + + return NULL; +} + +static int +DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id) +{ + DRI2DrawableRefPtr ref; + + ref = malloc(sizeof *ref); + if (ref == NULL) + return BadAlloc; + + if (!AddResource(dri2_id, dri2DrawableRes, pPriv)) + return BadAlloc; + if (!DRI2LookupDrawableRef(pPriv, id)) + if (!AddResource(id, dri2DrawableRes, pPriv)) + return BadAlloc; + + ref->id = id; + ref->dri2_id = dri2_id; + list_add(&ref->link, &pPriv->reference_list); + + return Success; +} + +int +DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id) +{ + DRI2DrawablePtr pPriv; + XID dri2_id; + int rc; + + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + pPriv = DRI2AllocateDrawable(pDraw); + if (pPriv == NULL) return BadAlloc; + + dri2_id = FakeClientID(client->index); + rc = DRI2AddDrawableRef(pPriv, id, dri2_id); + if (rc != Success) + return rc; return Success; } @@ -156,13 +230,45 @@ static int DRI2DrawableGone(pointer p, XID id) { DRI2DrawablePtr pPriv = p; DRI2ScreenPtr ds = pPriv->dri2_screen; - DrawablePtr root; + DRI2DrawableRefPtr ref, next; + WindowPtr pWin; + PixmapPtr pPixmap; + DrawablePtr pDraw; int i; - root = &WindowTable[ds->screen->myNum]->drawable; + list_for_each_entry_safe(ref, next, &pPriv->reference_list, link) { + if (ref->dri2_id == id) { + list_del(&ref->link); + /* If this was the last ref under this X drawable XID, + * unregister the X drawable resource. */ + if (!DRI2LookupDrawableRef(pPriv, ref->id)) + FreeResourceByType(ref->id, dri2DrawableRes, TRUE); + free(ref); + break; + } + + if (ref->id == id) { + list_del(&ref->link); + FreeResourceByType(ref->dri2_id, dri2DrawableRes, TRUE); + free(ref); + } + } + + if (!list_is_empty(&pPriv->reference_list)) + return Success; + + pDraw = pPriv->drawable; + if (pDraw->type == DRAWABLE_WINDOW) { + pWin = (WindowPtr) pDraw; + dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL); + } else { + pPixmap = (PixmapPtr) pDraw; + dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL); + } + if (pPriv->buffers != NULL) { for (i = 0; i < pPriv->bufferCount; i++) - (*ds->DestroyBuffer)(root, pPriv->buffers[i]); + (*ds->DestroyBuffer)(pDraw, pPriv->buffers[i]); xfree(pPriv->buffers); } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index ce8a5df..5415a0b 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -198,7 +198,8 @@ extern _X_EXPORT Bool DRI2Connect(ScreenPtr pScreen, extern _X_EXPORT Bool DRI2Authenticate(ScreenPtr pScreen, drm_magic_t magic); -extern _X_EXPORT int DRI2CreateDrawable(DrawablePtr pDraw); +extern _X_EXPORT int DRI2CreateDrawable(ClientPtr client, + DrawablePtr pDraw, XID id); extern _X_EXPORT void DRI2DestroyDrawable(DrawablePtr pDraw); diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 17df130..58eaa10 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -167,7 +167,7 @@ ProcDRI2CreateDrawable(ClientPtr client) &pDrawable, &status)) return status; - status = DRI2CreateDrawable(pDrawable); + status = DRI2CreateDrawable(client, pDrawable, stuff->drawable); if (status != Success) return status; commit 421f5dfdd8b566dc07b4606f0edec487b3ead3d9 Author: Peter Hutterer Date: Fri Jun 11 11:08:31 2010 +1000 DRI2: Don't return junk reply instead of blocking in glXWaitForSbcOML() DRI2WaitSBC() didn't block if requested targetSBC wasn't yet reached. Instead it returned a xreply with uninitialized junk return values, then blocked the connection until targetSBC was reached. Therefore the client didn't block, but continued with bogus return values from glXWaitForSbcOML. This patch fixes the problem by implementing DRI2WaitSBC similar to the clean and proven DRI2WaitMSC implementation. Signed-off-by: Mario Kleiner Reviewed-by: Jesse Barnes Signed-off-by: Keith Packard (cherry picked from commit b3548612c7943011f79a910f9a59bb975984d8a6) Conflicts: hw/xfree86/dri2/dri2ext.c diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 2bdb733..7d670ff 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -768,8 +768,7 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, } int -DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, - CARD64 *ust, CARD64 *msc, CARD64 *sbc) +DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc) { DRI2DrawablePtr pPriv; @@ -783,14 +782,13 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc, if (target_sbc == 0) target_sbc = pPriv->swap_count + pPriv->swapsPending; - /* If current swap count already >= target_sbc, + /* If current swap count already >= target_sbc, reply and * return immediately with (ust, msc, sbc) triplet of * most recent completed swap. */ if (pPriv->swap_count >= target_sbc) { - *sbc = pPriv->swap_count; - *msc = pPriv->last_swap_msc; - *ust = pPriv->last_swap_ust; + ProcDRI2WaitMSCReply(client, pPriv->last_swap_ust, + pPriv->last_swap_msc, pPriv->swap_count); return Success; } diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index ce8a5df..de94c0b 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -251,8 +251,7 @@ extern _X_EXPORT int DRI2WaitMSC(ClientPtr client, DrawablePtr pDrawable, extern _X_EXPORT int ProcDRI2WaitMSCReply(ClientPtr client, CARD64 ust, CARD64 msc, CARD64 sbc); extern _X_EXPORT int DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, - CARD64 target_sbc, CARD64 *ust, CARD64 *msc, - CARD64 *sbc); + CARD64 target_sbc); extern _X_EXPORT Bool DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw); extern _X_EXPORT Bool DRI2CanFlip(DrawablePtr pDraw); diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 094d54d..627dfc0 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -516,9 +516,8 @@ static int ProcDRI2WaitSBC(ClientPtr client) { REQUEST(xDRI2WaitSBCReq); - xDRI2MSCReply rep; DrawablePtr pDrawable; - CARD64 target, ust, msc, sbc; + CARD64 target; int status; REQUEST_SIZE_MATCH(xDRI2WaitSBCReq); @@ -528,18 +527,9 @@ ProcDRI2WaitSBC(ClientPtr client) return status; target = vals_to_card64(stuff->target_sbc_lo, stuff->target_sbc_hi); - status = DRI2WaitSBC(client, pDrawable, target, &ust, &msc, &sbc); - if (status != Success) - return status; + status = DRI2WaitSBC(client, pDrawable, target); - rep.type = X_Reply; - rep.length = 0; - rep.sequenceNumber = client->sequence; - load_msc_reply(&rep, ust, msc, sbc); - - WriteToClient(client, sizeof(xDRI2MSCReply), &rep); - - return client->noClientException; + return status; } static int commit cc9f6806ac0d45e122c24c0e99c1db70a6d5ca12 Author: Mario Kleiner Date: Sun Jun 13 18:05:26 2010 +0200 DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's (bug 28383) Detect if a drawable has been moved from an original crtc to a new crtc with a lower current vblank count than the original crtc inbetween glXSwapBuffers() calls. Reinitialize drawable's last_swap_target before scheduling next swap if such a move has taken place. last_swap_target defines the baseline for scheduling the next swap. If a movement between crtc's is not taken into account, the swap may schedule for a vblank count on the new crtc far in the future, resulting in a apparent "hang" of the drawable for a long time. Fixes Bugzilla bug #28383. Signed-off-by: Mario Kleiner Reviewed-by: Jesse Barnes Signed-off-by: Keith Packard (cherry picked from commit 75beadd766fed7b12a76e59e57c244e297c2d2cb) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 7d670ff..62734d1 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -614,6 +614,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, DRI2DrawablePtr pPriv; DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; int ret, i; + CARD64 ust, current_msc; pPriv = DRI2GetDrawable(pDraw); if (pPriv == NULL) { @@ -658,12 +659,26 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, * need to schedule a swap for the last swap target + the swap interval. */ if (target_msc == 0 && divisor == 0 && remainder == 0) { + /* If the current vblank count of the drawable's crtc is lower + * than the count stored in last_swap_target from a previous swap + * then reinitialize last_swap_target to the current crtc's msc, + * otherwise the swap will hang. This will happen if the drawable + * is moved to a crtc with a lower refresh rate, or a crtc that just + * got enabled. + */ + if (!(*ds->GetMSC)(pDraw, &ust, ¤t_msc)) + pPriv->last_swap_target = 0; + + if (current_msc < pPriv->last_swap_target) + pPriv->last_swap_target = current_msc; + /* * Swap target for this swap is last swap target + swap interval since * we have to account for the current swap count, interval, and the * number of pending swaps. */ *swap_target = pPriv->last_swap_target + pPriv->swap_interval; + } else { /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */ *swap_target = target_msc;