[PATCH v4 RESEND 0/4] Kernel PSR Fix-ups

[PATCH v4 RESEND 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

v3:
   * Re-ordered patches to put reviewed patches first.
   * Rebased to current drm-tip

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

 drivers/gpu/drm/i915/i915_reg.h               |  4 ++
 drivers/gpu/drm/i915/intel_dp.c               | 71 +++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_dp_link_training.c | 15 +++++-
 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            |  6 +++
 drivers/gpu/drm/i915/intel_psr.c              | 21 +++++++-
 9 files changed, 113 insertions(+), 15 deletions(-)

-- 
2.7.4

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

[PATCH v4 RESEND 1/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
v3-v4: * 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>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@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 c712d01..3e62429 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 v4 RESEND 2/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
v3: * Rebase
v4: * Switch from do-while to for loop when reading CRC values (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 | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2d42d09..c90ca1c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3906,6 +3906,11 @@ 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) {
+		return -ENOMEM;
+	}
 
 	ret = intel_dp_sink_crc_start(intel_dp);
 	if (ret)
@@ -3929,11 +3934,33 @@ 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;
+	/*
+	 * 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.
+	 */
+	for (attempts = 0; attempts < 6; attempts++) {
+		intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
+
+		if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR,
+				     crc, 6) < 0) {
+			ret = -EIO;
+			break;
+		}
+
+		if (attempts && memcmp(old_crc, crc, 6) == 0)
+			break;
+		memcpy(old_crc, crc, 6);
+
+		if (is_edp(intel_dp))
+			usleep_range(20000, 25000);
 	}
 
+	if (attempts == 6) {
+		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

[PATCH v4 RESEND 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 and v3: Rebase
v4: * Clean up accesses to train_set_valid a bit for easier reading. (Chris)
    * Rebase

Cc: Chris Wilson <chris@chris-wilson.co.uk>
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 | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h              |  2 ++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c90ca1c..7c0e530 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);
 
@@ -4738,6 +4738,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);
@@ -6008,6 +6009,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..d12200d 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,9 +163,18 @@ 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;
 	}
 
+	/*
+	 * The initial set of link parameters are set by this point, so go
+	 * ahead and set intel_dp->train_set_valid to false in case any of
+	 * the succeeding steps fail.  It will be set back to true if we were
+	 * able to achieve clock recovery in the specified configuration.
+	 */
+	intel_dp->train_set_valid = false;
+
 	voltage_tries = 1;
 	max_vswing_tries = 0;
 	for (;;) {
@@ -179,6 +189,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
 
 		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;
 		}
 
@@ -256,6 +267,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 +308,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 0902d7c..e45163a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -995,6 +995,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;
@@ -1570,6 +1571,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 v4 RESEND 4/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 and v3: Rebase
v4: * Fix up some leaky mode stuff (Chris)
    * Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
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 |  6 ++++++
 6 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7c0e530..c9db0e6 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;
@@ -5810,6 +5831,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;
@@ -5865,13 +5887,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 if (!alt_fixed_mode) {
+			alt_fixed_mode = drm_mode_duplicate(dev, scan);
 		}
 	}
 
@@ -5908,7 +5931,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 e45163a..3bd11e2 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 */
@@ -1698,6 +1699,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..aa59b12 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;
@@ -1937,6 +1939,10 @@ void intel_panel_fini(struct intel_panel *panel)
 	if (panel->fixed_mode)
 		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
 
+	if (panel->alt_fixed_mode)
+		drm_mode_destroy(intel_connector->base.dev,
+				panel->alt_fixed_mode);
+
 	if (panel->downclock_mode)
 		drm_mode_destroy(intel_connector->base.dev,
 				panel->downclock_mode);
-- 
2.7.4

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

Re: [PATCH v4 RESEND 0/4] Kernel PSR Fix-ups

[David Weinehall]

On Tue, Jul 25, 2017 at 09:48:07AM -0700, Jim Bride wrote:
> 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
> 
> v3:
>    * Re-ordered patches to put reviewed patches first.
>    * Rebased to current drm-tip
> 
> v4: 
>    * Addressed review feedback (see patches for details)
>    * Rebase

Is this a pure resend, or does it include any fixes on top of earlier
versions? As mentioned elsewhere I experienced issues with both your
previous patch series and the two before that one.

I'll run a new testrun with this series just in case (it might be that
the issues I noticed were caused by bad interaction with some other
component).


Kind regards, David Weinehall
_______________________________________________
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

[Patchwork]

== Series Details ==

Series: Kernel PSR Fix-ups
URL   : https://patchwork.freedesktop.org/series/27879/
State : failure

== Summary ==

Series 27879v1 Kernel PSR Fix-ups
https://patchwork.freedesktop.org/api/1.0/series/27879/revisions/1/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-skl-6700hq)
                pass       -> INCOMPLETE (fi-kbl-r)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-n2820) fdo#101705

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  time:448s
fi-blb-e6850     total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  time:363s
fi-bsw-n3050     total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  time:514s
fi-byt-j1900     total:280  pass:255  dwarn:1   dfail:0   fail:0   skip:24  time:493s
fi-byt-n2820     total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  time:486s
fi-hsw-4770      total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:439s
fi-hsw-4770r     total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:420s
fi-ilk-650       total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7500u     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:583s
fi-kbl-r         total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-pnv-d510      total:280  pass:222  dwarn:3   dfail:0   fail:0   skip:55  time:567s
fi-skl-6260u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:465s
fi-skl-6700hq    total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6700k     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-skl-6770hq    total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:280  pass:266  dwarn:1   dfail:0   fail:0   skip:13  time:436s
fi-skl-x1585l    total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  time:472s
fi-snb-2520m     total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  time:550s
fi-snb-2600      total:280  pass:251  dwarn:0   dfail:0   fail:0   skip:29  time:406s
fi-glk-2a failed to connect after reboot

92845e5dff13907ccebff6b9ae742e793ddc6a74 drm-tip: 2017y-07m-25d-14h-41m-13s UTC integration manifest
a6ed5a05fa72 drm/i915/edp: Allow alternate fixed mode for eDP if available.
898075eedd5c drm/i915/edp: Be less aggressive about changing link config on eDP
0d851135ba98 drm/i915/psr: Account for sink CRC raciness on some panels
59a3c66e6c05 drm/i915/psr: Clean-up intel_enable_source_psr1()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5275/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 RESEND 0/4] Kernel PSR Fix-ups

[Jim Bride]

On Tue, Jul 25, 2017 at 08:13:03PM +0300, David Weinehall wrote:
> On Tue, Jul 25, 2017 at 09:48:07AM -0700, Jim Bride wrote:
> > 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
> > 
> > v3:
> >    * Re-ordered patches to put reviewed patches first.
> >    * Rebased to current drm-tip
> > 
> > v4: 
> >    * Addressed review feedback (see patches for details)
> >    * Rebase
> 
> Is this a pure resend, or does it include any fixes on top of earlier
> versions? As mentioned elsewhere I experienced issues with both your
> previous patch series and the two before that one.

Yes, it's a pure resend (I didn't see that CI had picked them up.).
These patches still run fine for me on my HW (all RVPs) on HSW, BDW,
and SKL.  Have you tried to ssh into the systems to see if it's just
the display having issues vs. the whole system being hung?

Jim


> I'll run a new testrun with this series just in case (it might be that
> the issues I noticed were caused by bad interaction with some other
> component).
> 
> 
> Kind regards, David Weinehall
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v5] 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 and v3: Rebase
v4: * Fix up some leaky mode stuff (Chris)
    * Rebase
v5: * Fix a NULL pointer derefreence (David Weinehall)

Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c    | 38 +++++++++++++++++++++++++++++++++-----
 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 |  6 ++++++
 6 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7c0e530..60c4642 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1606,6 +1606,23 @@ 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)
+{
+	bool bres = false;
+	
+	if (m1 && m2)
+		bres =  (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);
+	return bres;
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config,
@@ -1652,8 +1669,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;
@@ -5810,6 +5835,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;
@@ -5865,13 +5891,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 if (!alt_fixed_mode) {
+			alt_fixed_mode = drm_mode_duplicate(dev, scan);
 		}
 	}
 
@@ -5908,7 +5935,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 046d358..00bde65 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 */
@@ -1682,6 +1683,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 fd2e081..e7df774 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1920,11 +1920,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;
@@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
 	if (panel->fixed_mode)
 		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
 
+	if (panel->alt_fixed_mode)
+		drm_mode_destroy(intel_connector->base.dev,
+				panel->alt_fixed_mode);
+
 	if (panel->downclock_mode)
 		drm_mode_destroy(intel_connector->base.dev,
 				panel->downclock_mode);
-- 
2.7.4

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

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

[Patchwork]

== Series Details ==

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

== Summary ==

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

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

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

fi-bdw-5557u     total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  time:436s
fi-blb-e6850     total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  time:352s
fi-bsw-n3050     total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  time:542s
fi-bxt-j4205     total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  time:518s
fi-byt-j1900     total:280  pass:255  dwarn:1   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:280  pass:251  dwarn:1   dfail:0   fail:0   skip:28  time:490s
fi-glk-2a        total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  time:604s
fi-hsw-4770      total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:436s
fi-hsw-4770r     total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  time:420s
fi-ilk-650       total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  time:416s
fi-ivb-3520m     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:506s
fi-ivb-3770      total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:477s
fi-kbl-7500u     total:280  pass:261  dwarn:1   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:587s
fi-kbl-r         total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:587s
fi-pnv-d510      total:280  pass:222  dwarn:3   dfail:0   fail:0   skip:55  time:558s
fi-skl-6260u     total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:461s
fi-skl-6700hq    total:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  time:588s
fi-skl-6700k     total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-skl-6770hq    total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  time:475s
fi-skl-gvtdvm    total:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-skl-x1585l    total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  time:476s
fi-snb-2520m     total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  time:555s
fi-snb-2600      total:280  pass:250  dwarn:0   dfail:0   fail:1   skip:29  time:407s

520e660a206f68917780f87db4e77e28e0b2a6f6 drm-tip: 2017y-07m-28d-12h-24m-40s UTC integration manifest
a72ec9e29f1a drm/i915/edp: Allow alternate fixed mode for eDP if available.
e2e627efb52f drm/i915/edp: Be less aggressive about changing link config on eDP
3e85beeba699 drm/i915/psr: Account for sink CRC raciness on some panels
3c1b5f76ea5a drm/i915/psr: Clean-up intel_enable_source_psr1()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5295/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5] drm/i915/edp: Allow alternate fixed mode for eDP if available.

[David Weinehall]

On Fri, Jul 28, 2017 at 12:22:29PM -0700, Jim Bride wrote:
> 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 and v3: Rebase
> v4: * Fix up some leaky mode stuff (Chris)
>     * Rebase
> v5: * Fix a NULL pointer derefreence (David Weinehall)

dereference

I've tested your patch series, and with the v5 version of this patch
plus the v4 versions of the other patches my Skylake laptop no longer
oopses on boot. I've also tested this on a Haswell laptop that had
working PSR even without this patch, to make sure that this doesn't
introduce any regressions.

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

with minor nitpicks.

> 
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c    | 38 +++++++++++++++++++++++++++++++++-----
>  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 |  6 ++++++
>  6 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7c0e530..60c4642 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1606,6 +1606,23 @@ 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)
> +{
> +	bool bres = false;
> +	

Whitespace error (git apply warns)

> +	if (m1 && m2)
> +		bres =  (m1->hdisplay == m2->hdisplay &&

Dunno if it warns about this too, but methinks that double space is
erroneous.

> +			 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);
> +	return bres;
> +}
> +
>  bool
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_state *pipe_config,
> @@ -1652,8 +1669,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;
> @@ -5810,6 +5835,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;
> @@ -5865,13 +5891,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 if (!alt_fixed_mode) {
> +			alt_fixed_mode = drm_mode_duplicate(dev, scan);
>  		}
>  	}
>  
> @@ -5908,7 +5935,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 046d358..00bde65 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 */
> @@ -1682,6 +1683,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 fd2e081..e7df774 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1920,11 +1920,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;
> @@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
>  	if (panel->fixed_mode)
>  		drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
>  
> +	if (panel->alt_fixed_mode)
> +		drm_mode_destroy(intel_connector->base.dev,
> +				panel->alt_fixed_mode);
> +
>  	if (panel->downclock_mode)
>  		drm_mode_destroy(intel_connector->base.dev,
>  				panel->downclock_mode);
> -- 
> 2.7.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 RESEND 1/4] drm/i915/psr: Clean-up intel_enable_source_psr1()

[Rodrigo Vivi]

what about the approach suggested by Chris and acked by Jani?
will we simply ignore? or did you discussed with them on irc?

On Tue, Jul 25, 2017 at 9:48 AM, 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
> v3-v4: * 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>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@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 c712d01..3e62429 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 v4 RESEND 3/4] drm/i915/edp: Be less aggressive about changing link config on eDP

[Rodrigo Vivi]

are all concerns from Chris and Manasi attended here on this resend?

On Tue, Jul 25, 2017 at 9:48 AM, Jim Bride <jim.bride@linux.intel.com> 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@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 and v3: Rebase
> v4: * Clean up accesses to train_set_valid a bit for easier reading. (Chris)
>     * Rebase
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 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 | 15 ++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h              |  2 ++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c90ca1c..7c0e530 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);
>
> @@ -4738,6 +4738,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);
> @@ -6008,6 +6009,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..d12200d 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,9 +163,18 @@ 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;
>         }
>
> +       /*
> +        * The initial set of link parameters are set by this point, so go
> +        * ahead and set intel_dp->train_set_valid to false in case any of
> +        * the succeeding steps fail.  It will be set back to true if we were
> +        * able to achieve clock recovery in the specified configuration.
> +        */
> +       intel_dp->train_set_valid = false;
> +
>         voltage_tries = 1;
>         max_vswing_tries = 0;
>         for (;;) {
> @@ -179,6 +189,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>
>                 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;
>                 }
>
> @@ -256,6 +267,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 +308,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 0902d7c..e45163a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -995,6 +995,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;
> @@ -1570,6 +1571,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



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