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

[Ville Syrjälä]

On Tue, Oct 17, 2017 at 03:46:24PM +0200, Michel Dänzer wrote:
> On 17/10/17 02:22 PM, Daniel Vetter wrote:
> > On Tue, Oct 17, 2017 at 12:28:17PM +0200, Michel Dänzer wrote:
> >> On 17/10/17 11:34 AM, Nicolai Hähnle wrote:
> > 
> >>> Common sense suggests that there need to be two side to FreeSync / VESA
> >>> Adaptive Sync support:
> >>>
> >>> 1. Query the display capabilities. This means querying minimum / maximum
> >>> refresh duration, plus possibly a query for when the earliest/latest
> >>> timing of the *next* refresh.
> >>>
> >>> 2. Signal desired present time. This means passing a target timer value
> >>> instead of a target vblank count, e.g. something like this for the KMS
> >>> interface:
> >>>
> >>>   int drmModePageFlipTarget64(int fd, uint32_t crtc_id, uint32_t fb_id,
> >>>                               uint32_t flags, void *user_data,
> >>>                               uint64_t target);
> >>>
> >>>   + a flag to indicate whether target is the vblank count or the
> >>> CLOCK_MONOTONIC (?) time in ns.
> >>
> >> drmModePageFlip(Target) is part of the pre-atomic KMS API, but adapative
> >> sync should probably only be supported via the atomic API, presumably
> >> via output properties.
> > 
> > +1
> > 
> > At least now that DC is on track to land properly, and you want to do this
> > for DC-only anyway there's no reason to pimp the legacy interfaces
> > further. And atomic is soooooo much easier to extend.
> > 
> > The big question imo is where we need to put the flag on the kms side,
> > since freesync is not just about presenting earlier, but also about
> > presenting later. But for backwards compat we can't stretch the refresh
> > rate by default for everyone, or clients that rely on high precision
> > timestamps and regular refresh will get a bad surprise.
> 
> The idea described above is that adaptive sync would be used for flips
> with a target timestamp. Apps which don't want to use adaptive sync
> wouldn't set a target timestamp.
> 
> 
> > I think a boolean enable_freesync property is probably what we want, which
> > enables freesync for as long as it's set.
> 
> The question then becomes under what circumstances the property is (not)
> set. Not sure offhand this will actually solve any problem, or just push
> it somewhere else.
> 
> 
> > Finally I'm not sure we want to insist on a target time for freesync. At
> > least as far as I understand things you just want "as soon as possible".
> > This might change with some of the VK/EGL/GLX extensions where you
> > specify a precise timing (media playback). But that needs a bit more work
> > to make it happen I think, so perhaps better to postpone.
> 
> I don't see why. There's an obvious use case for this now, for video
> playback. At least VDPAU already has target timestamps for this.
> 
> 
> > Also note that right now no driver expect amdgpu has support for a target
> > vblank on a flip. That's imo another reason for not requiring target
> > support for at least basic freesync support.
> 
> I think that's a bad reason. :) Adding it for atomic drivers shouldn't
> be that hard.

Apart from the actual implementation hurdles it does open up some new questions:
- Is it going to be per-plane or per-crtc?
- What happens if the target timestamp is already stale?
- What happens if the target timestamp is good when it gets scheduled,
  but can't be met once the fences and whatnot have signalled?
- What happens if another operation is already queued with a more
  recent timestamp?
- Apart from a pure timestamp do we want to move the OML_sync/swap_whatever
  msc remainder etc. semantics into the kernel as well? It's just
  another way to specify the target flip time after all.

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

-- 
Ville Syrjälä
Intel OTC