[PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw

[PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw

[Daniel Vetter]

The definitions for the error register should be valid on bdw/skl too,
but there we haven't even enabled DE_MISC handling yet.

Somewhat confusing the the moved register offset on bdw is only for
the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
on bdw.

v2: Fixes from Ville.

v3: Drop the REG_WRITE masking since edp interrupt still work and we
still enter PSR.

Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f0d941455bed..579f582ef987 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2185,6 +2185,26 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
 		ironlake_rps_change_irq_handler(dev_priv);
 }
 
+static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
+{
+	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+
+	if (edp_psr_iir & EDP_PSR_ERROR)
+		DRM_DEBUG_KMS("PSR error\n");
+
+	if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
+		DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
+		I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
+	}
+
+	if (edp_psr_iir & EDP_PSR_POST_EXIT) {
+		DRM_DEBUG_KMS("PSR exit completed\n");
+		I915_WRITE(EDP_PSR_IMR, 0);
+	}
+
+	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
+}
+
 static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
 				    u32 de_iir)
 {
@@ -2197,6 +2217,9 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
 	if (de_iir & DE_ERR_INT_IVB)
 		ivb_err_int_handler(dev_priv);
 
+	if (de_iir & DE_EDP_PSR_INT_HSW)
+		hsw_edp_psr_irq_handler(dev_priv);
+
 	if (de_iir & DE_AUX_CHANNEL_A_IVB)
 		dp_aux_irq_handler(dev_priv);
 
@@ -3381,6 +3404,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(EDP_PSR_IMR, 0xffffffff);
+		I915_WRITE(EDP_PSR_IIR, 0xffffffff);
+	}
+
 	gen5_gt_irq_reset(dev);
 
 	ibx_irq_reset(dev);
@@ -3676,6 +3704,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
 				DE_PLANEB_FLIP_DONE_IVB |
 				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
+
 		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
 			      DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
 			      DE_DP_A_HOTPLUG_IVB);
@@ -3690,6 +3719,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 			      DE_DP_A_HOTPLUG);
 	}
 
+	if (IS_HASWELL(dev)) {
+		gen5_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+		I915_WRITE(EDP_PSR_IMR, 0);
+		display_mask |= DE_EDP_PSR_INT_HSW;
+	}
+
 	dev_priv->irq_mask = ~display_mask;
 
 	I915_WRITE(HWSTAM, 0xeffe);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 86fbf723eca7..0f99e67f2114 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3225,6 +3225,13 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_TP1_TIME_0us			(3<<4)
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0
 
+/* Bspec claims those aren't shifted but stay at 0x64800 */
+#define EDP_PSR_IMR				_MMIO(0x64834)
+#define EDP_PSR_IIR				_MMIO(0x64838)
+#define   EDP_PSR_ERROR				(1<<2)
+#define   EDP_PSR_POST_EXIT			(1<<1)
+#define   EDP_PSR_PRE_ENTRY			(1<<0)
+
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
 
@@ -3259,6 +3266,7 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_DEBUG_MASK_LPSP	(1<<27)
 #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
 #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
+#define   EDP_PSR_DEBUG_MASK_REG_WRITE	(1<<16)
 
 #define EDP_PSR2_CTL			_MMIO(0x6f900)
 #define   EDP_PSR2_ENABLE		(1<<31)
@@ -5886,6 +5894,7 @@ enum skl_disp_power_wells {
 #define DE_PCH_EVENT_IVB		(1<<28)
 #define DE_DP_A_HOTPLUG_IVB		(1<<27)
 #define DE_AUX_CHANNEL_A_IVB		(1<<26)
+#define DE_EDP_PSR_INT_HSW		(1<<19)
 #define DE_SPRITEC_FLIP_DONE_IVB	(1<<14)
 #define DE_PLANEC_FLIP_DONE_IVB		(1<<13)
 #define DE_PIPEC_VBLANK_IVB		(1<<10)
@@ -5998,6 +6007,7 @@ enum skl_disp_power_wells {
 #define GEN8_DE_MISC_IIR _MMIO(0x44468)
 #define GEN8_DE_MISC_IER _MMIO(0x4446c)
 #define  GEN8_DE_MISC_GSE		(1 << 27)
+#define  GEN8_DE_EDP_PSR		(1 << 19)
 
 #define GEN8_PCU_ISR _MMIO(0x444e0)
 #define GEN8_PCU_IMR _MMIO(0x444e4)
-- 
2.8.1

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

[PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Daniel Vetter]

Oops. Hw default for programming these fields to 0 is "skip link
training". Display won't take that too well usually.

v2: Unbotch the math a bit.

v3: Drop debug hunk.

Tested-by: Lyude <cpaul@redhat.com>
Cc: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c3abae4bc596..a788d1e9589b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
+	uint32_t val = EDP_PSR_ENABLE;
+
+	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
+	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
 	if (IS_HASWELL(dev))
 		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
@@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 	if (dev_priv->psr.link_standby)
 		val |= EDP_PSR_LINK_STANDBY;
 
-	I915_WRITE(EDP_PSR_CTL, val |
-		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
-		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
-		   EDP_PSR_ENABLE);
+	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)
+		val |= EDP_PSR_TP1_TIME_500us;
+	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
+		val |= EDP_PSR_TP1_TIME_100us;
+	else
+		val |= EDP_PSR_TP1_TIME_0us;
+
+	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)
+		val |= EDP_PSR_TP2_TP3_TIME_500us;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
+		val |= EDP_PSR_TP2_TP3_TIME_100us;
+	else
+		val |= EDP_PSR_TP2_TP3_TIME_0us;
+
+	if (intel_dp_source_supports_hbr2(intel_dp) &&
+	    drm_dp_tps3_supported(intel_dp->dpcd))
+		val |= EDP_PSR_TP1_TP3_SEL;
+	else
+		val |= EDP_PSR_TP1_TP2_SEL;
+
+	I915_WRITE(EDP_PSR_CTL, val);
+
+	if (!dev_priv->psr.psr2_support)
+		return;
+
+	/* FIXME: selective update is probably totally broken because it doesn't
+	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
+	 * good enough. */
+	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
+
+	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
+		val |= EDP_PSR2_TP2_TIME_2500;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
+		val |= EDP_PSR2_TP2_TIME_500;
+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
+		val |= EDP_PSR2_TP2_TIME_100;
+	else
+		val |= EDP_PSR2_TP2_TIME_50;
 
-	if (dev_priv->psr.psr2_support)
-		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
-				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
+	I915_WRITE(EDP_PSR2_CTL, val);
 }
 
 static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
-- 
2.8.1

[PATCH 3/7] drm/i915/psr: Make idle_frames sensible again

[Daniel Vetter]

This reverts

commit dfaf37baa07513d2c37afff79978807d2d10221a
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
Date:   Mon Dec 7 14:45:20 2015 -0800

    drm/i915: Fix idle_frames counter.

and

commit 97173eaf5f33b1e85efdb06d593d333480b60bf3
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
Date:   Tue Jul 7 16:28:55 2015 -0700

    drm/i915: PSR: Increase idle_frames

and implements

commit d44b4dcbd1b44737462b77971d216d21a9413341
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
Date:   Fri Nov 14 08:52:31 2014 -0800

    drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1

without the hack to use 2 idle frames when VBT says 1. We keep the + 1
just for safety, although I haven't really figured out why that one
exists.

It's nonsense. idle_frames = number of frames where the screen is
entirely idle before we think about entering PSR.

idle_patter = part of link training, and we probably totally butchered
link training because we told the hw to entirely skip it. No wonder
PSR occasionally just fell over.

I suspect the reason we've increased idle frames is that it makes PSR
entry slightly less likely, and more likely to happen in a quite
system, which probably increased the changes the panel came back up
without link training. The proper fix is to implement link training
for PSR.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index a788d1e9589b..0295d8dd483f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -272,14 +272,14 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	uint32_t max_sleep_time = 0x1f;
-	/*
-	 * Let's respect VBT in case VBT asks a higher idle_frame value.
-	 * Let's use 6 as the minimum to cover all known cases including
-	 * the off-by-one issue that HW has in some cases. Also there are
-	 * cases where sink should be able to train
-	 * with the 5 or 6 idle patterns.
+	/* Lately it was identified that depending on panel idle frame count
+	 * calculated at HW can be off by 1. So let's use what came
+	 * from VBT + 1.
+	 * There are also other cases where panel demands at least 4
+	 * but VBT is not being set. To cover these 2 cases lets use
+	 * at least 5 when VBT isn't set to be on the safest side.
 	 */
-	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
+	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames + 1;
 	uint32_t val = EDP_PSR_ENABLE;
 
 	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
-- 
2.8.1

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

[PATCH 4/7] drm/i915/psr: Skip aux handeshake if the vbt tells us to

[Daniel Vetter]

Not sure we can trust VBT on this one, but let's try.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0295d8dd483f..a088be65a267 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -315,6 +315,9 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 	else
 		val |= EDP_PSR_TP1_TP2_SEL;
 
+	if (!dev_priv->vbt.psr.require_aux_wakeup)
+		val |= EDP_PSR_SKIP_AUX_EXIT;
+
 	I915_WRITE(EDP_PSR_CTL, val);
 
 	if (!dev_priv->psr.psr2_support)
-- 
2.8.1

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

[PATCH 5/7] drm/i915/psr: Order DP aux transactions correctly

[Daniel Vetter]

On bdw/hsw we have a separate psr dp aux registers to set up, but on
bdw it's shared with the main dp aux thing. Which means any subsequent
dp aux transaction will trample over it, and hence must be done
beforehand.

Also this means we can't do any dp aux transactions while PSR is
active, or at least we must restore the old state.

Probably need a psr disable/enable pair around dp aux transactions in
general.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index a088be65a267..87805078c8a3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -197,6 +197,13 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 				DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
 				DP_AUX_FRAME_SYNC_ENABLE);
 
+	if (dev_priv->psr.link_standby)
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
+	else
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE);
+
 	aux_ctl_reg = psr_aux_ctl_reg(dev_priv, port);
 
 	/* Setup AUX registers */
@@ -224,13 +231,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
 		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
 	}
-
-	if (dev_priv->psr.link_standby)
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
-	else
-		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE);
 }
 
 static void vlv_psr_enable_source(struct intel_dp *intel_dp)
-- 
2.8.1

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

[PATCH 6/7] drm/i915/psr: Use ->get_aux_send_ctl functions

[Daniel Vetter]

I just wanted to get rid of the rmw cycle for gen9, but this also
fixes some bugs we haven't carried over, like using recommended
precharge and timeout values.

Also I noticed that we don't set the fastwake sync length on skl, and
that's used by PSR2 selective updates. Fix that.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 25 ++++---------------------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 36330026ceff..cccf9bc7c7d6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -770,6 +770,7 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_TIME_OUT_1600us |
 	       DP_AUX_CH_CTL_RECEIVE_ERROR |
 	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+	       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 87805078c8a3..f9ce47135bb4 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -176,7 +176,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t aux_clock_divider;
 	i915_reg_t aux_ctl_reg;
-	int precharge = 0x3;
 	static const uint8_t aux_msg[] = {
 		[0] = DP_AUX_NATIVE_WRITE << 4,
 		[1] = DP_SET_POWER >> 8,
@@ -185,6 +184,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		[4] = DP_SET_POWER_D0,
 	};
 	enum port port = dig_port->port;
+	u32 aux_ctl;
 	int i;
 
 	BUILD_BUG_ON(sizeof(aux_msg) > 20);
@@ -211,26 +211,9 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		I915_WRITE(psr_aux_data_reg(dev_priv, port, i >> 2),
 			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
 
-	if (INTEL_INFO(dev)->gen >= 9) {
-		uint32_t val;
-
-		val = I915_READ(aux_ctl_reg);
-		val &= ~DP_AUX_CH_CTL_TIME_OUT_MASK;
-		val |= DP_AUX_CH_CTL_TIME_OUT_1600us;
-		val &= ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK;
-		val |= (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
-		/* Use hardcoded data values for PSR, frame sync and GTC */
-		val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
-		val &= ~DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL;
-		val &= ~DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL;
-		I915_WRITE(aux_ctl_reg, val);
-	} else {
-		I915_WRITE(aux_ctl_reg,
-		   DP_AUX_CH_CTL_TIME_OUT_400us |
-		   (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
-		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
-	}
+	aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
+					     aux_clock_divider);
+	I915_WRITE(aux_ctl_reg, aux_ctl);
 }
 
 static void vlv_psr_enable_source(struct intel_dp *intel_dp)
-- 
2.8.1

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

[PATCH 7/7] drm/i915/psr: Implement PSR2 w/a for skl/kbl

[Daniel Vetter]

Found this while browsing Bspec. Looks like it applies to both skl and
kbl.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0f99e67f2114..c51368744e9e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
 #define CHICKEN_PAR1_1		_MMIO(0x42080)
 #define  DPA_MASK_VBLANK_SRD	(1 << 15)
 #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
+#define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
 
 #define _CHICKEN_PIPESL_1_A	0x420b0
 #define _CHICKEN_PIPESL_1_B	0x420b4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e0d5405a8b15..c583d1de4555 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6845,6 +6845,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
 	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
 }
 
+static void skylake_init_clock_gating(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* See Bspec note for PSR2_CTL bit 31 */
+	I915_WRITE(CHICKEN_PAR1_1,
+		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
+}
+
 static void broadwell_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7307,9 +7316,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
 void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
 {
 	if (IS_SKYLAKE(dev_priv))
-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
 	else if (IS_KABYLAKE(dev_priv))
-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
 	else if (IS_BROXTON(dev_priv))
 		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
 	else if (IS_BROADWELL(dev_priv))
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw
URL   : https://patchwork.freedesktop.org/series/7357/
State : failure

== Summary ==

Series 7357v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7357/revisions/1/mbox

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-hsw-i7-4770r)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-FAIL (ro-skl-i7-6700hq)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (fi-hsw-i7-4770r)
Test kms_sink_crc_basic:
                pass       -> SKIP       (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
fi-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
fi-hsw-i7-4770k  total:219  pass:198  dwarn:0   dfail:0   fail:0   skip:21 
fi-hsw-i7-4770r  total:219  pass:192  dwarn:0   dfail:0   fail:0   skip:27 
fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:193  dwarn:0   dfail:0   fail:1   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-skl-i7-6700hq total:214  pass:188  dwarn:0   dfail:1   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
fi-bsw-n3050 failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_930/

1f8b54c drm-intel-nightly: 2016y-05m-18d-11h-49m-33s UTC integration manifest
955971e drm/i915/psr: Implement PSR2 w/a for skl/kbl
b6eb142 drm/i915/psr: Use ->get_aux_send_ctl functions
6022e2b drm/i915/psr: Order DP aux transactions correctly
4fc9942 drm/i915/psr: Skip aux handeshake if the vbt tells us to
5dabde3 drm/i915/psr: Make idle_frames sensible again
58e54d4 drm/i915/psr: Try to program link training times correctly
518be6c drm/i915: Enable edp psr error interrupts on hsw

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

Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote:
> Oops. Hw default for programming these fields to 0 is "skip link
> training". Display won't take that too well usually.

s/skip/500 usec/

> 
> v2: Unbotch the math a bit.
> 
> v3: Drop debug hunk.
> 
> Tested-by: Lyude <cpaul@redhat.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c3abae4bc596..a788d1e9589b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
> +	uint32_t val = EDP_PSR_ENABLE;
> +
> +	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> +	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
>  	if (IS_HASWELL(dev))
>  		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
> @@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>  	if (dev_priv->psr.link_standby)
>  		val |= EDP_PSR_LINK_STANDBY;
>  
> -	I915_WRITE(EDP_PSR_CTL, val |
> -		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> -		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> -		   EDP_PSR_ENABLE);
> +	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)
> +		val |= EDP_PSR_TP1_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
> +		val |= EDP_PSR_TP1_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP1_TIME_0us;
> +
> +	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)
> +		val |= EDP_PSR_TP2_TP3_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR_TP2_TP3_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP2_TP3_TIME_0us;

The current vbt spec is confusing. But after some head scratching this
does look correct.

Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

> +
> +	if (intel_dp_source_supports_hbr2(intel_dp) &&
> +	    drm_dp_tps3_supported(intel_dp->dpcd))
> +		val |= EDP_PSR_TP1_TP3_SEL;
> +	else
> +		val |= EDP_PSR_TP1_TP2_SEL;
> +
> +	I915_WRITE(EDP_PSR_CTL, val);
> +
> +	if (!dev_priv->psr.psr2_support)
> +		return;
> +
> +	/* FIXME: selective update is probably totally broken because it doesn't
> +	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> +	 * good enough. */
> +	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> +
> +	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> +		val |= EDP_PSR2_TP2_TIME_2500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> +		val |= EDP_PSR2_TP2_TIME_500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR2_TP2_TIME_100;
> +	else
> +		val |= EDP_PSR2_TP2_TIME_50;
>  
> -	if (dev_priv->psr.psr2_support)
> -		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> -				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> +	I915_WRITE(EDP_PSR2_CTL, val);
>  }
>  
>  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrj�l�
Intel OTC

Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote:
> Oops. Hw default for programming these fields to 0 is "skip link
> training". Display won't take that too well usually.

s/skip/500 usec/

> 
> v2: Unbotch the math a bit.
> 
> v3: Drop debug hunk.
> 
> Tested-by: Lyude <cpaul@redhat.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c3abae4bc596..a788d1e9589b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
> +	uint32_t val = EDP_PSR_ENABLE;
> +
> +	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> +	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
>  	if (IS_HASWELL(dev))
>  		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
> @@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>  	if (dev_priv->psr.link_standby)
>  		val |= EDP_PSR_LINK_STANDBY;
>  
> -	I915_WRITE(EDP_PSR_CTL, val |
> -		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> -		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> -		   EDP_PSR_ENABLE);
> +	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)
> +		val |= EDP_PSR_TP1_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
> +		val |= EDP_PSR_TP1_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP1_TIME_0us;
> +
> +	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)
> +		val |= EDP_PSR_TP2_TP3_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR_TP2_TP3_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP2_TP3_TIME_0us;

The current vbt spec is confusing. But after some head scratching this
does look correct.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	if (intel_dp_source_supports_hbr2(intel_dp) &&
> +	    drm_dp_tps3_supported(intel_dp->dpcd))
> +		val |= EDP_PSR_TP1_TP3_SEL;
> +	else
> +		val |= EDP_PSR_TP1_TP2_SEL;
> +
> +	I915_WRITE(EDP_PSR_CTL, val);
> +
> +	if (!dev_priv->psr.psr2_support)
> +		return;
> +
> +	/* FIXME: selective update is probably totally broken because it doesn't
> +	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> +	 * good enough. */
> +	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> +
> +	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> +		val |= EDP_PSR2_TP2_TIME_2500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> +		val |= EDP_PSR2_TP2_TIME_500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR2_TP2_TIME_100;
> +	else
> +		val |= EDP_PSR2_TP2_TIME_50;
>  
> -	if (dev_priv->psr.psr2_support)
> -		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> -				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> +	I915_WRITE(EDP_PSR2_CTL, val);
>  }
>  
>  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/7] drm/i915/psr: Make idle_frames sensible again

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:12PM +0200, Daniel Vetter wrote:
> This reverts
> 
> commit dfaf37baa07513d2c37afff79978807d2d10221a
> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Date:   Mon Dec 7 14:45:20 2015 -0800
> 
>     drm/i915: Fix idle_frames counter.
> 
> and
> 
> commit 97173eaf5f33b1e85efdb06d593d333480b60bf3
> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Date:   Tue Jul 7 16:28:55 2015 -0700
> 
>     drm/i915: PSR: Increase idle_frames
> 
> and implements
> 
> commit d44b4dcbd1b44737462b77971d216d21a9413341
> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Date:   Fri Nov 14 08:52:31 2014 -0800
> 
>     drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1
> 
> without the hack to use 2 idle frames when VBT says 1. We keep the + 1
> just for safety, although I haven't really figured out why that one
> exists.
> 
> It's nonsense. idle_frames = number of frames where the screen is
> entirely idle before we think about entering PSR.
> 
> idle_patter = part of link training, and we probably totally butchered
> link training because we told the hw to entirely skip it. No wonder
> PSR occasionally just fell over.
> 
> I suspect the reason we've increased idle frames is that it makes PSR
> entry slightly less likely, and more likely to happen in a quite
> system, which probably increased the changes the panel came back up
> without link training. The proper fix is to implement link training
> for PSR.

Quite the mess there. At the least this makes things look a bit more
sane.

FWIW
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index a788d1e9589b..0295d8dd483f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -272,14 +272,14 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	uint32_t max_sleep_time = 0x1f;
> -	/*
> -	 * Let's respect VBT in case VBT asks a higher idle_frame value.
> -	 * Let's use 6 as the minimum to cover all known cases including
> -	 * the off-by-one issue that HW has in some cases. Also there are
> -	 * cases where sink should be able to train
> -	 * with the 5 or 6 idle patterns.
> +	/* Lately it was identified that depending on panel idle frame count
> +	 * calculated at HW can be off by 1. So let's use what came
> +	 * from VBT + 1.
> +	 * There are also other cases where panel demands at least 4
> +	 * but VBT is not being set. To cover these 2 cases lets use
> +	 * at least 5 when VBT isn't set to be on the safest side.
>  	 */
> -	uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> +	uint32_t idle_frames = dev_priv->vbt.psr.idle_frames + 1;
>  	uint32_t val = EDP_PSR_ENABLE;
>  
>  	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/psr: Skip aux handeshake if the vbt tells us to

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:13PM +0200, Daniel Vetter wrote:
> Not sure we can trust VBT on this one, but let's try.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0295d8dd483f..a088be65a267 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -315,6 +315,9 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>  	else
>  		val |= EDP_PSR_TP1_TP2_SEL;
>  
> +	if (!dev_priv->vbt.psr.require_aux_wakeup)
> +		val |= EDP_PSR_SKIP_AUX_EXIT;

At least on my HSW HSB machine that did the wrong thing apparently. I'll
send out my patches to look at the DPCD as well here...

> +
>  	I915_WRITE(EDP_PSR_CTL, val);
>  
>  	if (!dev_priv->psr.psr2_support)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915/psr: Order DP aux transactions correctly

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:14PM +0200, Daniel Vetter wrote:
> On bdw/hsw we have a separate psr dp aux registers to set up, but on
> bdw it's shared with the main dp aux thing. Which means any subsequent
> dp aux transaction will trample over it, and hence must be done
> beforehand.
> 
> Also this means we can't do any dp aux transactions while PSR is
> active, or at least we must restore the old state.
> 
> Probably need a psr disable/enable pair around dp aux transactions in
> general.

Yep. Looks like there's a note even in the HSW spec to forbid AUX while
in PSR, even though it has the separate regs.

"SRD AUX channel transactions must not be sent while DDI A AUX is being
 used. SRD must be completely disabled before a DDI A AUX channel
 transaction can be sent."

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index a088be65a267..87805078c8a3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -197,6 +197,13 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  				DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
>  				DP_AUX_FRAME_SYNC_ENABLE);
>  
> +	if (dev_priv->psr.link_standby)
> +		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> +				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> +	else
> +		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> +				   DP_PSR_ENABLE);
> +
>  	aux_ctl_reg = psr_aux_ctl_reg(dev_priv, port);
>  
>  	/* Setup AUX registers */
> @@ -224,13 +231,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>  		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>  	}
> -
> -	if (dev_priv->psr.link_standby)
> -		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> -				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> -	else
> -		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> -				   DP_PSR_ENABLE);
>  }
>  
>  static void vlv_psr_enable_source(struct intel_dp *intel_dp)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Daniel Vetter]

On Wed, May 18, 2016 at 7:39 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote:
>> Oops. Hw default for programming these fields to 0 is "skip link
>> training". Display won't take that too well usually.
>
> s/skip/500 usec/

Yeah, my reading skills have reached an all time low ;-) But we
confirmed on irc that the hardcoded 500usec was indeed wrong, since
the fixed machines now run on 2.5ms of link training time. I'll update
the commit message when merging to reflect that. Also

Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: fritsch@kodi.tv
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 6/7] drm/i915/psr: Use ->get_aux_send_ctl functions

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:15PM +0200, Daniel Vetter wrote:
> I just wanted to get rid of the rmw cycle for gen9, but this also
> fixes some bugs we haven't carried over, like using recommended
> precharge and timeout values.
> 
> Also I noticed that we don't set the fastwake sync length on skl, and
> that's used by PSR2 selective updates. Fix that.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 25 ++++---------------------
>  2 files changed, 5 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 36330026ceff..cccf9bc7c7d6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -770,6 +770,7 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	       DP_AUX_CH_CTL_TIME_OUT_1600us |
>  	       DP_AUX_CH_CTL_RECEIVE_ERROR |
>  	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +	       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
>  	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 87805078c8a3..f9ce47135bb4 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -176,7 +176,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t aux_clock_divider;
>  	i915_reg_t aux_ctl_reg;
> -	int precharge = 0x3;
>  	static const uint8_t aux_msg[] = {
>  		[0] = DP_AUX_NATIVE_WRITE << 4,
>  		[1] = DP_SET_POWER >> 8,
> @@ -185,6 +184,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  		[4] = DP_SET_POWER_D0,
>  	};
>  	enum port port = dig_port->port;
> +	u32 aux_ctl;
>  	int i;
>  
>  	BUILD_BUG_ON(sizeof(aux_msg) > 20);
> @@ -211,26 +211,9 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  		I915_WRITE(psr_aux_data_reg(dev_priv, port, i >> 2),
>  			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
>  
> -	if (INTEL_INFO(dev)->gen >= 9) {
> -		uint32_t val;
> -
> -		val = I915_READ(aux_ctl_reg);
> -		val &= ~DP_AUX_CH_CTL_TIME_OUT_MASK;
> -		val |= DP_AUX_CH_CTL_TIME_OUT_1600us;
> -		val &= ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK;
> -		val |= (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
> -		/* Use hardcoded data values for PSR, frame sync and GTC */
> -		val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
> -		val &= ~DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL;
> -		val &= ~DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL;
> -		I915_WRITE(aux_ctl_reg, val);
> -	} else {
> -		I915_WRITE(aux_ctl_reg,
> -		   DP_AUX_CH_CTL_TIME_OUT_400us |
> -		   (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> -		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
> -	}
> +	aux_ctl = intel_dp->get_aux_send_ctl(intel_dp, 0, sizeof(aux_msg),
> +					     aux_clock_divider);
> +	I915_WRITE(aux_ctl_reg, aux_ctl);

The HSW/BDW EDP specific register doesn't have quite all the bits, but
looks like those bits are RO in the PSR register, so no harm in writing
1 to them.

The one difference I noticed was there's an "interrupt on error" bit
(bit 11) in the PSR register that doesn't exist in the normal register.
But we're not setting that bit here, so not a problem.

For SKL+ this makes a ton of sense to me.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

PS. perhaps someone should look into those interrupt bits a bit more?

>  }
>  
>  static void vlv_psr_enable_source(struct intel_dp *intel_dp)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Ville Syrjälä]

On Wed, May 18, 2016 at 08:04:02PM +0200, Daniel Vetter wrote:
> On Wed, May 18, 2016 at 7:39 PM, Ville Syrj�l�
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote:
> >> Oops. Hw default for programming these fields to 0 is "skip link
> >> training". Display won't take that too well usually.
> >
> > s/skip/500 usec/
> 
> Yeah, my reading skills have reached an all time low ;-) But we
> confirmed on irc that the hardcoded 500usec was indeed wrong, since
> the fixed machines now run on 2.5ms of link training time. I'll update
> the commit message when merging to reflect that. Also
> 
> Tested-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

That can be slapped onto the entire series.

> Tested-by: fritsch@kodi.tv
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrj�l�
Intel OTC

Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Ville Syrjälä]

On Wed, May 18, 2016 at 08:04:02PM +0200, Daniel Vetter wrote:
> On Wed, May 18, 2016 at 7:39 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote:
> >> Oops. Hw default for programming these fields to 0 is "skip link
> >> training". Display won't take that too well usually.
> >
> > s/skip/500 usec/
> 
> Yeah, my reading skills have reached an all time low ;-) But we
> confirmed on irc that the hardcoded 500usec was indeed wrong, since
> the fixed machines now run on 2.5ms of link training time. I'll update
> the commit message when merging to reflect that. Also
> 
> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

That can be slapped onto the entire series.

> Tested-by: fritsch@kodi.tv
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 7/7] drm/i915/psr: Implement PSR2 w/a for skl/kbl

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:16PM +0200, Daniel Vetter wrote:
> Found this while browsing Bspec. Looks like it applies to both skl and
> kbl.

BXT too perhaps. Actually PSR2_CTL says SKL+, but CHICKEN_MISC_1 seems
to say SKL only.

w/a db is having issues it seems, so I can't check if there's anything
relevant there.

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0f99e67f2114..c51368744e9e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
>  #define CHICKEN_PAR1_1		_MMIO(0x42080)
>  #define  DPA_MASK_VBLANK_SRD	(1 << 15)
>  #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> +#define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
>  
>  #define _CHICKEN_PIPESL_1_A	0x420b0
>  #define _CHICKEN_PIPESL_1_B	0x420b4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e0d5405a8b15..c583d1de4555 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6845,6 +6845,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>  	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>  }
>  
> +static void skylake_init_clock_gating(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* See Bspec note for PSR2_CTL bit 31 */
> +	I915_WRITE(CHICKEN_PAR1_1,
> +		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> +}
> +
>  static void broadwell_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7307,9 +7316,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
>  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_SKYLAKE(dev_priv))
> -		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> +		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>  	else if (IS_KABYLAKE(dev_priv))
> -		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> +		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>  	else if (IS_BROXTON(dev_priv))
>  		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
>  	else if (IS_BROADWELL(dev_priv))
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw

[Ville Syrjälä]

On Wed, May 18, 2016 at 06:47:10PM +0200, Daniel Vetter wrote:
> The definitions for the error register should be valid on bdw/skl too,
> but there we haven't even enabled DE_MISC handling yet.
> 
> Somewhat confusing the the moved register offset on bdw is only for
> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> on bdw.
> 
> v2: Fixes from Ville.
> 
> v3: Drop the REG_WRITE masking since edp interrupt still work and we
> still enter PSR.
> 
> Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I guess we might enable the error interrupt unconditionally, but the
entry/exit interrupts are going to cause constant dmesg spam, so those I
don't think we want enabled normally. Might be interesting to have some
kind of debug knob for those though.

Hmm. Actually is the PSR error interrupt tied to the "error interrupt"
bit in the PSR AUX register? We don't enable that bit, so maybe we can't
actually get the error interrupt?

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f0d941455bed..579f582ef987 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2185,6 +2185,26 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
>  		ironlake_rps_change_irq_handler(dev_priv);
>  }
>  
> +static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
> +{
> +	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> +
> +	if (edp_psr_iir & EDP_PSR_ERROR)
> +		DRM_DEBUG_KMS("PSR error\n");
> +
> +	if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> +		DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> +		I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> +	}
> +
> +	if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> +		DRM_DEBUG_KMS("PSR exit completed\n");
> +		I915_WRITE(EDP_PSR_IMR, 0);
> +	}
> +
> +	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> +}
> +
>  static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>  				    u32 de_iir)
>  {
> @@ -2197,6 +2217,9 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>  	if (de_iir & DE_ERR_INT_IVB)
>  		ivb_err_int_handler(dev_priv);
>  
> +	if (de_iir & DE_EDP_PSR_INT_HSW)
> +		hsw_edp_psr_irq_handler(dev_priv);
> +
>  	if (de_iir & DE_AUX_CHANNEL_A_IVB)
>  		dp_aux_irq_handler(dev_priv);
>  
> @@ -3381,6 +3404,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
>  	if (IS_GEN7(dev))
>  		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>  
> +	if (IS_HASWELL(dev)) {
> +		I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> +		I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> +	}
> +
>  	gen5_gt_irq_reset(dev);
>  
>  	ibx_irq_reset(dev);
> @@ -3676,6 +3704,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
>  				DE_PLANEB_FLIP_DONE_IVB |
>  				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
> +
>  		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
>  			      DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
>  			      DE_DP_A_HOTPLUG_IVB);
> @@ -3690,6 +3719,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  			      DE_DP_A_HOTPLUG);
>  	}
>  
> +	if (IS_HASWELL(dev)) {
> +		gen5_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> +		I915_WRITE(EDP_PSR_IMR, 0);
> +		display_mask |= DE_EDP_PSR_INT_HSW;
> +	}
> +
>  	dev_priv->irq_mask = ~display_mask;
>  
>  	I915_WRITE(HWSTAM, 0xeffe);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 86fbf723eca7..0f99e67f2114 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3225,6 +3225,13 @@ enum skl_disp_power_wells {
>  #define   EDP_PSR_TP1_TIME_0us			(3<<4)
>  #define   EDP_PSR_IDLE_FRAME_SHIFT		0
>  
> +/* Bspec claims those aren't shifted but stay at 0x64800 */
> +#define EDP_PSR_IMR				_MMIO(0x64834)
> +#define EDP_PSR_IIR				_MMIO(0x64838)
> +#define   EDP_PSR_ERROR				(1<<2)
> +#define   EDP_PSR_POST_EXIT			(1<<1)
> +#define   EDP_PSR_PRE_ENTRY			(1<<0)
> +
>  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
>  #define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
>  
> @@ -3259,6 +3266,7 @@ enum skl_disp_power_wells {
>  #define   EDP_PSR_DEBUG_MASK_LPSP	(1<<27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
>  #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
> +#define   EDP_PSR_DEBUG_MASK_REG_WRITE	(1<<16)
>  
>  #define EDP_PSR2_CTL			_MMIO(0x6f900)
>  #define   EDP_PSR2_ENABLE		(1<<31)
> @@ -5886,6 +5894,7 @@ enum skl_disp_power_wells {
>  #define DE_PCH_EVENT_IVB		(1<<28)
>  #define DE_DP_A_HOTPLUG_IVB		(1<<27)
>  #define DE_AUX_CHANNEL_A_IVB		(1<<26)
> +#define DE_EDP_PSR_INT_HSW		(1<<19)
>  #define DE_SPRITEC_FLIP_DONE_IVB	(1<<14)
>  #define DE_PLANEC_FLIP_DONE_IVB		(1<<13)
>  #define DE_PIPEC_VBLANK_IVB		(1<<10)
> @@ -5998,6 +6007,7 @@ enum skl_disp_power_wells {
>  #define GEN8_DE_MISC_IIR _MMIO(0x44468)
>  #define GEN8_DE_MISC_IER _MMIO(0x4446c)
>  #define  GEN8_DE_MISC_GSE		(1 << 27)
> +#define  GEN8_DE_EDP_PSR		(1 << 19)
>  
>  #define GEN8_PCU_ISR _MMIO(0x444e0)
>  #define GEN8_PCU_IMR _MMIO(0x444e4)
> -- 
> 2.8.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/psr: Implement PSR2 w/a for skl/kbl

[Daniel Vetter]

On Wed, May 18, 2016 at 8:22 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, May 18, 2016 at 06:47:16PM +0200, Daniel Vetter wrote:
>> Found this while browsing Bspec. Looks like it applies to both skl and
>> kbl.
>
> BXT too perhaps. Actually PSR2_CTL says SKL+, but CHICKEN_MISC_1 seems
> to say SKL only.
>
> w/a db is having issues it seems, so I can't check if there's anything
> relevant there.

Maybe Art can clarify? That entry didn't have the usual wa tags, so
maybe not in the wa db. Art, do we need to set bit3 in 0x42080 for
PSR2/selective update mode only on skl, or also on other platforms?
Bspec seems unclear.
-Daniel

>
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Durgadoss R <durgadoss.r@intel.com>
>> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++--
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 0f99e67f2114..c51368744e9e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
>>  #define CHICKEN_PAR1_1               _MMIO(0x42080)
>>  #define  DPA_MASK_VBLANK_SRD (1 << 15)
>>  #define  FORCE_ARB_IDLE_PLANES       (1 << 14)
>> +#define  SKL_EDP_PSR_FIX_RDWRAP      (1 << 3)
>>
>>  #define _CHICKEN_PIPESL_1_A  0x420b0
>>  #define _CHICKEN_PIPESL_1_B  0x420b4
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index e0d5405a8b15..c583d1de4555 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -6845,6 +6845,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>>       I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>>  }
>>
>> +static void skylake_init_clock_gating(struct drm_device *dev)
>> +{
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +     /* See Bspec note for PSR2_CTL bit 31 */
>> +     I915_WRITE(CHICKEN_PAR1_1,
>> +                I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
>> +}
>> +
>>  static void broadwell_init_clock_gating(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -7307,9 +7316,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
>>  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
>>  {
>>       if (IS_SKYLAKE(dev_priv))
>> -             dev_priv->display.init_clock_gating = nop_init_clock_gating;
>> +             dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>>       else if (IS_KABYLAKE(dev_priv))
>> -             dev_priv->display.init_clock_gating = nop_init_clock_gating;
>> +             dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>>       else if (IS_BROXTON(dev_priv))
>>               dev_priv->display.init_clock_gating = bxt_init_clock_gating;
>>       else if (IS_BROADWELL(dev_priv))
>> --
>> 2.8.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/psr: Implement PSR2 w/a for skl/kbl

[Runyan, Arthur J]

SKL+ really means "Gen9 onwards" in bspec terminology, so it includes BXT, KBL, etc.
I don't remember why this one is not labeled as a workaround in PSR2_CTL.  It is listed in the bspec workaround table.  In general everything is going in the table now, not in the associated registers.  I'll send you the internal link.

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] 
Sent: Wednesday, May 18, 2016 11:47 AM
To: Ville Syrjälä; Runyan, Arthur J
Cc: Vetter, Daniel; Intel Graphics Development; Pandiyan, Dhinakaran; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/psr: Implement PSR2 w/a for skl/kbl

On Wed, May 18, 2016 at 8:22 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, May 18, 2016 at 06:47:16PM +0200, Daniel Vetter wrote:
>> Found this while browsing Bspec. Looks like it applies to both skl and
>> kbl.
>
> BXT too perhaps. Actually PSR2_CTL says SKL+, but CHICKEN_MISC_1 seems
> to say SKL only.
>
> w/a db is having issues it seems, so I can't check if there's anything
> relevant there.

Maybe Art can clarify? That entry didn't have the usual wa tags, so
maybe not in the wa db. Art, do we need to set bit3 in 0x42080 for
PSR2/selective update mode only on skl, or also on other platforms?
Bspec seems unclear.
-Daniel

>
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Durgadoss R <durgadoss.r@intel.com>
>> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++--
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 0f99e67f2114..c51368744e9e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
>>  #define CHICKEN_PAR1_1               _MMIO(0x42080)
>>  #define  DPA_MASK_VBLANK_SRD (1 << 15)
>>  #define  FORCE_ARB_IDLE_PLANES       (1 << 14)
>> +#define  SKL_EDP_PSR_FIX_RDWRAP      (1 << 3)
>>
>>  #define _CHICKEN_PIPESL_1_A  0x420b0
>>  #define _CHICKEN_PIPESL_1_B  0x420b4
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index e0d5405a8b15..c583d1de4555 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -6845,6 +6845,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>>       I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>>  }
>>
>> +static void skylake_init_clock_gating(struct drm_device *dev)
>> +{
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> +     /* See Bspec note for PSR2_CTL bit 31 */
>> +     I915_WRITE(CHICKEN_PAR1_1,
>> +                I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
>> +}
>> +
>>  static void broadwell_init_clock_gating(struct drm_device *dev)
>>  {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -7307,9 +7316,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
>>  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
>>  {
>>       if (IS_SKYLAKE(dev_priv))
>> -             dev_priv->display.init_clock_gating = nop_init_clock_gating;
>> +             dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>>       else if (IS_KABYLAKE(dev_priv))
>> -             dev_priv->display.init_clock_gating = nop_init_clock_gating;
>> +             dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>>       else if (IS_BROXTON(dev_priv))
>>               dev_priv->display.init_clock_gating = bxt_init_clock_gating;
>>       else if (IS_BROADWELL(dev_priv))
>> --
>> 2.8.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/psr: Implement PSR2 w/a for gen9

[Daniel Vetter]

Found this while browsing Bspec. Looks like it applies to both skl and
kbl.

v2: Also for bxt (Art).

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0f99e67f2114..c51368744e9e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
 #define CHICKEN_PAR1_1		_MMIO(0x42080)
 #define  DPA_MASK_VBLANK_SRD	(1 << 15)
 #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
+#define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
 
 #define _CHICKEN_PIPESL_1_A	0x420b0
 #define _CHICKEN_PIPESL_1_B	0x420b4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e0d5405a8b15..8d42261daf1f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -58,6 +58,10 @@ static void bxt_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	/* See Bspec note for PSR2_CTL bit 31, Wa#828:bxt */
+	I915_WRITE(CHICKEN_PAR1_1,
+		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
+
 	/* WaDisableSDEUnitClockGating:bxt */
 	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
 		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
@@ -6845,6 +6849,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
 	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
 }
 
+static void skylake_init_clock_gating(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,kbl */
+	I915_WRITE(CHICKEN_PAR1_1,
+		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
+}
+
 static void broadwell_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7307,9 +7320,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
 void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
 {
 	if (IS_SKYLAKE(dev_priv))
-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
 	else if (IS_KABYLAKE(dev_priv))
-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
 	else if (IS_BROXTON(dev_priv))
 		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
 	else if (IS_BROADWELL(dev_priv))
-- 
2.8.1

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

✗ Ro.CI.BAT: warning for series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw (rev2)
URL   : https://patchwork.freedesktop.org/series/7357/
State : warning

== Summary ==

Series 7357v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/7357/revisions/2/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ro-ilk1-i5-650)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)

ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:174  dwarn:0   dfail:0   fail:3   skip:42 
ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_934/

a2499a0 drm-intel-nightly: 2016y-05m-18d-17h-17m-55s UTC integration manifest
7f28bec drm/i915/psr: Implement PSR2 w/a for gen9
3ed1f4a drm/i915/psr: Use ->get_aux_send_ctl functions
89aa4b8 drm/i915/psr: Order DP aux transactions correctly
f2227a1 drm/i915/psr: Skip aux handeshake if the vbt tells us to
63132de drm/i915/psr: Make idle_frames sensible again
c77bcd8 drm/i915/psr: Try to program link training times correctly
068aab2 drm/i915: Enable edp psr error interrupts on hsw

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

Re: [PATCH] drm/i915/psr: Implement PSR2 w/a for gen9

[Jindal, Sonika]

Looks good to me.

Reviewed-by: Sonika Jindal<sonika.jindal@intel.com>



On 5/19/2016 12:44 PM, Daniel Vetter wrote:
> Found this while browsing Bspec. Looks like it applies to both skl and
> kbl.
>
> v2: Also for bxt (Art).
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h |  1 +
>   drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++--
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0f99e67f2114..c51368744e9e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
>   #define CHICKEN_PAR1_1		_MMIO(0x42080)
>   #define  DPA_MASK_VBLANK_SRD	(1 << 15)
>   #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> +#define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
>   
>   #define _CHICKEN_PIPESL_1_A	0x420b0
>   #define _CHICKEN_PIPESL_1_B	0x420b4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e0d5405a8b15..8d42261daf1f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -58,6 +58,10 @@ static void bxt_init_clock_gating(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
> +	/* See Bspec note for PSR2_CTL bit 31, Wa#828:bxt */
> +	I915_WRITE(CHICKEN_PAR1_1,
> +		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> +
>   	/* WaDisableSDEUnitClockGating:bxt */
>   	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>   		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> @@ -6845,6 +6849,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>   	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>   }
>   
> +static void skylake_init_clock_gating(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,kbl */
> +	I915_WRITE(CHICKEN_PAR1_1,
> +		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> +}
> +
>   static void broadwell_init_clock_gating(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7307,9 +7320,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
>   void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
>   {
>   	if (IS_SKYLAKE(dev_priv))
> -		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> +		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>   	else if (IS_KABYLAKE(dev_priv))
> -		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> +		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
>   	else if (IS_BROXTON(dev_priv))
>   		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
>   	else if (IS_BROADWELL(dev_priv))

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

Re: [PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw

[Jindal, Sonika]

On 5/18/2016 11:56 PM, Ville Syrjälä wrote:
> On Wed, May 18, 2016 at 06:47:10PM +0200, Daniel Vetter wrote:
>> The definitions for the error register should be valid on bdw/skl too,
>> but there we haven't even enabled DE_MISC handling yet.
>>
>> Somewhat confusing the the moved register offset on bdw is only for
>> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
>> on bdw.
>>
>> v2: Fixes from Ville.
>>
>> v3: Drop the REG_WRITE masking since edp interrupt still work and we
>> still enter PSR.
>>
>> Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Durgadoss R <durgadoss.r@intel.com>
>> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> I guess we might enable the error interrupt unconditionally, but the
> entry/exit interrupts are going to cause constant dmesg spam, so those I
> don't think we want enabled normally. Might be interesting to have some
> kind of debug knob for those though.
>
> Hmm. Actually is the PSR error interrupt tied to the "error interrupt"
> bit in the PSR AUX register? We don't enable that bit, so maybe we can't
> actually get the error interrupt?
Only bit 2 (SRD Aux Error) must be tied to the error interrupt bit of 
SRD_AUX register.
The entry and exit interrupts should be independent as per my understanding.
I agree the entry/exit messages will be too much in dmesg.
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
>>   2 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index f0d941455bed..579f582ef987 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -2185,6 +2185,26 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
>>   		ironlake_rps_change_irq_handler(dev_priv);
>>   }
>>   
>> +static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
>> +{
>> +	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
>> +
>> +	if (edp_psr_iir & EDP_PSR_ERROR)
>> +		DRM_DEBUG_KMS("PSR error\n");
>> +
>> +	if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
>> +		DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
>> +		I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
>> +	}
>> +
>> +	if (edp_psr_iir & EDP_PSR_POST_EXIT) {
>> +		DRM_DEBUG_KMS("PSR exit completed\n");
>> +		I915_WRITE(EDP_PSR_IMR, 0);
>> +	}
>> +
>> +	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
>> +}
>> +
>>   static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>>   				    u32 de_iir)
>>   {
>> @@ -2197,6 +2217,9 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>>   	if (de_iir & DE_ERR_INT_IVB)
>>   		ivb_err_int_handler(dev_priv);
>>   
>> +	if (de_iir & DE_EDP_PSR_INT_HSW)
>> +		hsw_edp_psr_irq_handler(dev_priv);
>> +
>>   	if (de_iir & DE_AUX_CHANNEL_A_IVB)
>>   		dp_aux_irq_handler(dev_priv);
>>   
>> @@ -3381,6 +3404,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
>>   	if (IS_GEN7(dev))
>>   		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>>   
>> +	if (IS_HASWELL(dev)) {
>> +		I915_WRITE(EDP_PSR_IMR, 0xffffffff);
>> +		I915_WRITE(EDP_PSR_IIR, 0xffffffff);
>> +	}
>> +
>>   	gen5_gt_irq_reset(dev);
>>   
>>   	ibx_irq_reset(dev);
>> @@ -3676,6 +3704,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>>   				DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
>>   				DE_PLANEB_FLIP_DONE_IVB |
>>   				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
>> +
extra line.
>>   		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
>>   			      DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
>>   			      DE_DP_A_HOTPLUG_IVB);
>> @@ -3690,6 +3719,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>>   			      DE_DP_A_HOTPLUG);
>>   	}
>>   
>> +	if (IS_HASWELL(dev)) {
>> +		gen5_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
>> +		I915_WRITE(EDP_PSR_IMR, 0);
>> +		display_mask |= DE_EDP_PSR_INT_HSW;
>> +	}
>> +
>>   	dev_priv->irq_mask = ~display_mask;
>>   
>>   	I915_WRITE(HWSTAM, 0xeffe);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 86fbf723eca7..0f99e67f2114 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3225,6 +3225,13 @@ enum skl_disp_power_wells {
>>   #define   EDP_PSR_TP1_TIME_0us			(3<<4)
>>   #define   EDP_PSR_IDLE_FRAME_SHIFT		0
>>   
>> +/* Bspec claims those aren't shifted but stay at 0x64800 */
>> +#define EDP_PSR_IMR				_MMIO(0x64834)
>> +#define EDP_PSR_IIR				_MMIO(0x64838)
>> +#define   EDP_PSR_ERROR				(1<<2)
>> +#define   EDP_PSR_POST_EXIT			(1<<1)
>> +#define   EDP_PSR_PRE_ENTRY			(1<<0)
>> +
>>   #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
>>   #define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
>>   
>> @@ -3259,6 +3266,7 @@ enum skl_disp_power_wells {
>>   #define   EDP_PSR_DEBUG_MASK_LPSP	(1<<27)
>>   #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
>>   #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
>> +#define   EDP_PSR_DEBUG_MASK_REG_WRITE	(1<<16)
Is this remaining from some previous version? or left intentionally?
>>   
>>   #define EDP_PSR2_CTL			_MMIO(0x6f900)
>>   #define   EDP_PSR2_ENABLE		(1<<31)
>> @@ -5886,6 +5894,7 @@ enum skl_disp_power_wells {
>>   #define DE_PCH_EVENT_IVB		(1<<28)
>>   #define DE_DP_A_HOTPLUG_IVB		(1<<27)
>>   #define DE_AUX_CHANNEL_A_IVB		(1<<26)
>> +#define DE_EDP_PSR_INT_HSW		(1<<19)
>>   #define DE_SPRITEC_FLIP_DONE_IVB	(1<<14)
>>   #define DE_PLANEC_FLIP_DONE_IVB		(1<<13)
>>   #define DE_PIPEC_VBLANK_IVB		(1<<10)
>> @@ -5998,6 +6007,7 @@ enum skl_disp_power_wells {
>>   #define GEN8_DE_MISC_IIR _MMIO(0x44468)
>>   #define GEN8_DE_MISC_IER _MMIO(0x4446c)
>>   #define  GEN8_DE_MISC_GSE		(1 << 27)
>> +#define  GEN8_DE_EDP_PSR		(1 << 19)
We are not enabling it for bdw now right?
>>   
>>   #define GEN8_PCU_ISR _MMIO(0x444e0)
>>   #define GEN8_PCU_IMR _MMIO(0x444e4)
>> -- 
>> 2.8.1

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

Re: [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Jindal, Sonika]

[-- Attachment #1.1: Type: text/plain, Size: 3779 bytes --]



On 5/18/2016 10:17 PM, Daniel Vetter wrote:
> Oops. Hw default for programming these fields to 0 is "skip link
> training". Display won't take that too well usually.
But we were defaulting it to value 0, which means 500us for both TP1 and 
TP2 or TP3 time.
I dont think it means skip link training. This is just to set the time 
for the patterns.
Skip aux handshake can happen if bit 12 of SRD_CTL is set.

Does this solution help in fixing the bug mentioned here?

>
> v2: Unbotch the math a bit.
>
> v3: Drop debug hunk.
>
> Tested-by: Lyude <cpaul@redhat.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
>   1 file changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c3abae4bc596..a788d1e9589b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
> +	uint32_t val = EDP_PSR_ENABLE;
> +
> +	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> +	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>   
>   	if (IS_HASWELL(dev))
>   		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
> @@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>   	if (dev_priv->psr.link_standby)
>   		val |= EDP_PSR_LINK_STANDBY;
>   
> -	I915_WRITE(EDP_PSR_CTL, val |
> -		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> -		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> -		   EDP_PSR_ENABLE);
> +	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)
> +		val |= EDP_PSR_TP1_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
> +		val |= EDP_PSR_TP1_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP1_TIME_0us;
> +
> +	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)
> +		val |= EDP_PSR_TP2_TP3_TIME_500us;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR_TP2_TP3_TIME_100us;
> +	else
> +		val |= EDP_PSR_TP2_TP3_TIME_0us;
> +
> +	if (intel_dp_source_supports_hbr2(intel_dp) &&
> +	    drm_dp_tps3_supported(intel_dp->dpcd))
> +		val |= EDP_PSR_TP1_TP3_SEL;
> +	else
> +		val |= EDP_PSR_TP1_TP2_SEL;
> +
> +	I915_WRITE(EDP_PSR_CTL, val);
> +
> +	if (!dev_priv->psr.psr2_support)
> +		return;
> +
> +	/* FIXME: selective update is probably totally broken because it doesn't
> +	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> +	 * good enough. */
> +	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> +
> +	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> +		val |= EDP_PSR2_TP2_TIME_2500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> +		val |= EDP_PSR2_TP2_TIME_500;
> +	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> +		val |= EDP_PSR2_TP2_TIME_100;
> +	else
> +		val |= EDP_PSR2_TP2_TIME_50;
>   
> -	if (dev_priv->psr.psr2_support)
> -		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> -				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> +	I915_WRITE(EDP_PSR2_CTL, val);
>   }
>   
>   static bool intel_psr_match_conditions(struct intel_dp *intel_dp)


[-- Attachment #1.2: Type: text/html, Size: 5522 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Daniel Vetter]

On Thu, May 19, 2016 at 04:20:02PM +0530, Jindal, Sonika wrote:
> 
> 
> On 5/18/2016 10:17 PM, Daniel Vetter wrote:
> >Oops. Hw default for programming these fields to 0 is "skip link
> >training". Display won't take that too well usually.
> But we were defaulting it to value 0, which means 500us for both TP1 and TP2
> or TP3 time.
> I dont think it means skip link training. This is just to set the time for
> the patterns.
> Skip aux handshake can happen if bit 12 of SRD_CTL is set.
> 
> Does this solution help in fixing the bug mentioned here?

See the other thread, I misread and yes it does help.
-Daniel

> 
> >
> >v2: Unbotch the math a bit.
> >
> >v3: Drop debug hunk.
> >
> >Tested-by: Lyude <cpaul@redhat.com>
> >Cc: Lyude <cpaul@redhat.com>
> >Cc: stable@vger.kernel.org
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> >Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >Cc: Sonika Jindal <sonika.jindal@intel.com>
> >Cc: Durgadoss R <durgadoss.r@intel.com>
> >Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> >Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
> >  1 file changed, 47 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> >index c3abae4bc596..a788d1e9589b 100644
> >--- a/drivers/gpu/drm/i915/intel_psr.c
> >+++ b/drivers/gpu/drm/i915/intel_psr.c
> >@@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
> >+	uint32_t val = EDP_PSR_ENABLE;
> >+
> >+	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> >+	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> >  	if (IS_HASWELL(dev))
> >  		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
> >@@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> >  	if (dev_priv->psr.link_standby)
> >  		val |= EDP_PSR_LINK_STANDBY;
> >-	I915_WRITE(EDP_PSR_CTL, val |
> >-		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> >-		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> >-		   EDP_PSR_ENABLE);
> >+	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)
> >+		val |= EDP_PSR_TP1_TIME_500us;
> >+	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
> >+		val |= EDP_PSR_TP1_TIME_100us;
> >+	else
> >+		val |= EDP_PSR_TP1_TIME_0us;
> >+
> >+	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)
> >+		val |= EDP_PSR_TP2_TP3_TIME_500us;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> >+		val |= EDP_PSR_TP2_TP3_TIME_100us;
> >+	else
> >+		val |= EDP_PSR_TP2_TP3_TIME_0us;
> >+
> >+	if (intel_dp_source_supports_hbr2(intel_dp) &&
> >+	    drm_dp_tps3_supported(intel_dp->dpcd))
> >+		val |= EDP_PSR_TP1_TP3_SEL;
> >+	else
> >+		val |= EDP_PSR_TP1_TP2_SEL;
> >+
> >+	I915_WRITE(EDP_PSR_CTL, val);
> >+
> >+	if (!dev_priv->psr.psr2_support)
> >+		return;
> >+
> >+	/* FIXME: selective update is probably totally broken because it doesn't
> >+	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> >+	 * good enough. */
> >+	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> >+
> >+	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> >+		val |= EDP_PSR2_TP2_TIME_2500;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> >+		val |= EDP_PSR2_TP2_TIME_500;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> >+		val |= EDP_PSR2_TP2_TIME_100;
> >+	else
> >+		val |= EDP_PSR2_TP2_TIME_50;
> >-	if (dev_priv->psr.psr2_support)
> >-		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> >-				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> >+	I915_WRITE(EDP_PSR2_CTL, val);
> >  }
> >  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/7] drm/i915/psr: Try to program link training times correctly

[Daniel Vetter]

On Thu, May 19, 2016 at 04:20:02PM +0530, Jindal, Sonika wrote:
> 
> 
> On 5/18/2016 10:17 PM, Daniel Vetter wrote:
> >Oops. Hw default for programming these fields to 0 is "skip link
> >training". Display won't take that too well usually.
> But we were defaulting it to value 0, which means 500us for both TP1 and TP2
> or TP3 time.
> I dont think it means skip link training. This is just to set the time for
> the patterns.
> Skip aux handshake can happen if bit 12 of SRD_CTL is set.
> 
> Does this solution help in fixing the bug mentioned here?

See the other thread, I misread and yes it does help.
-Daniel

> 
> >
> >v2: Unbotch the math a bit.
> >
> >v3: Drop debug hunk.
> >
> >Tested-by: Lyude <cpaul@redhat.com>
> >Cc: Lyude <cpaul@redhat.com>
> >Cc: stable@vger.kernel.org
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176
> >Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >Cc: Sonika Jindal <sonika.jindal@intel.com>
> >Cc: Durgadoss R <durgadoss.r@intel.com>
> >Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> >Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------
> >  1 file changed, 47 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> >index c3abae4bc596..a788d1e9589b 100644
> >--- a/drivers/gpu/drm/i915/intel_psr.c
> >+++ b/drivers/gpu/drm/i915/intel_psr.c
> >@@ -280,7 +280,10 @@ static void hsw_psr_enable_source(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 = 0x0;
> >+	uint32_t val = EDP_PSR_ENABLE;
> >+
> >+	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> >+	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> >  	if (IS_HASWELL(dev))
> >  		val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
> >@@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> >  	if (dev_priv->psr.link_standby)
> >  		val |= EDP_PSR_LINK_STANDBY;
> >-	I915_WRITE(EDP_PSR_CTL, val |
> >-		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> >-		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> >-		   EDP_PSR_ENABLE);
> >+	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)
> >+		val |= EDP_PSR_TP1_TIME_500us;
> >+	else if (dev_priv->vbt.psr.tp1_wakeup_time > 0)
> >+		val |= EDP_PSR_TP1_TIME_100us;
> >+	else
> >+		val |= EDP_PSR_TP1_TIME_0us;
> >+
> >+	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)
> >+		val |= EDP_PSR_TP2_TP3_TIME_500us;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> >+		val |= EDP_PSR_TP2_TP3_TIME_100us;
> >+	else
> >+		val |= EDP_PSR_TP2_TP3_TIME_0us;
> >+
> >+	if (intel_dp_source_supports_hbr2(intel_dp) &&
> >+	    drm_dp_tps3_supported(intel_dp->dpcd))
> >+		val |= EDP_PSR_TP1_TP3_SEL;
> >+	else
> >+		val |= EDP_PSR_TP1_TP2_SEL;
> >+
> >+	I915_WRITE(EDP_PSR_CTL, val);
> >+
> >+	if (!dev_priv->psr.psr2_support)
> >+		return;
> >+
> >+	/* FIXME: selective update is probably totally broken because it doesn't
> >+	 * mesh at all with our frontbuffer tracking. And the hw alone isn't
> >+	 * good enough. */
> >+	val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> >+
> >+	if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> >+		val |= EDP_PSR2_TP2_TIME_2500;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> >+		val |= EDP_PSR2_TP2_TIME_500;
> >+	else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0)
> >+		val |= EDP_PSR2_TP2_TIME_100;
> >+	else
> >+		val |= EDP_PSR2_TP2_TIME_50;
> >-	if (dev_priv->psr.psr2_support)
> >-		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> >-				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> >+	I915_WRITE(EDP_PSR2_CTL, val);
> >  }
> >  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: warning for series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw (rev2)

[Daniel Vetter]

On Thu, May 19, 2016 at 08:01:33AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/7] drm/i915: Enable edp psr error interrupts on hsw (rev2)
> URL   : https://patchwork.freedesktop.org/series/7357/
> State : warning
> 
> == Summary ==
> 
> Series 7357v2 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/7357/revisions/2/mbox
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 fail       -> PASS       (ro-ilk1-i5-650)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (ro-ivb2-i7-3770)

This is a pretty good wtf since blanantly impossible. We have not bug for
this, but I think I've seen it in other places. I'm creating a patch to
make this code more resilient, hopefully that helps.

Definitely unrelated to PSR since ivb just plain doesn't have PSR ...
-Daniel

> 
> ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
> ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
> ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
> ro-bsw-n3050     total:219  pass:174  dwarn:0   dfail:0   fail:3   skip:42 
> ro-byt-n2820     total:218  pass:175  dwarn:0   dfail:0   fail:2   skip:41 
> ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
> ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
> ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
> ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
> ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
> ro-ivb2-i7-3770  total:219  pass:186  dwarn:1   dfail:0   fail:0   skip:32 
> ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_934/
> 
> a2499a0 drm-intel-nightly: 2016y-05m-18d-17h-17m-55s UTC integration manifest
> 7f28bec drm/i915/psr: Implement PSR2 w/a for gen9
> 3ed1f4a drm/i915/psr: Use ->get_aux_send_ctl functions
> 89aa4b8 drm/i915/psr: Order DP aux transactions correctly
> f2227a1 drm/i915/psr: Skip aux handeshake if the vbt tells us to
> 63132de drm/i915/psr: Make idle_frames sensible again
> c77bcd8 drm/i915/psr: Try to program link training times correctly
> 068aab2 drm/i915: Enable edp psr error interrupts on hsw
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/psr: Implement PSR2 w/a for gen9

[Daniel Vetter]

On Thu, May 19, 2016 at 02:25:59PM +0530, Jindal, Sonika wrote:
> Looks good to me.
> 
> Reviewed-by: Sonika Jindal<sonika.jindal@intel.com>

Ok, merged 2-7 except the aux handshake one - that needs the fixup from
Ville's series and that one needs an ack from Dave for the helper patches
still.
-Daniel

> 
> 
> 
> On 5/19/2016 12:44 PM, Daniel Vetter wrote:
> >Found this while browsing Bspec. Looks like it applies to both skl and
> >kbl.
> >
> >v2: Also for bxt (Art).
> >
> >Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >Cc: Sonika Jindal <sonika.jindal@intel.com>
> >Cc: Durgadoss R <durgadoss.r@intel.com>
> >Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> >Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
> >Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_reg.h |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++--
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >index 0f99e67f2114..c51368744e9e 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -6043,6 +6043,7 @@ enum skl_disp_power_wells {
> >  #define CHICKEN_PAR1_1		_MMIO(0x42080)
> >  #define  DPA_MASK_VBLANK_SRD	(1 << 15)
> >  #define  FORCE_ARB_IDLE_PLANES	(1 << 14)
> >+#define  SKL_EDP_PSR_FIX_RDWRAP	(1 << 3)
> >  #define _CHICKEN_PIPESL_1_A	0x420b0
> >  #define _CHICKEN_PIPESL_1_B	0x420b4
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index e0d5405a8b15..8d42261daf1f 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -58,6 +58,10 @@ static void bxt_init_clock_gating(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >+	/* See Bspec note for PSR2_CTL bit 31, Wa#828:bxt */
> >+	I915_WRITE(CHICKEN_PAR1_1,
> >+		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> >+
> >  	/* WaDisableSDEUnitClockGating:bxt */
> >  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> >  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> >@@ -6845,6 +6849,15 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
> >  	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> >  }
> >+static void skylake_init_clock_gating(struct drm_device *dev)
> >+{
> >+	struct drm_i915_private *dev_priv = dev->dev_private;
> >+
> >+	/* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,kbl */
> >+	I915_WRITE(CHICKEN_PAR1_1,
> >+		   I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
> >+}
> >+
> >  static void broadwell_init_clock_gating(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >@@ -7307,9 +7320,9 @@ static void nop_init_clock_gating(struct drm_device *dev)
> >  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
> >  {
> >  	if (IS_SKYLAKE(dev_priv))
> >-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> >+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
> >  	else if (IS_KABYLAKE(dev_priv))
> >-		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> >+		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
> >  	else if (IS_BROXTON(dev_priv))
> >  		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
> >  	else if (IS_BROADWELL(dev_priv))
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/psr: Make idle_frames sensible again

[Rodrigo Vivi]

On Wed, May 18, 2016 at 10:46 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, May 18, 2016 at 06:47:12PM +0200, Daniel Vetter wrote:
>> This reverts
>>
>> commit dfaf37baa07513d2c37afff79978807d2d10221a
>> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Date:   Mon Dec 7 14:45:20 2015 -0800
>>
>>     drm/i915: Fix idle_frames counter.
>>
>> and
>>
>> commit 97173eaf5f33b1e85efdb06d593d333480b60bf3
>> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Date:   Tue Jul 7 16:28:55 2015 -0700
>>
>>     drm/i915: PSR: Increase idle_frames
>>
>> and implements
>>
>> commit d44b4dcbd1b44737462b77971d216d21a9413341
>> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Date:   Fri Nov 14 08:52:31 2014 -0800
>>
>>     drm/i915: HSW/BDW PSR Set idle_frames = VBT + 1
>>
>> without the hack to use 2 idle frames when VBT says 1. We keep the + 1
>> just for safety, although I haven't really figured out why that one
>> exists.
>>
>> It's nonsense. idle_frames = number of frames where the screen is
>> entirely idle before we think about entering PSR.
>>
>> idle_patter = part of link training, and we probably totally butchered
>> link training because we told the hw to entirely skip it. No wonder
>> PSR occasionally just fell over.
>>
>> I suspect the reason we've increased idle frames is that it makes PSR
>> entry slightly less likely, and more likely to happen in a quite
>> system, which probably increased the changes the panel came back up
>> without link training. The proper fix is to implement link training
>> for PSR.
>
> Quite the mess there. At the least this makes things look a bit more
> sane.
>
> FWIW
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Durgadoss R <durgadoss.r@intel.com>
>> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index a788d1e9589b..0295d8dd483f 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -272,14 +272,14 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>
>>       uint32_t max_sleep_time = 0x1f;
>> -     /*
>> -      * Let's respect VBT in case VBT asks a higher idle_frame value.
>> -      * Let's use 6 as the minimum to cover all known cases including
>> -      * the off-by-one issue that HW has in some cases. Also there are
>> -      * cases where sink should be able to train
>> -      * with the 5 or 6 idle patterns.
>> +     /* Lately it was identified that depending on panel idle frame count
>> +      * calculated at HW can be off by 1. So let's use what came
>> +      * from VBT + 1.
>> +      * There are also other cases where panel demands at least 4
>> +      * but VBT is not being set. To cover these 2 cases lets use
>> +      * at least 5 when VBT isn't set to be on the safest side.
>>        */
>> -     uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>> +     uint32_t idle_frames = dev_priv->vbt.psr.idle_frames + 1;

Hm, I believe this is dangerous. First because this comment above
explain that in the past we found cases where panel demands 4 idles
but VBT was incorrectly unset. Also because at some point HW guys
found out they had a bug on the idle frames calculation so they were
off by one, so setting it to one would mean in some cases HW would use
0 and start entering PSR when it shouldn't yet.

But yeap, I know at least nowadays our sw tracking helper could help
in this case I believed we should stat on the safest side here...

>>       uint32_t val = EDP_PSR_ENABLE;
>>
>>       val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
>> --
>> 2.8.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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