| Message ID | 1425387898-3333-1-git-send-email-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 03 Mar 2015, Mika Kahola <mika.kahola@intel.com> wrote: > This is a first of series patches that optimize DP link > training. The first patch is for eDP only where we reuse > the previously trained link training values from cache > i.e. voltage swing and pre-emphasis levels. > > In case we are not able to train the link by reusing > the known values, the link training parameters are set > to zero and training is restarted. Apologies for taking about forever and a half to get back to this. :( See comments inline. BR, Jani. > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 67 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index d1141d3..9497eb6 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3294,6 +3294,25 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP, > } > > static bool > +intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP, > + uint8_t dp_train_pat) > +{ > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct drm_device *dev = intel_dig_port->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + intel_dp_set_signal_levels(intel_dp, DP); > + > + I915_WRITE(intel_dp->output_reg, *DP); > + POSTING_READ(intel_dp->output_reg); > + > + drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, > + intel_dp->train_set, intel_dp->lane_count); > + > + return intel_dp_set_link_train(intel_dp, DP, dp_train_pat); > +} So I'd think intel_dp_reuse_link_train should be exactly the same as intel_dp_reset_link_train, except for the memset part. Maybe just look at ->link_trained, and memset if not. And make sure you change ->link_trained at approapriate places *before* making the call. > + > +static bool > intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, > const uint8_t link_status[DP_LINK_STATUS_SIZE]) > { > @@ -3356,6 +3375,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > int voltage_tries, loop_tries; > uint32_t DP = intel_dp->DP; > uint8_t link_config[2]; > + uint8_t link_status[DP_LINK_STATUS_SIZE]; This is an unrelated change. > > if (HAS_DDI(dev)) > intel_ddi_prepare_link_retrain(encoder); > @@ -3373,20 +3393,33 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > > DP |= DP_PORT_EN; > > - /* clock recovery */ > - if (!intel_dp_reset_link_train(intel_dp, &DP, > - DP_TRAINING_PATTERN_1 | > - DP_LINK_SCRAMBLING_DISABLE)) { > - DRM_ERROR("failed to enable link training\n"); > - return; > + /* > + * check if eDP has already trained. Reset voltage swing and > + * pre-emphasis levels if that's not the case. > + */ > + if (intel_dp->link_trained) { I know what you mean with ->link_trained, but I think the naming is misleading. It doesn't mean we would have a link up and running. Maybe it should be ->train_set_valid or something? > + DRM_DEBUG_KMS("reuse current link train set\n"); > + if (!intel_dp_reuse_link_train(intel_dp, &DP, > + DP_TRAINING_PATTERN_1 | > + DP_LINK_SCRAMBLING_DISABLE)) { > + DRM_DEBUG_KMS("failed to set link training\n"); > + return; > + } > + } else { > + /* reset link training values */ > + DRM_DEBUG_KMS("reset link train set\n"); > + if (!intel_dp_reset_link_train(intel_dp, &DP, > + DP_TRAINING_PATTERN_1 | > + DP_LINK_SCRAMBLING_DISABLE)) { > + DRM_ERROR("failed to enable link training\n"); > + return; > + } > } > > voltage = 0xff; > voltage_tries = 0; > loop_tries = 0; > for (;;) { > - uint8_t link_status[DP_LINK_STATUS_SIZE]; > - > drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd); > if (!intel_dp_get_link_status(intel_dp, link_status)) { > DRM_ERROR("failed to get link status\n"); > @@ -3398,6 +3431,24 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > break; > } > > + /* > + * if we used previously trained voltage and pre-emphasis values > + * and we don't get clock recovery, reset link training values > + */ > + if (intel_dp->link_trained) { > + DRM_DEBUG_KMS("clock recovery not ok, reset"); > + if (!intel_dp_reset_link_train(intel_dp, &DP, > + DP_TRAINING_PATTERN_1 | > + DP_LINK_SCRAMBLING_DISABLE)) { > + DRM_ERROR("failed to enable link training\n"); > + return; > + } > + > + /* clear the flag as we are not reusing train set */ > + intel_dp->link_trained = false; Do this first in case we bail out in the above error handling. > + continue; > + } > + > /* Check to see if we've tried the max voltage */ > for (i = 0; i < intel_dp->lane_count; i++) > if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0) > @@ -3475,6 +3526,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > > /* Make sure clock is still ok */ > if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) { > + intel_dp->link_trained = false; > intel_dp_start_link_train(intel_dp); > intel_dp_set_link_train(intel_dp, &DP, > training_pattern | > @@ -3490,6 +3542,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > > /* Try 5 times, then try clock recovery if that fails */ > if (tries > 5) { > + intel_dp->link_trained = false; > intel_dp_start_link_train(intel_dp); > intel_dp_set_link_train(intel_dp, &DP, > training_pattern | > @@ -3511,8 +3564,10 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > > intel_dp->DP = DP; > > - if (channel_eq) > + if (channel_eq) { > DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); > + intel_dp->link_trained = is_edp(intel_dp); > + } > > } > > @@ -4450,6 +4505,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > intel_display_power_get(dev_priv, power_domain); > > if (long_hpd) { > + /* indicate that we need to restart link training */ > + intel_dp->link_trained = false; > > if (HAS_PCH_SPLIT(dev)) { > if (!ibx_digital_port_connected(dev_priv, intel_dig_port)) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 632df1c..f1e1318 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -668,6 +668,7 @@ struct intel_dp { > bool has_aux_irq, > int send_bytes, > uint32_t aux_clock_divider); > + bool link_trained; > }; > > struct intel_digital_port { > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d1141d3..9497eb6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3294,6 +3294,25 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP, } static bool +intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP, + uint8_t dp_train_pat) +{ + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct drm_device *dev = intel_dig_port->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + intel_dp_set_signal_levels(intel_dp, DP); + + I915_WRITE(intel_dp->output_reg, *DP); + POSTING_READ(intel_dp->output_reg); + + drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, + intel_dp->train_set, intel_dp->lane_count); + + return intel_dp_set_link_train(intel_dp, DP, dp_train_pat); +} + +static bool intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP, const uint8_t link_status[DP_LINK_STATUS_SIZE]) { @@ -3356,6 +3375,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) int voltage_tries, loop_tries; uint32_t DP = intel_dp->DP; uint8_t link_config[2]; + uint8_t link_status[DP_LINK_STATUS_SIZE]; if (HAS_DDI(dev)) intel_ddi_prepare_link_retrain(encoder); @@ -3373,20 +3393,33 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) DP |= DP_PORT_EN; - /* clock recovery */ - if (!intel_dp_reset_link_train(intel_dp, &DP, - DP_TRAINING_PATTERN_1 | - DP_LINK_SCRAMBLING_DISABLE)) { - DRM_ERROR("failed to enable link training\n"); - return; + /* + * check if eDP has already trained. Reset voltage swing and + * pre-emphasis levels if that's not the case. + */ + if (intel_dp->link_trained) { + DRM_DEBUG_KMS("reuse current link train set\n"); + if (!intel_dp_reuse_link_train(intel_dp, &DP, + DP_TRAINING_PATTERN_1 | + DP_LINK_SCRAMBLING_DISABLE)) { + DRM_DEBUG_KMS("failed to set link training\n"); + return; + } + } else { + /* reset link training values */ + DRM_DEBUG_KMS("reset link train set\n"); + if (!intel_dp_reset_link_train(intel_dp, &DP, + DP_TRAINING_PATTERN_1 | + DP_LINK_SCRAMBLING_DISABLE)) { + DRM_ERROR("failed to enable link training\n"); + return; + } } voltage = 0xff; voltage_tries = 0; loop_tries = 0; for (;;) { - uint8_t link_status[DP_LINK_STATUS_SIZE]; - drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd); if (!intel_dp_get_link_status(intel_dp, link_status)) { DRM_ERROR("failed to get link status\n"); @@ -3398,6 +3431,24 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) break; } + /* + * if we used previously trained voltage and pre-emphasis values + * and we don't get clock recovery, reset link training values + */ + if (intel_dp->link_trained) { + DRM_DEBUG_KMS("clock recovery not ok, reset"); + if (!intel_dp_reset_link_train(intel_dp, &DP, + DP_TRAINING_PATTERN_1 | + DP_LINK_SCRAMBLING_DISABLE)) { + DRM_ERROR("failed to enable link training\n"); + return; + } + + /* clear the flag as we are not reusing train set */ + intel_dp->link_trained = false; + continue; + } + /* Check to see if we've tried the max voltage */ for (i = 0; i < intel_dp->lane_count; i++) if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0) @@ -3475,6 +3526,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) /* Make sure clock is still ok */ if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) { + intel_dp->link_trained = false; intel_dp_start_link_train(intel_dp); intel_dp_set_link_train(intel_dp, &DP, training_pattern | @@ -3490,6 +3542,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) /* Try 5 times, then try clock recovery if that fails */ if (tries > 5) { + intel_dp->link_trained = false; intel_dp_start_link_train(intel_dp); intel_dp_set_link_train(intel_dp, &DP, training_pattern | @@ -3511,8 +3564,10 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) intel_dp->DP = DP; - if (channel_eq) + if (channel_eq) { DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); + intel_dp->link_trained = is_edp(intel_dp); + } } @@ -4450,6 +4505,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) intel_display_power_get(dev_priv, power_domain); if (long_hpd) { + /* indicate that we need to restart link training */ + intel_dp->link_trained = false; if (HAS_PCH_SPLIT(dev)) { if (!ibx_digital_port_connected(dev_priv, intel_dig_port)) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 632df1c..f1e1318 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -668,6 +668,7 @@ struct intel_dp { bool has_aux_irq, int send_bytes, uint32_t aux_clock_divider); + bool link_trained; }; struct intel_digital_port {
This is a first of series patches that optimize DP link training. The first patch is for eDP only where we reuse the previously trained link training values from cache i.e. voltage swing and pre-emphasis levels. In case we are not able to train the link by reusing the known values, the link training parameters are set to zero and training is restarted. Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 67 insertions(+), 9 deletions(-)