[Intel-gfx] [PATCH 07/12] drm/i915: Move DP link retraining into intel_dp_detect()

ville.syrjala at linux.intel.com ville.syrjala at linux.intel.com
Thu Jul 28 14:50:43 UTC 2016


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

DP link retraining needs to grab some modeset locks to not race with
modesets, so we can't really do it safely from the hpd_pulse, lest we
risk deadlocking due to MST sideband stuff.

Move the link retraining to happen from the hotplug work instead.
Doing at the end of intel_dp_detect() seems like a good place in case
the sink already got disconnected, in which case retraining is
pointless.

To determine if we need to schedule the hotplug work, we'll just check
the sink lane status without locks from hpd_pulse. A little racy
still eg. due to useing intel_dp->lane_count, but no less racy than
what we already had. We'll repeat the check in from intel_dp_detect()
with proper locking, where we'll also check if the link as actually
active or not.

Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
Cc: Jim Bride <jim.bride at linux.intel.com>
Cc: Manasi D Navare <manasi.d.navare at intel.com>
Cc: Durgadoss R <durgadoss.r at intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 154 +++++++++++++++++-----------------------
 1 file changed, 66 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4a4184c21989..675b83f57a07 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3842,15 +3842,6 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
 go_again:
 		if (bret == true) {
-
-			/* check link status - esi[10] = 0x200c */
-			if (intel_dp->active_streams &&
-			    !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
-				DRM_DEBUG_KMS("channel EQ not ok, retraining\n");
-				intel_dp_start_link_train(intel_dp);
-				intel_dp_stop_link_train(intel_dp);
-			}
-
 			DRM_DEBUG_KMS("got esi %3ph\n", esi);
 			ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
 
@@ -3886,34 +3877,42 @@ go_again:
 	return -EINVAL;
 }
 
-static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+static bool
+intel_dp_link_needs_retrain(struct intel_dp *intel_dp)
 {
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+	if (intel_dp->active_streams == 0)
+		return false;
+
+	if (intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING)
+		return true;
 
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
 		DRM_ERROR("Failed to get link status\n");
-		return;
+		return false;
 	}
 
-	if (!intel_encoder->base.crtc)
-		return;
+	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
+}
 
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		return;
+static void
+intel_dp_link_retrain(struct intel_dp *intel_dp)
+{
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 
-	/* if link training is requested we should perform it always */
-	if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
-	    (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
+	if (intel_dp_link_needs_retrain(intel_dp)) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
+			      encoder->base.name);
+
 		intel_dp_start_link_train(intel_dp);
 		intel_dp_stop_link_train(intel_dp);
 	}
+
+	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 }
 
 /*
@@ -3932,7 +3931,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 sink_irq_vector;
 	u8 old_sink_count = intel_dp->sink_count;
 	bool ret;
@@ -3972,10 +3970,6 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-	intel_dp_check_link_status(intel_dp);
-	drm_modeset_unlock(&dev->mode_config.connection_mutex);
-
 	return true;
 }
 
@@ -3986,6 +3980,8 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	uint8_t *dpcd = intel_dp->dpcd;
 	uint8_t type;
 
+	WARN_ON(is_edp(intel_dp));
+
 	if (!intel_dp_get_dpcd(intel_dp))
 		return connector_status_disconnected;
 
@@ -4218,7 +4214,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
-static void
+static enum drm_connector_status
 intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
 	struct drm_connector *connector = &intel_connector->base;
@@ -4273,18 +4269,12 @@ 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;
 	}
 
+	/* connected->connected, nothing to do */
+	if (connector->status == connector_status_connected)
+		goto out;
+
 	/*
 	 * Clearing NACK and defer counts to get their exact values
 	 * while reading EDID which are required by Compliance tests
@@ -4296,7 +4286,6 @@ 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 &&
@@ -4313,43 +4302,30 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	}
 
 out:
-	if ((status != connector_status_connected) &&
-	    (intel_dp->is_mst == false))
+	if (status != connector_status_connected)
 		intel_dp_unset_edid(intel_dp);
 
 	intel_display_power_put(to_i915(dev), power_domain);
-	return;
+
+	return status;
 }
 
 static enum drm_connector_status
 intel_dp_detect(struct drm_connector *connector, bool force)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct intel_connector *intel_connector = to_intel_connector(connector);
+	enum drm_connector_status status;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 
-	if (intel_dp->is_mst) {
-		/* MST devices are disconnected from a monitor POV */
-		intel_dp_unset_edid(intel_dp);
-		if (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DP;
-		return connector_status_disconnected;
-	}
-
-	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done)
-		intel_dp_long_pulse(intel_dp->attached_connector);
+	status = intel_dp_long_pulse(intel_connector);
 
-	intel_dp->detect_done = false;
+	if (status == connector_status_connected || intel_dp->is_mst)
+		intel_dp_link_retrain(intel_dp);
 
-	if (is_edp(intel_dp) || intel_connector->detect_edid)
-		return connector_status_connected;
-	else
-		return connector_status_disconnected;
+	return status;
 }
 
 static void
@@ -4706,39 +4682,41 @@ 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)
+		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_short_pulse(intel_dp)) {
-				intel_dp_long_pulse(intel_dp->attached_connector);
-				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);
+			goto put_power;
 		}
+	} else {
+		if (!intel_dp_short_pulse(intel_dp))
+			goto put_power;
 	}
 
+	/*
+	 * Link retraining happens from the hotplug work,
+	 * check if we might need to schdule it.
+	 *
+	 * There has been known issues of link loss
+	 * triggerring long pulse, so let's check both
+	 * for short and long pulse.
+	 */
+	if (intel_dp_link_needs_retrain(intel_dp))
+		goto put_power;
+
 	ret = IRQ_HANDLED;
 
 put_power:
-- 
2.7.4



More information about the Intel-gfx mailing list