On 22/04/17 01:23 AM, Mario Kleiner wrote:
> Make sure we do not program a hw pageflip inside vblank 'n' iff the
> atomic flip is comitted while inside the same vblank 'n'. We must
> defer such a flip by one refresh cycle to vblank 'n+1'.
> 
> Without this, pageflips programmed via X11 GLX_OML_sync_control extensions
> glXSwapBuffersMscOML(..., target_msc, ...); call and/or via DRI3/Present
> PresentPixmap(..., target_msc, ...); request will complete one vblank
> too early whenever target_msc > current_msc + 1, ie. more than 1 vblank
> in the future. In such a case, the call of the pageflip ioctl() would be
> triggered by a queued drmWaitVblank() vblank event, which itself gets
> dispatched inside the vblank one frame before the target_msc vblank.
> 
> Testing with this patch does no longer show any problems with
> OML_sync_control swap scheduling or flip completion timestamps.
> Tested on R9 380 Tonga.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner...@gmail.com>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: Michel Dänzer <michel.daen...@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> index 086a842..19be2d9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> @@ -2460,6 +2460,9 @@ static void amdgpu_dm_do_flip(
>       struct amdgpu_device *adev = crtc->dev->dev_private;
>       bool async_flip = (acrtc->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) != 0;
>  
> +     /* Prepare wait for target vblank early - before the fence-waits */
> +     target_vblank = target - drm_crtc_vblank_count(crtc) +
> +                     amdgpu_get_vblank_counter_kms(crtc->dev, 
> acrtc->crtc_id);
>  
>       /*TODO This might fail and hence better not used, wait
>        * explicitly on fences instead
> @@ -2478,13 +2481,9 @@ static void amdgpu_dm_do_flip(
>  
>       amdgpu_bo_unreserve(abo);
>  
> -     /* Wait for target vblank */
>       /* Wait until we're out of the vertical blank period before the one
>        * targeted by the flip
>        */
> -     target_vblank = target - drm_crtc_vblank_count(crtc) +
> -                     amdgpu_get_vblank_counter_kms(crtc->dev, 
> acrtc->crtc_id);
> -
>       while ((acrtc->enabled &&
>               (amdgpu_get_crtc_scanoutpos(adev->ddev, acrtc->crtc_id, 0,
>                                       &vpos, &hpos, NULL, NULL,

Makes sense.


> @@ -2763,7 +2762,7 @@ void amdgpu_dm_atomic_commit_tail(
>                       amdgpu_dm_do_flip(
>                                       crtc,
>                                       fb,
> -                                     drm_crtc_vblank_count(crtc));
> +                                     drm_crtc_vblank_count(crtc) + 1);
>  
>                       wait_for_vblank =
>                                       acrtc->flip_flags & 
> DRM_MODE_PAGE_FLIP_ASYNC ?
> 

I suspect this code runs relatively late, so if userspace calls
DRM_IOCTL_MODE_PAGE_FLIP shortly before the start of vertical blank, it
could result in the flip being unnecessarily delayed by one frame. I
added drm_crtc_funcs::page_flip_target to address this.

Anyway, this is okay for now.

Reviewed-by: Michel Dänzer <michel.daen...@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to