By the way, does PresentOptionAsync mean the same thing with the async flip 
capability in kernel side? Or is it just a flag to indicate X/DDX to do present 
immediately instead of waiting for the next vblank?

Regards,
Jammy

-----Original Message-----
From: Axel Davy [mailto:[email protected]] 
Sent: Thursday, October 29, 2015 3:42 PM
To: Michel Dänzer; Zhou, Jammy
Cc: [email protected]; Mario Kleiner
Subject: Re: [PATCH] present: Execute right away if target_msc equals 
current_msc

Hi,

What is the current conscensus for how should PresentOptionAsync work ?

If I remember correctly, the semantic used to be:

. if we present at the current or past msc with the flag, two options:
-> if the ddx doesn't support async swap, we do copy to the screen
pixmap right away
-> if the ddx does support it, we do an async swap right away

. if we present at another msc, things behave as without PresentOptionAsync, 
that is we schedule a swap, and ask at msc-1 to the ddx to swap for msc.

With "Fix use of vsynced pageflips", I get the impression it shifted to:

. Async requested, but driver not having the async option -> do screen copy (no 
flips, whatever the msc) . if Async not requested and flip planned, present at 
msc-1, else at msc. (so for flips and Async flag, we do always plan to flip at 
msc ? 
That means always tear, right ?)

That doesn't make a lot of sense to me, can someone clarify ?

I'm afraid this patch could be a workaround to currently broken behaviour, and 
not the correct fix.

I CCed Mario Kleiner.

Yours,

Axel Davy

On 29/10/2015 03:53, Michel Dänzer wrote:
> On 28.10.2015 19:39, Jammy Zhou wrote:
>> It is according to the protocol:
>>
>> "If 'options' contains PresentOptionAsync, and the 'target-msc'
>> is less than or equal to the current msc for 'window', then the 
>> operation will be performed as soon as possible, not necessarily 
>> waiting for the next vertical blank interval."
>>
>> Signed-off-by: Jammy Zhou <[email protected]>
>> ---
>>   present/present.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/present/present.c b/present/present.c index 
>> beb4ff0..5900c22 100644
>> --- a/present/present.c
>> +++ b/present/present.c
>> @@ -871,7 +871,7 @@ present_pixmap(WindowPtr window,
>>   
>>       xorg_list_add(&vblank->event_queue, &present_exec_queue);
>>       vblank->queued = TRUE;
>> -    if ((pixmap && target_msc >= crtc_msc) || (!pixmap && target_msc > 
>> crtc_msc)) {
>> +    if (target_msc > crtc_msc) {
>>           ret = present_queue_vblank(screen, target_crtc, vblank->event_id, 
>> target_msc);
>>           if (ret == Success)
>>               return Success;
>>
> Looks good to me, but Cc'ing Axel Davy, who made the last change to 
> this code.
>
> Reviewed-by: Michel Dänzer <[email protected]>
>
>

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to