[PATCH] DRI2: fixup handling of last_swap_target

[Mario Kleiner]

On Mar 8, 2010, at 6:07 PM, Jesse Barnes wrote:

> On Sun, 7 Mar 2010 21:16:21 +0100
> Florian Mickler <florian at mickler.org> wrote:
>
>> On Sun, 7 Mar 2010 09:10:51 -0800
>> Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
>>
>>> On Sun, 7 Mar 2010 08:44:51 +0100
>>> Mario Kleiner <mario.kleiner at tuebingen.mpg.de> wrote:
>>
>>> Yeah, this could also happen without OML I think, if a few swaps  
>>> were
>>> queued (resulting in a block) and then a SGI_video_sync call  
>>> occurred.
>>> I'll fix it up.
>>>
>>> Thanks,
>>
>> could this also happen with a single glxgears instance and no other
>> 3d clients running?
>
> I don't *think* glxgears does this; it's been running fine for me at
> least.
> But you could definitely be hitting one of the MSC races; you can add
> some debug output to I830DRI2ScheduleSwap in src/i830_dri.c in the DDX
> driver to see what's going on.  Presumably one of the paths there is
> putting the client to sleep and never waking it.

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).

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.

-mario