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

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

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

[Daniel Vetter]

On Wed, Oct 18, 2017 at 6:59 PM, Michel Dänzer  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 
> 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 soo 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 

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

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

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

Re: [Mesa-dev] 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  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 soo 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.
___
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  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: [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  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 soo 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:
> 
> - 

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

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

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

[Daniel Vetter]

On Tue, Oct 17, 2017 at 5:40 PM, Michel Dänzer  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 soo 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 

Re: [Mesa-dev] 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 soo 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
___
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 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 soo 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
___
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 soo 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: [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

[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 soo 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
___
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: [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 soo 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

[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

[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