Re: [PATCH 10/10] drm/i915: Leave FDI running after failed link training on LPT-H

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Dec 01, 2015 at 03:08:41PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we disable some parts of FDI setup after a failed link
> training. But despite that we continue with the modeset as if everything
> is fine. This results in tons of noise from the state checker, and
> it means we're not following the proper modeset sequence for the rest of
> crtc enabling, nor for crtc disabling.
> 
> Ideally we should abort the modeset and follow the proper disable
> suquenced to shut off everything we enabled so far, but that would
> require a big rework of the modeset code. So instead just leave FDI
> up and running in its untrained state, and log an error. This is
> what we do on older platforms too.

No, ideally we should light the pipe up and tell userspace through some
uevent that something bad has happened. Especially with async commits
failing to light up the pipe looks to userspace like the kernel just
randomly disabled it. And that usually results in random crashes when the
next flip or vblank wait fails and everything comes crashing down. And
even if we wouldn't -EINVAL these userspace would still have a problem
since the flip or vblank it wants to wait for simply never happens and
then it's just stalled.

We've had tons of problems with this in the past, and therefore removed
all the in-kernel sources for shutting down a pipe. E.g. DP re-training
also forces the pipe to on again, even if it fails.

Yup dp mst is an exception and I need to fix it eventually.

Cheers, Daniel
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 76ce7c2960b6..b312a47a0654 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -675,15 +675,16 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  		temp = I915_READ(DP_TP_STATUS(PORT_E));
>  		if (temp & DP_TP_STATUS_AUTOTRAIN_DONE) {
>  			DRM_DEBUG_KMS("FDI link training done on step %d\n", i);
> +			break;
> +		}
>  
> -			/* Enable normal pixel sending for FDI */
> -			I915_WRITE(DP_TP_CTL(PORT_E),
> -				   DP_TP_CTL_FDI_AUTOTRAIN |
> -				   DP_TP_CTL_LINK_TRAIN_NORMAL |
> -				   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> -				   DP_TP_CTL_ENABLE);
> -
> -			return;
> +		/*
> +		 * Leave things enabled even if we failed to train FDI.
> +		 * Results in less fireworks from the state checker.
> +		 */
> +		if (i == ARRAY_SIZE(hsw_ddi_translations_fdi) * 2 - 1) {
> +			DRM_ERROR("FDI link training failed!\n");
> +			break;
>  		}
>  
>  		temp = I915_READ(DDI_BUF_CTL(PORT_E));
> @@ -712,7 +713,12 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  		POSTING_READ(FDI_RX_MISC(PIPE_A));
>  	}
>  
> -	DRM_ERROR("FDI link training failed!\n");
> +	/* Enable normal pixel sending for FDI */
> +	I915_WRITE(DP_TP_CTL(PORT_E),
> +		   DP_TP_CTL_FDI_AUTOTRAIN |
> +		   DP_TP_CTL_LINK_TRAIN_NORMAL |
> +		   DP_TP_CTL_ENHANCED_FRAME_ENABLE |
> +		   DP_TP_CTL_ENABLE);
>  }
>  
>  void intel_ddi_init_dp_buf_reg(struct intel_encoder *encoder)
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx