[Intel-gfx] [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()

[Ville Syrjälä]

On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote:
> On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan wrote:
> > A crtc modeset lock was added for link retraining but
> > intel_dp_retrain_link() knows to take the necessary locks since
> > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the
> > ->post_hotplug() hook")
> > v2: Drop AUX power domain reference in the early return path
> > 
> > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")
> 
> This patch failed on cherry-pick for drm-intel-fixes targeting 4.19.
> 
> If it still makes sense to have it there please provide a backported version
> to drm-intel-fixes or let me know if it has dependency on other patches?

This is more of an optimization to not disturb page flips and whatnot
so much with detect. The double lock is otherwise harmless due to ww_mutex.

> 
> Thanks,
> Rodrigo.
> 
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 09229fc66dec..ef28cc0f122c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector,
> >  	 * Some external monitors do not signal loss of link synchronization
> >  	 * with an IRQ_HPD, so force a link status check.
> >  	 */
> > -	if (!intel_dp_is_edp(intel_dp))
> > -		intel_dp_retrain_link(encoder, ctx);
> > +	if (!intel_dp_is_edp(intel_dp)) {
> > +		int ret;
> > +
> > +		ret = intel_dp_retrain_link(encoder, ctx);
> > +		if (ret) {
> > +			intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > +			return ret;
> > +		}
> > +	}
> >  
> >  	/*
> >  	 * Clearing NACK and defer counts to get their exact values
> > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector,
> >  		      connector->base.id, connector->name);
> >  
> >  	/* If full detect is not performed yet, do a full detect */
> > -	if (!intel_dp->detect_done) {
> > -		struct drm_crtc *crtc;
> > -		int ret;
> > -
> > -		crtc = connector->state->crtc;
> > -		if (crtc) {
> > -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> > -			if (ret)
> > -				return ret;
> > -		}
> > -
> > +	if (!intel_dp->detect_done)
> >  		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
> > -	}
> >  
> >  	intel_dp->detect_done = false;
> >  
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel