Re: [PATCH 4/9] dri2: Add reference counting to DRI2

Christopher James Halse Rogers Thu, 03 Feb 2011 18:36:05 -0800

On Thu, 2011-02-03 at 19:48 +0200, Pauli wrote:
> From: Pauli Nieminen <[email protected]>
> 
> Asynchronous request like SwapBuffers can still reference Drawable after
> the Drawable has been freed. DRI2Drawable cleanup should be delayed
> until all asynchronous operations have completed.
> 
> Reference counted DRI2Drawable helps to keep DRI2Drawable around until
> all request on it has completed.
> 
> Signed-off-by: Pauli Nieminen <[email protected]>
> ---
>  hw/xfree86/dri2/dri2.c |   30 ++++++++++++++++++++++++++++--
>  1 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index f564822..21ee982 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -83,6 +83,7 @@ typedef struct _DRI2Drawable {
>      CARD64            last_swap_ust; /* ust at completion of most recent 
> swap */
>      int                       swap_limit; /* for N-buffering */
>      unsigned long     serialNumber;
> +    unsigned          refcnt;
>  } DRI2DrawableRec;
>  
>  typedef struct _DRI2Screen {
> @@ -190,12 +191,14 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
>      pPriv->last_swap_ust = 0;
>      list_init(&pPriv->reference_list);
>      pPriv->serialNumber = DRI2DrawableSerial(pDraw);
> +    pPriv->refcnt = 0;
>  
>      if (pDraw->type == DRAWABLE_WINDOW) {
>       pWin = (WindowPtr) pDraw;
>       dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv);
>      } else {
>       pPixmap = (PixmapPtr) pDraw;
> +     pPixmap->refcnt++;
>       dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, pPriv);
>      }
>  
> @@ -251,6 +254,8 @@ DRI2AddDrawableRef(DRI2DrawablePtr pPriv, XID id, XID 
> dri2_id,
>       if (!AddResource(id, dri2DrawableRes, pPriv))
>           return BadAlloc;
>  
> +    pPriv->refcnt++;
> +
>      ref->id = id;
>      ref->dri2_id = dri2_id; 
>      ref->invalidate = invalidate;
> @@ -309,6 +314,7 @@ static int DRI2DrawableGone(pointer p, XID id)
>  
>      list_for_each_entry_safe(ref, next, &pPriv->reference_list, link) {
>       if (ref->dri2_id == id) {
> +         pPriv->refcnt--;
>           list_del(&ref->link);
>           /* If this was the last ref under this X drawable XID,
>            * unregister the X drawable resource. */
> @@ -319,13 +325,15 @@ static int DRI2DrawableGone(pointer p, XID id)
>       }
>  
>       if (ref->id == id) {
> +         pPriv->refcnt--;
>           list_del(&ref->link);
>           FreeResourceByType(ref->dri2_id, dri2DrawableRes, TRUE);
>           free(ref);
>       }
>      }
>  
> -    if (!list_is_empty(&pPriv->reference_list))
> +
> +    if (pPriv->refcnt > 0)
>       return Success;
>  
>      pDraw = pPriv->drawable;
> @@ -343,6 +351,7 @@ static int DRI2DrawableGone(pointer p, XID id)
>      } else {
>       pPixmap = (PixmapPtr) pDraw;
>       dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL);
> +     pDraw->pScreen->DestroyPixmap(pPixmap);
>      }
>  
>      free(pPriv);
> @@ -694,7 +703,12 @@ void
>  DRI2WaitMSCComplete(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
>                   unsigned int tv_sec, unsigned int tv_usec)
>  {
> +    pPriv->refcnt--;
>  
> +    if (pPriv->refcnt == 0) {


In DRI2DrawableGone you compare to > 0.  Although I'm convinced that
WaitMSCComplete won't be called with pPriv->refcnt <= 0, it wouldn't
hurt to be similarly paranoid here, and compare <= 0 rather than == 0.

> +     DRI2DrawableGone(pPriv, 0);
> +     return;
> +    }
>  
>      ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec,
>                        frame, pPriv->swap_count);
> @@ -752,7 +766,12 @@ DRI2SwapComplete(ClientPtr client, DRI2DrawablePtr 
> pPriv, int frame,
>  
>      pPriv->swapsPending--;
>      pPriv->swap_count++;
> +    pPriv->refcnt--;
>  
> +    if (pPriv->refcnt == 0) {
> +     DRI2DrawableGone(pPriv, 0);
> +     return;
> +    }
>  
>      if (pDraw) {
>       BoxRec          box;
> @@ -836,6 +855,7 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, 
> CARD64 target_msc,
>       RegionInit(&region, &box, 0);
>  
>       pPriv->swapsPending++;
> +     pPriv->refcnt++;
>  
>       (*ds->CopyRegion)(pDraw, &region, pDestBuffer, pSrcBuffer);
>       DRI2SwapComplete(client, pPriv, target_msc, 0, 0, DRI2_BLIT_COMPLETE,
> @@ -877,10 +897,12 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr 
> pPriv, CARD64 target_msc,
>      }
>  
>      pPriv->swapsPending++;
> +    pPriv->refcnt++;
>      ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer,
>                             swap_target, divisor, remainder, func, data);
>      if (!ret) {
>       pPriv->swapsPending--; /* didn't schedule */
> +     pPriv->refcnt--;
>          xf86DrvMsg(pScreen->myNum, X_ERROR,
>                  "[DRI2] %s: driver failed to schedule swap\n", __func__);
>       return BadDrawable;
> @@ -951,6 +973,8 @@ DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, 
> CARD64 target_msc,
>      if (pDraw == NULL)
>       return BadDrawable;
>  
> +    pPriv->refcnt++;
> +
>      /* Old DDX just completes immediately */
>      if (!ds->ScheduleWaitMSC) {
>       DRI2WaitMSCComplete(client, pPriv, target_msc, 0, 0);
> @@ -959,8 +983,10 @@ DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, 
> CARD64 target_msc,
>      }
>  
>      ret = (*ds->ScheduleWaitMSC)(client, pDraw, target_msc, divisor, 
> remainder);
> -    if (!ret)
> +    if (!ret) {
> +     pPriv->refcnt--;
>       return BadDrawable;
> +    }
>  
>      return Success;
>  }

Reviewed-By: Christopher James Halse Rogers
<[email protected]>

signature.asc
Description: This is a digitally signed message part

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 4/9] dri2: Add reference counting to DRI2

Reply via email to