From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruno Milreu Subject: Re: [PATCH] drm/amd/display: Use vrr friendly pageflip throttling in DC Date: Fri, 22 Feb 2019 00:30:17 -0300 Message-ID: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1322163453==" Return-path: List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org --===============1322163453== Content-Type: multipart/alternative; boundary="0000000000005e201e0582733683" --0000000000005e201e0582733683 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 2/9/19 1:52 AM, 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 > Cc: > Cc: Nicholas Kazlauskas > Cc: Harry Wentland > Cc: Alex Deucher > Cc: Michel D=C3=A4nzer This gets rid of stuttering with FreeSync for me. The current behavior shows constant periodic stutters at lower than maximum framerates. Please try pushing this into 5.1. Tested-by: Bruno Filipe --0000000000005e201e0582733683 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On 2/9/19 1:52 AM, Mario Kleiner wrote:
> In VRR mod= e, keep track of the vblank count of the last
> completed pageflip in= amdgpu_crtc->last_flip_vblank, as
> recorded in the pageflip comp= letion handler after each
> completed flip.
>
> Use that= count to prevent mmio programming a new pageflip
> within the same v= blank in which the last pageflip completed,
> iow. to throttle pagefl= ips to at most one flip per video
> frame, while at the same time all= owing to request a flip
> not only before start of vblank, but also a= nywhere 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 hu= ge vblank, thereby
> reducing framerate in vrr mode instead of improv= ing it, by
> delaying a slightly delayed flip requests up to a maximu= m
> 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 metho= d a flip can be both requested and - after
> fences have completed - = executed, ie. it doesn't matter if
> the request (amdgpu_dm_do_fl= ip()) gets delayed until deep
> into the extended vblank due to cpu e= xecution delays. This
> also allows clients which want to regulate fr= amerate within
> the vrr range a much more fine-grained control of fl= ip timing,
> a feature that might be useful for video playback, and i= s
> 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
> OpenG= L clients intact, if they use the GLX_OML_sync_control
> extensions g= lXSwapBufferMscOML(, ..., 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. T= hey'd often flip one
> frame too early. However, this limitation = should not matter
> much in VRR mode, as scheduling based on vblank c= ounts is
> pretty futile/unusable under variable refresh duration
= > anyway, so no real extra harm is done.
>
> According to s= ome testing already done with this patch by
> Nicholas on top of my t= ests, IGT tests didn't report any
> problems. If fixes stuttering= and flickering when flipping
> at rates below the minimum vrr refres= h rate.
>
> Fixes: bb47de736661 ("drm/amdgpu: Set FreeSyn= c state using drm VRR
> properties")
> Signed-off-by: Mari= o Kleiner <mario.kleiner.de at <= a href=3D"http://gmail.com">gmail.com>
> Cc: <stable at vger.kernel.org>
> Cc: Nichola= s Kazlauskas <nicholas.kazlauskas at amd.com<= /a>>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Alex Deucher <alexander.deucher at = amd.com>
> Cc: Michel D=C3=A4nzer &= lt;michel at daenzer.net>

This gets rid of stuttering with FreeSync for me. The current behavior sh= ows constant periodic stutters at lower than maximum framerates. Please try= pushing this into 5.1.

= --0000000000005e201e0582733683-- --===============1322163453== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4 --===============1322163453==--