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

[Daniel Vetter]

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?

- 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.

- 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