On Mon, Jan 02, 2017 at 02:09:13PM +0200, Laurent Pinchart wrote:
On Wednesday 08 Jun 2016 17:15:36 Daniel Vetter wrote:
+void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state) +{
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_crtc_commit *commit;
- int i;
- long ret;
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
commit = state->crtcs[i].commit;if (WARN_ON(!commit))continue;spin_lock(&crtc->commit_lock);complete_all(&commit->cleanup_done);WARN_ON(!try_wait_for_completion(&commit->hw_done));/* commit_list borrows our reference, need to remove before we* clean up our drm_atomic_state. But only after it actually* completed, otherwise subsequent commits won't stallproperly. */
if (try_wait_for_completion(&commit->flip_done)) {list_del(&commit->commit_entry);spin_unlock(&crtc->commit_lock);continue;}spin_unlock(&crtc->commit_lock);/* We must wait for the vblank event to signal our completion* before releasing our reference, since the vblank work does* not hold a reference of its own. */ret = wait_for_completion_timeout(&commit->flip_done,10*HZ);Why is this needed ? drm_atomic_helper_commit_cleanup_done() is called in commit_tail() after the call to drm_atomic_helper_commit_tail() or to the driver's .atomic_commit_tail() handler. If I'm not mistaken both already wait for the page flip to complete, either with a call to drm_atomic_helper_wait_for_vblanks() in drm_atomic_helper_commit_tail() or with a custom method in the driver's .atomic_commit_tail() handler.
You might still race with the event handling, and for legacy cursor updates we've ommitted the vblank waits. Anyway there's a patch from me on the m-l to switch over to refcounting, which makes this code here unecessary. I guess I should apply it to drm-misc. -Daniel