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

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

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>

> 
> 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;
>  		}
>  	}
>