[Intel-gfx] [PATCH v3] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect

[Ville Syrjälä]

On Tue, Jan 17, 2017 at 09:40:42AM -0800, Manasi Navare wrote:
> 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.

Why are you resetting the failure values in this function? You should
only do that when a long pulse is actually detected IMO (and yes, 
calling the function intel_dp_long_pulse() name is pretty much wrong
now).

> 
> 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 at intel.com>
> > 
> > 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 at vger.kernel.org
> > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > Cc: Ander Conselvande Oliveira <ander.conselvan.de.oliveira at intel.com>
> > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > Fixes: commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  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
> > 

-- 
Ville Syrjälä
Intel OTC