Re: [PATCH v3 4/4] drm/i915/edp: Be less aggressive about changing link config on eDP

Chris Wilson
2017-07-11T23:16:22Z

Quoting Jim Bride (2017-07-11 23:19:56)
> @@ -174,21 +176,25 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>
> if (!intel_dp_get_link_status(intel_dp, link_status)) {
> DRM_ERROR("failed to get link status\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> if (drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
> DRM_DEBUG_KMS("clock recovery OK\n");
> + intel_dp->train_set_valid = is_edp(intel_dp);

Ouch, that was hard to spot amongst the decoys. How about setting
intel_dp->train_set_valid = false at the very start of training, and
only on success set it to true, something like

@@ -316,6 +316,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
{
struct intel_connector *intel_connector = intel_dp->attached_connector;

+ intel_dp->train_set_valid = false;
+
if (!intel_dp_link_training_clock_recovery(intel_dp))
goto failure_handling;
if (!intel_dp_link_training_channel_equalization(intel_dp))
@@ -323,6 +325,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)

DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
intel_dp->link_rate, intel_dp->lane_count);
+ /*
+ * Record the last known working parameters for quick retraining
+ * next time. We only trust eDP for now.
+ */
+ intel_dp->train_set_valid = is_edp(intel_dp);
return;