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

Reply via email to