Op 31-10-2019 om 23:36 schreef Rob Clark:
From: Rob Clark robdclark@chromium.org
Stalling on cleanup_done ensures that any atomic state related to a nonblock commit no longer has dangling references to per-object state that can be freed.
Otherwise, if a !nonblock commit completes after a nonblock commit has swapped state (ie. the synchronous part of the nonblock commit comes before the !nonblock commit), but before the asynchronous part of the nonblock commit completes, what was the new per-object state in the nonblock commit can be freed.
This shows up with the new self-refresh helper, as _update_avg_times() dereferences the original old and new crtc_state.
Fixes: d4da4e33341c ("drm: Measure Self Refresh Entry/Exit times to avoid thrashing") Cc: Sean Paul seanpaul@chromium.org Signed-off-by: Rob Clark robdclark@chromium.org
Other possibilities:
- maybe block later before freeing atomic state?
- refcount individual per-object state
drivers/gpu/drm/drm_atomic_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 3ef2ac52ce94..a5d95429f91b 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2711,7 +2711,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, if (!commit) continue;
ret = wait_for_completion_interruptible(&commit->hw_done);
ret = wait_for_completion_interruptible(&commit->cleanup_done); if (ret) return ret;@@ -2722,7 +2722,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, if (!commit) continue;
ret = wait_for_completion_interruptible(&commit->hw_done);
ret = wait_for_completion_interruptible(&commit->cleanup_done); if (ret) return ret;@@ -2733,7 +2733,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state, if (!commit) continue;
ret = wait_for_completion_interruptible(&commit->hw_done);
ret = wait_for_completion_interruptible(&commit->cleanup_done); if (ret) return ret;
From setup_commit():
* Completion of the hardware commit step must be signalled using * drm_atomic_helper_commit_hw_done(). After this step the driver is not allowed * to read or change any permanent software or hardware modeset state. The only * exception is state protected by other means than &drm_modeset_lock locks. * Only the free standing @state with pointers to the old state structures can * be inspected, e.g. to clean up old buffers using * drm_atomic_helper_cleanup_planes().
And the hw_done function says pretty much the same thing. :)