On Tue, 1 Nov 2011 23:20:29 -0700
Keith Packard <keithp@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
