On Tue, Oct 17, 2017 at 09:01:52PM +0200, Nicolai Hähnle wrote: > On 17.10.2017 19:16, Daniel Vetter wrote: > > On Tue, Oct 17, 2017 at 5:40 PM, Michel Dänzer <michel@xxxxxxxxxxx> wrote: > > > On 17/10/17 05:04 PM, Daniel Vetter wrote: > > > > On Tue, Oct 17, 2017 at 03:46:24PM +0200, Michel Dänzer wrote: > > > > > On 17/10/17 02:22 PM, Daniel Vetter wrote: > > > > > > On Tue, Oct 17, 2017 at 12:28:17PM +0200, Michel Dänzer wrote: > > > > > > > On 17/10/17 11:34 AM, Nicolai Hähnle wrote: > > > > > > > > > > > > > > Common sense suggests that there need to be two side to FreeSync / VESA > > > > > > > > Adaptive Sync support: > > > > > > > > > > > > > > > > 1. Query the display capabilities. This means querying minimum / maximum > > > > > > > > refresh duration, plus possibly a query for when the earliest/latest > > > > > > > > timing of the *next* refresh. > > > > > > > > > > > > > > > > 2. Signal desired present time. This means passing a target timer value > > > > > > > > instead of a target vblank count, e.g. something like this for the KMS > > > > > > > > interface: > > > > > > > > > > > > > > > > int drmModePageFlipTarget64(int fd, uint32_t crtc_id, uint32_t fb_id, > > > > > > > > uint32_t flags, void *user_data, > > > > > > > > uint64_t target); > > > > > > > > > > > > > > > > + a flag to indicate whether target is the vblank count or the > > > > > > > > CLOCK_MONOTONIC (?) time in ns. > > > > > > > > > > > > > > drmModePageFlip(Target) is part of the pre-atomic KMS API, but adapative > > > > > > > sync should probably only be supported via the atomic API, presumably > > > > > > > via output properties. > > > > > > > > > > > > +1 > > > > > > > > > > > > At least now that DC is on track to land properly, and you want to do this > > > > > > for DC-only anyway there's no reason to pimp the legacy interfaces > > > > > > further. And atomic is soooooo much easier to extend. > > > > > > > > > > > > The big question imo is where we need to put the flag on the kms side, > > > > > > since freesync is not just about presenting earlier, but also about > > > > > > presenting later. But for backwards compat we can't stretch the refresh > > > > > > rate by default for everyone, or clients that rely on high precision > > > > > > timestamps and regular refresh will get a bad surprise. > > > > > > > > > > The idea described above is that adaptive sync would be used for flips > > > > > with a target timestamp. Apps which don't want to use adaptive sync > > > > > wouldn't set a target timestamp. > > > > > > > > > > > > > > > > I think a boolean enable_freesync property is probably what we want, which > > > > > > enables freesync for as long as it's set. > > > > > > > > > > The question then becomes under what circumstances the property is (not) > > > > > set. Not sure offhand this will actually solve any problem, or just push > > > > > it somewhere else. > > > > > > > > I thought that's what the driconf switch is for, with a policy of "please > > > > schedule asap" instead of a specific timestamp. > > > > > > The driconf switch is just for the user's intention to use adaptive sync > > > when possible. A property as you suggest cannot be set by the client > > > directly, because it can't know when adaptive sync can actually be used > > > (only when its window is fullscreen and using page flipping). So the > > > property would have to be set by the X server/driver / Wayland > > > compositor / ... instead. The question is whether such a property is > > > actually needed, or whether the kernel could just enable adaptive sync > > > when there's a flip with a target timestamp, and disable it when there's > > > a flip without a target timestamp, or something like that. > > > > If your adaptive sync also supports extending the vblank beyond the > > nominal limit, then you can't do that with a per-flip flag. Because > > absent of a userspace requesting adaptive sync you must flip at the > > nominal vrefresh rate. So if your userspace is a tad bit late with the > > frame and would like to extend the frame to avoid missing a frame > > entirely it'll be too late by the time the vblank actually gets > > submitted. That's a bit a variation of what Ville brought up about > > what we're going to do when the timestamp was missed by the time all > > the depending fences signalled. > > These are very good points. It does sound like we'd need both an > "AdaptiveSync" boolean property and an (optional) "DesiredPresentTime" > property. > > The DesiredPresentTime property applies only to a single commit and could > perhaps be left out in a first version. The AdaptiveSync property is > persistent. When enabled, it means: > > - handle page flip requests as soon as possible > - while no page flip is requested, delay vblank as long as possible > > How does that sound? Yeah, that's what I had in mind. No idea it'll work out on real hw/full stack. > > Given all that I'm not sure whether requiring that userspace submits a > > timestamp to get adaptive sync is a good idea (if we don't have an "as > > soon as ready" flag at least), and the timestamp/flag at flip time > > isn't enough either. > > > > > > > > Finally I'm not sure we want to insist on a target time for freesync. At > > > > > > least as far as I understand things you just want "as soon as possible". > > > > > > This might change with some of the VK/EGL/GLX extensions where you > > > > > > specify a precise timing (media playback). But that needs a bit more work > > > > > > to make it happen I think, so perhaps better to postpone. > > > > > > > > > > I don't see why. There's an obvious use case for this now, for video > > > > > playback. At least VDPAU already has target timestamps for this. > > > > > > > > > > > > > > > > Also note that right now no driver expect amdgpu has support for a target > > > > > > vblank on a flip. That's imo another reason for not requiring target > > > > > > support for at least basic freesync support. > > > > > > > > > > I think that's a bad reason. :) Adding it for atomic drivers shouldn't > > > > > be that hard. > > > > > > > > I thought the primary reason for adaptive sync is the adaptive frame rate > > > > to cope with the occasional stall in games. If the intended use-case is > > > > vr/media, then I agree going with timestamps from the beginning makes > > > > sense. That still leaves the "schedule asap, with some leeway" mode. Or is > > > > that (no longer) something we want? > > > > > > Both are use cases for adaptive sync. Both can be covered by a target > > > timestamp. There may be other possible solutions which work for both though. > > > > Hm, how do you make the "flip as soon as ready" semantics work with > > timestamps, without requiring userspace to wait for the fences to > > signal before submitting? Set the timestamp to now and force the miss? > > Like I wrote in my reply to Ville, I think it makes sense to always treat > stale timestamps as "flip as soon as ready". Makes sense, and matches what we do with the vblank target right now. But with stuff like VR it might be that we need a window, and when things are delayed too much it's better to re-render a newly distorted frame instead of motion sickness. We'll see. VR's real tough anyway :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
