Hi, commit 75beadd766fed7b12a76e59e57c244e297c2d2cb Author: Mario Kleiner <mario.klei...@tuebingen.mpg.de> Date: Sun Jun 13 18:05:26 2010 +0200
DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's (bug 28383) Detect if a drawable has been moved from an original crtc to a new crtc with a lower current vblank count than the original crtc inbetween glXSwapBuffers() calls. Reinitialize drawable's last_swap_target before scheduling next swap if such a move has taken place. last_swap_target defines the baseline for scheduling the next swap. If a movement between crtc's is not taken into account, the swap may schedule for a vblank count on the new crtc far in the future, resulting in a apparent "hang" of the drawable for a long time. Fixes Bugzilla bug #28383. Signed-off-by: Mario Kleiner <mario.klei...@tuebingen.mpg.de> Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org> Signed-off-by: Keith Packard <kei...@keithp.com> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index a8dedfa..f9ba8e7 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -759,6 +759,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, DRI2DrawablePtr pPriv; DRI2BufferPtr pDestBuffer = NULL, pSrcBuffer = NULL; int ret, i; + CARD64 ust, current_msc; pPriv = DRI2GetDrawable(pDraw); if (pPriv == NULL) { @@ -803,12 +804,26 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc, * need to schedule a swap for the last swap target + the swap interval. */ if (target_msc == 0 && divisor == 0 && remainder == 0) { + /* If the current vblank count of the drawable's crtc is lower + * than the count stored in last_swap_target from a previous swap + * then reinitialize last_swap_target to the current crtc's msc, + * otherwise the swap will hang. This will happen if the drawable + * is moved to a crtc with a lower refresh rate, or a crtc that just + * got enabled. + */ + if (!(*ds->GetMSC)(pDraw, &ust, ¤t_msc)) + pPriv->last_swap_target = 0; + + if (current_msc < pPriv->last_swap_target) + pPriv->last_swap_target = current_msc; + /* * Swap target for this swap is last swap target + swap interval since * we have to account for the current swap count, interval, and the * number of pending swaps. */ *swap_target = pPriv->last_swap_target + pPriv->swap_interval; + } else { /* glXSwapBuffersMscOML could have a 0 target_msc, honor it */ This commit assumes that MSC maybe different depending on where the drawable is. While specification says "For a multi-monitor system, the monitor used to determine MSC is screen 0 of <display>." which IMO means that there should be single MSC everywhere were target drawable can move. So correct fix would be making MSC value in-depend of CRTC where drawable is. I don't know if correct place for that is DRI2 or driver code. In current DRI2 way od doing stuff it would be driver code but this would results duplicating same code to all ddx drivers. I would prefer DRI2 side implementation helpers that can be configured easily by driver to work correctly with underlying hw. But actual patch has two problems. First, GetMSC is not NULL checked before calling it. This causes crash with driver that is using flipping without OML_sync_control. Second, same freeze would happen for application that is using OML_sync_control if it was moved between areas where is different MSC. In my option actual patch should be reverted because it is not correct fix for the bug. Pauli _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel