Attachment 35364 Details for Bug 27767 – roll-up patch

[patch] roll-up patch

0001-dri2-Take-an-XID-for-tracking-the-DRI2-drawable.patch (text/plain), 15.51 KB, created by Kristian Høgsberg on 2010-05-01 09:14:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kristian Høgsberg

Created: 2010-05-01 09:14:36 UTC

Size: 15.51 KB

patch

obsolete

>From 2b3f755254675933475b068ff20bd322ffb8631f Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Kristian=20H=C3=B8gsberg?= <krh@bitplanet.net>
>Date: Thu, 29 Apr 2010 16:36:10 -0400
>Subject: [PATCH] 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.
>---
> dix/resource.c            |    1 +
> glx/glxcmds.c             |   23 ++++---
> glx/glxdri.c              |    8 ++-
> glx/glxdri2.c             |   12 ++--
> glx/glxdriswrast.c        |    8 ++-
> glx/glxscreens.h          |    6 +-
> hw/xfree86/dri2/dri2.c    |  150 ++++++++++++++++++++++++++++++++++++++-------
> hw/xfree86/dri2/dri2.h    |    3 +-
> hw/xfree86/dri2/dri2ext.c |    2 +-
> include/list.h            |    6 ++
> 10 files changed, 173 insertions(+), 46 deletions(-)
>
>diff --git a/dix/resource.c b/dix/resource.c
>index 91d0cfb..aec2a5b 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);
> 
>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 9810a73..1d8c902 100644
>--- a/glx/glxdri.c
>+++ b/glx/glxdri.c
>@@ -683,10 +683,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;
>@@ -700,7 +702,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 74d6ebc..40825c8 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;
>     }
>@@ -717,7 +719,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
> 		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];
> 	}
>     }
>diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
>index 918383c..4ba448a 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 6c4dabc..11442d0 100644
>--- a/hw/xfree86/dri2/dri2.c
>+++ b/hw/xfree86/dri2/dri2.c
>@@ -37,6 +37,7 @@
> #include <errno.h>
> #include <xf86drm.h>
> #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;
>+    WindowPtr pWin;
>+    PixmapPtr pPixmap;
> 
>-    rc = dixLookupResourceByType((pointer *) &pPriv, pDraw->id,
>-				 dri2DrawableRes, NULL, DixReadAccess);
>-    if (rc != Success)
>-	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)
>+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 = 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 (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;
>+}
> 
>-    if (!AddResource(pDraw->id, dri2DrawableRes, pPriv))
>+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;
> 
>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
>-- 
>1.7.0.1
>

Actions: View | Splinter Review

Attachments on bug 27767: 35194 | 35338 | 35350 | 35363 | 35364

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.