diff mbox

drm/i915: Cleaning up intel_dp_hpd_pulse

Message ID 1447742832-1303-1-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shubhangi Shrivastava Nov. 17, 2015, 6:47 a.m. UTC
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)

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Ander Conselvan de Oliveira Nov. 20, 2015, 2:03 p.m. UTC | #1
On Tue, 2015-11-17 at 12:17 +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)
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dd2b9da..e4d6d33 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4668,6 +4668,16 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>  		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;
>  	}
>  
>  	intel_dp_set_edid(intel_dp);
> @@ -4715,7 +4725,8 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  		return connector_status_disconnected;
>  	}
>  
> -	intel_dp_long_pulse(intel_dp->attached_connector);
> +	if (force)
> +		intel_dp_long_pulse(intel_dp->attached_connector);
>  
>  	if (intel_connector->detect_edid)
>  		return connector_status_connected;
> @@ -5045,21 +5056,9 @@ 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);
> +		goto put_power;

This has the problem with handling an mst device that is gone which Sivakumar
and I discussed on IRC.

I'm thinking whether it makes sense to move the code in the mst_fail label to 
intel_dp_detect(), since it gets called every time we reach that label. We would
just need a way to tell it about the failure, since we can't rely on connector
status being disconnected (because of MST). That could help us clean up
intel_dp_hpd_pulse even more.
Thoughts?

Ander

>  
> -		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)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dd2b9da..e4d6d33 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4668,6 +4668,16 @@  intel_dp_long_pulse(struct intel_connector *intel_connector)
 			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
 		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;
 	}
 
 	intel_dp_set_edid(intel_dp);
@@ -4715,7 +4725,8 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 		return connector_status_disconnected;
 	}
 
-	intel_dp_long_pulse(intel_dp->attached_connector);
+	if (force)
+		intel_dp_long_pulse(intel_dp->attached_connector);
 
 	if (intel_connector->detect_edid)
 		return connector_status_connected;
@@ -5045,21 +5056,9 @@  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);
+		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)