Op 22-11-18 om 15:34 schreef Ville Syrjala: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Consider the following scenario: > 1. nonblocking enable crtc > 2. wait for the event > 3. nonblocking disable crtc > > On i915 this can lead to a spurious -EBUSY from step 3 on > account of non-enabled planes getting the fake_commit in step 1 > and we don't complete the fake_commit-> flip_done until > drm_atomic_helper_commit_hw_done() which can happen a long > time after the flip event was sent out. > > This will become somewhat easy to hit on SKL+ once we start > to add all the planes for the crtc to every modeset commit > for the purposes of forcing a watermark register programming > [1]. > > To make the race a little less pronounced let's complete > fake_commit->flip_done after drm_atomic_helper_wait_for_flip_done(). > For the single crtc case this should make the race quite > theoretical, assuming drm_atomic_helper_wait_for_flip_done() > actually has to wait for the real commit flip_done. In case > the real commit flip_done gets completed singificantly before > drm_atomic_helper_wait_for_flip_done(), or we are dealing with > multiple crtcs whose vblanks don't line up nicely the race still > exists. > > [1] https://patchwork.freedesktop.org/patch/262670/ > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Fixes: 080de2e5be2d ("drm/atomic: Check for busy planes/connectors before setting the commit") > Testcase: igt/kms_cursor_legacy/*nonblocking-modeset-vs-cursor-atomic > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/drm_atomic_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index fa95f9974f6d..47398662b895 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1460,6 +1460,9 @@ void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, > DRM_ERROR("[CRTC:%d:%s] flip_done timed out\n", > crtc->base.id, crtc->name); > } > + > + if (old_state->fake_commit) > + complete_all(&old_state->fake_commit->flip_done); > } > EXPORT_SYMBOL(drm_atomic_helper_wait_for_flip_done); > Yeah as long as we don't enable hw_done sooner, this should be fine. For both patches: Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
