[PATCH] DRI2/xserver: Don't hang in glXSwapBuffers if drawable moves between crtc's

[Jesse Barnes]

On Sun, 13 Jun 2010 18:05:26 +0200
Mario Kleiner <mario.kleiner at tuebingen.mpg.de> wrote:

> 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.kleiner at tuebingen.mpg.de>
> ---
>  hw/xfree86/dri2/dri2.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index d33b0d1..1f80022 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -756,6 +756,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) {
> @@ -800,12 +801,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, &current_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 */
>  	*swap_target = target_msc;

Yeah, this looks ok.  Really we should write up GLX extension that
clarifies behavior in multi-head and display on/off situations too.  Is
that something you could do?

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center