[PATCH] present: Fix Async swap logic

[davyaxel at free.fr]

On 04/11/2015 11:01, Chris Wilson wrote:
> On Wed, Nov 04, 2015 at 10:48:40AM +0100, Axel Davy wrote:
>>
>>
>> Could you explain:
>> . Why you increase target_msc when the Async option is requested ?
>
> It's the fallthrough path where the client requested an async swap but
> we cannot perform one and so must use a synchronous wait for the vblank
> (in which case we actually wait for the vblank before target_msc, hence
> the increment here to compenstae).
>
>> . Why you check for Async flips first (isn't sync flips better when
>> possible) ?
>
> The choice is between using native async flips or emulating them through
> sync flips. Native wins.
>  

Ok, I think I see to what you refers. I don't know enough of hardware to
know if that generalises to other hardware. Also if the async flip is
triggered too late, you'd get a tear (though I guess we are very
unlikely to be that late). As I expect sync flip emulation with async
flips to be very lightweight, I guess it doesn't hurt to try it first.
But I'm ok to favor Async if you think we should.


>> 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.
>
> It is described as being "perform the flip as soon as possible (if
> target < current msc) without regard to synchronisation to vrefresh".
> That says use tearing native async flips to me.

To me triggering an Async flip for current msc is equivalent to
fullscreen copy, which will tear -except if DDX enforces Tearfree-,
the main difference for the user being that a copy is slower.

>> 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.
>
> Exactly, hence why we need to fudge it in the code to maintain the
> requested msc (as the code fudges it again later on).
> -Chris
>

If that path is taken, we will trigger a copy.
The copy will be triggered at msc == target_msc.
I don't see why we should increase target_msc there.

For example, If I present to a window at the current msc with the async option,
incrementing target_msc means you wait vblank to present. Ie you enforce
swap interval == 1, which is not what we want. However not increasing
target_msc will trigger the copy directly, which is what the user expects.

Yours,

Axel Davy