| Message ID | 1532036858-11743-1-git-send-email-nathan.d.ciobanu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 19, 2018 at 02:47:38PM -0700, Nathan Ciobanu wrote: > Limit the link training clock recovery loop to 10 attempts at > LANEx_CR_DONE per DP 1.4 spec section 3.5.1.2.2 and 80 attempts for > pre-DP 1.4 (4 voltage levels x 4 preemphasis levels x > x 5 identical voltages tries). Some faulty USB-C MST hubs can > cause us to get stuck in this loop indefinitely requesting something > like: > > voltage swing: 0, pre-emphasis level: 2 > voltage swing: 1, pre-emphasis level: 2 > voltage swing: 0, pre-emphasis level: 3 > > over and over so max_vswing would never be reached, > drm_dp_clock_recovery_ok() would never return true and voltage_tries > would always get reset to 1. The driver sends those values to the hub > but the hub keeps requesting new values every time. > > 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 > > Changes in v4: > - Updated the title to reflect the change > - Updated the commit message > - Added 80 attempts for pre-DP 1.4 devices > > Changes in v5: > - Removed DP_DP14_MAX_CR_TRIES from drm > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Marc Herbert <marc.herbert@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp_link_training.c | 16 ++++++++++++++-- > 1 file changed, 14 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..7903de7a54c9 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, max_cr_tries; > uint8_t link_config[2]; > uint8_t link_bw, rate_select; > > @@ -170,9 +170,19 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp) > return false; > } > > + /* DP 1.4 spec clock recovery retries defined but nip: Preferred kernel multi-line comment is: /* * Start comment.. * second line.. */ but yeah, I understand we have many precedent cases using the other style... Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (better with comment updated ;)) > + * for devices pre-DP 1.4 we set the retry limit > + * to 4 (voltage levels) x 4 (preemphasis levels) x > + * x 5 (same voltage retries) = 80 (max iterations) > + */ > + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14) > + max_cr_tries = 10; > + else > + max_cr_tries = 80; > + > voltage_tries = 1; > max_vswing_tries = 0; > - for (;;) { > + for (cr_tries = 0; cr_tries < max_cr_tries; ++cr_tries) { > uint8_t link_status[DP_LINK_STATUS_SIZE]; > > drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd); > @@ -216,6 +226,8 @@ 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", max_cr_tries); > + return false; > } > > /* > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jul 20, 2018 at 12:22:15PM -0700, Rodrigo Vivi wrote: > On Thu, Jul 19, 2018 at 02:47:38PM -0700, Nathan Ciobanu wrote: > > Limit the link training clock recovery loop to 10 attempts at > > LANEx_CR_DONE per DP 1.4 spec section 3.5.1.2.2 and 80 attempts for > > pre-DP 1.4 (4 voltage levels x 4 preemphasis levels x > > x 5 identical voltages tries). Some faulty USB-C MST hubs can > > cause us to get stuck in this loop indefinitely requesting something > > like: > > > > voltage swing: 0, pre-emphasis level: 2 > > voltage swing: 1, pre-emphasis level: 2 > > voltage swing: 0, pre-emphasis level: 3 > > > > over and over so max_vswing would never be reached, > > drm_dp_clock_recovery_ok() would never return true and voltage_tries > > would always get reset to 1. The driver sends those values to the hub > > but the hub keeps requesting new values every time. > > > > 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 > > > > Changes in v4: > > - Updated the title to reflect the change > > - Updated the commit message > > - Added 80 attempts for pre-DP 1.4 devices > > > > Changes in v5: > > - Removed DP_DP14_MAX_CR_TRIES from drm > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Marc Herbert <marc.herbert@intel.com> > > Cc: Manasi Navare <manasi.d.navare@intel.com> > > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 16 ++++++++++++++-- > > 1 file changed, 14 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..7903de7a54c9 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, max_cr_tries; > > uint8_t link_config[2]; > > uint8_t link_bw, rate_select; > > > > @@ -170,9 +170,19 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp) > > return false; > > } > > > > + /* DP 1.4 spec clock recovery retries defined but > > nip: Preferred kernel multi-line comment is: > > /* > * Start comment.. > * second line.. > */ > > but yeah, I understand we have many precedent cases using the other style... > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > (better with comment updated ;)) Absolutely. I made the change and sent the patch again. Thanks, Nathan. > > > + * for devices pre-DP 1.4 we set the retry limit > > + * to 4 (voltage levels) x 4 (preemphasis levels) x > > + * x 5 (same voltage retries) = 80 (max iterations) > > + */ > > + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14) > > + max_cr_tries = 10; > > + else > > + max_cr_tries = 80; > > + > > voltage_tries = 1; > > max_vswing_tries = 0; > > - for (;;) { > > + for (cr_tries = 0; cr_tries < max_cr_tries; ++cr_tries) { > > uint8_t link_status[DP_LINK_STATUS_SIZE]; > > > > drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd); > > @@ -216,6 +226,8 @@ 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", max_cr_tries); > > + return false; > > } > > > > /* > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 4da6e33c7fa1..7903de7a54c9 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, max_cr_tries; uint8_t link_config[2]; uint8_t link_bw, rate_select; @@ -170,9 +170,19 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp) return false; } + /* DP 1.4 spec clock recovery retries defined but + * for devices pre-DP 1.4 we set the retry limit + * to 4 (voltage levels) x 4 (preemphasis levels) x + * x 5 (same voltage retries) = 80 (max iterations) + */ + if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14) + max_cr_tries = 10; + else + max_cr_tries = 80; + voltage_tries = 1; max_vswing_tries = 0; - for (;;) { + for (cr_tries = 0; cr_tries < max_cr_tries; ++cr_tries) { uint8_t link_status[DP_LINK_STATUS_SIZE]; drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd); @@ -216,6 +226,8 @@ 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", max_cr_tries); + return false; } /*
Limit the link training clock recovery loop to 10 attempts at LANEx_CR_DONE per DP 1.4 spec section 3.5.1.2.2 and 80 attempts for pre-DP 1.4 (4 voltage levels x 4 preemphasis levels x x 5 identical voltages tries). Some faulty USB-C MST hubs can cause us to get stuck in this loop indefinitely requesting something like: voltage swing: 0, pre-emphasis level: 2 voltage swing: 1, pre-emphasis level: 2 voltage swing: 0, pre-emphasis level: 3 over and over so max_vswing would never be reached, drm_dp_clock_recovery_ok() would never return true and voltage_tries would always get reset to 1. The driver sends those values to the hub but the hub keeps requesting new values every time. 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 Changes in v4: - Updated the title to reflect the change - Updated the commit message - Added 80 attempts for pre-DP 1.4 devices Changes in v5: - Removed DP_DP14_MAX_CR_TRIES from drm Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Marc Herbert <marc.herbert@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp_link_training.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)