Re: Upstream support for FreeSync / Adaptive Sync

["Nicolai Hähnle" <nhaehnle@gmail.com>]

On 18.10.2017 10:10, Daniel Vetter wrote:
> On Tue, Oct 17, 2017 at 09:01:52PM +0200, Nicolai Hähnle wrote:
>> On 17.10.2017 19:16, Daniel Vetter wrote:
>>> On Tue, Oct 17, 2017 at 5:40 PM, Michel Dänzer <michel@daenzer.net> wrote:
>>>> On 17/10/17 05:04 PM, Daniel Vetter 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.
>>>>>
>>>>> I thought that's what the driconf switch is for, with a policy of "please
>>>>> schedule asap" instead of a specific timestamp.
>>>>
>>>> The driconf switch is just for the user's intention to use adaptive sync
>>>> when possible. A property as you suggest cannot be set by the client
>>>> directly, because it can't know when adaptive sync can actually be used
>>>> (only when its window is fullscreen and using page flipping). So the
>>>> property would have to be set by the X server/driver / Wayland
>>>> compositor / ... instead. The question is whether such a property is
>>>> actually needed, or whether the kernel could just enable adaptive sync
>>>> when there's a flip with a target timestamp, and disable it when there's
>>>> a flip without a target timestamp, or something like that.
>>>
>>> If your adaptive sync also supports extending the vblank beyond the
>>> nominal limit, then you can't do that with a per-flip flag. Because
>>> absent of a userspace requesting adaptive sync you must flip at the
>>> nominal vrefresh rate. So if your userspace is a tad bit late with the
>>> frame and would like to extend the frame to avoid missing a frame
>>> entirely it'll be too late by the time the vblank actually gets
>>> submitted. That's a bit a variation of what Ville brought up about
>>> what we're going to do when the timestamp was missed by the time all
>>> the depending fences signalled.
>>
>> These are very good points. It does sound like we'd need both an
>> "AdaptiveSync" boolean property and an (optional) "DesiredPresentTime"
>> property.
>>
>> The DesiredPresentTime property applies only to a single commit and could
>> perhaps be left out in a first version. The AdaptiveSync property is
>> persistent. When enabled, it means:
>>
>> - handle page flip requests as soon as possible
>> - while no page flip is requested, delay vblank as long as possible
>>
>> How does that sound?
> 
> Yeah, that's what I had in mind. No idea it'll work out on real hw/full
> stack.

Here's another question that occurred to me while thinking about how all 
this could be prototyped.

What happens when a FreeSync aware application / compositor enables the 
FreeSync property and then shuts down (crashes) without disabling it again?

It seems to me that a non-FreeSync aware compositor would then be 
operating in FreeSync mode without expecting it.

Can we fix that somehow? Do we care?

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev