On Mon, Oct 16, 2017 at 03:59:38PM +0200, Maarten Lankhorst wrote:
> Op 16-10-17 om 15:42 schreef Ville Syrjälä:
> > On Mon, Oct 16, 2017 at 03:29:27PM +0200, Maarten Lankhorst wrote:
> >> Commit 669c9215afea ("drm/atomic: Make async plane update checks work as
> >> intended, v2.") forced planes to always be tracked, but forgot to
> >> explicitly get the crtc commit from the new crtc when available.
> >>
> >> This broke plane commit tracking, and caused kms_atomic_transitions
> >> to randomly fail with -EBUSY.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> >> Fixes: 669c9215afea ("drm/atomic: Make async plane update checks work as 
> >> intended, v2.")
> >> Cc: Gustavo Padovan <gustavo.pado...@collabora.com>
> >> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102671
> >> Testcase: kms_atomic_transitions
> >> ---
> >>  drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> >> b/drivers/gpu/drm/drm_atomic_helper.c
> >> index d59441f1dcd4..b64c8f5bc940 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -1804,7 +1804,7 @@ int drm_atomic_helper_setup_commit(struct 
> >> drm_atomic_state *state,
> >>                
> >> !try_wait_for_completion(&old_plane_state->commit->flip_done))
> >>                    return -EBUSY;
> >>  
> >> -          commit = crtc_or_fake_commit(state, old_plane_state->crtc);
> >> +          commit = crtc_or_fake_commit(state, old_plane_state->crtc ?: 
> >> new_plane_state->crtc);
> > Shouldn't old vs. new state be the other way around?
> Hmm to be honest, could be. We don't allow crtc's to switch planes directly. 
> So in practice it doesn't matter.

Not sure where we actually prevent that. A quick trawl through the code
didn't reveal anything like that.

> But if we ever did allow moving crtc's, it's up for debate what crtc we want 
> to use here..

new is the one it'd be hanging off at the end so that seems like the
right choice. It would also match what we do in i915 code.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to