[PATCH] drm: ensure atomic messages consistently include the name of the component

[Maarten Lankhorst]

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 at 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 at linux.intel.com>