On 18/10/17 12:15 PM, Nicolai Hähnle wrote: > On 18.10.2017 10:10, Daniel Vetter wrote: >> On Tue, Oct 17, 2017 at 09:01:52PM +0200, Nicolai Hähnle wrote: >>> On 17.10.2017 19:16, Daniel Vetter wrote: >>>> On Tue, Oct 17, 2017 at 5:40 PM, Michel Dänzer <michel@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. > > Here's another question that occurred to me while thinking about how all > this could be prototyped. > > What happens when a FreeSync aware application / compositor enables the > FreeSync property and then shuts down (crashes) without disabling it again? > > It seems to me that a non-FreeSync aware compositor would then be > operating in FreeSync mode without expecting it. AFAIU the atomic KMS API (Daniel et al please correct me if I'm wrong), this might not be an issue: When a process which enabled the property dies, I think any other process has to do a modeset to display anything else on that CRTC. The modeset will set the property's value to what's expected by that process. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
