Re: [PATCH 1/2] drm/i915: Move long hpd handling into the hotplug work

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 03, 2016 at 02:39:16PM +0300, Ander Conselvan De Oliveira wrote:
> On Mon, 2016-10-03 at 10:55 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > 
> > We can't rely on connector->status in the detect() hook if the long hpd
> > was already handled by the dig_port_work as that won't update
> > connector->status. Thus we have to defer the long hpd handling entirely
> > until the hotplug work runs to avoid the double long hpd handling
> > the "detect_done" flag is trying to prevent.
> 
> This is better indeed. But perhaps add a note here about the next patch, since
> this one doesn't actually change the use of connector->status usage in detect().
> 
> For both patches:
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Thanks. I added a small note to 1/2 and pushed both patches to dinq.

> 
> > 
> > Cc: Damien Cassou <damien@cassou.me>
> > Cc: freedesktop.org@gp.mailgun.org
> > Cc: Arno <blouin.arno@gmail.com>
> > Cc: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> > Cc: stable@vger.kernel.org
> > Tested-by: Arno <blouin.arno@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83348
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> > �drivers/gpu/drm/i915/intel_dp.c | 48 ++++++++++++++++++++------------------
> > ---
> > �1 file changed, 23 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 9448d898d80b..96caa469e3a8 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4827,36 +4827,34 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > *intel_dig_port, bool long_hpd)
> > �		������port_name(intel_dig_port->port),
> > �		������long_hpd ? "long" : "short");
> > �
> > +	if (long_hpd) {
> > +		intel_dp->detect_done = false;
> > +		return IRQ_NONE;
> > +	}
> > +
> > �	power_domain = intel_display_port_aux_power_domain(intel_encoder);
> > �	intel_display_power_get(dev_priv, power_domain);
> > �
> > -	if (long_hpd) {
> > -		intel_dp_long_pulse(intel_dp->attached_connector);
> > -		if (intel_dp->is_mst)
> > -			ret = IRQ_HANDLED;
> > -		goto put_power;
> > -
> > -	} else {
> > -		if (intel_dp->is_mst) {
> > -			if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> > -				/*
> > -				�* If we were in MST mode, and device is not
> > -				�* there, get out of MST mode
> > -				�*/
> > -				DRM_DEBUG_KMS("MST device may have
> > disappeared %d vs %d\n",
> > -					������intel_dp->is_mst, intel_dp-
> > >mst_mgr.mst_state);
> > -				intel_dp->is_mst = false;
> > -				drm_dp_mst_topology_mgr_set_mst(&intel_dp-
> > >mst_mgr,
> > -								intel_dp-
> > >is_mst);
> > -				goto put_power;
> > -			}
> > +	if (intel_dp->is_mst) {
> > +		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> > +			/*
> > +			�* If we were in MST mode, and device is not
> > +			�* there, get out of MST mode
> > +			�*/
> > +			DRM_DEBUG_KMS("MST device may have disappeared %d vs
> > %d\n",
> > +				������intel_dp->is_mst, intel_dp-
> > >mst_mgr.mst_state);
> > +			intel_dp->is_mst = false;
> > +			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> > +							intel_dp->is_mst);
> > +			intel_dp->detect_done = false;
> > +			goto put_power;
> > �		}
> > +	}
> > �
> > -		if (!intel_dp->is_mst) {
> > -			if (!intel_dp_short_pulse(intel_dp)) {
> > -				intel_dp_long_pulse(intel_dp-
> > >attached_connector);
> > -				goto put_power;
> > -			}
> > +	if (!intel_dp->is_mst) {
> > +		if (!intel_dp_short_pulse(intel_dp)) {
> > +			intel_dp->detect_done = false;
> > +			goto put_power;
> > �		}
> > �	}
> > �

-- 
Ville Syrj�l�
Intel OTC