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

[Takashi Iwai]

At Wed, 04 Dec 2013 12:17:03 +0200,
Jani Nikula wrote:
> 
> On Fri, 22 Nov 2013, Takashi Iwai <tiwai at suse.de> 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.
> 
> If it's not related to crtc disable/enable state changes, then I assume
> this might happen with any native aux reads at any time, and your patch
> would just paper over one instance of it.

Yes, exactly, that's the purpose :)

> Does this occur with several different displays? With the displays this
> occurs with, does it also occur on non-Haswell?

I haven't seen on IvyBridge machines, at least.
And I have no other DP monitor, so cannot test right now.

> Have you observed that you'd ever need to try again more than once?
> I.e. does the first retry sometimes return garbage too?

I don't remember whether more than once were needed.


Takashi

> Instinctively I'd say we need to resist the urge to apply this
> workaround, and try to find the root cause.
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >
> >  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
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
>