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

From: Matt Roper <matthew.d.roper@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/5] drm/i915: Provide more information on DP AUX failures
Date: Fri, 25 Oct 2019 16:25:47 -0700	[thread overview]
Message-ID: <20191025232547.GG18842@mdroper-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <20191025231933.xfjp6ut4p364ngjx@ldmartin-desk1.jf.intel.com>

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

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx