[PATCH] present: Fix Async swap logic

[Axel Davy]

On 04/11/2015 10:40, Chris Wilson wrote:
> On Tue, Nov 03, 2015 at 09:14:51AM +0100, Axel Davy wrote:
>> +    if (pixmap != NULL &&
>> +        !(options & PresentOptionCopy) &&
>> +        screen_priv->info) {
>> +        if (target_msc > crtc_msc &&
>> +            present_check_flip (target_crtc, window, pixmap, TRUE, valid, x_off, y_off))
>> +        {
>> +          vblank->flip = TRUE;
>> +          vblank->sync_flip = TRUE;
>> +          target_msc--;
>> +        } else if (target_msc == crtc_msc &&
>> +            (options & PresentOptionAsync) &&
>> +            (screen_priv->info->capabilities & PresentCapabilityAsync) &&
>> +            present_check_flip (target_crtc, window, pixmap, FALSE, valid, x_off, y_off))
>> +        {
>> +          vblank->flip = TRUE;
>> +        }
>>       }
> For reference, this is how I fixed the async flip + swap elision:
>
> t a/present/present.c b/present/present.c
> index e9ccfb8..32522af 100644
> --- a/present/present.c
> +++ b/present/present.c
> @@ -861,19 +861,19 @@ present_pixmap(WindowPtr window,
>       vblank->notifies = notifies;
>       vblank->num_notifies = num_notifies;
>   
> -    if (!(options & PresentOptionAsync))
> -        vblank->sync_flip = TRUE;
> -
> -    if (!(options & PresentOptionCopy) &&
> -        !((options & PresentOptionAsync) &&
> -          (!screen_priv->info ||
> -           !(screen_priv->info->capabilities & PresentCapabilityAsync))) &&
> -        pixmap != NULL &&
> -        present_check_flip (target_crtc, window, pixmap, vblank->sync_flip, valid, x_off, y_off))
> -    {
> -        vblank->flip = TRUE;
> -        if (vblank->sync_flip)
> +    if (!(options & PresentOptionCopy) && pixmap != NULL) {
> +        if (options & PresentOptionAsync &&
> +            (screen_priv->info &&
> +             screen_priv->info->capabilities & PresentCapabilityAsync) &&
> +            present_check_flip (target_crtc, window, pixmap, FALSE, valid, x_off, y_off)) {
> +            vblank->flip = TRUE;
> +        }
> +        else if (present_check_flip (target_crtc, window, pixmap, TRUE, valid, x_off, y_off)) {
> +            vblank->flip = TRUE;
> +            vblank->sync_flip = TRUE;
>               target_msc--;
> +        } else if (options & PresentOptionAsync)
> +            target_msc++;
>       }
>   
>       if (wait_fence) {
>
>
Hi,

Could you explain:
. Why you increase target_msc when the Async option is requested ?
. Why you check for Async flips first (isn't sync flips better when 
possible) ?

To me the Async option isn't related particularly to Async flips. Async 
flips is just an optimization to handle it without a copy in the case 
you would need one.


http://cgit.freedesktop.org/xorg/proto/presentproto/tree/presentproto.txt#n212
Given the spec, I believe when target_msc > crtc_msc, the behaviour 
should be the same with or without the flag, and thus you shouldn't 
increase target_msc.

Yours,

Axel Davy