Re: [PATCH] drm/i915: Retry reading DPCD when bogus values are read

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 22 Nov 2013, Takashi Iwai <tiwai@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.

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

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

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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center