[Intel-gfx] [PATCH 3/4] drm/i915/edp: Be less aggressive about changing link config on eDP
Mika Kahola
mika.kahola at intel.com
Mon May 8 13:07:09 UTC 2017
On Fri, 2017-05-05 at 14:02 -0700, Jim Bride wrote:
> This set of changes has some history to them. There were several
> attempts
> to add what was called "fast link training" to i915, which actually
> wasn't
> fast link training as per the DP spec. These changes were
>
> 5fa836a9d859 ("drm/i915: DP link training optimization")
> 4e96c97742f4 ("drm/i915: eDP link training optimization")
>
> which were eventually hand-reverted by
>
> 34511dce4 ("drm/i915: Revert DisplayPort fast link training feature")
>
> in kernel 4.7-rc4. The eDP pieces of the above revert, however, had
> some
> very bad side-effects on PSR functionality on Skylake. The issue at
> hand is that when PSR exits i915 briefly emits TP1 followed by TP2/3
> (depending on the original link configuration) in order to quickly
> get
> the source and sink back in synchronization across the link before
> handing
> control back to the i915. There's an assumption that none of the
> link
> configuration information has changed (and thus it's still valid)
> since the
> last full link training operation. The revert above was identified
> via a
> bisect as the cause of some of Skylake's PSR woes. This patch,
> largely
> based on
>
> commit 4e96c97742f4201edf1b0f8e1b1b6b2ac6ff33e7
> Author: Mika Kahola <mika.kahola at intel.com>
> Date: Wed Apr 29 09:17:39 2015 +0300
> drm/i915: eDP link training optimization
>
> puts the eDP portions of this patch back in place. None of the
> flickering
> issues that spurred the revert have been seen, and I suspect the real
> culprits here were addressed by some of the recent link training
> changes
> that Manasi has implemented, and PSR on Skylake is definitely more
> happy
> with these changes in-place.
We reverted this back in the day due to bug reports we received from
the community concerning flickering. However, you have to remember that
during that time also the watermark work was ongoing. We couldn't
pinpoint one issue or root cause that triggered these reported
flickering cases. Therefore we decided to revert these optimizations.
Personally, I feel that this would have been safe for eDP case.
Acked-by: Mika Kahola <mika.kahola at intel.com>
>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Manasi D Navare <manasi.d.navare at intel.com>
> Cc: Mika Kahola <mika.kahola at intel.com>
> Fixes: 34511dce4 ("drm/i915: Revert DisplayPort fast link training
> feature")
> Signed-off-by: Jim Bride <jim.bride at linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> drivers/gpu/drm/i915/intel_dp_link_training.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index d46f72d..06b8bd4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -106,7 +106,7 @@ static const int default_rates[] = { 162000,
> 270000, 540000 };
> * If a CPU or PCH DP output is attached to an eDP panel, this
> function
> * will return true, and false otherwise.
> */
> -static bool is_edp(struct intel_dp *intel_dp)
> +bool is_edp(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
>
> @@ -4690,6 +4690,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> intel_dp->max_link_rate =
> intel_dp_max_common_rate(intel_dp);
>
> intel_dp->reset_link_params = false;
> + intel_dp->train_set_valid = false;
> }
>
> intel_dp_print_rates(intel_dp);
> @@ -6052,6 +6053,7 @@ intel_dp_init_connector(struct
> intel_digital_port *intel_dig_port,
> intel_dp_set_source_rates(intel_dp);
>
> intel_dp->reset_link_params = true;
> + intel_dp->train_set_valid = false;
> intel_dp->pps_pipe = INVALID_PIPE;
> intel_dp->active_pipe = INVALID_PIPE;
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index b79c1c0..60233e2 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -94,7 +94,8 @@ static bool
> intel_dp_reset_link_train(struct intel_dp *intel_dp,
> uint8_t dp_train_pat)
> {
> - memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> + if (!intel_dp->train_set_valid)
> + memset(intel_dp->train_set, 0, sizeof(intel_dp-
> >train_set));
> intel_dp_set_signal_levels(intel_dp);
> return intel_dp_set_link_train(intel_dp, dp_train_pat);
> }
> @@ -162,6 +163,7 @@ intel_dp_link_training_clock_recovery(struct
> intel_dp *intel_dp)
> DP_TRAINING_PATTERN_1 |
> DP_LINK_SCRAMBLING_DISABLE))
> {
> DRM_ERROR("failed to enable link training\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> @@ -174,21 +176,25 @@ intel_dp_link_training_clock_recovery(struct
> intel_dp *intel_dp)
>
> if (!intel_dp_get_link_status(intel_dp,
> link_status)) {
> DRM_ERROR("failed to get link status\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> if (drm_dp_clock_recovery_ok(link_status, intel_dp-
> >lane_count)) {
> DRM_DEBUG_KMS("clock recovery OK\n");
> + intel_dp->train_set_valid =
> is_edp(intel_dp);
> return true;
> }
>
> if (voltage_tries == 5) {
> DRM_DEBUG_KMS("Same voltage tried 5
> times\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> if (max_vswing_tries == 1) {
> DRM_DEBUG_KMS("Max Voltage Swing
> reached\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> @@ -198,6 +204,7 @@ intel_dp_link_training_clock_recovery(struct
> intel_dp *intel_dp)
> intel_get_adjust_train(intel_dp, link_status);
> if (!intel_dp_update_link_train(intel_dp)) {
> DRM_ERROR("failed to update link
> training\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> @@ -256,6 +263,7 @@
> intel_dp_link_training_channel_equalization(struct intel_dp
> *intel_dp)
> training_pattern |
> DP_LINK_SCRAMBLING_DISABLE)) {
> DRM_ERROR("failed to start channel equalization\n");
> + intel_dp->train_set_valid = false;
> return false;
> }
>
> @@ -296,6 +304,7 @@
> intel_dp_link_training_channel_equalization(struct intel_dp
> *intel_dp)
> /* Try 5 times, else fail and try at lower BW */
> if (tries == 5) {
> intel_dp_dump_link_status(link_status);
> + intel_dp->train_set_valid = false;
> DRM_DEBUG_KMS("Channel equalization failed 5
> times\n");
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 19d0c8f..149d4c9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -977,6 +977,7 @@ struct intel_dp {
> struct drm_dp_aux aux;
> enum intel_display_power_domain aux_power_domain;
> uint8_t train_set[4];
> + bool train_set_valid;
> int panel_power_up_delay;
> int panel_power_down_delay;
> int panel_power_cycle_delay;
> @@ -1551,6 +1552,7 @@ bool intel_dp_read_dpcd(struct intel_dp
> *intel_dp);
> bool __intel_dp_read_desc(struct intel_dp *intel_dp,
> struct intel_dp_desc *desc);
> bool intel_dp_read_desc(struct intel_dp *intel_dp);
> +bool is_edp(struct intel_dp *intel_dp);
> int intel_dp_link_required(int pixel_clock, int bpp);
> int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
--
Mika Kahola - Intel OTC
More information about the Intel-gfx
mailing list