[PATCH 4/4] dri2: Take an XID for tracking the DRI2 drawable

Kristian Høgsberg krh at bitplanet.net
Sun May 2 10:47:27 PDT 2010


2010/5/1 Kristian Høgsberg <krh at bitplanet.net>:
> 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.
>
> Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>

I forgot to add this:

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

Kristian

>  glx/glxcmds.c             |   23 ++++---
>  glx/glxdri.c              |    8 ++-
>  glx/glxdri2.c             |   10 ++-
>  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 +-
>  8 files changed, 165 insertions(+), 45 deletions(-)
>
> 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 c34e29a..bad4516 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 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;
>
> --
> 1.7.0.1
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel


More information about the xorg-devel mailing list