[PATCH] drm/amd/display: Use vrr friendly pageflip throttling in DC.

[Daniel Vetter]

On Mon, Feb 11, 2019 at 04:01:12PM +0100, Michel Dänzer wrote:
> On 2019-02-09 7:52 a.m., Mario Kleiner wrote:
> > In VRR mode, keep track of the vblank count of the last
> > completed pageflip in amdgpu_crtc->last_flip_vblank, as
> > recorded in the pageflip completion handler after each
> > completed flip.
> > 
> > Use that count to prevent mmio programming a new pageflip
> > within the same vblank in which the last pageflip completed,
> > iow. to throttle pageflips to at most one flip per video
> > frame, while at the same time allowing to request a flip
> > not only before start of vblank, but also anywhere within
> > vblank.
> > 
> > The old logic did the same, and made sense for regular fixed
> > refresh rate flipping, but in vrr mode it prevents requesting
> > a flip anywhere inside the possibly huge vblank, thereby
> > reducing framerate in vrr mode instead of improving it, by
> > delaying a slightly delayed flip requests up to a maximum
> > vblank duration + 1 scanout duration. This would limit VRR
> > usefulness to only help applications with a very high GPU
> > demand, which can submit the flip request before start of
> > vblank, but then have to wait long for fences to complete.
> > 
> > With this method a flip can be both requested and - after
> > fences have completed - executed, ie. it doesn't matter if
> > the request (amdgpu_dm_do_flip()) gets delayed until deep
> > into the extended vblank due to cpu execution delays. This
> > also allows clients which want to regulate framerate within
> > the vrr range a much more fine-grained control of flip timing,
> > a feature that might be useful for video playback, and is
> > very useful for neuroscience/vision research applications.
> > 
> > In regular non-VRR mode, retain the old flip submission
> > behavior. This to keep flip scheduling for fullscreen X11/GLX
> > OpenGL clients intact, if they use the GLX_OML_sync_control
> > extensions glXSwapBufferMscOML(, ..., target_msc,...) function
> > with a specific target_msc target vblank count.
> > 
> > glXSwapBuffersMscOML() or DRI3/Present PresentPixmap() will
> > not flip at the proper target_msc for a non-zero target_msc
> > if VRR mode is active with this patch. They'd often flip one
> > frame too early. However, this limitation should not matter
> > much in VRR mode, as scheduling based on vblank counts is
> > pretty futile/unusable under variable refresh duration
> > anyway, so no real extra harm is done.
> > 
> > According to some testing already done with this patch by
> > Nicholas on top of my tests, IGT tests didn't report any
> > problems. If fixes stuttering and flickering when flipping
> > at rates below the minimum vrr refresh rate.
> > 
> > Fixes: bb47de736661 ("drm/amdgpu: Set FreeSync state using drm VRR
> > properties")
> > Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> > Cc: <stable at vger.kernel.org>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> > Cc: Harry Wentland <harry.wentland at amd.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Cc: Michel Dänzer <michel at daenzer.net>
> 
> I wonder if this couldn't be solved in a simpler / cleaner way by making
> use of the target MSC passed to the page_flip_target hook.

Requiring that all compositors that use VRR also have to use page_flip
target (which is not yet exposed on the atomic side yet at all) just
because amdgpu doesn't sound like a great idea. I think better to handle
this in the amdgpu kernel driver, for similar reasons we've originally
added this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch