[PATCH] drm/atomic-helper: roll out commit synchronization

[Daniel Vetter]

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 stall 
> properly. */
> > +		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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch