[PATCH] DRI2: fixup handling of last_swap_target

Jesse Barnes jbarnes at virtuousgeek.org
Mon Mar 8 11:19:38 PST 2010

On Mon, 8 Mar 2010 20:13:21 +0100
Mario Kleiner <mario.kleiner at tuebingen.mpg.de> wrote:
> The glxgears.c of mesa only does draw -> glXSwapBuffers -> draw ->  
> glXSwapBuffers -> ...
> It doesn't use any of the new api's, so the only way it can block is  
> probably via the DRI2ThrottleClient call when requesting a "new"  
> backbuffer.
> I see at least one problematic thing. In the xserver's dri2.c file in  
> DRI2SwapBuffers(), the pPriv->swapsPending++ statement is *after* the  
> call into the ddx's I830DRI2ScheduleSwap() function, instead of  
> *before* it. If a swap completes inside the ddx, it will call  
> DRI2SwapComplete() which will do a pPriv->swapsPending-- before it  
> was incremented to mark a swap as pending. That is, if swapsPending  
> was zero before, it will now wrap around to 0xffffffff. This will  
> happen if the fallback_blit path inside the ddx is used, e.g., if the  
> drawable is not visible (yet).

Or if composited; it may be offscreen.

> Not sure if this by itself is sufficient for the hang, because the  
> call to pPriv->swapsPending++ after return from I830DRI2ScheduleSwap  
> () should fix this, but if something else goes wrong and an error  
> path is taken or an event not delivered, then swapsPending could get  
> stuck at 0xffffffff or a similar high number, which would trigger  
> DRI2ThrottleClient and block the client connection without ever  
> waking it up again, as clients are only woken up if a swap completes,  
> which can't happen if no swap is pending. -> hang in _XReply on the  
> client side.
> So we should probable move pPriv->swapsPending++ before the call to...
>      ret = (*ds->ScheduleSwap)(client, pDraw, pDestBuffer, pSrcBuffer,
>                                swap_target, divisor, remainder, func,  
> data);
> ... in DRI2SwapBuffers() and probably add a pPriv->swapsPending--  
> into the error handling path directly after the call to ds- 
>  >ScheduleSwap.

Yep, that's the safer thing to do.  Florian, can you give that a try
and see if it helps your situation?

Jesse Barnes, Intel Open Source Technology Center

More information about the xorg-devel mailing list