[v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port

Commit Message

Imre Deak June 5, 2020, 9:16 a.m. UTC

Currently MST on a port can get enabled/disabled from the hotplug work
and get disabled from the short pulse work in a racy way. Fix this by
relying on the MST state checking in the hotplug work and just schedule
a hotplug work from the short pulse handler if some problem happened
during the MST interrupt handling.

This removes the explicit MST disabling in case of an AUX failure, but
if AUX fails, then probably the detection will also fail during the
scheduled hotplug work and it's not guaranteed that we'll see
intermittent errors anyway.

While at it also simplify the error checking of the MST interrupt
handler.

v2:
- Convert intel_dp_check_mst_status() to return bool. (Ville)
- Change the intel_dp->is_mst check to an assert, since after this patch
  the condition can't change after we checked it previously.
- Document the return value from intel_dp_check_mst_status().
v3:
- Remove the intel_dp->is_mst check from intel_dp_check_mst_status().
  There is no point in checking the same condition twice, even though
  there is a chance that the hotplug work running concurrently changes
  it.

Cc: José Roberto de Souza <[e-mail]>
Cc: Ville Syrjälä <[e-mail]>
Reviewed-by: José Roberto de Souza <[e-mail]> (v1)
Signed-off-by: Imre Deak <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 66 ++++++++++---------------
 1 file changed, 26 insertions(+), 40 deletions(-)

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7ef60af8308b..ade21157f29b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5556,14 +5556,24 @@  static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 			    "Could not write test response to sink\n");
 }
 
-static int
+/**
+ * intel_dp_check_mst_status - service any pending MST interrupts, check link status
+ * @intel_dp: Intel DP struct
+ *
+ * Read any pending MST interrupts, call MST core to handle these and ack the
+ * interrupts. Check if the main and AUX link state is ok.
+ *
+ * Returns:
+ * - %true if pending interrupts were serviced (or no interrupts were
+ *   pending) w/o detecting an error condition.
+ * - %false if an error condition - like AUX failure or a loss of link - is
+ *   detected, which needs servicing from the hotplug work.
+ */
+static bool
 intel_dp_check_mst_status(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
-	bool need_retrain = false;
-
-	if (!intel_dp->is_mst)
-		return -EINVAL;
+	bool link_ok = true;
 
 	drm_WARN_ON_ONCE(&i915->drm, intel_dp->active_mst_links < 0);
 
@@ -5591,22 +5601,23 @@  intel_dp_check_mst_status(struct intel_dp *intel_dp)
 
 	for (;;) {
 		u8 esi[DP_DPRX_ESI_LEN] = {};
-		bool bret, handled;
+		bool handled;
 		int retry;
 
-		bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
-		if (!bret) {
+		if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) {
 			drm_dbg_kms(&i915->drm,
 				    "failed to get ESI - device may have failed\n");
-			return -EINVAL;
+			link_ok = false;
+
+			break;
 		}
 
 		/* check link status - esi[10] = 0x200c */
-		if (intel_dp->active_mst_links > 0 && !need_retrain &&
+		if (intel_dp->active_mst_links > 0 && link_ok &&
 		    !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
 			drm_dbg_kms(&i915->drm,
 				    "channel EQ not ok, retraining\n");
-			need_retrain = true;
+			link_ok = false;
 		}
 
 		drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
@@ -5626,7 +5637,7 @@  intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		}
 	}
 
-	return need_retrain;
+	return link_ok;
 }
 
 static bool
@@ -7277,35 +7288,10 @@  intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (intel_dp->is_mst) {
-		switch (intel_dp_check_mst_status(intel_dp)) {
-		case -EINVAL:
-			/*
-			 * If we were in MST mode, and device is not
-			 * there, get out of MST mode
-			 */
-			drm_dbg_kms(&i915->drm,
-				    "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);
-
-			return IRQ_NONE;
-		case 1:
-			return IRQ_NONE;
-		default:
-			break;
-		}
-	}
-
-	if (!intel_dp->is_mst) {
-		bool handled;
-
-		handled = intel_dp_short_pulse(intel_dp);
-
-		if (!handled)
+		if (!intel_dp_check_mst_status(intel_dp))
 			return IRQ_NONE;
+	} else if (!intel_dp_short_pulse(intel_dp)) {
+		return IRQ_NONE;
 	}
 
 	return IRQ_HANDLED;