[PATCH] present: Fix Async swap logic

[Axel Davy]

On 04/11/2015 04:17, Michel Dänzer wrote:
> On 03.11.2015 17:14, Axel Davy wrote:
>> According to the spec, PresentOptionAsync should only
>> trigger a different behaviour when the target msc has been reached.
>>
>> In this case if the driver is able to do async swaps, we use
>> them to avoid a screen copy.
>>
>> When the target msc hasn't been reached yet, we want to use sync swaps.
>>
>> Signed-off-by: Axel Davy <axel.davy at ens.fr>
>> ---
>> I'm not sure for indentation, I tried to do the same than previous check
>>   present/present.c | 29 ++++++++++++++++-------------
>>   1 file changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/present/present.c b/present/present.c
>> index beb4ff0..3b8679c 100644
>> --- a/present/present.c
>> +++ b/present/present.c
>> @@ -836,19 +836,22 @@ 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)
>> -            target_msc--;
>> +    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--;
> These three statements should be indented by two more spaces.
Good catch
>
> Also, shouldn't we take this case if !(options & PresentOptionAsync)
> even if target_msc <= crtc_msc?
At this point of the code, we have !(option & PresentOptionAsync) implies
target_msc > crtc_msc. See the part "Adjust target_msc to match modulus"
>
>
>> +        } 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;
>> +        }
> Why only take this case if target_msc == crtc_msc?
At this point of the code, when we have (option & PresentOptionAsync), 
target_msc >= crtc_msc

If target_msc > crtc_msc, then we want to use a sync flip (An async flip 
can cause tears),
and we use the previous if statement.
But in the case target_msc == crtc_msc, we want to copy immediately (via 
an async flip, or a copy).

I realize that the "(option & PresentOptionAsync)" part of the check is 
unneccessary.
>
>
Yours,

Axel