This workaround is not fixing the issue. Regards, Shirish S -----Original Message----- From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Harry Wentland Sent: Friday, September 28, 2018 6:46 PM To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; S, Shirish <Shirish.S@xxxxxxx>; Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx> Cc: Wentland, Harry <Harry.Wentland@xxxxxxx> Subject: [PATCH] drm/amd/display: Work around race beetween hw_done and wait_for_flip [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@xxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
