I have verified this patch with the latest drm-tip and it is also absolutely necessary to ensure the link gets properly retrained after link-status is BAD and after anew modeset is triggered by userspace. Without this patch, since intel_dp_long_pulse gets called it resets the link failure values and link retraining does not happen at lower link rates. Ville/Jani could you please review this patch? Thi is definitely a bug in the existing codebase and we need to get this merged soon to get it fixed. Regards Manasi On Thu, Dec 29, 2016 at 11:14:28PM -0800, Dhinakaran Pandiyan wrote: > From: "Navare, Manasi D" <manasi.d.navare@xxxxxxxxx> > > The detect_done flag was introduced in the 'commit 7d23e3c37bb3 > ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple > detects on hotplug where intel_dp_long_pulse() was called from HPD handler > as well as intel_dp_detect(). Later, 'commit 1015811609c0 > ("drm/i915: Move long hpd handling into the hotplug work")' deferred long > hpd handling to hotplug work to avoid handling it twice. But, resetting the > flag after long hpd handling leads to the code being executed again during > mode enumeration. > > So, do not reset the detect_done flag to False in intel_dp_detect(). The > flag is reset in intel_dp_hpd_pulse() to allow a full detect and set when > the hotplug work does a full DPCD detect. However if ->detect() gets called > during mode enumeration after a DPCD detect, return the cached connector > status. > > Resetting the flag in the encoder's reset callback should take care of > hotplug between suspend/resume. > > v2: > Allow full detect after encoder reset. (Ville) > Set the detect_done flag for connector disconnected case too. (DK) > Commit message changes. > > Cc: stable@xxxxxxxxxxxxxxx > Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Ander Conselvande Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Fixes: commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index fb12896..6732c17 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4516,7 +4516,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > intel_dp_set_edid(intel_dp); > if (is_edp(intel_dp) || intel_connector->detect_edid) > status = connector_status_connected; > - intel_dp->detect_done = true; > > /* Try to read the source of the interrupt */ > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > @@ -4551,10 +4550,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) > connector->base.id, connector->name); > > /* If full detect is not performed yet, do a full detect */ > - if (!intel_dp->detect_done) > + if (!intel_dp->detect_done) { > + intel_dp->detect_done = true; > status = intel_dp_long_pulse(intel_dp->attached_connector); > - > - intel_dp->detect_done = false; > + } > > return status; > } > @@ -4859,6 +4858,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) > if (lspcon->active) > lspcon_resume(lspcon); > > + intel_dp->detect_done = false; > + > pps_lock(intel_dp); > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html