Upstream support for FreeSync / Adaptive Sync

Upstream support for FreeSync / Adaptive Sync

[Nicolai Hähnle]

Hi all,

I just sent out a patch that enables FreeSync in Mesa for the somewhat 
hacked implementation of FreeSync that exists in our hybrid (amdgpu-pro) 
stack's DDX and kernel module. [0]

While this patch isn't meant for upstream, that's as good a time as any 
to raise the issue of how a proper upstream solution would look like. It 
needs to cut across the entire stack, and we should try to align the KMS 
interface with the X11 protocol and the Wayland protocol.

Prior art that I'm aware of includes:

1. The Present protocol extension has a PresentOptionUST bit for 
requesting a specific present time, but the reality is that the 
implementation of that is even less than what the protocol docs claim. [1]

2. There's a VK_GOOGLE_display_timing extension which similarly allows 
providing a desiredPresentTime (in ns).

3. Keith Packard's CRTC_{GET,QUEUE}_SEQUENCE is not specific to Adaptive 
Sync, but seems like something Adaptive Sync-aware applications would 
want to use. [2]

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.

These two sides then need to be plumbed through the entire stack.

I guess for now the main questions are: Is there more "prior art" that 
we should be aware of? Does anybody have partial prototypes or very 
strong feelings about what the interfaces and protocols should look like?

Cheers,
Nicolai

[0] https://patchwork.freedesktop.org/patch/183117/

[1] 
https://cgit.freedesktop.org/xorg/proto/presentproto/tree/presentproto.txt

[2] https://lists.freedesktop.org/archives/dri-devel/2017-August/148905.html
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Michel Dänzer]

On 17/10/17 11:34 AM, Nicolai Hähnle wrote:
> Hi all,
> 
> I just sent out a patch that enables FreeSync in Mesa for the somewhat
> hacked implementation of FreeSync that exists in our hybrid (amdgpu-pro)
> stack's DDX and kernel module. [0]
> 
> While this patch isn't meant for upstream, that's as good a time as any
> to raise the issue of how a proper upstream solution would look like. It
> needs to cut across the entire stack, and we should try to align the KMS
> interface with the X11 protocol and the Wayland protocol.
> 
> Prior art that I'm aware of includes:
> 
> 1. The Present protocol extension has a PresentOptionUST bit for
> requesting a specific present time, but the reality is that the
> implementation of that is even less than what the protocol docs claim. [1]

FWIW, I do think this could be a good way for clients to signal that
they want a frame to be displayed ASAP. It would also allow for e.g.
video players to naturally adapt the refresh rate to the video frame
rate (the VDPAU presentation API has a target timestamp for this).


> 3. Keith Packard's CRTC_{GET,QUEUE}_SEQUENCE is not specific to Adaptive
> Sync, but seems like something Adaptive Sync-aware applications would
> want to use. [2]

Not sure I can agree with that. Applications should use higher level
APIs, not low level ones like these directly. (Also, they're basically
just KMS variants of DRM_IOCTL_WAIT_VBLANK, not directly related to
adaptive sync)


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


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Nicolai Hähnle]

On 17.10.2017 12:28, Michel Dänzer wrote:
> On 17/10/17 11:34 AM, Nicolai Hähnle wrote:
>> Hi all,
>>
>> I just sent out a patch that enables FreeSync in Mesa for the somewhat
>> hacked implementation of FreeSync that exists in our hybrid (amdgpu-pro)
>> stack's DDX and kernel module. [0]
>>
>> While this patch isn't meant for upstream, that's as good a time as any
>> to raise the issue of how a proper upstream solution would look like. It
>> needs to cut across the entire stack, and we should try to align the KMS
>> interface with the X11 protocol and the Wayland protocol.
>>
>> Prior art that I'm aware of includes:
>>
>> 1. The Present protocol extension has a PresentOptionUST bit for
>> requesting a specific present time, but the reality is that the
>> implementation of that is even less than what the protocol docs claim. [1]
> 
> FWIW, I do think this could be a good way for clients to signal that
> they want a frame to be displayed ASAP. It would also allow for e.g.
> video players to naturally adapt the refresh rate to the video frame
> rate (the VDPAU presentation API has a target timestamp for this).

Agreed. The point is that a lot of implementation work needs to be done, 
and the protocol docs need to be fixed (the doc claims that every 
implementation will treat PresentOptionUST reasonably, rounding to the 
nearest MSC when PresentCapabilityUST is missing, but that's false as 
far as I can tell).


>> 3. Keith Packard's CRTC_{GET,QUEUE}_SEQUENCE is not specific to Adaptive
>> Sync, but seems like something Adaptive Sync-aware applications would
>> want to use. [2]
> 
> Not sure I can agree with that. Applications should use higher level
> APIs, not low level ones like these directly. (Also, they're basically
> just KMS variants of DRM_IOCTL_WAIT_VBLANK, not directly related to
> adaptive sync)
 >
>> 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.

Time for xf86-video-amdgpu to grow atomic support, then? ;)

How is a target vblank count communicated via the atomic API? Or is that 
simply not part of the design and up to user space?

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

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

[Daniel Vetter]

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:
> > Hi all,
> > 
> > I just sent out a patch that enables FreeSync in Mesa for the somewhat
> > hacked implementation of FreeSync that exists in our hybrid (amdgpu-pro)
> > stack's DDX and kernel module. [0]
> > 
> > While this patch isn't meant for upstream, that's as good a time as any
> > to raise the issue of how a proper upstream solution would look like. It
> > needs to cut across the entire stack, and we should try to align the KMS
> > interface with the X11 protocol and the Wayland protocol.
> > 
> > Prior art that I'm aware of includes:
> > 
> > 1. The Present protocol extension has a PresentOptionUST bit for
> > requesting a specific present time, but the reality is that the
> > implementation of that is even less than what the protocol docs claim. [1]
> 
> FWIW, I do think this could be a good way for clients to signal that
> they want a frame to be displayed ASAP. It would also allow for e.g.
> video players to naturally adapt the refresh rate to the video frame
> rate (the VDPAU presentation API has a target timestamp for this).
> 
> 
> > 3. Keith Packard's CRTC_{GET,QUEUE}_SEQUENCE is not specific to Adaptive
> > Sync, but seems like something Adaptive Sync-aware applications would
> > want to use. [2]
> 
> Not sure I can agree with that. Applications should use higher level
> APIs, not low level ones like these directly. (Also, they're basically
> just KMS variants of DRM_IOCTL_WAIT_VBLANK, not directly related to
> adaptive sync)

+1.

We already accidentally exposed the legacy vblank ioctl to clients, and
they abuse it badly (second-guessing the compositor, which works so well).
If you're a client app and want timings, you must query the compositor.
Neither the client nor the kernel know enough to make this happen
directly.

Ofc the compositor will want to query timings through ioctls, but we
provide them all already (flip timestamp and vblank timestamp), Keith's
new ioctl simply provides a bit of an uapi cleanup + nanosecond accuracy
(not that any driver can do that anyway right now).

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

I think a boolean enable_freesync property is probably what we want, which
enables freesync for as long as it's set.

The other side is communicating to userspace which modes are freesync
capable. We might want to extend the mode struct with a min/max vrefresh
rate, or something similar.

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.

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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 17/10/17 01:04 PM, Nicolai Hähnle wrote:
> On 17.10.2017 12:28, 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.
> 
> Time for xf86-video-amdgpu to grow atomic support, then? ;)

Yes, that will likely be part of an upstreamable solution. There are
already patches for this for the modesetting driver, adapting those
might not be that hard.


> How is a target vblank count communicated via the atomic API? Or is that
> simply not part of the design and up to user space?

From Daniel's followup it sounds like there's no support for this yet in
the atomic API, but I'm assuming it would be communicated via
properties, as is the case for most things in the atomic API.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Upstream support for FreeSync / Adaptive Sync

[Michel Dänzer]

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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Christian König]

Am 17.10.2017 um 15:46 schrieb Michel Dänzer:
> On 17/10/17 02:22 PM, Daniel Vetter wrote:
> [SNIP]
>> 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.

Application calculate their frames for a certain point in time.

As far as I know this is very important for any VR application if you 
don't want to get sea sick.

Regards,
Christian.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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:
- Is it going to be per-plane or per-crtc?
- 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?
- What happens if another operation is already queued with a more
  recent timestamp?
- 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.

I do like the idea, but clearly there's a bit of thought require to
make sure the semantics are good.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Upstream support for FreeSync / Adaptive Sync

[Daniel Vetter]

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.

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

I thought the primary reason for adaptive sync is the adaptive frame rate
to cope with the occasional stall in games. If the intended use-case is
vr/media, then I agree going with timestamps from the beginning makes
sense. That still leaves the "schedule asap, with some leeway" mode. Or is
that (no longer) something we want?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: Upstream support for FreeSync / Adaptive Sync

[Michel Dänzer]

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.


>>> 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.
> 
> I thought the primary reason for adaptive sync is the adaptive frame rate
> to cope with the occasional stall in games. If the intended use-case is
> vr/media, then I agree going with timestamps from the beginning makes
> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
> that (no longer) something we want?

Both are use cases for adaptive sync. Both can be covered by a target
timestamp. There may be other possible solutions which work for both though.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Daniel Vetter]

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.

Given all that I'm not sure whether requiring that userspace submits a
timestamp to get adaptive sync is a good idea (if we don't have an "as
soon as ready" flag at least), and the timestamp/flag at flip time
isn't enough either.

>>>> 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.
>>
>> I thought the primary reason for adaptive sync is the adaptive frame rate
>> to cope with the occasional stall in games. If the intended use-case is
>> vr/media, then I agree going with timestamps from the beginning makes
>> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
>> that (no longer) something we want?
>
> Both are use cases for adaptive sync. Both can be covered by a target
> timestamp. There may be other possible solutions which work for both though.

Hm, how do you make the "flip as soon as ready" semantics work with
timestamps, without requiring userspace to wait for the fences to
signal before submitting? Set the timestamp to now and force the miss?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Nicolai Hähnle]

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.


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


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


> I do like the idea, but clearly there's a bit of thought require to
> make sure the semantics are good.

Agreed!

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

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

[Nicolai Hähnle]

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?


> Given all that I'm not sure whether requiring that userspace submits a
> timestamp to get adaptive sync is a good idea (if we don't have an "as
> soon as ready" flag at least), and the timestamp/flag at flip time
> isn't enough either.
> 
>>>>> 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.
>>>
>>> I thought the primary reason for adaptive sync is the adaptive frame rate
>>> to cope with the occasional stall in games. If the intended use-case is
>>> vr/media, then I agree going with timestamps from the beginning makes
>>> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
>>> that (no longer) something we want?
>>
>> Both are use cases for adaptive sync. Both can be covered by a target
>> timestamp. There may be other possible solutions which work for both though.
> 
> Hm, how do you make the "flip as soon as ready" semantics work with
> timestamps, without requiring userspace to wait for the fences to
> signal before submitting? Set the timestamp to now and force the miss?

Like I wrote in my reply to Ville, I think it makes sense to always 
treat stale timestamps as "flip as soon as ready".

Cheers,
Nicolai

> -Daniel
> 


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

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

[Ville Syrjälä]

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.

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

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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.

> > Given all that I'm not sure whether requiring that userspace submits a
> > timestamp to get adaptive sync is a good idea (if we don't have an "as
> > soon as ready" flag at least), and the timestamp/flag at flip time
> > isn't enough either.
> > 
> > > > > > 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.
> > > > 
> > > > I thought the primary reason for adaptive sync is the adaptive frame rate
> > > > to cope with the occasional stall in games. If the intended use-case is
> > > > vr/media, then I agree going with timestamps from the beginning makes
> > > > sense. That still leaves the "schedule asap, with some leeway" mode. Or is
> > > > that (no longer) something we want?
> > > 
> > > Both are use cases for adaptive sync. Both can be covered by a target
> > > timestamp. There may be other possible solutions which work for both though.
> > 
> > Hm, how do you make the "flip as soon as ready" semantics work with
> > timestamps, without requiring userspace to wait for the fences to
> > signal before submitting? Set the timestamp to now and force the miss?
> 
> Like I wrote in my reply to Ville, I think it makes sense to always treat
> stale timestamps as "flip as soon as ready".

Makes sense, and matches what we do with the vblank target right now. But
with stuff like VR it might be that we need a window, and when things are
delayed too much it's better to re-render a newly distorted frame instead
of motion sickness. We'll see. VR's real tough anyway :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 18/10/17 10:10 AM, 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:
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>> I thought the primary reason for adaptive sync is the adaptive frame rate
>>>>> to cope with the occasional stall in games. If the intended use-case is
>>>>> vr/media, then I agree going with timestamps from the beginning makes
>>>>> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
>>>>> that (no longer) something we want?
>>>>
>>>> Both are use cases for adaptive sync. Both can be covered by a target
>>>> timestamp. There may be other possible solutions which work for both though.
>>>
>>> Hm, how do you make the "flip as soon as ready" semantics work with
>>> timestamps, without requiring userspace to wait for the fences to
>>> signal before submitting? Set the timestamp to now and force the miss?
>>
>> Like I wrote in my reply to Ville, I think it makes sense to always treat
>> stale timestamps as "flip as soon as ready".
> 
> Makes sense, and matches what we do with the vblank target right now. But
> with stuff like VR it might be that we need a window, and when things are
> delayed too much it's better to re-render a newly distorted frame instead
> of motion sickness. We'll see. VR's real tough anyway :-)

I suspect a VR compositor will have to deal with this before submitting
the flip to the kernel, i.e. wait for the frame to finish rendering, and
if it takes too long, render a reprojected frame and flip to that instead.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Upstream support for FreeSync / Adaptive Sync

[Nicolai Hähnle]

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

Re: [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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 18/10/17 12:15 PM, Nicolai Hähnle wrote:
> 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.

AFAIU the atomic KMS API (Daniel et al please correct me if I'm wrong),
this might not be an issue: When a process which enabled the property
dies, I think any other process has to do a modeset to display anything
else on that CRTC. The modeset will set the property's value to what's
expected by that process.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Harry Wentland]

On 2017-10-18 04:10 AM, 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.
> 

A bit late to the thread but whatever has been suggested sounds quite good.

Our experience generally has been that we don't want games to do framepacing but flip as soon as the render finishes, as games have no clue how long a render takes, even though some might pretend to.

As for targeted applications, like video, a timestamp should work. A timestamp might also work as a power-saving feature on the desktop where the compositor would basically force a reasonable refresh rate (like 60 instead of 144Hz), possibly ramping that rate up or down based on activity. The thing to keep in mind here is that a lot of Freesync displays might flicker if the actual refresh rate has large jumps.

>>> Given all that I'm not sure whether requiring that userspace submits a
>>> timestamp to get adaptive sync is a good idea (if we don't have an "as
>>> soon as ready" flag at least), and the timestamp/flag at flip time
>>> isn't enough either.
>>>
>>>>>>> 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.
>>>>>
>>>>> I thought the primary reason for adaptive sync is the adaptive frame rate
>>>>> to cope with the occasional stall in games. If the intended use-case is
>>>>> vr/media, then I agree going with timestamps from the beginning makes
>>>>> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
>>>>> that (no longer) something we want?
>>>>
>>>> Both are use cases for adaptive sync. Both can be covered by a target
>>>> timestamp. There may be other possible solutions which work for both though.
>>>
>>> Hm, how do you make the "flip as soon as ready" semantics work with
>>> timestamps, without requiring userspace to wait for the fences to
>>> signal before submitting? Set the timestamp to now and force the miss?
>>
>> Like I wrote in my reply to Ville, I think it makes sense to always treat
>> stale timestamps as "flip as soon as ready".
> 
> Makes sense, and matches what we do with the vblank target right now. But
> with stuff like VR it might be that we need a window, and when things are
> delayed too much it's better to re-render a newly distorted frame instead
> of motion sickness. We'll see. VR's real tough anyway :-)

I think currently we don't do Freesync on VR since the VR app wants to be able to control the refresh rate to avoid motion sickness.

Harry

> -Daniel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Oct 18, 2017 at 6:59 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 18/10/17 12:15 PM, Nicolai Hähnle wrote:
>> 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.
>
> AFAIU the atomic KMS API (Daniel et al please correct me if I'm wrong),
> this might not be an issue: When a process which enabled the property
> dies, I think any other process has to do a modeset to display anything
> else on that CRTC. The modeset will set the property's value to what's
> expected by that process.

All the atomic commits are incremental (this was needed to be able to
implement the legacy stuff on top of atomic), so if you don't reset
the prop, you'll inherit whatever's there.

But one design principle for new properties we're tryng to uphold is
that userspace can just read them all and force-restore all atomic
properties when it takes over again, even if it doesn't understand
them, to be able to recover from something else having crashed. That
means special properties that cause a one-off action like fences, or
the link status stuff give you a value on read that won't cause
anything to happen when writing it back. But the kernel won't restore
stuff for you. fbcon does restore some of the things it deems
relevant, and we migh want to add a reset for the freesync to the list
of things it sets. But you can't rely on fbcon everywhere.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Manasi Navare]

On Wed, Oct 18, 2017 at 03:20:57PM -0400, Harry Wentland wrote:
> On 2017-10-18 04:10 AM, 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?

Thanks for initiating this discussion. I had some initial patches of creating these
properties in DRM for Adaptive Sync but never got a chance to complete the testing and
submitting them upstream.
So the initial thought was to add two properties:

1. bool enable_adaptive_sync - one for allowing userspace to
enable VRR and 
2. bool supports_adaptive_sync - the other immutable property for exposing
hardware's capability of supporting VRR.

The second property will be set based on the VRR capabilities of the connected
monitor. If this is set, userspace can request to enable adaptive-sync by setting
the first property and through the atomic modeset, the driver will check if the
requested frame rate falls within the monitor range and enable it.

I have some prototype patches as well...But I would like to see where this
discussion heads and then submit those some day.

Manasi

Manasi

> > 
> > Yeah, that's what I had in mind. No idea it'll work out on real hw/full
> > stack.
> > 
> 
> A bit late to the thread but whatever has been suggested sounds quite good.
> 
> Our experience generally has been that we don't want games to do framepacing but flip as soon as the render finishes, as games have no clue how long a render takes, even though some might pretend to.
> 
> As for targeted applications, like video, a timestamp should work. A timestamp might also work as a power-saving feature on the desktop where the compositor would basically force a reasonable refresh rate (like 60 instead of 144Hz), possibly ramping that rate up or down based on activity. The thing to keep in mind here is that a lot of Freesync displays might flicker if the actual refresh rate has large jumps.
> 
> >>> Given all that I'm not sure whether requiring that userspace submits a
> >>> timestamp to get adaptive sync is a good idea (if we don't have an "as
> >>> soon as ready" flag at least), and the timestamp/flag at flip time
> >>> isn't enough either.
> >>>
> >>>>>>> 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.
> >>>>>
> >>>>> I thought the primary reason for adaptive sync is the adaptive frame rate
> >>>>> to cope with the occasional stall in games. If the intended use-case is
> >>>>> vr/media, then I agree going with timestamps from the beginning makes
> >>>>> sense. That still leaves the "schedule asap, with some leeway" mode. Or is
> >>>>> that (no longer) something we want?
> >>>>
> >>>> Both are use cases for adaptive sync. Both can be covered by a target
> >>>> timestamp. There may be other possible solutions which work for both though.
> >>>
> >>> Hm, how do you make the "flip as soon as ready" semantics work with
> >>> timestamps, without requiring userspace to wait for the fences to
> >>> signal before submitting? Set the timestamp to now and force the miss?
> >>
> >> Like I wrote in my reply to Ville, I think it makes sense to always treat
> >> stale timestamps as "flip as soon as ready".
> > 
> > Makes sense, and matches what we do with the vblank target right now. But
> > with stuff like VR it might be that we need a window, and when things are
> > delayed too much it's better to re-render a newly distorted frame instead
> > of motion sickness. We'll see. VR's real tough anyway :-)
> 
> I think currently we don't do Freesync on VR since the VR app wants to be able to control the refresh rate to avoid motion sickness.
> 
> Harry
> 
> > -Daniel
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Manasi Navare]

On Wed, Oct 18, 2017 at 09:28:01PM +0200, Daniel Vetter wrote:
> On Wed, Oct 18, 2017 at 6:59 PM, Michel Dänzer <michel@daenzer.net> wrote:
> > On 18/10/17 12:15 PM, Nicolai Hähnle wrote:
> >> 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.
> >
> > AFAIU the atomic KMS API (Daniel et al please correct me if I'm wrong),
> > this might not be an issue: When a process which enabled the property
> > dies, I think any other process has to do a modeset to display anything
> > else on that CRTC. The modeset will set the property's value to what's
> > expected by that process.
> 
> All the atomic commits are incremental (this was needed to be able to
> implement the legacy stuff on top of atomic), so if you don't reset
> the prop, you'll inherit whatever's there.
> 
> But one design principle for new properties we're tryng to uphold is
> that userspace can just read them all and force-restore all atomic
> properties when it takes over again, even if it doesn't understand
> them, to be able to recover from something else having crashed. That
> means special properties that cause a one-off action like fences, or
> the link status stuff give you a value on read that won't cause
> anything to happen when writing it back. But the kernel won't restore
> stuff for you. fbcon does restore some of the things it deems
> relevant, and we migh want to add a reset for the freesync to the list
> of things it sets. But you can't rely on fbcon everywhere.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Thanks for initiating this discussion. I had some initial patches of creating these
properties in DRM for Adaptive Sync but never got a chance to complete the testing and
submitting them upstream.
So the initial thought was to add two properties:

1. bool enable_adaptive_sync - one for allowing userspace to
enable VRR and
2. bool supports_adaptive_sync - the other immutable property for exposing
hardware's capability of supporting VRR.

The second property will be set based on the VRR capabilities of the connected
monitor. If this is set, userspace can request to enable adaptive-sync by setting
the first property and through the atomic modeset, the driver will check if the
requested frame rate falls within the monitor range and enable it.

I have some prototype patches as well...But I would like to see where this
discussion heads and then submit those some day.

Manasi

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel