On Mon, Jul 16, 2018 at 11:14:45AM -0700, Nathan Ciobanu wrote:
> Limit the link training clock recovery loop to 10 failed attempts at
> LANEx_CR_DONE per DP 1.4 spec section 3.5.1.2.2.
Thanks... so this made me look to DP 1.3 and DP 1.4 differences here.
Are we already addressing all new conditions?
I wonder if that should be at drm level.
I noticed that one difference, 100us wait difference is already
addressed there at drm level...
> Some USB-C MST hubs
> cause us to get stuck in this loop indefinitely requesting
>
> voltage swing: 0, pre-emphasis level: 2
> voltage swing: 1, pre-emphasis level: 2
> voltage swing: 0, pre-emphasis level: 3
>
> over and over: max_vswing is never reached, drm_dp_clock_recovery_ok()
> never returns true and voltage_tries always gets reset to 1. The driver
> sends those values to the hub but the hub keeps requesting new values
> every time in the pattern shown above.
>
> Changes in v2:
> - updated commit message (DK, Manasi)
> - defined DP_DP14_MAX_CR_TRIES (Marc)
> - made the loop iterate for max 10 times (Rodrigo, Marc)
>
> Changes in v3:
> - changed error message to use DP_DP14_MAX_CR_TRIES
>
> Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp_link_training.c | 7 +++++--
> include/drm/drm_dp_helper.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 4da6e33c7fa1..4bfba65c431c 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -129,7 +129,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> {
> uint8_t voltage;
> - int voltage_tries, max_vswing_tries;
> + int voltage_tries, max_vswing_tries, cr_tries;
> uint8_t link_config[2];
> uint8_t link_bw, rate_select;
>
> @@ -172,7 +172,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
>
> voltage_tries = 1;
> max_vswing_tries = 0;
> - for (;;) {
> + for (cr_tries = 0; cr_tries < DP_DP14_MAX_CR_TRIES; ++cr_tries) {
I know that I was the on that asked you to make this global here,
but just now with better spec pointers and definition is that I noticed
that this is only for DP 1.4 spec while this code here runs for all
DP... So we need to change in a way that we check the spec version somehow...
and possibly at drm level?!
> uint8_t link_status[DP_LINK_STATUS_SIZE];
>
> drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd);
> @@ -216,6 +216,9 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> ++max_vswing_tries;
>
> }
> + DRM_ERROR("Failed clock recovery %d times, giving up!\n",
> + DP_DP14_MAX_CR_TRIES);
> + return false;
> }
>
> /*
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c01564991a9f..2303ad8ed24e 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -820,6 +820,7 @@
>
> #define DP_DP13_DPCD_REV 0x2200
> #define DP_DP13_MAX_LINK_RATE 0x2201
> +#define DP_DP14_MAX_CR_TRIES 10
>
> #define DP_DPRX_FEATURE_ENUMERATION_LIST 0x2210 /* DP 1.3 */
> # define DP_GTC_CAP (1 << 0) /* DP 1.3 */
> --
> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
