[Intel-gfx] [PATCH] drm/i915: Retry reading DPCD when bogus values are read

[Takashi Iwai]

At Mon, 25 Nov 2013 16:47:25 +0100,
Daniel Vetter wrote:
> 
> On Fri, Nov 22, 2013 at 09:36:09AM +0100, Takashi Iwai wrote:
> > I got kernel WARNINGs frequently on Haswell laptops complaining about
> > invalid max DP link bw.  With drm.debug=0x0e, it turned out that the
> > obtained DPCD is utterly bogus when it happens:
> >   [drm:intel_dp_get_dpcd], DPCD: 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d 4d
> > 
> > This seems happening intermittently without plug state changes and
> > even after the first few reads succeeded.  As a workaround, add a
> > sanity check for such bogus values and retry reading if it hits.
> > 
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> 
> *insert lament about how hw never fails to disappoint*
> 
> A few questions:
> - Has anyone checked whether the same panel (I guess this is about edp)
>   also fails in the same way on other chips? Maybe with a dual gpu
>   solution?

I should have mentioned more clearly that this was about the real DP
monitor, not eDP.  I haven't seen this problem with eDP.

> - Jesse has a patch somewhere, please test the diff embedded in this bz
>   comment:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=67245#c29
> 
>   Could be that we have an issue with the panel power timings.

I guess this is only about eDP?


Takashi

> - Shouldn't we instead clamp the values read from dpcd to a sane range?
>   Worst case we'll do a few bogus link training rounds ...
> 
> Also we really should be extracting this stuff from drivers into common
> code. Somewhen next century maybe ;-)
> 
> Cheers, Daniel
> 
> > ---
> > 
> >  drivers/gpu/drm/i915/intel_dp.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index eb8139da9763..f596353da557 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2792,13 +2792,26 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_device *dev = dig_port->base.base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > +	int retry = 0;
> >  	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
> >  
> > + again:
> >  	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
> >  					   sizeof(intel_dp->dpcd)) == 0)
> >  		return false; /* aux transfer failed */
> >  
> > +	/* sanity check: sometimes DPCD contains a series of same bogus value
> > +	 * even though the above returned success.  Retry reading in that case.
> > +	 */
> > +	if (intel_dp->dpcd[0]) {
> > +		int i;
> > +		for (i = 1; i < sizeof(intel_dp->dpcd); i++)
> > +			if (intel_dp->dpcd[i] != intel_dp->dpcd[0])
> > +				break;
> > +		if (i >= sizeof(intel_dp->dpcd) && retry++ < 2)
> > +			goto again;
> > +	}
> > +
> >  	hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
> >  			   32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
> >  	DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
> > -- 
> > 1.8.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>