[PATCH 1/2] drm/amd/display: Fix race between vblank irq and pageflip irq. (v2)

[PATCH 1/2] drm/amd/display: Fix race between vblank irq and pageflip irq. (v2)

[Mario Kleiner]

Since DC now uses CRTC_VERTICAL_INTERRUPT0 as VBLANK irq trigger
and vblank interrupts actually happen earliest at start of vblank,
instead of a bit before vblank, we no longer need some of the
fudging logic to deal with too early vblank irq handling (grep for
lb_vblank_lead_lines). This itself fixes a pageflip scheduling
bug in DC, caused by uninitialized  use of lb_vblank_lead_lines,
with a wrong startup value of 0. Thanks to the new vblank irq
trigger this value of zero is now actually correct for DC :).

A new problem is that vblank irq's race against pflip irq's,
and as both can fire at first line of vblank, it is no longer
guaranteed that vblank irq handling (therefore -> drm_handle_vblank()
-> drm_update_vblank_count()) executes before pflip irq handling
for a given vblank interval when a pageflip completes. Therefore
the vblank count and timestamps emitted to user-space as part of
the pageflip completion event will be often stale and cause new
timestamping and swap scheduling errors in user-space.

This was observed with large frequency on R9 380 Tonga Pro.

Fix this by enforcing a vblank count+timestamp update right
before emitting the pageflip completion event from the pflip
irq handler. The logic in core drm_update_vblank_count() makes
sure that no redundant or conflicting updates happen, iow. the
call turns into a no-op if it wasn't needed for that vblank,
burning a few microseconds of cpu time though.

Successfully tested on AMD R9 380 "Tonga Pro" (VI/DCE 10)
with DC enabled on the current DC staging branch. Independent
measurement of pageflip completion timing with special hardware
measurement equipment now confirms correct pageflip timestamps
and counts in the pageflip completion events.

v2: Review comments by Michel, drop outdated paragraph
    about problem already fixed in 2nd patch of the series.
    Add acked/r-b by Harry and Michel.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Acked-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

Cc: Andrey Grodzovsky <Andrey.Grodzovsky@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.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 794362e..0d77b0a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -208,6 +208,9 @@ static void dm_pflip_high_irq(void *interrupt_params)
 	if (amdgpu_crtc->event
 			&& amdgpu_crtc->event->event.base.type
 			== DRM_EVENT_FLIP_COMPLETE) {
+		/* Update to correct count/ts if racing with vblank irq */
+		drm_accurate_vblank_count(&amdgpu_crtc->base);
+
 		drm_crtc_send_vblank_event(&amdgpu_crtc->base, amdgpu_crtc->event);
 		/* page flip completed. clean up */
 		amdgpu_crtc->event = NULL;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/amd/display: Prevent premature pageflip when comitting in vblank. (v2)

[Mario Kleiner]

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.

v2: Add acked/r-b by Harry and Michel.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Acked-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.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,
@@ -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 ?
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx