On Wed, 23 Sep 2020 12:57:37 +0200
Daniel Vetter <daniel.vet...@ffwll.ch> wrote:

> Hopefully we'll have the drm crash recorder RSN, but meanwhile
> compositors would like to know a bit better why they get an EBUSY.
> 

These debug messages will be especially useful with the flight
recorder, but also without. :-)

...

> ---
>  drivers/gpu/drm/drm_atomic.c        |  4 ++--
>  drivers/gpu/drm/drm_atomic_helper.c | 20 +++++++++++++++++---
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index e22669b64521..6864e520269d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1272,7 +1272,7 @@ int drm_atomic_check_only(struct drm_atomic_state 
> *state)
>  
>       DRM_DEBUG_ATOMIC("checking %p\n", state);
>  
> -     for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> +     for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
>               requested_crtc |= drm_crtc_mask(crtc);
>  
>       for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
> new_plane_state, i) {
> @@ -1322,7 +1322,7 @@ int drm_atomic_check_only(struct drm_atomic_state 
> *state)
>               }
>       }
>  
> -     for_each_new_crtc_in_state(state, crtc, old_crtc_state, i)
> +     for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
>               affected_crtc |= drm_crtc_mask(crtc);

Oops, these belong in the previous patch?

>  
>       /*
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index e8abaaaa7fd1..6b3bfabac26c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1740,8 +1740,11 @@ int drm_atomic_helper_async_check(struct drm_device 
> *dev,
>        * overridden by a previous synchronous update's state.
>        */
>       if (old_plane_state->commit &&
> -         !try_wait_for_completion(&old_plane_state->commit->hw_done))
> +         !try_wait_for_completion(&old_plane_state->commit->hw_done)) {
> +             DRM_DEBUG_ATOMIC("[PLANE:%d:%s] inflight previous commit 
> preventing async commit\n",
> +                     plane->base.id, plane->name);
>               return -EBUSY;
> +     }
>  
>       return funcs->atomic_async_check(plane, new_plane_state);
>  }
> @@ -1964,6 +1967,9 @@ static int stall_checks(struct drm_crtc *crtc, bool 
> nonblock)
>                        * commit with nonblocking ones. */
>                       if (!completed && nonblock) {
>                               spin_unlock(&crtc->commit_lock);
> +                             DRM_DEBUG_ATOMIC("[CRTC:%d:%s] busy with a 
> previous commit\n",
> +                                     crtc->base.id, crtc->name);
> +
>                               return -EBUSY;
>                       }
>               } else if (i == 1) {
> @@ -2132,8 +2138,12 @@ int drm_atomic_helper_setup_commit(struct 
> drm_atomic_state *state,
>               /* Userspace is not allowed to get ahead of the previous
>                * commit with nonblocking ones. */
>               if (nonblock && old_conn_state->commit &&
> -                 
> !try_wait_for_completion(&old_conn_state->commit->flip_done))
> +                 
> !try_wait_for_completion(&old_conn_state->commit->flip_done)) {
> +                     DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] busy with a 
> previous commit\n",
> +                             conn->base.id, conn->name);
> +
>                       return -EBUSY;
> +             }
>  
>               /* Always track connectors explicitly for e.g. link retraining. 
> */
>               commit = crtc_or_fake_commit(state, new_conn_state->crtc ?: 
> old_conn_state->crtc);
> @@ -2147,8 +2157,12 @@ int drm_atomic_helper_setup_commit(struct 
> drm_atomic_state *state,
>               /* Userspace is not allowed to get ahead of the previous
>                * commit with nonblocking ones. */
>               if (nonblock && old_plane_state->commit &&
> -                 
> !try_wait_for_completion(&old_plane_state->commit->flip_done))
> +                 
> !try_wait_for_completion(&old_plane_state->commit->flip_done)) {
> +                     DRM_DEBUG_ATOMIC("[PLANE:%d:%s] busy with a previous 
> commit\n",
> +                             plane->base.id, plane->name);
> +
>                       return -EBUSY;
> +             }
>  
>               /* Always track planes explicitly for async pageflip support. */
>               commit = crtc_or_fake_commit(state, new_plane_state->crtc ?: 
> old_plane_state->crtc);

The new debug messages sound good to me.


Thanks,
pq

Attachment: pgphIBH1_gBHb.pgp
Description: OpenPGP digital signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to