[PATCH 024/108] drm/i915/dp_link_training: Disallow autoretrains after failed modeset

[Imre Deak <imre.deak@intel.com>]

intel_dp_check_link_state() and intel_dp_link_params_valid() perform
only a coarse validation of the link configuration used by the active
mode against the available configurations (as constrained by the link
training fallback code after a previous LT failure). Even if these
coarse checks find a seemingly usable configuration, the modeset check,
which performs full verification, may still fail.

Disallow further autoretrain attempts if an autoretrain modeset fails.
Further attempts would just reuse the same modeset parameters and fail
in the same way. Autoretrain will be reallowed unconditionally when the
sink reports a change in its capabilities. This allows an autoretrain to
proceed once both the link validation and modeset checks confirm a
usable configuration.

Also clarify in intel_dp_check_link_state() and
intel_dp_link_params_valid() that these checks are coarse and that a
full validation is only performed by the subsequent atomic modeset
check.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_dp_link_training.c | 98 ++++++++++++++++++-
 1 file changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index b446a3523b94c..37862b915cf6e 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -99,6 +99,7 @@
  *   - link_recovery_autoretrain_allowed()
  *   - link_recovery_has_no_fallback()
  *   - link_recovery_mark_train_failure()
+ *   - link_recovery_mark_autoretrain_modeset_failure()
  *   - link_recovery_mark_no_fallback()
  *   - link_recovery_reset()
  */
@@ -1354,6 +1355,13 @@ link_recovery_has_no_fallback(struct intel_dp_link_training *link_training)
  * is no longer possible, via userspace modesets after fallback
  * selection.
  *
+ * Note that the error reported via this function is the error seen by
+ * the link training failure handler proper after an actual link
+ * training failure indicated by the sink device, and so the error and
+ * corresponding actions required are distinct from an autoretrain
+ * modeset failure. See link_recovery_mark_autoretrain_modeset_failure() to
+ * report a modeset failure.
+ *
  * Return:
  * - %true  if recovery should continue via automatic retraining.
  * - %false if automatic retraining is no longer possible and recovery
@@ -1370,7 +1378,30 @@ link_recovery_mark_train_failure(struct intel_dp_link_training *link_training)
 }
 
 /**
- * link_recovery_mark_no_fallback - record that fallback is unavailable
+ * link_recovery_mark_autoretrain_modeset_failure - record an autoretrain modeset failure
+ * @link_training: link training state
+ *
+ * Record a failure of the autoretrain modeset before link training
+ * itself could run.
+ *
+ * Note that the error reported via this function and the corresponding
+ * expected actions are distinct from an actual link training failure:
+ * the modeset failed before a link training attempt could be performed.
+ * See link_recovery_mark_train_failure() to report an actual link
+ * training failure.
+ *
+ * Update the state to indicate that further recovery is to be delegated to
+ * userspace via a regular modeset.
+ */
+static void
+link_recovery_mark_autoretrain_modeset_failure(struct intel_dp_link_training *link_training)
+{
+	if (link_recovery_autoretrain_allowed(link_training))
+		link_training->recovery_state = INTEL_DP_LINK_RECOVERY_AUTORETRAIN_DISABLED;
+}
+
+/**
+ * link_recovery_mark_no_fallback - record that no fallback is possible
  * @link_training: link training state
  *
  * Record that no more link fallback configuration is available.
@@ -2021,6 +2052,23 @@ bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
 	 * FIXME: we need to synchronize the current link parameters with
 	 * hardware readout. Currently fast link training doesn't work on
 	 * boot-up.
+	 *
+	 * NOTE:
+	 * This may be called from both serialized (locked and synced against
+	 * async commit tails) and unserialized (e.g. HPD IRQ) contexts. It
+	 * uses the current max link limits as upper bounds to reject
+	 * obviously bogus values, even if those bounds may be observed in a
+	 * transient or slightly stale state.
+	 *
+	 * This is not a full validation of the link configuration. Even in
+	 * serialized contexts, additional constraints (e.g. source limitations,
+	 * bandwidth checks, and other atomic state dependencies) are only
+	 * verified during the atomic check of the subsequent commit.
+	 *
+	 * max_link_limits only provides independent upper bounds for rate and
+	 * lane count. Callers must not assume it is itself an allowed link
+	 * configuration. Although that happens to be true for now, it will
+	 * stop being guaranteed once fallback depends only on disabled configs.
 	 */
 	if (link_rate == 0 ||
 	    link_rate > intel_dp->link.max_rate)
@@ -2151,6 +2199,22 @@ static bool intel_dp_is_connected(struct intel_dp *intel_dp)
 		intel_dp->is_mst;
 }
 
+static void queue_modeset_retry_for_links_in_state(struct intel_atomic_state *state,
+						   struct intel_encoder *encoder,
+						   u8 pipe_mask)
+{
+	const struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int i;
+
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		if (!(BIT(crtc->pipe) & pipe_mask))
+			continue;
+
+		intel_dp_queue_modeset_retry_for_link(state, encoder, crtc_state);
+	}
+}
+
 static int intel_dp_retrain_link(struct intel_encoder *encoder,
 				 struct drm_modeset_acquire_ctx *ctx)
 {
@@ -2201,11 +2265,23 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder,
 
 	intel_dp_link_training_set_force_retrain(link_training, false);
 
-	if (ret)
+	if (ret) {
 		drm_dbg_kms(display->drm,
 			    "[ENCODER:%d:%s] link retraining failed: %pe\n",
 			    encoder->base.base.id, encoder->base.name,
 			    ERR_PTR(ret));
+		/*
+		 * intel_dp_needs_link_retrain() only performs a coarse check of
+		 * retrainability, so the modeset commit may still fail. Disable
+		 * further auto-retrain attempts in that case.
+		 *
+		 * A sink capability change may restore the retrainable state (see
+		 * intel_dp_update_sink_caps(), intel_dp_reset_link_params()),
+		 * allowing retraining to be attempted again.
+		 */
+		link_recovery_mark_autoretrain_modeset_failure(link_training);
+		queue_modeset_retry_for_links_in_state(state, encoder, pipe_mask);
+	}
 out:
 	drm_atomic_state_put(&state->base);
 
@@ -2229,6 +2305,24 @@ void intel_dp_check_link_state(struct intel_dp *intel_dp)
 	if (!intel_dp_is_connected(intel_dp))
 		return;
 
+	/*
+	 * NOTE:
+	 * This may race with an ongoing modeset updating the max link limits
+	 * and, with that, the link's retrainability, so
+	 * intel_dp_needs_link_retrain() may observe stale state.
+	 *
+	 * This is harmless: stale params captured as valid may spuriously
+	 * allow retraining here, but the decision is rechecked later in a
+	 * properly serialized context.
+	 *
+	 * Conversely, stale params captured as invalid may skip retraining,
+	 * but that can only happen before the modeset has completed its own
+	 * link training for the new, valid configuration, after which the
+	 * link state is rechecked.
+	 *
+	 * See intel_dp_link_params_valid() for capturing and validating the
+	 * params.
+	 */
 	if (!intel_dp_needs_link_retrain(intel_dp))
 		return;
 
-- 
2.49.1