| Message ID | 1492791786-24543-3-git-send-email-mario.kleiner.de@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.de@gmail.com> > Cc: Harry Wentland <Harry.Wentland@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Michel Dänzer <michel.daenzer@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.daenzer@amd.com>
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, @@ -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 ?
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.de@gmail.com> Cc: Harry Wentland <Harry.Wentland@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Michel Dänzer <michel.daenzer@amd.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)