Re: [Intel-gfx] [PATCH 6/7] drm/i915: Try harder during dp pattern 1 link training

On Tue, 1 Nov 2011 23:20:29 -0700 Keith Packard keithp@keithp.com wrote:

Instead of going through the sequence just once, run through the whole set up to 5 times to see if something can work. This isn't part of the DP spec, but the BIOS seems to do it, and given that link training failure is so bad, it seems reasonable to follow suit.

Signed-off-by: Keith Packard keithp@keithp.com

drivers/gpu/drm/i915/intel_dp.c | 41 +++++++++++++++++++++++++------------- 1 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 6be6a04..bf20a35 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1576,8 +1576,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,

ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,

• 			intel_dp->train_set, 4);
  

• if (ret != 4)

• 			intel_dp->train_set,
  

• 			intel_dp->lane_count);
  

• if (ret != intel_dp->lane_count) return false;

  return true;

Sneaky putting this bug fix into this patch. :)

@@ -1593,7 +1594,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) int i; uint8_t voltage; bool clock_recovery = false;

• int tries;

• int voltage_tries, loop_tries; u32 reg; uint32_t DP = intel_dp->DP;

@@ -1620,7 +1621,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) DP &= ~DP_LINK_TRAIN_MASK; memset(intel_dp->train_set, 0, 4); voltage = 0xff;

• tries = 0;

• voltage_tries = 0;
• loop_tries = 0; clock_recovery = false; for (;;) { /* Use intel_dp->train_set[0] to set the voltage and pre emphasis values */

@@ -1663,17 +1665,28 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) for (i = 0; i < intel_dp->lane_count; i++) if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0) break;

• if (i == intel_dp->lane_count)
  

• 	break;
  

• /* Check to see if we've tried the same voltage 5 times */
  

• if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) == voltage) {
  

• 	++tries;
  

• 	if (tries == 5)
  

• if (i == intel_dp->lane_count) {
  

• 	++loop_tries;
  

• 	if (loop_tries == 5) {
  

• 		DRM_DEBUG_KMS("too many full retries, give up\n");
  	break;
  

• } else
  

• 	tries = 0;
  

• voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
  

• 	}
  

• 	memset(intel_dp->train_set, 0, 4);
  

• 	voltage_tries = 0;
  

• 	continue;
  

• } else {
  

• 	/* Check to see if we've tried the same voltage 5 times */
  

• 	if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) == voltage) {
  

• 		++voltage_tries;
  

• 		if (voltage_tries == 5) {
  

• 			DRM_DEBUG_KMS("too many voltage retries, give up\n");
  

• 			break;
  

• 		}
  

• 	} else
  

• 		voltage_tries = 0;
  

• 	voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
  

• }
  
  

  /* Compute new intel_dp->train_set as requested by target */ intel_get_adjust_train(intel_dp, link_status);

Don't you love the training state machine? I think this looks ok, and the DP compliance test should catch any grievous errors, so aside from the bug fix hunk which should be split out:

Reviewed-by: Jesse Barnes jbarnes@virtuousgeek.org