[PATCH] drm/amd/display: Work around race beetween hw_done and wait_for_flip

[Harry Wentland <harry.wentland-5C7GfCeVMHo@public.gmane.org>]

[Why]
There is a race that between drm_crtc_commit_hw_done and
drm_atomic_helper_wait_for_flip where the possibilty exist for the
crtc->commit to be cleared after the null check in wait_for_flip_done
but before the call to wait_for_completion_timeout on commit->flip_done.

[How]
Take a reference to all commits in the state before
drm_crtc_commit_hw_done is called and release those after
drm_atomic_helper_wait_for_flip has finished.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---

Would something like this work? I get the strong sense that this happens
because Intel and IMX use the helpers in the other order, hence the
dependency between hw_done and wait_for_flip was missed.

I'd rather make it obvious that there's (a) no reason to reorder these
two calls on AMD HW (other than this unexpected dependency) and (b) this
is something we'll probably want to fix in DRM.

Sorry it took me a while to understand what was happening here. Been
busy at XDC until Jordan and Leo reminded me to take another look.

Harry

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++++++++++++++
 1 file changed, 15 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 0f10d920a785..ed9a7d680b63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4626,12 +4626,27 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 	}
 	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
 
+	/*
+	 * WORKAROUND: Take a ref for all crtc_state commits to avoid
+	 * a race where the commit gets freed before commit_hw_done had
+	 * a chance to look for commit->flip_done
+	 */
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
+		drm_crtc_commit_get(new_crtc_state->commit);
+
 	/* Signal HW programming completion */
 	drm_atomic_helper_commit_hw_done(state);
 
 	if (wait_for_vblank)
 		drm_atomic_helper_wait_for_flip_done(dev, state);
 
+	/*
+	 * WORKAROUND: put the commit refs from above (see comment on
+	 * the drm_crtc_commit_get call above)
+	 */
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
+		drm_crtc_commit_put(new_crtc_state->commit);
+
 	drm_atomic_helper_cleanup_planes(dev, state);
 
 	/*
-- 
2.17.1

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