[PATCH] present: Fix Async swap logic

[Michel Dänzer]

On 05.11.2015 01:51, davyaxel at free.fr wrote:
> On 04/11/2015 11:01, Chris Wilson wrote:
>> On Wed, Nov 04, 2015 at 10:48:40AM +0100, Axel Davy wrote:
>>>
>>> . 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.

I think we should:

If we schedule a sync flip for crtc_msc+1, there's a chance that it'll
actually miss that and only take effect on crtc_msc+2 (if we're already
towards the end of the crtc_msc frame). It's probably very unlikely to
happen, but if an unlucky client triggers that for every frame, this
would effectively halve the visible frame update rate. But the client
specifying PresentOptionAsync indicates it would rather risk tearing
instead.


>>> 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).

Where does it get fudged again later on? I can't see that.


> 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.

Makes sense to me.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer