[PATCH xserver 2/4] dri2: Split resource tracking for DRI2Drawable and references to them
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Feb 3 15:29:35 UTC 2016
On Wed, Feb 03, 2016 at 09:54:44AM +0000, Chris Wilson wrote:
> In order to ease resource tracking, disentangle DRI2Drawable XIDs from
> their references. This will be used in later patches to first limit the
> object leak from unnamed references created on behalf of Clients and
> then never destroy, and then to allow Clients to explicit manage their
> references to DRI2Drawables.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> glx/glxdri2.c | 10 ++--
> hw/xfree86/dri2/dri2.c | 136 ++++++++++++++++++----------------------------
> hw/xfree86/dri2/dri2.h | 11 ++--
> hw/xfree86/dri2/dri2ext.c | 6 +-
> 4 files changed, 66 insertions(+), 97 deletions(-)
>
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 89ad808..d7f1436 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -105,7 +105,7 @@ __glXDRIdrawableDestroy(__GLXdrawable * drawable)
> __GLXDRIdrawable *private = (__GLXDRIdrawable *) drawable;
> const __DRIcoreExtension *core = private->screen->core;
>
> - FreeResource(private->dri2_id, FALSE);
> + DRI2DestroyDrawable(NULL, private->base.pDraw, private->dri2_id);
>
> (*core->destroyDrawable) (private->driDrawable);
>
> @@ -602,7 +602,7 @@ __glXDRIscreenCreateContext(__GLXscreen * baseScreen,
> }
>
> static void
> -__glXDRIinvalidateBuffers(DrawablePtr pDraw, void *priv, XID id)
> +__glXDRIinvalidateBuffers(DrawablePtr pDraw, void *priv)
> {
> __GLXDRIdrawable *private = priv;
> __GLXDRIscreen *screen = private->screen;
> @@ -641,9 +641,9 @@ __glXDRIscreenCreateDrawable(ClientPtr client,
> private->base.waitGL = __glXDRIdrawableWaitGL;
> private->base.waitX = __glXDRIdrawableWaitX;
>
> - ret = DRI2CreateDrawable2(client, pDraw, drawId,
> - __glXDRIinvalidateBuffers, private,
> - &private->dri2_id);
> + private->dri2_id = FakeClientID(client->index);
> + ret = DRI2CreateDrawable(client, pDraw, private->dri2_id,
> + __glXDRIinvalidateBuffers, private);
I was a but confused by the change to not passing the drawable id
separately, but after another look it seems it should end up being
exactly the same as before. The most special case being the glx pbuffer,
but that seems to have the pixmap drawable id set to the glxdrawable id,
and that is what was passed in also. And everywone else seemed to pass
the relevant X drawable id.
Otherwise adding the references resource looks sane enough to me, so
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> if (cx != lastGLContext) {
> lastGLContext = cx;
> cx->makeCurrent(cx);
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index bbff11c..4dc40f8 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -68,16 +68,16 @@ static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
>
> static DevPrivateKeyRec dri2ClientPrivateKeyRec;
>
> -#define dri2ClientPrivateKey (&dri2ClientPrivateKeyRec)
> -
> -#define dri2ClientPrivate(_pClient) (dixLookupPrivate(&(_pClient)->devPrivates, \
> - dri2ClientPrivateKey))
> -
> typedef struct _DRI2Client {
> int prime_id;
> } DRI2ClientRec, *DRI2ClientPtr;
>
> -static RESTYPE dri2DrawableRes;
> +static DRI2ClientPtr dri2ClientPrivate(ClientPtr pClient)
> +{
> + return dixLookupPrivate(&pClient->devPrivates, &dri2ClientPrivateKeyRec);
> +}
> +
> +static RESTYPE dri2DrawableRes, dri2ReferenceRes;
>
> typedef struct _DRI2Screen *DRI2ScreenPtr;
>
> @@ -203,6 +203,11 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
> if (pPriv == NULL)
> return NULL;
>
> + if (!AddResource(pDraw->id, dri2DrawableRes, pPriv)) {
> + free(pPriv);
> + return NULL;
> + }
> +
> pPriv->dri2_screen = ds;
> pPriv->drawable = pDraw;
> pPriv->width = pDraw->width;
> @@ -269,49 +274,37 @@ DRI2SwapLimit(DrawablePtr pDraw, int swap_limit)
> }
>
> typedef struct DRI2DrawableRefRec {
> - XID id;
> - XID dri2_id;
> + XID pid;
> DRI2InvalidateProcPtr invalidate;
> void *priv;
> struct xorg_list link;
> } DRI2DrawableRefRec, *DRI2DrawableRefPtr;
>
> -static DRI2DrawableRefPtr
> -DRI2LookupDrawableRef(DRI2DrawablePtr pPriv, XID id)
> +int
> +DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID pid,
> + DRI2InvalidateProcPtr invalidate, void *priv)
> {
> + DRI2DrawablePtr pPriv;
> DRI2DrawableRefPtr ref;
>
> - xorg_list_for_each_entry(ref, &pPriv->reference_list, link) {
> - if (ref->id == id)
> - return ref;
> - }
> -
> - return NULL;
> -}
> + pPriv = DRI2GetDrawable(pDraw);
> + if (pPriv == NULL)
> + pPriv = DRI2AllocateDrawable(pDraw);
> + if (pPriv == NULL)
> + return BadAlloc;
>
> -static int
> -DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id,
> - DRI2InvalidateProcPtr invalidate, void *priv)
> -{
> - DRI2DrawableRefPtr ref;
> + pPriv->prime_id = dri2ClientPrivate(client)->prime_id;
>
> ref = malloc(sizeof *ref);
> if (ref == NULL)
> return BadAlloc;
>
> - if (!AddResource(dri2_id, dri2DrawableRes, pPriv)) {
> + if (!AddResource(pid, dri2ReferenceRes, ref)) {
> free(ref);
> return BadAlloc;
> }
> - if (!DRI2LookupDrawableRef(pPriv, id))
> - if (!AddResource(id, dri2DrawableRes, pPriv)) {
> - FreeResourceByType(dri2_id, dri2DrawableRes, TRUE);
> - free(ref);
> - return BadAlloc;
> - }
>
> - ref->id = id;
> - ref->dri2_id = dri2_id;
> + ref->pid = pid;
> ref->invalidate = invalidate;
> ref->priv = priv;
> xorg_list_add(&ref->link, &pPriv->reference_list);
> @@ -320,71 +313,32 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID dri2_id,
> }
>
> int
> -DRI2CreateDrawable2(ClientPtr client, DrawablePtr pDraw, XID id,
> - DRI2InvalidateProcPtr invalidate, void *priv,
> - XID *dri2_id_out)
> +DRI2DestroyDrawable(ClientPtr client, DrawablePtr pDraw, XID id)
> {
> - DRI2DrawablePtr pPriv;
> - DRI2ClientPtr dri2_client = dri2ClientPrivate(client);
> - XID dri2_id;
> - int rc;
> -
> - pPriv = DRI2GetDrawable(pDraw);
> - if (pPriv == NULL)
> - pPriv = DRI2AllocateDrawable(pDraw);
> - if (pPriv == NULL)
> - return BadAlloc;
> -
> - pPriv->prime_id = dri2_client->prime_id;
> -
> - dri2_id = FakeClientID(client->index);
> - rc = DRI2AddDrawableRef(pPriv, id, dri2_id, invalidate, priv);
> - if (rc != Success)
> - return rc;
> -
> - if (dri2_id_out)
> - *dri2_id_out = dri2_id;
> -
> + FreeResourceByType(id, dri2ReferenceRes, FALSE);
> return Success;
> }
>
> -int
> -DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
> - DRI2InvalidateProcPtr invalidate, void *priv)
> -{
> - return DRI2CreateDrawable2(client, pDraw, id, invalidate, priv, NULL);
> -}
> -
> static int
> DRI2DrawableGone(void *p, XID id)
> {
> DRI2DrawablePtr pPriv = p;
> - DRI2DrawableRefPtr ref, next;
> WindowPtr pWin;
> PixmapPtr pPixmap;
> DrawablePtr pDraw;
> int i;
>
> - xorg_list_for_each_entry_safe(ref, next, &pPriv->reference_list, link) {
> - if (ref->dri2_id == id) {
> - xorg_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;
> - }
> + while (!xorg_list_is_empty(&pPriv->reference_list)) {
> + DRI2DrawableRefPtr ref;
>
> - if (ref->id == id) {
> - xorg_list_del(&ref->link);
> - FreeResourceByType(ref->dri2_id, dri2DrawableRes, TRUE);
> - free(ref);
> - }
> - }
> + ref = xorg_list_first_entry(&pPriv->reference_list,
> + DRI2DrawableRefRec,
> + link);
> + xorg_list_del(&ref->link);
>
> - if (!xorg_list_is_empty(&pPriv->reference_list))
> - return Success;
> + FreeResourceByType(ref->pid, dri2ReferenceRes, TRUE);
> + free(ref);
> + }
>
> pDraw = pPriv->drawable;
> if (pDraw->type == DRAWABLE_WINDOW) {
> @@ -418,6 +372,20 @@ DRI2DrawableGone(void *p, XID id)
> return Success;
> }
>
> +static int
> +DRI2ReferenceGone(void *p, XID id)
> +{
> + DRI2DrawableRefPtr ref = p;
> + DRI2DrawablePtr pPriv = ref->priv;
> +
> + xorg_list_del(&ref->link);
> + if (xorg_list_is_empty(&pPriv->reference_list))
> + FreeResourceByType(pPriv->drawable->id, dri2DrawableRes, FALSE);
> + free(ref);
> +
> + return Success;
> +}
> +
> static DRI2BufferPtr
> create_buffer(DRI2ScreenPtr ds, DrawablePtr pDraw,
> unsigned int attachment, unsigned int format)
> @@ -684,7 +652,7 @@ DRI2InvalidateDrawable(DrawablePtr pDraw)
> pPriv->needInvalidate = FALSE;
>
> xorg_list_for_each_entry(ref, &pPriv->reference_list, link)
> - ref->invalidate(pDraw, ref->priv, ref->id);
> + ref->invalidate(pDraw, ref->priv);
> }
>
> /*
> @@ -1646,6 +1614,10 @@ DRI2ModuleSetup(void)
> if (!dri2DrawableRes)
> return FALSE;
>
> + dri2ReferenceRes = CreateNewResourceType(DRI2ReferenceGone, "DRI2Reference");
> + if (!dri2ReferenceRes)
> + return FALSE;
> +
> return TRUE;
> }
>
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index 1e7afdd..df50443 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -161,7 +161,7 @@ typedef int (*DRI2ScheduleWaitMSCProcPtr) (ClientPtr client,
> CARD64 target_msc,
> CARD64 divisor, CARD64 remainder);
>
> -typedef void (*DRI2InvalidateProcPtr) (DrawablePtr pDraw, void *data, XID id);
> +typedef void (*DRI2InvalidateProcPtr) (DrawablePtr pDraw, void *data);
>
> /**
> * DRI2 calls this hook when ever swap_limit is going to be changed. Default
> @@ -270,16 +270,13 @@ extern _X_EXPORT Bool DRI2Authenticate(ClientPtr client, ScreenPtr pScreen, uint
>
> extern _X_EXPORT int DRI2CreateDrawable(ClientPtr client,
> DrawablePtr pDraw,
> - XID id,
> + XID pid,
> DRI2InvalidateProcPtr invalidate,
> void *priv);
>
> -extern _X_EXPORT int DRI2CreateDrawable2(ClientPtr client,
> +extern _X_EXPORT int DRI2DestroyDrawable(ClientPtr client,
> DrawablePtr pDraw,
> - XID id,
> - DRI2InvalidateProcPtr invalidate,
> - void *priv,
> - XID *dri2_id_out);
> + XID id);
>
> extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffers(DrawablePtr pDraw,
> int *width,
> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
> index 520b7cf..c42dcbc 100644
> --- a/hw/xfree86/dri2/dri2ext.c
> +++ b/hw/xfree86/dri2/dri2ext.c
> @@ -161,12 +161,12 @@ ProcDRI2Authenticate(ClientPtr client)
> }
>
> static void
> -DRI2InvalidateBuffersEvent(DrawablePtr pDraw, void *priv, XID id)
> +DRI2InvalidateBuffersEvent(DrawablePtr pDraw, void *priv)
> {
> ClientPtr client = priv;
> xDRI2InvalidateBuffers event = {
> .type = DRI2EventBase + DRI2_InvalidateBuffers,
> - .drawable = id
> + .drawable = pDraw->id,
> };
>
> WriteEventsToClient(client, 1, (xEvent *) &event);
> @@ -185,7 +185,7 @@ ProcDRI2CreateDrawable(ClientPtr client)
> &pDrawable, &status))
> return status;
>
> - status = DRI2CreateDrawable(client, pDrawable, stuff->drawable,
> + status = DRI2CreateDrawable(client, pDrawable, FakeClientID(client->index),
> DRI2InvalidateBuffersEvent, client);
> if (status != Success)
> return status;
> --
> 2.7.0
--
Ville Syrjälä
Intel OTC
More information about the xorg-devel
mailing list