On Tue, Dec 01, 2015 at 03:08:41PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx