public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
@ 2014-05-23 20:16 Jesse Barnes
  2014-05-23 20:16 ` [PATCH 2/6] drm/i915/vlv: drop power well enable in uncore_sanitize Jesse Barnes
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
that it resets the whole common lane section of the PHY.  This is
required on machines where the BIOS doesn't do this for us on boot or
resume to properly re-calibrate and get the PHY ready to transmit data.

Without this patch, such machines won't resume correctly much of the time,
with the symptom being a 'port ready' timeout and/or a link training
failure.

Note that simply asserting reset at suspend and de-asserting at resume
is not sufficient, nor is simply de-asserting at boot.  Both of these
cases have been tested and have still been found to have failures on
some configurations.

v2: extract simpler set_power_well function for use in reset_dpio (Imre)
    move to reset_dpio (Daniel & Ville)
v3: don't reset if DPIO reset is already de-asserted (Imre)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_pm.c      | 13 ++++++++++---
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index df58afc..bdb4624 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
 
 	} else {
 		/*
+		 * If DPIO has already been reset, e.g. by BIOS, just skip all
+		 * this.
+		 */
+		if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
+			return;
+
+		/*
+		 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
+		 * Need to assert and de-assert PHY SB reset by gating the
+		 * common lane power, then un-gating it.
+		 * Simply ungating isn't enough to reset the PHY enough to get
+		 * ports and lanes running.
+		 */
+		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+				     false);
+		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
+				     true);
+
+		/*
 		 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
 		 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
 		 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0ef2777..feb6165 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -966,7 +966,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
 void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
 void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
 void ilk_wm_get_hw_state(struct drm_device *dev);
-
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+			  enum punit_power_well power_well_id, bool enable);
 
 /* intel_sdvo.c */
 bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b59e8ab..8f7dbb9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5724,10 +5724,9 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-			       struct i915_power_well *power_well, bool enable)
+void __vlv_set_power_well(struct drm_i915_private *dev_priv,
+			  enum punit_power_well power_well_id, bool enable)
 {
-	enum punit_power_well power_well_id = power_well->data;
 	u32 mask;
 	u32 state;
 	u32 ctrl;
@@ -5760,6 +5759,14 @@ out:
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void vlv_set_power_well(struct drm_i915_private *dev_priv,
+			       struct i915_power_well *power_well, bool enable)
+{
+	enum punit_power_well power_well_id = power_well->data;
+
+	__vlv_set_power_well(dev_priv, power_well_id, enable);
+}
+
 static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/6] drm/i915/vlv: drop power well enable in uncore_sanitize
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
@ 2014-05-23 20:16 ` Jesse Barnes
  2014-05-23 20:16 ` [PATCH 3/6] drm/i915/vlv: move CRI refclk enable into __vlv_set_power_well Jesse Barnes
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

We do this at runtime and later on now.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_uncore.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 27fe2df..bcd6945 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -393,26 +393,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
 
 void intel_uncore_sanitize(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 reg_val;
-
 	/* BIOS often leaves RC6 enabled, but disable it for hw init */
 	intel_disable_gt_powersave(dev);
-
-	/* Turn off power gate, require especially for the BIOS less system */
-	if (IS_VALLEYVIEW(dev)) {
-
-		mutex_lock(&dev_priv->rps.hw_lock);
-		reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS);
-
-		if (reg_val & (PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_RENDER) |
-			       PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_MEDIA) |
-			       PUNIT_PWRGT_PWR_GATE(PUNIT_POWER_WELL_DISP2D)))
-			vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0);
-
-		mutex_unlock(&dev_priv->rps.hw_lock);
-
-	}
 }
 
 /*
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/6] drm/i915/vlv: move CRI refclk enable into __vlv_set_power_well
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
  2014-05-23 20:16 ` [PATCH 2/6] drm/i915/vlv: drop power well enable in uncore_sanitize Jesse Barnes
@ 2014-05-23 20:16 ` Jesse Barnes
  2014-05-23 20:16 ` [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX Jesse Barnes
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

This needs to be done before we power back on the CMN_BC well so the PHY
can calibrate properly.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |  8 --------
 drivers/gpu/drm/i915/intel_pm.c      | 11 +++++++++++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bdb4624..369cd1e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1477,14 +1477,6 @@ static void intel_reset_dpio(struct drm_device *dev)
 	if (!IS_VALLEYVIEW(dev))
 		return;
 
-	/*
-	 * Enable the CRI clock source so we can get at the display and the
-	 * reference clock for VGA hotplug / manual detection.
-	 */
-	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
-		   DPLL_REFA_CLK_ENABLE_VLV |
-		   DPLL_INTEGRATED_CRI_CLK_VLV);
-
 	if (IS_CHERRYVIEW(dev)) {
 		enum dpio_phy phy;
 		u32 val;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8f7dbb9..e8f0c85 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5731,6 +5731,17 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 	u32 state;
 	u32 ctrl;
 
+	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable) {
+		/*
+		 * Enable the CRI clock source so we can get at the display
+		 * and the reference clock for VGA hotplug / manual detection.
+		 */
+		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+			   DPLL_REFA_CLK_ENABLE_VLV |
+			   DPLL_INTEGRATED_CRI_CLK_VLV);
+		udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
+	}
+
 	mask = PUNIT_PWRGT_MASK(power_well_id);
 	state = enable ? PUNIT_PWRGT_PWR_ON(power_well_id) :
 			 PUNIT_PWRGT_PWR_GATE(power_well_id);
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
  2014-05-23 20:16 ` [PATCH 2/6] drm/i915/vlv: drop power well enable in uncore_sanitize Jesse Barnes
  2014-05-23 20:16 ` [PATCH 3/6] drm/i915/vlv: move CRI refclk enable into __vlv_set_power_well Jesse Barnes
@ 2014-05-23 20:16 ` Jesse Barnes
  2014-05-27 20:21   ` Daniel Vetter
  2014-05-23 20:16 ` [PATCH 5/6] drm/i915/vlv: move DPIO common reset de-assert into __vlv_set_power_well Jesse Barnes
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

There may be a dependency here.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8f0c85..fb7e23e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6122,12 +6122,6 @@ static struct i915_power_well vlv_power_wells[] = {
 		.ops = &vlv_display_power_well_ops,
 	},
 	{
-		.name = "dpio-common",
-		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
-		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
-		.ops = &vlv_dpio_power_well_ops,
-	},
-	{
 		.name = "dpio-tx-b-01",
 		.domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
 			   VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS |
@@ -6163,6 +6157,12 @@ static struct i915_power_well vlv_power_wells[] = {
 		.ops = &vlv_dpio_power_well_ops,
 		.data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
 	},
+	{
+		.name = "dpio-common",
+		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
+		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
+		.ops = &vlv_dpio_power_well_ops,
+	},
 };
 
 #define set_power_wells(power_domains, __power_wells) ({		\
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/6] drm/i915/vlv: move DPIO common reset de-assert into __vlv_set_power_well
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
                   ` (2 preceding siblings ...)
  2014-05-23 20:16 ` [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX Jesse Barnes
@ 2014-05-23 20:16 ` Jesse Barnes
  2014-05-23 20:16 ` [PATCH 6/6] drm/i915/vlv: add pll assertion when disabling DPIO common well Jesse Barnes
  2014-05-27 19:32 ` [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Ville Syrjälä
  5 siblings, 0 replies; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

We need to do this anytime we power gate the DPIO common well.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 13 ------------
 drivers/gpu/drm/i915/intel_pm.c      | 39 +++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 369cd1e..bfa1fc7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1518,19 +1518,6 @@ static void intel_reset_dpio(struct drm_device *dev)
 				     false);
 		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
 				     true);
-
-		/*
-		 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
-		 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
-		 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
-		 *   b.	The other bits such as sfr settings / modesel may all
-		 *	be set to 0.
-		 *
-		 * This should only be done on init and resume from S3 with
-		 * both PLLs disabled, or we risk losing DPIO and PLL
-		 * synchronization.
-		 */
-		I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fb7e23e..948a4aa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5731,15 +5731,22 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 	u32 state;
 	u32 ctrl;
 
-	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable) {
-		/*
-		 * Enable the CRI clock source so we can get at the display
-		 * and the reference clock for VGA hotplug / manual detection.
-		 */
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
-			   DPLL_REFA_CLK_ENABLE_VLV |
-			   DPLL_INTEGRATED_CRI_CLK_VLV);
-		udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
+	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC) {
+		if (enable) {
+			/*
+			 * Enable the CRI clock source so we can get at the
+			 * display and the reference clock for VGA
+			 * hotplug / manual detection.
+			 */
+			I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+				   DPLL_REFA_CLK_ENABLE_VLV |
+				   DPLL_INTEGRATED_CRI_CLK_VLV);
+			udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
+		} else {
+			/* Assert common reset */
+			I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) &
+				   ~DPIO_CMNRST);
+		}
 	}
 
 	mask = PUNIT_PWRGT_MASK(power_well_id);
@@ -5768,6 +5775,20 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 
 out:
 	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	/*
+	 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
+	 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
+	 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
+	 *   b.	The other bits such as sfr settings / modesel may all
+	 *	be set to 0.
+	 *
+	 * This should only be done on init and resume from S3 with
+	 * both PLLs disabled, or we risk losing DPIO and PLL
+	 * synchronization.
+	 */
+	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC && enable)
+		I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_CMNRST);
 }
 
 static void vlv_set_power_well(struct drm_i915_private *dev_priv,
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/6] drm/i915/vlv: add pll assertion when disabling DPIO common well
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
                   ` (3 preceding siblings ...)
  2014-05-23 20:16 ` [PATCH 5/6] drm/i915/vlv: move DPIO common reset de-assert into __vlv_set_power_well Jesse Barnes
@ 2014-05-23 20:16 ` Jesse Barnes
  2014-05-27 19:32 ` [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Ville Syrjälä
  5 siblings, 0 replies; 10+ messages in thread
From: Jesse Barnes @ 2014-05-23 20:16 UTC (permalink / raw)
  To: intel-gfx

When doing this, all PLLs should be disabled.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 948a4aa..452518f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5727,9 +5727,11 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
 void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 			  enum punit_power_well power_well_id, bool enable)
 {
+	struct drm_device *dev = dev_priv->dev;
 	u32 mask;
 	u32 state;
 	u32 ctrl;
+	enum pipe pipe;
 
 	if (power_well_id == PUNIT_POWER_WELL_DPIO_CMN_BC) {
 		if (enable) {
@@ -5743,6 +5745,8 @@ void __vlv_set_power_well(struct drm_i915_private *dev_priv,
 				   DPLL_INTEGRATED_CRI_CLK_VLV);
 			udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 		} else {
+			for_each_pipe(pipe)
+				assert_pll_disabled(dev_priv, pipe);
 			/* Assert common reset */
 			I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) &
 				   ~DPIO_CMNRST);
-- 
1.8.4.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
  2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
                   ` (4 preceding siblings ...)
  2014-05-23 20:16 ` [PATCH 6/6] drm/i915/vlv: add pll assertion when disabling DPIO common well Jesse Barnes
@ 2014-05-27 19:32 ` Ville Syrjälä
  2014-05-27 20:24   ` Daniel Vetter
  5 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2014-05-27 19:32 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Fri, May 23, 2014 at 01:16:40PM -0700, Jesse Barnes wrote:
> This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
> that it resets the whole common lane section of the PHY.  This is
> required on machines where the BIOS doesn't do this for us on boot or
> resume to properly re-calibrate and get the PHY ready to transmit data.
> 
> Without this patch, such machines won't resume correctly much of the time,
> with the symptom being a 'port ready' timeout and/or a link training
> failure.
> 
> Note that simply asserting reset at suspend and de-asserting at resume
> is not sufficient, nor is simply de-asserting at boot.  Both of these
> cases have been tested and have still been found to have failures on
> some configurations.
> 
> v2: extract simpler set_power_well function for use in reset_dpio (Imre)
>     move to reset_dpio (Daniel & Ville)
> v3: don't reset if DPIO reset is already de-asserted (Imre)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

The series matches my understanding of the limitations of the PHY, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

But if these limitations are real, then I think we would also need to
adjust the power domains to power up all the wells whenever even a
single one is required.

This should be testable I think by simply:
1. disable both ports
2. enable one port
3. enable the other port

At step 3. the common well is already up, so the TX wells for the second
port should come up in some kind of poor state.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
>  drivers/gpu/drm/i915/intel_pm.c      | 13 ++++++++++---
>  3 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index df58afc..bdb4624 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1509,6 +1509,25 @@ static void intel_reset_dpio(struct drm_device *dev)
>  
>  	} else {
>  		/*
> +		 * If DPIO has already been reset, e.g. by BIOS, just skip all
> +		 * this.
> +		 */
> +		if (I915_READ(DPIO_CTL) & DPIO_CMNRST)
> +			return;
> +
> +		/*
> +		 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> +		 * Need to assert and de-assert PHY SB reset by gating the
> +		 * common lane power, then un-gating it.
> +		 * Simply ungating isn't enough to reset the PHY enough to get
> +		 * ports and lanes running.
> +		 */
> +		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> +				     false);
> +		__vlv_set_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_BC,
> +				     true);
> +
> +		/*
>  		 * From VLV2A0_DP_eDP_DPIO_driver_vbios_notes_10.docx -
>  		 *  6.	De-assert cmn_reset/side_reset. Same as VLV X0.
>  		 *   a.	GUnit 0x2110 bit[0] set to 1 (def 0)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0ef2777..feb6165 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -966,7 +966,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
> -
> +void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> +			  enum punit_power_well power_well_id, bool enable);
>  
>  /* intel_sdvo.c */
>  bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b59e8ab..8f7dbb9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5724,10 +5724,9 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> -			       struct i915_power_well *power_well, bool enable)
> +void __vlv_set_power_well(struct drm_i915_private *dev_priv,
> +			  enum punit_power_well power_well_id, bool enable)
>  {
> -	enum punit_power_well power_well_id = power_well->data;
>  	u32 mask;
>  	u32 state;
>  	u32 ctrl;
> @@ -5760,6 +5759,14 @@ out:
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> +			       struct i915_power_well *power_well, bool enable)
> +{
> +	enum punit_power_well power_well_id = power_well->data;
> +
> +	__vlv_set_power_well(dev_priv, power_well_id, enable);
> +}
> +
>  static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX
  2014-05-23 20:16 ` [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX Jesse Barnes
@ 2014-05-27 20:21   ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-05-27 20:21 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Fri, May 23, 2014 at 01:16:43PM -0700, Jesse Barnes wrote:
> There may be a dependency here.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Hm, this implicit ordering is a bit funky imo ... If this increase in
complexity in furture platforms I think we should switch to epxlicit
depencies and grab references in turn. Turtles all the way down and all.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e8f0c85..fb7e23e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6122,12 +6122,6 @@ static struct i915_power_well vlv_power_wells[] = {
>  		.ops = &vlv_display_power_well_ops,
>  	},
>  	{
> -		.name = "dpio-common",
> -		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
> -		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
> -		.ops = &vlv_dpio_power_well_ops,
> -	},
> -	{
>  		.name = "dpio-tx-b-01",
>  		.domains = VLV_DPIO_TX_B_LANES_01_POWER_DOMAINS |
>  			   VLV_DPIO_TX_B_LANES_23_POWER_DOMAINS |
> @@ -6163,6 +6157,12 @@ static struct i915_power_well vlv_power_wells[] = {
>  		.ops = &vlv_dpio_power_well_ops,
>  		.data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
>  	},
> +	{
> +		.name = "dpio-common",
> +		.domains = VLV_DPIO_CMN_BC_POWER_DOMAINS,
> +		.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
> +		.ops = &vlv_dpio_power_well_ops,
> +	},
>  };
>  
>  #define set_power_wells(power_domains, __power_wells) ({		\
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
  2014-05-27 19:32 ` [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Ville Syrjälä
@ 2014-05-27 20:24   ` Daniel Vetter
  2014-05-28  8:00     ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2014-05-27 20:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, May 27, 2014 at 10:32:47PM +0300, Ville Syrjälä wrote:
> On Fri, May 23, 2014 at 01:16:40PM -0700, Jesse Barnes wrote:
> > This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
> > that it resets the whole common lane section of the PHY.  This is
> > required on machines where the BIOS doesn't do this for us on boot or
> > resume to properly re-calibrate and get the PHY ready to transmit data.
> > 
> > Without this patch, such machines won't resume correctly much of the time,
> > with the symptom being a 'port ready' timeout and/or a link training
> > failure.
> > 
> > Note that simply asserting reset at suspend and de-asserting at resume
> > is not sufficient, nor is simply de-asserting at boot.  Both of these
> > cases have been tested and have still been found to have failures on
> > some configurations.
> > 
> > v2: extract simpler set_power_well function for use in reset_dpio (Imre)
> >     move to reset_dpio (Daniel & Ville)
> > v3: don't reset if DPIO reset is already de-asserted (Imre)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> The series matches my understanding of the limitations of the PHY, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

All merged to dinq, thanks.
 
> But if these limitations are real, then I think we would also need to
> adjust the power domains to power up all the wells whenever even a
> single one is required.
> 
> This should be testable I think by simply:
> 1. disable both ports
> 2. enable one port
> 3. enable the other port
> 
> At step 3. the common well is already up, so the TX wells for the second
> port should come up in some kind of poor state.

Hm, if we need this we might forc a modeset for _all_ pipes on vlv, even
for unchanged ports. At elast as long as we enable something new. That
should make this work properly I hope.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3
  2014-05-27 20:24   ` Daniel Vetter
@ 2014-05-28  8:00     ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2014-05-28  8:00 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, May 27, 2014 at 10:24:14PM +0200, Daniel Vetter wrote:
> On Tue, May 27, 2014 at 10:32:47PM +0300, Ville Syrjälä wrote:
> > On Fri, May 23, 2014 at 01:16:40PM -0700, Jesse Barnes wrote:
> > > This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
> > > that it resets the whole common lane section of the PHY.  This is
> > > required on machines where the BIOS doesn't do this for us on boot or
> > > resume to properly re-calibrate and get the PHY ready to transmit data.
> > > 
> > > Without this patch, such machines won't resume correctly much of the time,
> > > with the symptom being a 'port ready' timeout and/or a link training
> > > failure.
> > > 
> > > Note that simply asserting reset at suspend and de-asserting at resume
> > > is not sufficient, nor is simply de-asserting at boot.  Both of these
> > > cases have been tested and have still been found to have failures on
> > > some configurations.
> > > 
> > > v2: extract simpler set_power_well function for use in reset_dpio (Imre)
> > >     move to reset_dpio (Daniel & Ville)
> > > v3: don't reset if DPIO reset is already de-asserted (Imre)
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > The series matches my understanding of the limitations of the PHY, so:
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All merged to dinq, thanks.
>  
> > But if these limitations are real, then I think we would also need to
> > adjust the power domains to power up all the wells whenever even a
> > single one is required.
> > 
> > This should be testable I think by simply:
> > 1. disable both ports
> > 2. enable one port
> > 3. enable the other port
> > 
> > At step 3. the common well is already up, so the TX wells for the second
> > port should come up in some kind of poor state.
> 
> Hm, if we need this we might forc a modeset for _all_ pipes on vlv, even
> for unchanged ports. At elast as long as we enable something new. That
> should make this work properly I hope.

Yeah that would work too, but obviously would cause some blinking that
might be a bit disturbing. But we may have such blinking already due to
adjusting cdclk. If the blinking is disturbing for users we might want
to have a knob for controlling it: either use less power but blink more,
or waste a bit of power and blink less. But I don't know if anyone would
really want to waste power.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-05-28  8:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-23 20:16 [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Jesse Barnes
2014-05-23 20:16 ` [PATCH 2/6] drm/i915/vlv: drop power well enable in uncore_sanitize Jesse Barnes
2014-05-23 20:16 ` [PATCH 3/6] drm/i915/vlv: move CRI refclk enable into __vlv_set_power_well Jesse Barnes
2014-05-23 20:16 ` [PATCH 4/6] drm/i915/vlv: re-order power wells so DPIO common comes after TX Jesse Barnes
2014-05-27 20:21   ` Daniel Vetter
2014-05-23 20:16 ` [PATCH 5/6] drm/i915/vlv: move DPIO common reset de-assert into __vlv_set_power_well Jesse Barnes
2014-05-23 20:16 ` [PATCH 6/6] drm/i915/vlv: add pll assertion when disabling DPIO common well Jesse Barnes
2014-05-27 19:32 ` [PATCH 1/6] drm/i915/vlv: assert and de-assert sideband reset at boot and resume v3 Ville Syrjälä
2014-05-27 20:24   ` Daniel Vetter
2014-05-28  8:00     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox