diff mbox series

[4/5] drm/i915: Provide more information on DP AUX failures

Message ID 20191025230623.27829-5-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series DP AUX updates | expand

Commit Message

Matt Roper Oct. 25, 2019, 11:06 p.m. UTC
We're seeing some failures where an aux transaction still shows as
'busy' well after the timeout limit that the hardware is supposed to
enforce.  Improve the error message so that we can see exactly which aux
channel this error happened on and what the status bits were during this
case that isn't supposed to happen.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Matt Roper Oct. 25, 2019, 11:25 p.m. UTC | #1
On Fri, Oct 25, 2019 at 04:19:33PM -0700, Lucas De Marchi wrote:
> On Fri, Oct 25, 2019 at 04:06:22PM -0700, Matt Roper wrote:
> > We're seeing some failures where an aux transaction still shows as
> > 'busy' well after the timeout limit that the hardware is supposed to
> > enforce.  Improve the error message so that we can see exactly which aux
> > channel this error happened on and what the status bits were during this
> > case that isn't supposed to happen.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 65bab46f7b43..2b4915b5cf52 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1190,7 +1190,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> > 
> > 	if (!done)
> > -		DRM_ERROR("dp aux hw did not signal timeout!\n");
> > +		DRM_ERROR("%s did not complete or timeout within 10ms (status 0x%08x)\n",
> > +			  intel_dp->aux.name ?: "AUX", status);
> 
> maybe a "const timeout_msec = 10" and use it both here and above to
> avoid mismatch in future? However I'm not sure it's worth... up to you.
> 
> intel_dp_aux_init set aux.name to kasprintf() and we can't possibly
> initiate aux transactions before that init.
> intel_dp_connector_register() also doesn't handle aux.name == NULL, so
> whay are we checking it heere?

kasprintf() could technically fail to allocate the string and leave the
name as NULL.  But I guess if that happens we've probably got bigger
problems anyway.


Matt


> 
> Lucas De Marchi
> 
> > #undef C
> > 
> > 	return status;
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Oct. 28, 2019, 4:43 p.m. UTC | #2
On Fri, Oct 25, 2019 at 04:06:22PM -0700, Matt Roper wrote:
> We're seeing some failures where an aux transaction still shows as
> 'busy' well after the timeout limit that the hardware is supposed to
> enforce.  Improve the error message so that we can see exactly which aux
> channel this error happened on and what the status bits were during this
> case that isn't supposed to happen.

Pretty sure I have a patch somewhere that adds the aux name
to all the messages. I should probably dig that up and post it.

> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 65bab46f7b43..2b4915b5cf52 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1190,7 +1190,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>  	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>  
>  	if (!done)
> -		DRM_ERROR("dp aux hw did not signal timeout!\n");
> +		DRM_ERROR("%s did not complete or timeout within 10ms (status 0x%08x)\n",
> +			  intel_dp->aux.name ?: "AUX", status);
>  #undef C
>  
>  	return status;
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 65bab46f7b43..2b4915b5cf52 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1190,7 +1190,8 @@  intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
 
 	if (!done)
-		DRM_ERROR("dp aux hw did not signal timeout!\n");
+		DRM_ERROR("%s did not complete or timeout within 10ms (status 0x%08x)\n",
+			  intel_dp->aux.name ?: "AUX", status);
 #undef C
 
 	return status;