[PATCH v2 0/4] Kernel PSR Fix-ups

[PATCH v2 0/4] Kernel PSR Fix-ups

[Jim Bride]

These patches, along with an upcoming series for IGT, enable our
PSR IGT tests to run reliably once again on HSW, BDW, and SKL.
The first change enables us to run the PSR tests on some RVP platforms
whose panels have too slow of a setup time when running in their
preferred mode.  The second fixes a minor problem with the way that
we were initializing SRD_CTL that caused us to clobber a bit that we
are not supposed to change in that register on SKL and KBL.  The third
change re-introduces some changes to our link training code to be less
aggressive about changing link state for eDP, because PSR depends on
the link state being the same at PSR exit as it was at PSR entry.
The fourth change greatly increases the reliability of reading the
sink CRC generated by the eDP panel.

v2 Highlights:

   * Rebased to current drm-tip
   * Greatly reduced looping around trying to read sink CRC (Jani)
   * Reduce amount of changes in the sink CRC patch (Jani)
   * Field-wise init of EDP_PSR_MAX_SLEEP_TIME (Rodrigo)
   * Minor commit message / cover letter tweaks


Jim Bride (4):
  drm/i915/edp: Allow alternate fixed mode for eDP if available.
  drm/i915/psr: Clean-up intel_enable_source_psr1()
  drm/i915/edp: Be less aggressive about changing link config on eDP
  drm/i915/psr: Account for sink CRC raciness on some panels

 drivers/gpu/drm/i915/i915_reg.h               |  4 ++
 drivers/gpu/drm/i915/intel_dp.c               | 78 +++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_dp_link_training.c | 11 +++-
 drivers/gpu/drm/i915/intel_drv.h              |  4 ++
 drivers/gpu/drm/i915/intel_dsi.c              |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c              |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c             |  3 +-
 drivers/gpu/drm/i915/intel_panel.c            |  2 +
 drivers/gpu/drm/i915/intel_psr.c              | 21 +++++++-
 9 files changed, 111 insertions(+), 16 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/4] drm/i915/edp: Allow alternate fixed mode for eDP if available.

[Jim Bride]

Some fixed resolution panels actually support more than one mode,
with the only thing different being the refresh rate.  Having this
alternate mode available to us is desirable, because it allows us to
test PSR on panels whose setup time at the preferred mode is too long.
With this patch we allow the use of the alternate mode if it's
available and it was specifically requested.

v2: Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c    | 34 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_dsi.c   |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c   |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c  |  3 ++-
 drivers/gpu/drm/i915/intel_panel.c |  2 ++
 6 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 67bc8a7a..3560e97 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1606,6 +1606,19 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
 	return bpp;
 }
 
+static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
+				       struct drm_display_mode *m2)
+{
+	return (m1->hdisplay == m2->hdisplay &&
+		m1->hsync_start == m2->hsync_start &&
+		m1->hsync_end == m2->hsync_end &&
+		m1->htotal == m2->htotal &&
+		m1->vdisplay == m2->vdisplay &&
+		m1->vsync_start == m2->vsync_start &&
+		m1->vsync_end == m2->vsync_end &&
+		m1->vtotal == m2->vtotal);
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config,
@@ -1652,8 +1665,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
 
 	if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
-		intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
-				       adjusted_mode);
+		struct drm_display_mode *panel_mode =
+			intel_connector->panel.alt_fixed_mode;
+		struct drm_display_mode *req_mode = &pipe_config->base.mode;
+
+		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
+			panel_mode = intel_connector->panel.fixed_mode;
+
+		drm_mode_debug_printmodeline(panel_mode);
+
+		intel_fixed_panel_mode(panel_mode, adjusted_mode);
 
 		if (INTEL_GEN(dev_priv) >= 9) {
 			int ret;
@@ -5772,6 +5793,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_display_mode *fixed_mode = NULL;
+	struct drm_display_mode *alt_fixed_mode = NULL;
 	struct drm_display_mode *downclock_mode = NULL;
 	bool has_dpcd;
 	struct drm_display_mode *scan;
@@ -5827,13 +5849,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	}
 	intel_connector->edid = edid;
 
-	/* prefer fixed mode from EDID if available */
+	/* prefer fixed mode from EDID if available, save an alt mode also */
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
 			fixed_mode = drm_mode_duplicate(dev, scan);
 			downclock_mode = intel_dp_drrs_init(
 						intel_connector, fixed_mode);
-			break;
+		} else {
+			alt_fixed_mode = drm_mode_duplicate(dev, scan);
 		}
 	}
 
@@ -5870,7 +5893,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 			      pipe_name(pipe));
 	}
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
+			 downclock_mode);
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a324..0c14b05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -265,6 +265,7 @@ struct intel_encoder {
 
 struct intel_panel {
 	struct drm_display_mode *fixed_mode;
+	struct drm_display_mode *alt_fixed_mode;
 	struct drm_display_mode *downclock_mode;
 
 	/* backlight */
@@ -1696,6 +1697,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
 /* intel_panel.c */
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
+		     struct drm_display_mode *alt_fixed_mode,
 		     struct drm_display_mode *downclock_mode);
 void intel_panel_fini(struct intel_panel *panel);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 50ec836..d3a6608 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1851,7 +1851,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	connector->display_info.width_mm = fixed_mode->width_mm;
 	connector->display_info.height_mm = fixed_mode->height_mm;
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+	intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	intel_dsi_add_properties(intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index c1544a5..39fd4f3 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -554,7 +554,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
 			 */
 			intel_panel_init(&intel_connector->panel,
 					 intel_dvo_get_current_mode(connector),
-					 NULL);
+					 NULL, NULL);
 			intel_dvo->panel_wants_dither = true;
 		}
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6fe5d7c..a995ccb 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1140,7 +1140,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
+			 downclock_mode);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 96c2cbd..4e7ba93 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1919,11 +1919,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
+		     struct drm_display_mode *alt_fixed_mode,
 		     struct drm_display_mode *downclock_mode)
 {
 	intel_panel_init_backlight_funcs(panel);
 
 	panel->fixed_mode = fixed_mode;
+	panel->alt_fixed_mode = alt_fixed_mode;
 	panel->downclock_mode = downclock_mode;
 
 	return 0;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/4] drm/i915/psr: Clean-up intel_enable_source_psr1()

[Jim Bride]

On SKL+ there is a bit in SRD_CTL that software is not supposed to
modify, but we currently clobber that bit when we enable PSR.  In
order to preserve the value of that bit, go ahead and read SRD_CTL and
do a field-wise setting of the various bits that we need to initialize
before writing the register back out.  Additionally, go ahead and
explicitly disable single-frame update since we aren't currently
supporting it.

v2: * Do a field-wise init on EDP_PSR_MAX_SLEEP_TIME even though we
      always set it to the max value. (Rodrigo)
    * Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Wayne Boyer <wayne.boyer@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 ++++
 drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 64cc674..df58818 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3789,18 +3789,22 @@ enum {
 #define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES	(1<<25)
 #define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES	(2<<25)
 #define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES	(3<<25)
+#define   EDP_PSR_MAX_SLEEP_TIME_MASK           (0x1f<<20)
 #define   EDP_PSR_MAX_SLEEP_TIME_SHIFT		20
 #define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
 #define   EDP_PSR_TP1_TP2_SEL			(0<<11)
 #define   EDP_PSR_TP1_TP3_SEL			(1<<11)
+#define   EDP_PSR_TP2_TP3_TIME_MASK             (3<<8)
 #define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
 #define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
 #define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
 #define   EDP_PSR_TP2_TP3_TIME_0us		(3<<8)
+#define   EDP_PSR_TP1_TIME_MASK                 (0x3<<4)
 #define   EDP_PSR_TP1_TIME_500us		(0<<4)
 #define   EDP_PSR_TP1_TIME_100us		(1<<4)
 #define   EDP_PSR_TP1_TIME_2500us		(2<<4)
 #define   EDP_PSR_TP1_TIME_0us			(3<<4)
+#define   EDP_PSR_IDLE_FRAME_MASK               (0xf<<0)
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 559f1ab..132987b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -280,17 +280,32 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
 	 * with the 5 or 6 idle patterns.
 	 */
 	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
-	uint32_t val = EDP_PSR_ENABLE;
+	uint32_t val = I915_READ(EDP_PSR_CTL);
 
+	val |= EDP_PSR_ENABLE;
+
+	val &= ~EDP_PSR_MAX_SLEEP_TIME_MASK;
 	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
+
+	val &= ~EDP_PSR_IDLE_FRAME_MASK;
 	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
+	val &= ~EDP_PSR_MIN_LINK_ENTRY_TIME_MASK;
 	if (IS_HASWELL(dev_priv))
 		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
 
-	if (dev_priv->psr.link_standby)
+	if (dev_priv->psr.link_standby) {
 		val |= EDP_PSR_LINK_STANDBY;
 
+		/* SFU should only be enabled with link standby, but for
+		 * now we do not support it. */
+		val &= ~BDW_PSR_SINGLE_FRAME;
+	} else {
+		val &= ~EDP_PSR_LINK_STANDBY;
+		val &= ~BDW_PSR_SINGLE_FRAME;
+	}
+
+	val &= ~EDP_PSR_TP1_TIME_MASK;
 	if (dev_priv->vbt.psr.tp1_wakeup_time > 5)
 		val |= EDP_PSR_TP1_TIME_2500us;
 	else if (dev_priv->vbt.psr.tp1_wakeup_time > 1)
@@ -300,6 +315,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
 	else
 		val |= EDP_PSR_TP1_TIME_0us;
 
+	val &= ~EDP_PSR_TP2_TP3_TIME_MASK;
 	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
 		val |= EDP_PSR_TP2_TP3_TIME_2500us;
 	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
@@ -309,6 +325,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
 	else
 		val |= EDP_PSR_TP2_TP3_TIME_0us;
 
+	val &= ~EDP_PSR_TP1_TP3_SEL;
 	if (intel_dp_source_supports_hbr2(intel_dp) &&
 	    drm_dp_tps3_supported(intel_dp->dpcd))
 		val |= EDP_PSR_TP1_TP3_SEL;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/4] drm/i915/edp: Be less aggressive about changing link config on eDP

[Jim Bride]

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@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.

v2: Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Manasi D Navare <manasi.d.navare@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Fixes: 34511dce4 ("drm/i915: Revert DisplayPort fast link training feature")
Signed-off-by: Jim Bride <jim.bride@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 3560e97..b46fa03 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);
 
@@ -4732,6 +4732,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);
@@ -5995,6 +5996,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 0c14b05..a1b4b5d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -996,6 +996,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;
@@ -1571,6 +1572,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 }
 
 bool intel_dp_read_dpcd(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,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/4] drm/i915/psr: Account for sink CRC raciness on some panels

[Jim Bride]

According to the eDP spec, when the count field in TEST_SINK_MISC
increments then the six bytes of sink CRC information in the DPCD
should be valid.  Unfortunately, this doesn't seem to be the case
on some panels, and as a result we get some incorrect and inconsistent
values from the sink CRC DPCD locations at times.  This problem exhibits
itself more on faster processors (relative failure rates HSW < SKL < KBL.)
In order to try and account for this, we try a lot harder to read the sink
CRC until we get consistent values twice in a row before returning what we
read and delay for a time before trying to read.  We still see some
occasional failures, but reading the sink CRC is much more reliable,
particularly on SKL and KBL, with these changes than without.

v2: * Reduce number of retries when reading the sink CRC (Jani)
    * Refactor to minimize changes to the code (Jani)
    * Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b46fa03..1fe0975 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3927,6 +3927,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 	u8 buf;
 	int count, ret;
 	int attempts = 6;
+	u8 old_crc[6];
+
+	if (crc != NULL) {
+		memset(crc, 0, 6);
+		memset(old_crc, 0xff, 6);
+	} else {
+		return -ENOMEM;
+	}
 
 	ret = intel_dp_sink_crc_start(intel_dp);
 	if (ret)
@@ -3950,11 +3958,35 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 		goto stop;
 	}
 
-	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
-		ret = -EIO;
-		goto stop;
-	}
+	attempts = 6;
+
+	/*
+	 * Sometimes it takes a while for the "real" CRC values to land in
+	 * the DPCD, so try several times until we get two reads in a row
+	 * that are the same.  If we're an eDP panel, delay between reads
+	 * for a while since the values take a bit longer to propagate.
+	 */
+	do {
+		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
+		if (is_edp(intel_dp))
+			usleep_range(20000, 25000);
+
+		if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR,
+				     crc, 6) < 0) {
+			ret = -EIO;
+			goto stop;
+		}
+
+		if (memcmp(old_crc, crc, 6) == 0) {
+			ret = 0;
+			goto stop;
+		} else {
+			memcpy(old_crc, crc, 6);
+		}
+	} while (--attempts);
 
+	DRM_DEBUG_KMS("Failed to get CRC after 6 attempts.\n");
+	ret = -ETIMEDOUT;
 stop:
 	intel_dp_sink_crc_stop(intel_dp);
 	return ret;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Kernel PSR Fix-ups (rev2)

[Patchwork]

== Series Details ==

Series: Kernel PSR Fix-ups (rev2)
URL   : https://patchwork.freedesktop.org/series/24049/
State : failure

== Summary ==

Series 24049v2 Kernel PSR Fix-ups
https://patchwork.freedesktop.org/api/1.0/series/24049/revisions/2/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-kbl-r)
Test gem_ctx_create:
        Subgroup basic-files:
                pass       -> INCOMPLETE (fi-byt-n2820)
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597 +1
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101516
                pass       -> FAIL       (fi-skl-6700k)

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101516 https://bugs.freedesktop.org/show_bug.cgi?id=101516

fi-bdw-5557u     total:279  pass:264  dwarn:0   dfail:0   fail:3   skip:11  time:436s
fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:279  pass:239  dwarn:0   dfail:0   fail:3   skip:36  time:520s
fi-bxt-j4205     total:279  pass:256  dwarn:0   dfail:0   fail:3   skip:19  time:502s
fi-byt-j1900     total:279  pass:250  dwarn:1   dfail:0   fail:3   skip:24  time:480s
fi-byt-n2820     total:17   pass:16   dwarn:0   dfail:0   fail:0   skip:0  
fi-hsw-4770      total:279  pass:259  dwarn:0   dfail:0   fail:3   skip:16  time:427s
fi-hsw-4770r     total:279  pass:259  dwarn:0   dfail:0   fail:3   skip:16  time:417s
fi-ilk-650       total:279  pass:225  dwarn:0   dfail:0   fail:3   skip:50  time:414s
fi-ivb-3520m     total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:492s
fi-ivb-3770      total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:470s
fi-kbl-7500u     total:279  pass:257  dwarn:0   dfail:0   fail:3   skip:18  time:454s
fi-kbl-7560u     total:279  pass:265  dwarn:0   dfail:0   fail:3   skip:10  time:570s
fi-kbl-r         total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:553s
fi-skl-6260u     total:279  pass:265  dwarn:0   dfail:0   fail:3   skip:10  time:449s
fi-skl-6700k     total:279  pass:256  dwarn:0   dfail:0   fail:4   skip:18  time:445s
fi-skl-6770hq    total:279  pass:265  dwarn:0   dfail:0   fail:3   skip:10  time:467s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-snb-2520m     total:279  pass:247  dwarn:0   dfail:0   fail:3   skip:28  time:529s
fi-snb-2600      total:279  pass:246  dwarn:0   dfail:0   fail:3   skip:29  time:404s
fi-skl-6700hq failed to connect after reboot

6412bcb8e279d62b9d8ae2840712d6c85806369b drm-tip: 2017y-06m-30d-14h-56m-41s UTC integration manifest
ddb903c drm/i915/psr: Account for sink CRC raciness on some panels
b8169fa drm/i915/edp: Be less aggressive about changing link config on eDP
b92ab9e8 drm/i915/psr: Clean-up intel_enable_source_psr1()
2eb6962 drm/i915/edp: Allow alternate fixed mode for eDP if available.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5084/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/i915/psr: Clean-up intel_enable_source_psr1()

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Fri, Jun 30, 2017 at 12:08 PM, Jim Bride <jim.bride@linux.intel.com> wrote:
> On SKL+ there is a bit in SRD_CTL that software is not supposed to
> modify, but we currently clobber that bit when we enable PSR.  In
> order to preserve the value of that bit, go ahead and read SRD_CTL and
> do a field-wise setting of the various bits that we need to initialize
> before writing the register back out.  Additionally, go ahead and
> explicitly disable single-frame update since we aren't currently
> supporting it.
>
> v2: * Do a field-wise init on EDP_PSR_MAX_SLEEP_TIME even though we
>       always set it to the max value. (Rodrigo)
>     * Rebase
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Wayne Boyer <wayne.boyer@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  4 ++++
>  drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 64cc674..df58818 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3789,18 +3789,22 @@ enum {
>  #define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES  (1<<25)
>  #define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES  (2<<25)
>  #define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES  (3<<25)
> +#define   EDP_PSR_MAX_SLEEP_TIME_MASK           (0x1f<<20)
>  #define   EDP_PSR_MAX_SLEEP_TIME_SHIFT         20
>  #define   EDP_PSR_SKIP_AUX_EXIT                        (1<<12)
>  #define   EDP_PSR_TP1_TP2_SEL                  (0<<11)
>  #define   EDP_PSR_TP1_TP3_SEL                  (1<<11)
> +#define   EDP_PSR_TP2_TP3_TIME_MASK             (3<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_500us           (0<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_100us           (1<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_2500us          (2<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_0us             (3<<8)
> +#define   EDP_PSR_TP1_TIME_MASK                 (0x3<<4)
>  #define   EDP_PSR_TP1_TIME_500us               (0<<4)
>  #define   EDP_PSR_TP1_TIME_100us               (1<<4)
>  #define   EDP_PSR_TP1_TIME_2500us              (2<<4)
>  #define   EDP_PSR_TP1_TIME_0us                 (3<<4)
> +#define   EDP_PSR_IDLE_FRAME_MASK               (0xf<<0)
>  #define   EDP_PSR_IDLE_FRAME_SHIFT             0
>
>  #define EDP_PSR_AUX_CTL                                _MMIO(dev_priv->psr_mmio_base + 0x10)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 559f1ab..132987b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,17 +280,32 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>          * with the 5 or 6 idle patterns.
>          */
>         uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> -       uint32_t val = EDP_PSR_ENABLE;
> +       uint32_t val = I915_READ(EDP_PSR_CTL);
>
> +       val |= EDP_PSR_ENABLE;
> +
> +       val &= ~EDP_PSR_MAX_SLEEP_TIME_MASK;
>         val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> +
> +       val &= ~EDP_PSR_IDLE_FRAME_MASK;
>         val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>
> +       val &= ~EDP_PSR_MIN_LINK_ENTRY_TIME_MASK;
>         if (IS_HASWELL(dev_priv))
>                 val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
> -       if (dev_priv->psr.link_standby)
> +       if (dev_priv->psr.link_standby) {
>                 val |= EDP_PSR_LINK_STANDBY;
>
> +               /* SFU should only be enabled with link standby, but for
> +                * now we do not support it. */
> +               val &= ~BDW_PSR_SINGLE_FRAME;
> +       } else {
> +               val &= ~EDP_PSR_LINK_STANDBY;
> +               val &= ~BDW_PSR_SINGLE_FRAME;
> +       }
> +
> +       val &= ~EDP_PSR_TP1_TIME_MASK;
>         if (dev_priv->vbt.psr.tp1_wakeup_time > 5)
>                 val |= EDP_PSR_TP1_TIME_2500us;
>         else if (dev_priv->vbt.psr.tp1_wakeup_time > 1)
> @@ -300,6 +315,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>         else
>                 val |= EDP_PSR_TP1_TIME_0us;
>
> +       val &= ~EDP_PSR_TP2_TP3_TIME_MASK;
>         if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>                 val |= EDP_PSR_TP2_TP3_TIME_2500us;
>         else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> @@ -309,6 +325,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>         else
>                 val |= EDP_PSR_TP2_TP3_TIME_0us;
>
> +       val &= ~EDP_PSR_TP1_TP3_SEL;
>         if (intel_dp_source_supports_hbr2(intel_dp) &&
>             drm_dp_tps3_supported(intel_dp->dpcd))
>                 val |= EDP_PSR_TP1_TP3_SEL;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/4] drm/i915/psr: Account for sink CRC raciness on some panels

[Rodrigo Vivi]

my only bikeshed here is the subject since this is not the psr feature itself...

anyways, I don't mind much since sink crc is only used to test psr...

Also yes, based on what I saw on sink crc behaviour I know that this
doesn't fix things for real because I remember reading hundreds or
thousand incorrect values with on increment on count...
But anyways I believe that this improve the situation

So,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


On Fri, Jun 30, 2017 at 12:08 PM, Jim Bride <jim.bride@linux.intel.com> wrote:
> According to the eDP spec, when the count field in TEST_SINK_MISC
> increments then the six bytes of sink CRC information in the DPCD
> should be valid.  Unfortunately, this doesn't seem to be the case
> on some panels, and as a result we get some incorrect and inconsistent
> values from the sink CRC DPCD locations at times.  This problem exhibits
> itself more on faster processors (relative failure rates HSW < SKL < KBL.)
> In order to try and account for this, we try a lot harder to read the sink
> CRC until we get consistent values twice in a row before returning what we
> read and delay for a time before trying to read.  We still see some
> occasional failures, but reading the sink CRC is much more reliable,
> particularly on SKL and KBL, with these changes than without.
>
> v2: * Reduce number of retries when reading the sink CRC (Jani)
>     * Refactor to minimize changes to the code (Jani)
>     * Rebase
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b46fa03..1fe0975 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3927,6 +3927,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>         u8 buf;
>         int count, ret;
>         int attempts = 6;
> +       u8 old_crc[6];
> +
> +       if (crc != NULL) {
> +               memset(crc, 0, 6);
> +               memset(old_crc, 0xff, 6);
> +       } else {
> +               return -ENOMEM;
> +       }
>
>         ret = intel_dp_sink_crc_start(intel_dp);
>         if (ret)
> @@ -3950,11 +3958,35 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>                 goto stop;
>         }
>
> -       if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
> -               ret = -EIO;
> -               goto stop;
> -       }
> +       attempts = 6;
> +
> +       /*
> +        * Sometimes it takes a while for the "real" CRC values to land in
> +        * the DPCD, so try several times until we get two reads in a row
> +        * that are the same.  If we're an eDP panel, delay between reads
> +        * for a while since the values take a bit longer to propagate.
> +        */
> +       do {
> +               intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> +               if (is_edp(intel_dp))
> +                       usleep_range(20000, 25000);
> +
> +               if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR,
> +                                    crc, 6) < 0) {
> +                       ret = -EIO;
> +                       goto stop;
> +               }
> +
> +               if (memcmp(old_crc, crc, 6) == 0) {
> +                       ret = 0;
> +                       goto stop;
> +               } else {
> +                       memcpy(old_crc, crc, 6);
> +               }
> +       } while (--attempts);
>
> +       DRM_DEBUG_KMS("Failed to get CRC after 6 attempts.\n");
> +       ret = -ETIMEDOUT;
>  stop:
>         intel_dp_sink_crc_stop(intel_dp);
>         return ret;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx