[Intel-gfx] [PATCH 2/5] drm/i915: Cleaning up intel_dp_hpd_pulse

[Ander Conselvan De Oliveira]

On Wed, 2016-02-10 at 14:34 +0530, Shubhangi Shrivastava wrote:
> Current DP detection has DPCD operations split across
> intel_dp_hpd_pulse and intel_dp_detect which contains
> duplicates as well. Also intel_dp_detect is called
> during modes enumeration as well which will result
> in multiple dpcd operations. So this patch tries
> to solve both these by bringing all DPCD operations
> in one single function and make intel_dp_detect
> use existing values instead of repeating same steps.
> 
> v2: Pulled in a hunk from last patch of the series to
>     this patch. (Ander)
> v3: Added MST hotplug handling. (Ander)

The change log for this version of the patch is missing. With that, this is

Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>

> 
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu at intel.com>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani at intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 72 +++++++++++++++++++++++++--------------
> -
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 042283a..ad5ec3b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4653,6 +4653,16 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else if (connector->status == connector_status_connected) {
> +		/*
> +		 * If display was connected already and is still connected
> +		 * check links status, there has been known issues of
> +		 * link loss triggerring long pulse!!!!
> +		 */
> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +		intel_dp_check_link_status(intel_dp);
> +		drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +		goto out;
>  	}
>  
>  	/*
> @@ -4666,6 +4676,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  	intel_dp_set_edid(intel_dp);
>  
>  	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 &&
> @@ -4682,8 +4693,21 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  	}
>  
>  out:
> -	if (status != connector_status_connected)
> +	if (status != connector_status_connected) {
>  		intel_dp_unset_edid(intel_dp);
> +		/*
> +		 * If we were in MST mode, and device is not there,
> +		 * get out of MST mode
> +		 */
> +		if (intel_dp->is_mst) {
> +			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_display_power_put(to_i915(dev), power_domain);
>  	return;
>  }
> @@ -4707,7 +4731,11 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  		return connector_status_disconnected;
>  	}
>  
> -	intel_dp_long_pulse(intel_dp->attached_connector);
> +	/* If full detect is not performed yet, do a full detect */
> +	if (!intel_dp->detect_done)
> +		intel_dp_long_pulse(intel_dp->attached_connector);
> +
> +	intel_dp->detect_done = false;
>  
>  	if (intel_connector->detect_edid)
>  		return connector_status_connected;
> @@ -5040,25 +5068,25 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>  		/* indicate that we need to restart link training */
>  		intel_dp->train_set_valid = false;
>  
> -		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
> -			goto mst_fail;
> -
> -		if (!intel_dp_get_dpcd(intel_dp)) {
> -			goto mst_fail;
> -		}
> -
> -		intel_dp_probe_oui(intel_dp);
> +		intel_dp_long_pulse(intel_dp->attached_connector);
> +		if (intel_dp->is_mst)
> +			ret = IRQ_HANDLED;
> +		goto put_power;
>  
> -		if (!intel_dp_probe_mst(intel_dp)) {
> -			drm_modeset_lock(&dev->mode_config.connection_mutex,
> NULL);
> -			intel_dp_check_link_status(intel_dp);
> -			drm_modeset_unlock(&dev
> ->mode_config.connection_mutex);
> -			goto mst_fail;
> -		}
>  	} else {
>  		if (intel_dp->is_mst) {
> -			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> -				goto mst_fail;
> +			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) {
> @@ -5070,14 +5098,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>  
>  	ret = IRQ_HANDLED;
>  
> -	goto put_power;
> -mst_fail:
> -	/* if we were in MST mode, and device is not there get out of MST
> mode */
> -	if (intel_dp->is_mst) {
> -		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);
> -	}
>  put_power:
>  	intel_display_power_put(dev_priv, power_domain);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 878172a..3d003d6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -753,6 +753,7 @@ struct intel_dp {
>  	int link_rate;
>  	uint8_t lane_count;
>  	bool has_audio;
> +	bool detect_done;
>  	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;
>  	bool color_range_auto;