Op 16-10-17 om 16:48 schreef Ville Syrjälä:
> 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.
plane_switching_crtc(), called from
drm_atomic_check_only->drm_atomic_plane_check().
So I wouldn't worry. :)
>> 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.
Ok new one it is, with that changed do I have your r-b?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
