Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 good points. It does sound like we'd need b
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 no page flip is reque
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 games to do framepacing
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 version. The AdaptiveSync property is persi
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 long as possible >>> >>>
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 at
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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. ___ 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
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 ___ 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
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 reques
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 to
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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. :) Adding it for atomic drive
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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. ___ 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
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 intended use-case is
Re: [Mesa-dev] Upstream support for FreeSync / Adaptive Sync
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 ___ 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
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 ___ 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
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 ___ 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
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. ___ 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
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 ___ 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
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 ___ 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
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 ___ 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
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. ___ 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
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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev