[Mesa-dev] Upstream support for FreeSync / Adaptive Sync

[Nicolai Hähnle]

On 17.10.2017 21:53, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 09:00:56PM +0200, Nicolai Hähnle wrote:
>> On 17.10.2017 16:09, Ville Syrjälä wrote:
>>> On Tue, Oct 17, 2017 at 03:46:24PM +0200, Michel Dänzer wrote:
>>>> On 17/10/17 02:22 PM, Daniel Vetter wrote:
>>>>> On Tue, Oct 17, 2017 at 12:28:17PM +0200, Michel Dänzer wrote:
>>>>>> On 17/10/17 11:34 AM, Nicolai Hähnle wrote:
>>>>>
>>>>>>> Common sense suggests that there need to be two side to FreeSync / VESA
>>>>>>> Adaptive Sync support:
>>>>>>>
>>>>>>> 1. Query the display capabilities. This means querying minimum / maximum
>>>>>>> refresh duration, plus possibly a query for when the earliest/latest
>>>>>>> timing of the *next* refresh.
>>>>>>>
>>>>>>> 2. Signal desired present time. This means passing a target timer value
>>>>>>> instead of a target vblank count, e.g. something like this for the KMS
>>>>>>> interface:
>>>>>>>
>>>>>>>     int drmModePageFlipTarget64(int fd, uint32_t crtc_id, uint32_t fb_id,
>>>>>>>                                 uint32_t flags, void *user_data,
>>>>>>>                                 uint64_t target);
>>>>>>>
>>>>>>>     + a flag to indicate whether target is the vblank count or the
>>>>>>> CLOCK_MONOTONIC (?) time in ns.
>>>>>>
>>>>>> drmModePageFlip(Target) is part of the pre-atomic KMS API, but adapative
>>>>>> sync should probably only be supported via the atomic API, presumably
>>>>>> via output properties.
>>>>>
>>>>> +1
>>>>>
>>>>> At least now that DC is on track to land properly, and you want to do this
>>>>> for DC-only anyway there's no reason to pimp the legacy interfaces
>>>>> further. And atomic is soooooo much easier to extend.
>>>>>
>>>>> The big question imo is where we need to put the flag on the kms side,
>>>>> since freesync is not just about presenting earlier, but also about
>>>>> presenting later. But for backwards compat we can't stretch the refresh
>>>>> rate by default for everyone, or clients that rely on high precision
>>>>> timestamps and regular refresh will get a bad surprise.
>>>>
>>>> The idea described above is that adaptive sync would be used for flips
>>>> with a target timestamp. Apps which don't want to use adaptive sync
>>>> wouldn't set a target timestamp.
>>>>
>>>>
>>>>> I think a boolean enable_freesync property is probably what we want, which
>>>>> enables freesync for as long as it's set.
>>>>
>>>> The question then becomes under what circumstances the property is (not)
>>>> set. Not sure offhand this will actually solve any problem, or just push
>>>> it somewhere else.
>>>>
>>>>
>>>>> Finally I'm not sure we want to insist on a target time for freesync. At
>>>>> least as far as I understand things you just want "as soon as possible".
>>>>> This might change with some of the VK/EGL/GLX extensions where you
>>>>> specify a precise timing (media playback). But that needs a bit more work
>>>>> to make it happen I think, so perhaps better to postpone.
>>>>
>>>> I don't see why. There's an obvious use case for this now, for video
>>>> playback. At least VDPAU already has target timestamps for this.
>>>>
>>>>
>>>>> Also note that right now no driver expect amdgpu has support for a target
>>>>> vblank on a flip. That's imo another reason for not requiring target
>>>>> support for at least basic freesync support.
>>>>
>>>> I think that's a bad reason. :) Adding it for atomic drivers shouldn't
>>>> be that hard.
>>>
>>> Apart from the actual implementation hurdles it does open up some new questions:
>>
>> All good questions, thanks! Let me try to take a crack at them:
>>
>>
>>> - Is it going to be per-plane or per-crtc?
>>
>> My understanding is that planes are combined to form a single signal
>> that goes out to the monitor(s). The planes are scanned out together by
>> a crtc, so it should be per-crtc.
> 
> I guess one might imagine a compositor with one video player type of
> client, and another game/benchmark type of client. If both clients queue
> their next frames around the same time, the compositor might think to
> combine them to a single atomic ioctl call. But it's possible the
> video player client would want its frame presented much later than
> the other client, which would require a per-plane timestamp.
> But I guess it's not totally unreasonable to ask the compositor to
> do two ioctls in this case since we aren't actually looking for a
> single atomic update of two planes.

Right. And remember that the desired time stamp isn't about when the 
planes get switched out, but about when the vblank happens. You can't 
have different vblank times for different planes going to the same monitor.


>>> - What happens if the target timestamp is already stale?
>>> - What happens if the target timestamp is good when it gets scheduled,
>>>     but can't be met once the fences and whatnot have signalled?
>>
>> Treat it as "flip as soon as possible" in both cases.
>>
>>
>>> - What happens if another operation is already queued with a more
>>>     recent timestamp?
>>
>> This is a problem already today, isn't it? You could have two page flips
>> being queued before the next vblank. What happens in that case?
> 
> I think currently we get -EBUSY. But there's has been talk about
> replacing queued flips, async flips, etc. so it seems like people
> are starting to want something a bit different.
> 
> I guess it's always possible to start with the EBUSY idea and change
> it later with some kind of flags or something. Not sure how well flags
> work with atomic though since generally everything is a property. Having
> flags as a property feels funky. I guess we do have flags in the ioctl
> struct itself, but those would have to affect the entire operation rather
> than just one plane or crtc.

Starting with EBUSY for the first version seems reasonable to me.


>>> - Apart from a pure timestamp do we want to move the OML_sync/swap_whatever
>>>     msc remainder etc. semantics into the kernel as well? It's just
>>>     another way to specify the target flip time after all.
>>
>> A related question:
>>
>> - What happens if the target timestamp is too late for the next vblank?
>>
>> There's an argument to be made that late timestamps should just be
>> treated as "delay the next vblank as late as possible". Such an option
>> could be used by compositors for a power-saving mode.
> 
> Hmm. So this seems to get into adaptive sync specific territory. Without
> adaptive sync I would imagine we'd just try to flip as soon as the
> target timestamp has been reached, which could be several frames into
> the future (with some reasonable upper bould I suppose).
> 
> With adaptive sync I guess we could always try to adjust the vblank
> interval up or down to try and meet the target as closely as possible
> either on the next vblank, or potentially after N frames.

Right, that would be the alternative.


> But IIRC
> there's a delay in how fast we can ramp the vblank interval up/down,
> so not quite sure how accurately we could predict it all.

Right. Having the option of letting the driver take care of the ramp 
would be nice.

I'm just a bit hesitant to allow it in a first version, because it seems 
to imply the possibility of somebody queuing up a commit arbitrarily far 
into the future, without any way of kicking that change out again :)

And besides, simply clamping a time that is too far into the future is 
surely easier to implement and can be useful as well.

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.