All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: ensure atomic messages consistently include the name of the component
@ 2017-02-13 12:27 Russell King
  2017-02-13 13:19 ` Thierry Reding
  2017-02-13 13:37 ` Maarten Lankhorst
  0 siblings, 2 replies; 5+ messages in thread
From: Russell King @ 2017-02-13 12:27 UTC (permalink / raw)
  To: dri-devel, Thierry Reding; +Cc: Daniel Vetter

Most DRM messages include three pieces of information: the type of the
component (CRTC, ENCODER, CONNECTOR etc), the DRM object ID of the
component, and the component name.  However, there are some messages
which omit the last piece of identifying information.  This makes it
harder to debug failures when these messages are printed, because the
DRM object ID doesn't supply enough information to know which piece of
hardware had a problem.

Update the atomic modeset code to always print the component name along
with the type and DRM object ID.

Fixes: 4cba68507cf5 ("drm/atomic-helper: Reject legacy flips on a disabled pipe")
Fixes: 8d4d0d700dda ("drm/atomic-helper: Print an error if vblank wait times out")
Fixes: 5481c8fb1da2 ("drm/atomic-helper: Check encoder/crtc constraints")
Fixes: 99cf4a29fa24 ("drm/atomic: Add current-mode blob to CRTC state")
Fixes: cc4ceb484b37 ("drm: Global atomic state handling")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/drm_atomic.c        |  9 +++++----
 drivers/gpu/drm/drm_atomic_helper.c | 12 +++++++-----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index fdfb1ec17e66..70d31f800410 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -628,8 +628,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
 	 * pipe.
 	 */
 	if (state->event && !state->active && !crtc->state->active) {
-		DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event but off\n",
-				 crtc->base.id);
+		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
+				 crtc->base.id, crtc->name);
 		return -EINVAL;
 	}
 
@@ -1039,8 +1039,9 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
 	state->connectors[index].ptr = connector;
 	connector_state->state = state;
 
-	DRM_DEBUG_ATOMIC("Added [CONNECTOR:%d] %p state to %p\n",
-			 connector->base.id, connector_state, state);
+	DRM_DEBUG_ATOMIC("Added [CONNECTOR:%d:%s] %p state to %p\n",
+			 connector->base.id, connector->name,
+			 connector_state, state);
 
 	if (connector_state->crtc) {
 		struct drm_crtc_state *crtc_state;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4594477dee00..fc4434fe28e8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -315,10 +315,11 @@ update_connector_routing(struct drm_atomic_state *state,
 	}
 
 	if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
-		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
+		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
 				 new_encoder->base.id,
 				 new_encoder->name,
-				 connector_state->crtc->base.id);
+				 connector_state->crtc->base.id,
+				 connector_state->crtc->name);
 		return -EINVAL;
 	}
 
@@ -1146,7 +1147,8 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 					drm_crtc_vblank_count(crtc),
 				msecs_to_jiffies(50));
 
-		WARN(!ret, "[CRTC:%d] vblank wait timed out\n", crtc->base.id);
+		WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n",
+		     crtc->base.id, crtc->name);
 
 		drm_crtc_vblank_put(crtc);
 	}
@@ -2783,8 +2785,8 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 	/* Make sure we don't accidentally do a full modeset. */
 	state->allow_modeset = false;
 	if (!crtc_state->active) {
-		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
-				 crtc->base.id);
+		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
+				 crtc->base.id, crtc->name);
 		ret = -EINVAL;
 		goto fail;
 	}
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm: ensure atomic messages consistently include the name of the component
  2017-02-13 12:27 [PATCH] drm: ensure atomic messages consistently include the name of the component Russell King
@ 2017-02-13 13:19 ` Thierry Reding
  2017-02-13 13:37 ` Maarten Lankhorst
  1 sibling, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2017-02-13 13:19 UTC (permalink / raw)
  To: Russell King; +Cc: Daniel Vetter, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1326 bytes --]

On Mon, Feb 13, 2017 at 12:27:03PM +0000, Russell King wrote:
> Most DRM messages include three pieces of information: the type of the
> component (CRTC, ENCODER, CONNECTOR etc), the DRM object ID of the
> component, and the component name.  However, there are some messages
> which omit the last piece of identifying information.  This makes it
> harder to debug failures when these messages are printed, because the
> DRM object ID doesn't supply enough information to know which piece of
> hardware had a problem.
> 
> Update the atomic modeset code to always print the component name along
> with the type and DRM object ID.
> 
> Fixes: 4cba68507cf5 ("drm/atomic-helper: Reject legacy flips on a disabled pipe")
> Fixes: 8d4d0d700dda ("drm/atomic-helper: Print an error if vblank wait times out")
> Fixes: 5481c8fb1da2 ("drm/atomic-helper: Check encoder/crtc constraints")
> Fixes: 99cf4a29fa24 ("drm/atomic: Add current-mode blob to CRTC state")
> Fixes: cc4ceb484b37 ("drm: Global atomic state handling")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/gpu/drm/drm_atomic.c        |  9 +++++----
>  drivers/gpu/drm/drm_atomic_helper.c | 12 +++++++-----
>  2 files changed, 12 insertions(+), 9 deletions(-)

Looks good:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm: ensure atomic messages consistently include the name of the component
  2017-02-13 12:27 [PATCH] drm: ensure atomic messages consistently include the name of the component Russell King
  2017-02-13 13:19 ` Thierry Reding
@ 2017-02-13 13:37 ` Maarten Lankhorst
  2017-02-13 16:47   ` Russell King - ARM Linux
  1 sibling, 1 reply; 5+ messages in thread
From: Maarten Lankhorst @ 2017-02-13 13:37 UTC (permalink / raw)
  To: Russell King, dri-devel, Thierry Reding; +Cc: Daniel Vetter

Op 13-02-17 om 13:27 schreef Russell King:
> Most DRM messages include three pieces of information: the type of the
> component (CRTC, ENCODER, CONNECTOR etc), the DRM object ID of the
> component, and the component name.  However, there are some messages
> which omit the last piece of identifying information.  This makes it
> harder to debug failures when these messages are printed, because the
> DRM object ID doesn't supply enough information to know which piece of
> hardware had a problem.
>
> Update the atomic modeset code to always print the component name along
> with the type and DRM object ID.
>
> Fixes: 4cba68507cf5 ("drm/atomic-helper: Reject legacy flips on a disabled pipe")
> Fixes: 8d4d0d700dda ("drm/atomic-helper: Print an error if vblank wait times out")
> Fixes: 5481c8fb1da2 ("drm/atomic-helper: Check encoder/crtc constraints")
> Fixes: 99cf4a29fa24 ("drm/atomic: Add current-mode blob to CRTC state")
> Fixes: cc4ceb484b37 ("drm: Global atomic state handling")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/gpu/drm/drm_atomic.c        |  9 +++++----
>  drivers/gpu/drm/drm_atomic_helper.c | 12 +++++++-----
>  2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index fdfb1ec17e66..70d31f800410 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -628,8 +628,8 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
>  	 * pipe.
>  	 */
>  	if (state->event && !state->active && !crtc->state->active) {
> -		DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event but off\n",
> -				 crtc->base.id);
> +		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requesting event but off\n",
> +				 crtc->base.id, crtc->name);
>  		return -EINVAL;
>  	}
>  
> @@ -1039,8 +1039,9 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  	state->connectors[index].ptr = connector;
>  	connector_state->state = state;
>  
> -	DRM_DEBUG_ATOMIC("Added [CONNECTOR:%d] %p state to %p\n",
> -			 connector->base.id, connector_state, state);
> +	DRM_DEBUG_ATOMIC("Added [CONNECTOR:%d:%s] %p state to %p\n",
> +			 connector->base.id, connector->name,
> +			 connector_state, state);
>  
>  	if (connector_state->crtc) {
>  		struct drm_crtc_state *crtc_state;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4594477dee00..fc4434fe28e8 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -315,10 +315,11 @@ update_connector_routing(struct drm_atomic_state *state,
>  	}
>  
>  	if (!drm_encoder_crtc_ok(new_encoder, connector_state->crtc)) {
> -		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d]\n",
> +		DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] incompatible with [CRTC:%d:%s]\n",
>  				 new_encoder->base.id,
>  				 new_encoder->name,
> -				 connector_state->crtc->base.id);
> +				 connector_state->crtc->base.id,
> +				 connector_state->crtc->name);
>  		return -EINVAL;
>  	}
>  
> @@ -1146,7 +1147,8 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>  					drm_crtc_vblank_count(crtc),
>  				msecs_to_jiffies(50));
>  
> -		WARN(!ret, "[CRTC:%d] vblank wait timed out\n", crtc->base.id);
> +		WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n",
> +		     crtc->base.id, crtc->name);
>  
>  		drm_crtc_vblank_put(crtc);
>  	}
> @@ -2783,8 +2785,8 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  	/* Make sure we don't accidentally do a full modeset. */
>  	state->allow_modeset = false;
>  	if (!crtc_state->active) {
> -		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> -				 crtc->base.id);
> +		DRM_DEBUG_ATOMIC("[CRTC:%d:%s] disabled, rejecting legacy flip\n",
> +				 crtc->base.id, crtc->name);
>  		ret = -EINVAL;
>  		goto fail;
>  	}

All for it, looks sane. The last hunk fails to apply because it's based on
an older version of page_flip, but easy enough to fix.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm: ensure atomic messages consistently include the name of the component
  2017-02-13 13:37 ` Maarten Lankhorst
@ 2017-02-13 16:47   ` Russell King - ARM Linux
  2017-02-14 20:18     ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2017-02-13 16:47 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: Daniel Vetter, dri-devel

On Mon, Feb 13, 2017 at 02:37:31PM +0100, Maarten Lankhorst wrote:
> All for it, looks sane. The last hunk fails to apply because it's based on
> an older version of page_flip, but easy enough to fix.

Yes, it's based on v4.10-rc7.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm: ensure atomic messages consistently include the name of the component
  2017-02-13 16:47   ` Russell King - ARM Linux
@ 2017-02-14 20:18     ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2017-02-14 20:18 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: Daniel Vetter, dri-devel

On Mon, Feb 13, 2017 at 04:47:01PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 13, 2017 at 02:37:31PM +0100, Maarten Lankhorst wrote:
> > All for it, looks sane. The last hunk fails to apply because it's based on
> > an older version of page_flip, but easy enough to fix.
> 
> Yes, it's based on v4.10-rc7.

Resolved conflict and merged for 4.12 into drm-misc-next.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-02-14 20:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-13 12:27 [PATCH] drm: ensure atomic messages consistently include the name of the component Russell King
2017-02-13 13:19 ` Thierry Reding
2017-02-13 13:37 ` Maarten Lankhorst
2017-02-13 16:47   ` Russell King - ARM Linux
2017-02-14 20:18     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.