diff mbox series

[2/2] drm/atomic: debug output for EBUSY

Message ID 20200923105737.2943649-2-daniel.vetter@ffwll.ch
State New, archived
Headers show
Series [1/2] drm/atomic: document and enforce rules around "spurious" EBUSY | expand

Commit Message

Daniel Vetter Sept. 23, 2020, 10:57 a.m. UTC
Hopefully we'll have the drm crash recorder RSN, but meanwhile
compositors would like to know a bit better why they get an EBUSY.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: Simon Ser <contact@emersion.fr>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  4 ++--
 drivers/gpu/drm/drm_atomic_helper.c | 20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

Comments

Pekka Paalanen Sept. 25, 2020, 8:27 a.m. UTC | #1
On Wed, 23 Sep 2020 12:57:37 +0200
Daniel Vetter <daniel.vetter@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
diff mbox series

Patch

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);
 
 	/*
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);