[PATCH 0/5] drm/i915/icl: Fix combo PHY HW context loss

[PATCH 0/5] drm/i915/icl: Fix combo PHY HW context loss

[Imre Deak]

This patchset fixes an issue where DMC/PCODE doesn't retain the HW
context for the port B combo PHY as one would expect (and as it happens
correctly for the port A combo PHY).

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Imre Deak (5):
  drm/i915/icl: Fix combo PHY uninit
  drm/i915/cnl+: Move the combo PHY init/uninit code to a new file
  drm/i915/cnl+: Verify combo PHY HW state during PHY uninit
  drm/i915/icl: Skip init for an already enabled combo PHY
  drm/i915/icl: Fix port B combo PHY context loss after DC transitions

 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_drv.h         |   6 +
 drivers/gpu/drm/i915/intel_combo_phy.c  | 264 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 131 ++--------------
 4 files changed, 287 insertions(+), 115 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c

-- 
2.13.2

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

[PATCH 1/5] drm/i915/icl: Fix combo PHY uninit

[Imre Deak]

BSpec says to clear the comp init HW flag too during combo PHY uninit,
so do that. The lack of this could badly interact with the PHY reinit
after a DC6/9 transition at least, where (after a follow-up patch fixing
the init code) we'd skip the initialization incorrectly due to this flag
being set.

BSpec: 21257
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.c
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6c453366cd24..a7eea8423580 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3668,6 +3668,10 @@ void icl_display_core_uninit(struct drm_i915_private *dev_priv)
 		val = I915_READ(ICL_PHY_MISC(port));
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);
+
+		val = I915_READ(ICL_PORT_COMP_DW0(port));
+		val &= ~COMP_INIT;
+		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
 	}
 }
 
-- 
2.13.2

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

[PATCH 2/5] drm/i915/cnl+: Move the combo PHY init/uninit code to a new file

[Imre Deak]

Similarly to the GEN9_LP DPIO PHY code keep the CNL+ combo PHY code in a
separate file.

No functional change.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_drv.h         |   6 ++
 drivers/gpu/drm/i915/intel_combo_phy.c  | 159 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 127 ++-----------------------
 4 files changed, 174 insertions(+), 119 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 28c7d7884e88..1e7e9513bb10 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -113,6 +113,7 @@ i915-y += intel_audio.o \
 	  intel_bios.o \
 	  intel_cdclk.o \
 	  intel_color.o \
+	  intel_combo_phy.o \
 	  intel_connector.o \
 	  intel_display.o \
 	  intel_dpio_phy.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6157f8128cc5..62882e1ddbee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3571,6 +3571,12 @@ void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder,
 void vlv_phy_reset_lanes(struct intel_encoder *encoder,
 			 const struct intel_crtc_state *old_crtc_state);
 
+/* intel_combo_phy.c */
+void icl_combo_phys_init(struct drm_i915_private *dev_priv);
+void icl_combo_phys_uninit(struct drm_i915_private *dev_priv);
+void cnl_combo_phys_init(struct drm_i915_private *dev_priv);
+void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv);
+
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
 u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
new file mode 100644
index 000000000000..13184ae5a217
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -0,0 +1,159 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "intel_drv.h"
+
+enum {
+	PROCMON_0_85V_DOT_0,
+	PROCMON_0_95V_DOT_0,
+	PROCMON_0_95V_DOT_1,
+	PROCMON_1_05V_DOT_0,
+	PROCMON_1_05V_DOT_1,
+};
+
+static const struct cnl_procmon {
+	u32 dw1, dw9, dw10;
+} cnl_procmon_values[] = {
+	[PROCMON_0_85V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, },
+	[PROCMON_0_95V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, },
+	[PROCMON_0_95V_DOT_1] =
+		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, },
+	[PROCMON_1_05V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, },
+	[PROCMON_1_05V_DOT_1] =
+		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },
+};
+
+/*
+ * CNL has just one set of registers, while ICL has two sets: one for port A and
+ * the other for port B. The CNL registers are equivalent to the ICL port A
+ * registers, that's why we call the ICL macros even though the function has CNL
+ * on its name.
+ */
+static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
+				       enum port port)
+{
+	const struct cnl_procmon *procmon;
+	u32 val;
+
+	val = I915_READ(ICL_PORT_COMP_DW3(port));
+	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
+	default:
+		MISSING_CASE(val);
+		/* fall through */
+	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
+		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
+		break;
+	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
+		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
+		break;
+	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
+		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
+		break;
+	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
+		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
+		break;
+	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
+		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
+		break;
+	}
+
+	val = I915_READ(ICL_PORT_COMP_DW1(port));
+	val &= ~((0xff << 16) | 0xff);
+	val |= procmon->dw1;
+	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
+
+	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
+	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
+}
+
+void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(CHICKEN_MISC_2);
+	val &= ~CNL_COMP_PWR_DOWN;
+	I915_WRITE(CHICKEN_MISC_2, val);
+
+	/* Dummy PORT_A to get the correct CNL register from the ICL macro */
+	cnl_set_procmon_ref_values(dev_priv, PORT_A);
+
+	val = I915_READ(CNL_PORT_COMP_DW0);
+	val |= COMP_INIT;
+	I915_WRITE(CNL_PORT_COMP_DW0, val);
+
+	val = I915_READ(CNL_PORT_CL1CM_DW5);
+	val |= CL_POWER_DOWN_ENABLE;
+	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
+}
+
+void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(CHICKEN_MISC_2);
+	val |= CNL_COMP_PWR_DOWN;
+	I915_WRITE(CHICKEN_MISC_2, val);
+}
+
+void icl_combo_phys_init(struct drm_i915_private *dev_priv)
+{
+	enum port port;
+
+	for (port = PORT_A; port <= PORT_B; port++) {
+		u32 val;
+
+		val = I915_READ(ICL_PHY_MISC(port));
+		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
+		I915_WRITE(ICL_PHY_MISC(port), val);
+
+		cnl_set_procmon_ref_values(dev_priv, port);
+
+		val = I915_READ(ICL_PORT_COMP_DW0(port));
+		val |= COMP_INIT;
+		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
+
+		val = I915_READ(ICL_PORT_CL_DW5(port));
+		val |= CL_POWER_DOWN_ENABLE;
+		I915_WRITE(ICL_PORT_CL_DW5(port), val);
+	}
+}
+
+void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
+{
+	enum port port;
+
+	for (port = PORT_A; port <= PORT_B; port++) {
+		u32 val;
+
+		val = I915_READ(ICL_PHY_MISC(port));
+		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
+		I915_WRITE(ICL_PHY_MISC(port), val);
+
+		val = I915_READ(ICL_PORT_COMP_DW0(port));
+		val &= ~COMP_INIT;
+		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index a7eea8423580..f8da471e81aa 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3436,99 +3436,18 @@ void bxt_display_core_uninit(struct drm_i915_private *dev_priv)
 	usleep_range(10, 30);		/* 10 us delay per Bspec */
 }
 
-enum {
-	PROCMON_0_85V_DOT_0,
-	PROCMON_0_95V_DOT_0,
-	PROCMON_0_95V_DOT_1,
-	PROCMON_1_05V_DOT_0,
-	PROCMON_1_05V_DOT_1,
-};
-
-static const struct cnl_procmon {
-	u32 dw1, dw9, dw10;
-} cnl_procmon_values[] = {
-	[PROCMON_0_85V_DOT_0] =
-		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, },
-	[PROCMON_0_95V_DOT_0] =
-		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, },
-	[PROCMON_0_95V_DOT_1] =
-		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, },
-	[PROCMON_1_05V_DOT_0] =
-		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, },
-	[PROCMON_1_05V_DOT_1] =
-		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },
-};
-
-/*
- * CNL has just one set of registers, while ICL has two sets: one for port A and
- * the other for port B. The CNL registers are equivalent to the ICL port A
- * registers, that's why we call the ICL macros even though the function has CNL
- * on its name.
- */
-static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
-				       enum port port)
-{
-	const struct cnl_procmon *procmon;
-	u32 val;
-
-	val = I915_READ(ICL_PORT_COMP_DW3(port));
-	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
-	default:
-		MISSING_CASE(val);
-		/* fall through */
-	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
-		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
-		break;
-	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
-		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
-		break;
-	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
-		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
-		break;
-	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
-		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
-		break;
-	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
-		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
-		break;
-	}
-
-	val = I915_READ(ICL_PORT_COMP_DW1(port));
-	val &= ~((0xff << 16) | 0xff);
-	val |= procmon->dw1;
-	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
-
-	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
-	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
-}
-
 static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	struct i915_power_well *well;
-	u32 val;
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
 	/* 1. Enable PCH Reset Handshake */
 	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
-	/* 2. Enable Comp */
-	val = I915_READ(CHICKEN_MISC_2);
-	val &= ~CNL_COMP_PWR_DOWN;
-	I915_WRITE(CHICKEN_MISC_2, val);
-
-	/* Dummy PORT_A to get the correct CNL register from the ICL macro */
-	cnl_set_procmon_ref_values(dev_priv, PORT_A);
-
-	val = I915_READ(CNL_PORT_COMP_DW0);
-	val |= COMP_INIT;
-	I915_WRITE(CNL_PORT_COMP_DW0, val);
-
-	/* 3. */
-	val = I915_READ(CNL_PORT_CL1CM_DW5);
-	val |= CL_POWER_DOWN_ENABLE;
-	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
+	/* 2-3. */
+	cnl_combo_phys_init(dev_priv);
 
 	/*
 	 * 4. Enable Power Well 1 (PG1).
@@ -3553,7 +3472,6 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	struct i915_power_well *well;
-	u32 val;
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
@@ -3577,10 +3495,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
 
 	usleep_range(10, 30);		/* 10 us delay per Bspec */
 
-	/* 5. Disable Comp */
-	val = I915_READ(CHICKEN_MISC_2);
-	val |= CNL_COMP_PWR_DOWN;
-	I915_WRITE(CHICKEN_MISC_2, val);
+	/* 5. */
+	cnl_combo_phys_uninit(dev_priv);
 }
 
 void icl_display_core_init(struct drm_i915_private *dev_priv,
@@ -3588,31 +3504,14 @@ void icl_display_core_init(struct drm_i915_private *dev_priv,
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	struct i915_power_well *well;
-	enum port port;
-	u32 val;
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
 	/* 1. Enable PCH reset handshake. */
 	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
-	for (port = PORT_A; port <= PORT_B; port++) {
-		/* 2. Enable DDI combo PHY comp. */
-		val = I915_READ(ICL_PHY_MISC(port));
-		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
-		I915_WRITE(ICL_PHY_MISC(port), val);
-
-		cnl_set_procmon_ref_values(dev_priv, port);
-
-		val = I915_READ(ICL_PORT_COMP_DW0(port));
-		val |= COMP_INIT;
-		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
-
-		/* 3. Set power down enable. */
-		val = I915_READ(ICL_PORT_CL_DW5(port));
-		val |= CL_POWER_DOWN_ENABLE;
-		I915_WRITE(ICL_PORT_CL_DW5(port), val);
-	}
+	/* 2-3. */
+	icl_combo_phys_init(dev_priv);
 
 	/*
 	 * 4. Enable Power Well 1 (PG1).
@@ -3640,8 +3539,6 @@ void icl_display_core_uninit(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 	struct i915_power_well *well;
-	enum port port;
-	u32 val;
 
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
@@ -3663,16 +3560,8 @@ void icl_display_core_uninit(struct drm_i915_private *dev_priv)
 	intel_power_well_disable(dev_priv, well);
 	mutex_unlock(&power_domains->lock);
 
-	/* 5. Disable Comp */
-	for (port = PORT_A; port <= PORT_B; port++) {
-		val = I915_READ(ICL_PHY_MISC(port));
-		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
-		I915_WRITE(ICL_PHY_MISC(port), val);
-
-		val = I915_READ(ICL_PORT_COMP_DW0(port));
-		val &= ~COMP_INIT;
-		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
-	}
+	/* 5. */
+	icl_combo_phys_uninit(dev_priv);
 }
 
 static void chv_phy_control_init(struct drm_i915_private *dev_priv)
-- 
2.13.2

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

[PATCH 3/5] drm/i915/cnl+: Verify combo PHY HW state during PHY uninit

[Imre Deak]

Verify on CNL, ICL that the combo PHY HW state stayed intact after PHY
initialization.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103 ++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index 13184ae5a217..1522e2a25390 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -52,8 +52,8 @@ static const struct cnl_procmon {
  * registers, that's why we call the ICL macros even though the function has CNL
  * on its name.
  */
-static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
-				       enum port port)
+static const struct cnl_procmon *
+cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum port port)
 {
 	const struct cnl_procmon *procmon;
 	u32 val;
@@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
 		break;
 	}
 
+	return procmon;
+}
+
+static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
+				       enum port port)
+{
+	const struct cnl_procmon *procmon;
+	u32 val;
+
+	procmon = cnl_get_procmon_ref_values(dev_priv, port);
+
 	val = I915_READ(ICL_PORT_COMP_DW1(port));
 	val &= ~((0xff << 16) | 0xff);
 	val |= procmon->dw1;
@@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
 	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
 }
 
+static bool check_phy_reg(struct drm_i915_private *dev_priv,
+			  enum port port, i915_reg_t reg, u32 mask,
+			  u32 expected_val)
+{
+	u32 val = I915_READ(reg);
+
+	if ((val & mask) != expected_val) {
+		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state mismatch: "
+				 "current %08x mask %08x expected %08x\n",
+				 port_name(port),
+				 reg.reg, val, mask, expected_val);
+		return false;
+	}
+
+	return true;
+}
+
+static bool cnl_verify_procmon_ref_values(struct drm_i915_private *dev_priv,
+					  enum port port)
+{
+	const struct cnl_procmon *procmon;
+	bool ret;
+
+	procmon = cnl_get_procmon_ref_values(dev_priv, port);
+
+	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
+			    (0xff << 16) | 0xff, procmon->dw1);
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
+			     -1U, procmon->dw9);
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
+			     -1U, procmon->dw10);
+
+	return ret;
+}
+
+static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
+{
+	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
+		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
+}
+
+static bool cnl_combo_phy_verify_state(struct drm_i915_private *dev_priv)
+{
+	enum port port = PORT_A;
+	bool ret;
+
+	if (!cnl_combo_phy_enabled(dev_priv))
+		return false;
+
+	ret = cnl_verify_procmon_ref_values(dev_priv, port);
+
+	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
+			     CL_POWER_DOWN_ENABLE, CL_POWER_DOWN_ENABLE);
+
+	return ret;
+}
+
 void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	u32 val;
@@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 {
 	u32 val;
 
+	if (!cnl_combo_phy_verify_state(dev_priv))
+		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
+
 	val = I915_READ(CHICKEN_MISC_2);
 	val |= CNL_COMP_PWR_DOWN;
 	I915_WRITE(CHICKEN_MISC_2, val);
 }
 
+static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
+				  enum port port)
+{
+	return !(I915_READ(ICL_PHY_MISC(port)) &
+		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
+		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
+}
+
+static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
+				       enum port port)
+{
+	bool ret;
+
+	if (!icl_combo_phy_enabled(dev_priv, port))
+		return false;
+
+	ret = cnl_verify_procmon_ref_values(dev_priv, port);
+
+	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
+			     CL_POWER_DOWN_ENABLE, CL_POWER_DOWN_ENABLE);
+
+	return ret;
+}
+
 void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 {
 	enum port port;
@@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
 	for (port = PORT_A; port <= PORT_B; port++) {
 		u32 val;
 
+		if (!icl_combo_phy_verify_state(dev_priv, port))
+			DRM_WARN("Port-%c combo PHY HW state changed unexpectedly\n",
+				 port_name(port));
+
 		val = I915_READ(ICL_PHY_MISC(port));
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);
-- 
2.13.2

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

[PATCH 4/5] drm/i915/icl: Skip init for an already enabled combo PHY

[Imre Deak]

Bspec says we should skip the initialization of combo PHYs that are
already initialized. We'll need to reinit the PHYs more frequently
when exiting from DC6 (after the next patch), so let's make sure the
uninit sequence complies with the spec. For safety skip the init only if
all the PHY register fields have their expected values.

Bspec: 21257
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
index 1522e2a25390..7b911702f834 100644
--- a/drivers/gpu/drm/i915/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/intel_combo_phy.c
@@ -220,6 +220,12 @@ void icl_combo_phys_init(struct drm_i915_private *dev_priv)
 	for (port = PORT_A; port <= PORT_B; port++) {
 		u32 val;
 
+		if (icl_combo_phy_verify_state(dev_priv, port)) {
+			DRM_DEBUG_DRIVER("Port-%c combo PHY already enabled, won't reprogram it.\n",
+					 port_name(port));
+			continue;
+		}
+
 		val = I915_READ(ICL_PHY_MISC(port));
 		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);
-- 
2.13.2

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

[PATCH 5/5] drm/i915/icl: Fix port B combo PHY context loss after DC transitions

[Imre Deak]

On ICL DMC/PCODE retains the HW context only for port A across DC
transitions, for the other port B combo PHY, it doesn't. So we need to
do this manually after exiting from DC6. Do the reinit even after
exiting from DC5, it won't hurt since we only reinit the PHY in case
it's needed (in case it was disabled to begin with).

As can be guessed from the bugzilla report leaving the PHY uninited will
lead to a later timeout during the port B specific AUX and DDI_IO power
well enabling.

Bspec: 21257
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f8da471e81aa..763912c0245c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -843,6 +843,14 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
 
 	if (IS_GEN9_LP(dev_priv))
 		bxt_verify_ddi_phy_power_wells(dev_priv);
+
+	if (IS_ICELAKE(dev_priv))
+		/*
+		 * DMC retains HW context only for port A, the other combo
+		 * PHY's HW context for port B is lost after DC transitions,
+		 * so we need to restore it manually.
+		 */
+		icl_combo_phys_init(dev_priv);
 }
 
 static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
-- 
2.13.2

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: Fix combo PHY HW context loss

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix combo PHY HW context loss
URL   : https://patchwork.freedesktop.org/series/51970/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
59293d4a4153 drm/i915/icl: Fix combo PHY uninit
-:19: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Rodrigo Vivi <rodrigo.vivi@intel.c'
#19: 
Cc: Rodrigo Vivi <rodrigo.vivi@intel.c

total: 1 errors, 0 warnings, 0 checks, 10 lines checked
8b4de5e03e41 drm/i915/cnl+: Move the combo PHY init/uninit code to a new file
-:52: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#52: 
new file mode 100644

-:57: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#57: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:1:
+/*

-:94: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#94: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:38:
+	[PROCMON_0_85V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, },

-:96: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#96: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:40:
+	[PROCMON_0_95V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, },

-:98: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#98: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:42:
+	[PROCMON_0_95V_DOT_1] =
+		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, },

-:100: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#100: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:44:
+	[PROCMON_1_05V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, },

-:102: ERROR:OPEN_BRACE: that open brace { should be on the previous line
#102: FILE: drivers/gpu/drm/i915/intel_combo_phy.c:46:
+	[PROCMON_1_05V_DOT_1] =
+		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },

total: 5 errors, 2 warnings, 0 checks, 357 lines checked
3b95ec9736d7 drm/i915/cnl+: Verify combo PHY HW state during PHY uninit
b6aea4a716dd drm/i915/icl: Skip init for an already enabled combo PHY
162b1a44d002 drm/i915/icl: Fix port B combo PHY context loss after DC transitions

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

✗ Fi.CI.SPARSE: warning for drm/i915/icl: Fix combo PHY HW context loss

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix combo PHY HW context loss
URL   : https://patchwork.freedesktop.org/series/51970/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/icl: Fix combo PHY uninit
Okay!

Commit: drm/i915/cnl+: Move the combo PHY init/uninit code to a new file
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3705:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3711:16: warning: expression using sizeof(void)
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Commit: drm/i915/cnl+: Verify combo PHY HW state during PHY uninit
Okay!

Commit: drm/i915/icl: Skip init for an already enabled combo PHY
Okay!

Commit: drm/i915/icl: Fix port B combo PHY context loss after DC transitions
Okay!

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

Re: [PATCH 5/5] drm/i915/icl: Fix port B combo PHY context loss after DC transitions

[Rodrigo Vivi]

On Fri, Nov 02, 2018 at 08:07:06PM +0200, Imre Deak wrote:
> On ICL DMC/PCODE retains the HW context only for port A across DC
> transitions, for the other port B combo PHY, it doesn't. So we need to
> do this manually after exiting from DC6. Do the reinit even after
> exiting from DC5, it won't hurt since we only reinit the PHY in case
> it's needed (in case it was disabled to begin with).
> 
> As can be guessed from the bugzilla report leaving the PHY uninited will
> lead to a later timeout during the port B specific AUX and DDI_IO power
> well enabling.
> 
> Bspec: 21257
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f8da471e81aa..763912c0245c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -843,6 +843,14 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	if (IS_GEN9_LP(dev_priv))
>  		bxt_verify_ddi_phy_power_wells(dev_priv);
> +
> +	if (IS_ICELAKE(dev_priv))

maybe INTEL_GEN(dev_priv) >= 11?


with or without:

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



> +		/*
> +		 * DMC retains HW context only for port A, the other combo
> +		 * PHY's HW context for port B is lost after DC transitions,
> +		 * so we need to restore it manually.
> +		 */
> +		icl_combo_phys_init(dev_priv);
>  }
>  
>  static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> -- 
> 2.13.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/icl: Fix combo PHY HW context loss

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix combo PHY HW context loss
URL   : https://patchwork.freedesktop.org/series/51970/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5081 -> Patchwork_10714 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51970/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10714 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107556, fdo#107774, fdo#107859)

    igt@kms_busy@basic-flip-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#103182)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-icl-u2:          DMESG-WARN (fdo#108070) -> PASS

    igt@gem_cpu_reloc@basic:
      fi-kbl-7560u:       INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_contexts:
      fi-icl-u:           DMESG-FAIL (fdo#108569) -> INCOMPLETE (fdo#108315)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103182 https://bugs.freedesktop.org/show_bug.cgi?id=103182
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
  fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
  fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
  fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569


== Participating hosts (50 -> 46) ==

  Additional (1): fi-kbl-soraka 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5081 -> Patchwork_10714

  CI_DRM_5081: f5e16acf6c85d38756c3efdb77ec6aede55df0ba @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4705: 7983e19ed62ec8db1884f55e07e458a62cc51e37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10714: 162b1a44d002f2bd5df1b4f1b70f5b744d1f5733 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

162b1a44d002 drm/i915/icl: Fix port B combo PHY context loss after DC transitions
b6aea4a716dd drm/i915/icl: Skip init for an already enabled combo PHY
3b95ec9736d7 drm/i915/cnl+: Verify combo PHY HW state during PHY uninit
8b4de5e03e41 drm/i915/cnl+: Move the combo PHY init/uninit code to a new file
59293d4a4153 drm/i915/icl: Fix combo PHY uninit

== Logs ==

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

Re: [PATCH 2/5] drm/i915/cnl+: Move the combo PHY init/uninit code to a new file

[Rodrigo Vivi]

On Fri, Nov 02, 2018 at 08:07:03PM +0200, Imre Deak wrote:
> Similarly to the GEN9_LP DPIO PHY code keep the CNL+ combo PHY code in a
> separate file.
> 
> No functional change.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile           |   1 +
>  drivers/gpu/drm/i915/i915_drv.h         |   6 ++
>  drivers/gpu/drm/i915/intel_combo_phy.c  | 159 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 127 ++-----------------------
>  4 files changed, 174 insertions(+), 119 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 28c7d7884e88..1e7e9513bb10 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -113,6 +113,7 @@ i915-y += intel_audio.o \
>  	  intel_bios.o \
>  	  intel_cdclk.o \
>  	  intel_color.o \
> +	  intel_combo_phy.o \
>  	  intel_connector.o \
>  	  intel_display.o \
>  	  intel_dpio_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6157f8128cc5..62882e1ddbee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3571,6 +3571,12 @@ void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder,
>  void vlv_phy_reset_lanes(struct intel_encoder *encoder,
>  			 const struct intel_crtc_state *old_crtc_state);
>  
> +/* intel_combo_phy.c */
> +void icl_combo_phys_init(struct drm_i915_private *dev_priv);
> +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> +void cnl_combo_phys_init(struct drm_i915_private *dev_priv);
> +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> +
>  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
>  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
>  u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> new file mode 100644
> index 000000000000..13184ae5a217
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */

SPDX identifiers or not?!

anyway:


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



> +
> +#include "intel_drv.h"
> +
> +enum {
> +	PROCMON_0_85V_DOT_0,
> +	PROCMON_0_95V_DOT_0,
> +	PROCMON_0_95V_DOT_1,
> +	PROCMON_1_05V_DOT_0,
> +	PROCMON_1_05V_DOT_1,
> +};
> +
> +static const struct cnl_procmon {
> +	u32 dw1, dw9, dw10;
> +} cnl_procmon_values[] = {
> +	[PROCMON_0_85V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, },
> +	[PROCMON_0_95V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, },
> +	[PROCMON_0_95V_DOT_1] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, },
> +	[PROCMON_1_05V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, },
> +	[PROCMON_1_05V_DOT_1] =
> +		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },
> +};
> +
> +/*
> + * CNL has just one set of registers, while ICL has two sets: one for port A and
> + * the other for port B. The CNL registers are equivalent to the ICL port A
> + * registers, that's why we call the ICL macros even though the function has CNL
> + * on its name.
> + */
> +static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	val = I915_READ(ICL_PORT_COMP_DW3(port));
> +	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> +	default:
> +		MISSING_CASE(val);
> +		/* fall through */
> +	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> +		break;
> +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> +		break;
> +	}
> +
> +	val = I915_READ(ICL_PORT_COMP_DW1(port));
> +	val &= ~((0xff << 16) | 0xff);
> +	val |= procmon->dw1;
> +	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> +
> +	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> +	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> +}
> +
> +void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	val = I915_READ(CHICKEN_MISC_2);
> +	val &= ~CNL_COMP_PWR_DOWN;
> +	I915_WRITE(CHICKEN_MISC_2, val);
> +
> +	/* Dummy PORT_A to get the correct CNL register from the ICL macro */
> +	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> +
> +	val = I915_READ(CNL_PORT_COMP_DW0);
> +	val |= COMP_INIT;
> +	I915_WRITE(CNL_PORT_COMP_DW0, val);
> +
> +	val = I915_READ(CNL_PORT_CL1CM_DW5);
> +	val |= CL_POWER_DOWN_ENABLE;
> +	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> +}
> +
> +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	val = I915_READ(CHICKEN_MISC_2);
> +	val |= CNL_COMP_PWR_DOWN;
> +	I915_WRITE(CHICKEN_MISC_2, val);
> +}
> +
> +void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> +{
> +	enum port port;
> +
> +	for (port = PORT_A; port <= PORT_B; port++) {
> +		u32 val;
> +
> +		val = I915_READ(ICL_PHY_MISC(port));
> +		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +		I915_WRITE(ICL_PHY_MISC(port), val);
> +
> +		cnl_set_procmon_ref_values(dev_priv, port);
> +
> +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> +		val |= COMP_INIT;
> +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> +
> +		val = I915_READ(ICL_PORT_CL_DW5(port));
> +		val |= CL_POWER_DOWN_ENABLE;
> +		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> +	}
> +}
> +
> +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> +{
> +	enum port port;
> +
> +	for (port = PORT_A; port <= PORT_B; port++) {
> +		u32 val;
> +
> +		val = I915_READ(ICL_PHY_MISC(port));
> +		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +		I915_WRITE(ICL_PHY_MISC(port), val);
> +
> +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> +		val &= ~COMP_INIT;
> +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index a7eea8423580..f8da471e81aa 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3436,99 +3436,18 @@ void bxt_display_core_uninit(struct drm_i915_private *dev_priv)
>  	usleep_range(10, 30);		/* 10 us delay per Bspec */
>  }
>  
> -enum {
> -	PROCMON_0_85V_DOT_0,
> -	PROCMON_0_95V_DOT_0,
> -	PROCMON_0_95V_DOT_1,
> -	PROCMON_1_05V_DOT_0,
> -	PROCMON_1_05V_DOT_1,
> -};
> -
> -static const struct cnl_procmon {
> -	u32 dw1, dw9, dw10;
> -} cnl_procmon_values[] = {
> -	[PROCMON_0_85V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 = 0x51914F96, },
> -	[PROCMON_0_95V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 = 0x77CA5EAB, },
> -	[PROCMON_0_95V_DOT_1] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 = 0x8AE871C5, },
> -	[PROCMON_1_05V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 = 0x89E46DC1, },
> -	[PROCMON_1_05V_DOT_1] =
> -		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 = 0x8AE38FF1, },
> -};
> -
> -/*
> - * CNL has just one set of registers, while ICL has two sets: one for port A and
> - * the other for port B. The CNL registers are equivalent to the ICL port A
> - * registers, that's why we call the ICL macros even though the function has CNL
> - * on its name.
> - */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv,
> -				       enum port port)
> -{
> -	const struct cnl_procmon *procmon;
> -	u32 val;
> -
> -	val = I915_READ(ICL_PORT_COMP_DW3(port));
> -	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> -	default:
> -		MISSING_CASE(val);
> -		/* fall through */
> -	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> -		break;
> -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> -		break;
> -	}
> -
> -	val = I915_READ(ICL_PORT_COMP_DW1(port));
> -	val &= ~((0xff << 16) | 0xff);
> -	val |= procmon->dw1;
> -	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> -
> -	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> -	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> -}
> -
>  static void cnl_display_core_init(struct drm_i915_private *dev_priv, bool resume)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* 1. Enable PCH Reset Handshake */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> -	/* 2. Enable Comp */
> -	val = I915_READ(CHICKEN_MISC_2);
> -	val &= ~CNL_COMP_PWR_DOWN;
> -	I915_WRITE(CHICKEN_MISC_2, val);
> -
> -	/* Dummy PORT_A to get the correct CNL register from the ICL macro */
> -	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> -
> -	val = I915_READ(CNL_PORT_COMP_DW0);
> -	val |= COMP_INIT;
> -	I915_WRITE(CNL_PORT_COMP_DW0, val);
> -
> -	/* 3. */
> -	val = I915_READ(CNL_PORT_CL1CM_DW5);
> -	val |= CL_POWER_DOWN_ENABLE;
> -	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> +	/* 2-3. */
> +	cnl_combo_phys_init(dev_priv);
>  
>  	/*
>  	 * 4. Enable Power Well 1 (PG1).
> @@ -3553,7 +3472,6 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> @@ -3577,10 +3495,8 @@ static void cnl_display_core_uninit(struct drm_i915_private *dev_priv)
>  
>  	usleep_range(10, 30);		/* 10 us delay per Bspec */
>  
> -	/* 5. Disable Comp */
> -	val = I915_READ(CHICKEN_MISC_2);
> -	val |= CNL_COMP_PWR_DOWN;
> -	I915_WRITE(CHICKEN_MISC_2, val);
> +	/* 5. */
> +	cnl_combo_phys_uninit(dev_priv);
>  }
>  
>  void icl_display_core_init(struct drm_i915_private *dev_priv,
> @@ -3588,31 +3504,14 @@ void icl_display_core_init(struct drm_i915_private *dev_priv,
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
> -	enum port port;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* 1. Enable PCH reset handshake. */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> -	for (port = PORT_A; port <= PORT_B; port++) {
> -		/* 2. Enable DDI combo PHY comp. */
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> -
> -		cnl_set_procmon_ref_values(dev_priv, port);
> -
> -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> -		val |= COMP_INIT;
> -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> -
> -		/* 3. Set power down enable. */
> -		val = I915_READ(ICL_PORT_CL_DW5(port));
> -		val |= CL_POWER_DOWN_ENABLE;
> -		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> -	}
> +	/* 2-3. */
> +	icl_combo_phys_init(dev_priv);
>  
>  	/*
>  	 * 4. Enable Power Well 1 (PG1).
> @@ -3640,8 +3539,6 @@ void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  	struct i915_power_well *well;
> -	enum port port;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> @@ -3663,16 +3560,8 @@ void icl_display_core_uninit(struct drm_i915_private *dev_priv)
>  	intel_power_well_disable(dev_priv, well);
>  	mutex_unlock(&power_domains->lock);
>  
> -	/* 5. Disable Comp */
> -	for (port = PORT_A; port <= PORT_B; port++) {
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> -
> -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> -		val &= ~COMP_INIT;
> -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> -	}
> +	/* 5. */
> +	icl_combo_phys_uninit(dev_priv);
>  }
>  
>  static void chv_phy_control_init(struct drm_i915_private *dev_priv)
> -- 
> 2.13.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915/icl: Skip init for an already enabled combo PHY

[Rodrigo Vivi]

On Fri, Nov 02, 2018 at 08:07:05PM +0200, Imre Deak wrote:
> Bspec says we should skip the initialization of combo PHYs that are
> already initialized. We'll need to reinit the PHYs more frequently
> when exiting from DC6 (after the next patch), so let's make sure the
> uninit sequence complies with the spec. For safety skip the init only if
> all the PHY register fields have their expected values.
> 
> Bspec: 21257
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 1522e2a25390..7b911702f834 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -220,6 +220,12 @@ void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (icl_combo_phy_verify_state(dev_priv, port)) {
> +			DRM_DEBUG_DRIVER("Port-%c combo PHY already enabled, won't reprogram it.\n",
> +					 port_name(port));

Note since I didn't rv-b the previous patch:
I have checked the previous patch that introduces this and it is indeed correct:

"If PORT_COMP_DW0 Comp Init == 1b, skip the rest of this sequence since it is already initialized"

so,

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

> +			continue;
> +		}
> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
> -- 
> 2.13.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915/icl: Fix combo PHY uninit

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> BSpec says to clear the comp init HW flag too during combo PHY
> uninit,
> so do that. The lack of this could badly interact with the PHY reinit
> after a DC6/9 transition at least, where (after a follow-up patch
> fixing
> the init code) we'd skip the initialization incorrectly due to this
> flag
> being set.
> 
> BSpec: 21257
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.c
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6c453366cd24..a7eea8423580 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3668,6 +3668,10 @@ void icl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
> +
> +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> +		val &= ~COMP_INIT;
> +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);

As DDIA PHY is the master maybe would be more safe clear it by last?

Other than that:
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>


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

Re: [PATCH 2/5] drm/i915/cnl+: Move the combo PHY init/uninit code to a new file

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Similarly to the GEN9_LP DPIO PHY code keep the CNL+ combo PHY code
> in a
> separate file.
> 
> No functional change.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile           |   1 +
>  drivers/gpu/drm/i915/i915_drv.h         |   6 ++
>  drivers/gpu/drm/i915/intel_combo_phy.c  | 159
> ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 127 ++------------------
> -----
>  4 files changed, 174 insertions(+), 119 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> index 28c7d7884e88..1e7e9513bb10 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -113,6 +113,7 @@ i915-y += intel_audio.o \
>  	  intel_bios.o \
>  	  intel_cdclk.o \
>  	  intel_color.o \
> +	  intel_combo_phy.o \
>  	  intel_connector.o \
>  	  intel_display.o \
>  	  intel_dpio_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 6157f8128cc5..62882e1ddbee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3571,6 +3571,12 @@ void vlv_phy_pre_encoder_enable(struct
> intel_encoder *encoder,
>  void vlv_phy_reset_lanes(struct intel_encoder *encoder,
>  			 const struct intel_crtc_state
> *old_crtc_state);
>  
> +/* intel_combo_phy.c */
> +void icl_combo_phys_init(struct drm_i915_private *dev_priv);
> +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> +void cnl_combo_phys_init(struct drm_i915_private *dev_priv);
> +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> +
>  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
>  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
>  u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> new file mode 100644
> index 000000000000..13184ae5a217
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "intel_drv.h"
> +
> +enum {
> +	PROCMON_0_85V_DOT_0,
> +	PROCMON_0_95V_DOT_0,
> +	PROCMON_0_95V_DOT_1,
> +	PROCMON_1_05V_DOT_0,
> +	PROCMON_1_05V_DOT_1,
> +};
> +
> +static const struct cnl_procmon {
> +	u32 dw1, dw9, dw10;
> +} cnl_procmon_values[] = {
> +	[PROCMON_0_85V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> 0x51914F96, },
> +	[PROCMON_0_95V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> 0x77CA5EAB, },
> +	[PROCMON_0_95V_DOT_1] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> 0x8AE871C5, },
> +	[PROCMON_1_05V_DOT_0] =
> +		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> 0x89E46DC1, },
> +	[PROCMON_1_05V_DOT_1] =
> +		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> 0x8AE38FF1, },
> +};
> +
> +/*
> + * CNL has just one set of registers, while ICL has two sets: one
> for port A and
> + * the other for port B. The CNL registers are equivalent to the ICL
> port A
> + * registers, that's why we call the ICL macros even though the
> function has CNL
> + * on its name.
> + */
> +static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	val = I915_READ(ICL_PORT_COMP_DW3(port));
> +	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> +	default:
> +		MISSING_CASE(val);
> +		/* fall through */
> +	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> +		break;
> +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> +		break;
> +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> +		break;
> +	}
> +
> +	val = I915_READ(ICL_PORT_COMP_DW1(port));
> +	val &= ~((0xff << 16) | 0xff);
> +	val |= procmon->dw1;
> +	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> +
> +	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> +	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> +}
> +
> +void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	val = I915_READ(CHICKEN_MISC_2);
> +	val &= ~CNL_COMP_PWR_DOWN;
> +	I915_WRITE(CHICKEN_MISC_2, val);
> +
> +	/* Dummy PORT_A to get the correct CNL register from the ICL
> macro */
> +	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> +
> +	val = I915_READ(CNL_PORT_COMP_DW0);
> +	val |= COMP_INIT;
> +	I915_WRITE(CNL_PORT_COMP_DW0, val);
> +
> +	val = I915_READ(CNL_PORT_CL1CM_DW5);
> +	val |= CL_POWER_DOWN_ENABLE;
> +	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> +}
> +
> +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> +{
> +	u32 val;
> +
> +	val = I915_READ(CHICKEN_MISC_2);
> +	val |= CNL_COMP_PWR_DOWN;
> +	I915_WRITE(CHICKEN_MISC_2, val);
> +}
> +
> +void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> +{
> +	enum port port;
> +
> +	for (port = PORT_A; port <= PORT_B; port++) {
> +		u32 val;
> +
> +		val = I915_READ(ICL_PHY_MISC(port));
> +		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +		I915_WRITE(ICL_PHY_MISC(port), val);
> +
> +		cnl_set_procmon_ref_values(dev_priv, port);
> +
> +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> +		val |= COMP_INIT;
> +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> +
> +		val = I915_READ(ICL_PORT_CL_DW5(port));
> +		val |= CL_POWER_DOWN_ENABLE;
> +		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> +	}
> +}
> +
> +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> +{
> +	enum port port;
> +
> +	for (port = PORT_A; port <= PORT_B; port++) {
> +		u32 val;
> +
> +		val = I915_READ(ICL_PHY_MISC(port));
> +		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> +		I915_WRITE(ICL_PHY_MISC(port), val);
> +
> +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> +		val &= ~COMP_INIT;
> +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index a7eea8423580..f8da471e81aa 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3436,99 +3436,18 @@ void bxt_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  	usleep_range(10, 30);		/* 10 us delay per Bspec */
>  }
>  
> -enum {
> -	PROCMON_0_85V_DOT_0,
> -	PROCMON_0_95V_DOT_0,
> -	PROCMON_0_95V_DOT_1,
> -	PROCMON_1_05V_DOT_0,
> -	PROCMON_1_05V_DOT_1,
> -};
> -
> -static const struct cnl_procmon {
> -	u32 dw1, dw9, dw10;
> -} cnl_procmon_values[] = {
> -	[PROCMON_0_85V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> 0x51914F96, },
> -	[PROCMON_0_95V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> 0x77CA5EAB, },
> -	[PROCMON_0_95V_DOT_1] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> 0x8AE871C5, },
> -	[PROCMON_1_05V_DOT_0] =
> -		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> 0x89E46DC1, },
> -	[PROCMON_1_05V_DOT_1] =
> -		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> 0x8AE38FF1, },
> -};
> -
> -/*
> - * CNL has just one set of registers, while ICL has two sets: one
> for port A and
> - * the other for port B. The CNL registers are equivalent to the ICL
> port A
> - * registers, that's why we call the ICL macros even though the
> function has CNL
> - * on its name.
> - */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> -				       enum port port)
> -{
> -	const struct cnl_procmon *procmon;
> -	u32 val;
> -
> -	val = I915_READ(ICL_PORT_COMP_DW3(port));
> -	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> -	default:
> -		MISSING_CASE(val);
> -		/* fall through */
> -	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> -		break;
> -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> -		break;
> -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> -		break;
> -	}
> -
> -	val = I915_READ(ICL_PORT_COMP_DW1(port));
> -	val &= ~((0xff << 16) | 0xff);
> -	val |= procmon->dw1;
> -	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> -
> -	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> -	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> -}
> -
>  static void cnl_display_core_init(struct drm_i915_private *dev_priv,
> bool resume)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv-
> >power_domains;
>  	struct i915_power_well *well;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* 1. Enable PCH Reset Handshake */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> -	/* 2. Enable Comp */
> -	val = I915_READ(CHICKEN_MISC_2);
> -	val &= ~CNL_COMP_PWR_DOWN;
> -	I915_WRITE(CHICKEN_MISC_2, val);
> -
> -	/* Dummy PORT_A to get the correct CNL register from the ICL
> macro */
> -	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> -
> -	val = I915_READ(CNL_PORT_COMP_DW0);
> -	val |= COMP_INIT;
> -	I915_WRITE(CNL_PORT_COMP_DW0, val);
> -
> -	/* 3. */
> -	val = I915_READ(CNL_PORT_CL1CM_DW5);
> -	val |= CL_POWER_DOWN_ENABLE;
> -	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> +	/* 2-3. */
> +	cnl_combo_phys_init(dev_priv);
>  
>  	/*
>  	 * 4. Enable Power Well 1 (PG1).
> @@ -3553,7 +3472,6 @@ static void cnl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv-
> >power_domains;
>  	struct i915_power_well *well;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> @@ -3577,10 +3495,8 @@ static void cnl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  
>  	usleep_range(10, 30);		/* 10 us delay per Bspec */
>  
> -	/* 5. Disable Comp */
> -	val = I915_READ(CHICKEN_MISC_2);
> -	val |= CNL_COMP_PWR_DOWN;
> -	I915_WRITE(CHICKEN_MISC_2, val);
> +	/* 5. */
> +	cnl_combo_phys_uninit(dev_priv);
>  }
>  
>  void icl_display_core_init(struct drm_i915_private *dev_priv,
> @@ -3588,31 +3504,14 @@ void icl_display_core_init(struct
> drm_i915_private *dev_priv,
>  {
>  	struct i915_power_domains *power_domains = &dev_priv-
> >power_domains;
>  	struct i915_power_well *well;
> -	enum port port;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* 1. Enable PCH reset handshake. */
>  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
> -	for (port = PORT_A; port <= PORT_B; port++) {
> -		/* 2. Enable DDI combo PHY comp. */
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> -
> -		cnl_set_procmon_ref_values(dev_priv, port);
> -
> -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> -		val |= COMP_INIT;
> -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> -
> -		/* 3. Set power down enable. */

If respining this patch, please consider also move the step comments to
the new functions.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>


> -		val = I915_READ(ICL_PORT_CL_DW5(port));
> -		val |= CL_POWER_DOWN_ENABLE;
> -		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> -	}
> +	/* 2-3. */
> +	icl_combo_phys_init(dev_priv);
>  
>  	/*
>  	 * 4. Enable Power Well 1 (PG1).
> @@ -3640,8 +3539,6 @@ void icl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv-
> >power_domains;
>  	struct i915_power_well *well;
> -	enum port port;
> -	u32 val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> @@ -3663,16 +3560,8 @@ void icl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  	intel_power_well_disable(dev_priv, well);
>  	mutex_unlock(&power_domains->lock);
>  
> -	/* 5. Disable Comp */
> -	for (port = PORT_A; port <= PORT_B; port++) {
> -		val = I915_READ(ICL_PHY_MISC(port));
> -		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> -		I915_WRITE(ICL_PHY_MISC(port), val);
> -
> -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> -		val &= ~COMP_INIT;
> -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> -	}
> +	/* 5. */
> +	icl_combo_phys_uninit(dev_priv);
>  }
>  
>  static void chv_phy_control_init(struct drm_i915_private *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/i915/cnl+: Verify combo PHY HW state during PHY uninit

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Verify on CNL, ICL that the combo PHY HW state stayed intact after
> PHY
> initialization.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> ++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 13184ae5a217..1522e2a25390 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -52,8 +52,8 @@ static const struct cnl_procmon {
>   * registers, that's why we call the ICL macros even though the
> function has CNL
>   * on its name.
>   */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> -				       enum port port)
> +static const struct cnl_procmon *
> +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> port port)
>  {
>  	const struct cnl_procmon *procmon;
>  	u32 val;
> @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  		break;
>  	}
>  
> +	return procmon;
> +}
> +
> +static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
>  	val = I915_READ(ICL_PORT_COMP_DW1(port));
>  	val &= ~((0xff << 16) | 0xff);
>  	val |= procmon->dw1;
> @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
>  }
>  
> +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> +			  enum port port, i915_reg_t reg, u32 mask,
> +			  u32 expected_val)
> +{
> +	u32 val = I915_READ(reg);
> +
> +	if ((val & mask) != expected_val) {
> +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> mismatch: "
> +				 "current %08x mask %08x expected
> %08x\n",
> +				 port_name(port),
> +				 reg.reg, val, mask, expected_val);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +					  enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	bool ret;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
> +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> +			    (0xff << 16) | 0xff, procmon->dw1);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> +			     -1U, procmon->dw9);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> +			     -1U, procmon->dw10);
> +
> +	return ret;
> +}
> +
> +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> +{
> +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);


Minor but would be better add parenthesis in the first part:


return (!(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN)) &&
(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);

> +}
> +
> +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv)
> +{
> +	enum port port = PORT_A;
> +	bool ret;
> +
> +	if (!cnl_combo_phy_enabled(dev_priv))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
> @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> +	if (!cnl_combo_phy_verify_state(dev_priv))
> +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> +
>  	val = I915_READ(CHICKEN_MISC_2);
>  	val |= CNL_COMP_PWR_DOWN;
>  	I915_WRITE(CHICKEN_MISC_2, val);
>  }
>  
> +static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> +				  enum port port)
> +{
> +	return !(I915_READ(ICL_PHY_MISC(port)) &
> +		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
> +		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);

Same in here:

return (!(I915_READ(ICL_PHY_MISC(port)) &
ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN)) &&
(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);

Other than that:
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> +}
> +
> +static bool icl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	bool ret;
> +
> +	if (!icl_combo_phy_enabled(dev_priv, port))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
> @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (!icl_combo_phy_verify_state(dev_priv, port))
> +			DRM_WARN("Port-%c combo PHY HW state changed
> unexpectedly\n",
> +				 port_name(port));
> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915/icl: Skip init for an already enabled combo PHY

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Bspec says we should skip the initialization of combo PHYs that are
> already initialized. We'll need to reinit the PHYs more frequently
> when exiting from DC6 (after the next patch), so let's make sure the
> uninit sequence complies with the spec. For safety skip the init only
> if
> all the PHY register fields have their expected values.
> 
> Bspec: 21257
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 1522e2a25390..7b911702f834 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -220,6 +220,12 @@ void icl_combo_phys_init(struct drm_i915_private
> *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (icl_combo_phy_verify_state(dev_priv, port)) {
> +			DRM_DEBUG_DRIVER("Port-%c combo PHY already
> enabled, won't reprogram it.\n",
> +					 port_name(port));
> +			continue;
> +		}
> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/i915/cnl+: Verify combo PHY HW state during PHY uninit

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> Verify on CNL, ICL that the combo PHY HW state stayed intact after
> PHY
> initialization.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> ++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> b/drivers/gpu/drm/i915/intel_combo_phy.c
> index 13184ae5a217..1522e2a25390 100644
> --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> @@ -52,8 +52,8 @@ static const struct cnl_procmon {
>   * registers, that's why we call the ICL macros even though the
> function has CNL
>   * on its name.
>   */
> -static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> -				       enum port port)
> +static const struct cnl_procmon *
> +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> port port)
>  {
>  	const struct cnl_procmon *procmon;
>  	u32 val;
> @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  		break;
>  	}
>  
> +	return procmon;
> +}
> +
> +static void cnl_set_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	u32 val;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
>  	val = I915_READ(ICL_PORT_COMP_DW1(port));
>  	val &= ~((0xff << 16) | 0xff);
>  	val |= procmon->dw1;
> @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> drm_i915_private *dev_priv,
>  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
>  }
>  
> +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> +			  enum port port, i915_reg_t reg, u32 mask,
> +			  u32 expected_val)
> +{
> +	u32 val = I915_READ(reg);
> +
> +	if ((val & mask) != expected_val) {
> +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> mismatch: "
> +				 "current %08x mask %08x expected
> %08x\n",
> +				 port_name(port),
> +				 reg.reg, val, mask, expected_val);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> *dev_priv,
> +					  enum port port)
> +{
> +	const struct cnl_procmon *procmon;
> +	bool ret;
> +
> +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> +
> +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> +			    (0xff << 16) | 0xff, procmon->dw1);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> +			     -1U, procmon->dw9);
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> +			     -1U, procmon->dw10);
> +
> +	return ret;
> +}
> +
> +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> +{
> +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
> +}
> +
> +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv)
> +{
> +	enum port port = PORT_A;
> +	bool ret;
> +
> +	if (!cnl_combo_phy_enabled(dev_priv))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
> @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> +	if (!cnl_combo_phy_verify_state(dev_priv))
> +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> +
>  	val = I915_READ(CHICKEN_MISC_2);
>  	val |= CNL_COMP_PWR_DOWN;
>  	I915_WRITE(CHICKEN_MISC_2, val);
>  }
>  
> +static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> +				  enum port port)
> +{
> +	return !(I915_READ(ICL_PHY_MISC(port)) &
> +		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
> +		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
> +}
> +
> +static bool icl_combo_phy_verify_state(struct drm_i915_private
> *dev_priv,
> +				       enum port port)
> +{
> +	bool ret;
> +
> +	if (!icl_combo_phy_enabled(dev_priv, port))
> +		return false;
> +
> +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> +
> +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> +			     CL_POWER_DOWN_ENABLE,
> CL_POWER_DOWN_ENABLE);
> +
> +	return ret;
> +}
> +
>  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
>  {
>  	enum port port;
> @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> drm_i915_private *dev_priv)
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		u32 val;
>  
> +		if (!icl_combo_phy_verify_state(dev_priv, port))
> +			DRM_WARN("Port-%c combo PHY HW state changed
> unexpectedly\n",
> +				 port_name(port));

Minor: In most of the places the port indentification is printed like
this: "port %c"

> +
>  		val = I915_READ(ICL_PHY_MISC(port));
>  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
>  		I915_WRITE(ICL_PHY_MISC(port), val);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915/icl: Fix port B combo PHY context loss after DC transitions

[Souza, Jose]

On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> On ICL DMC/PCODE retains the HW context only for port A across DC
> transitions, for the other port B combo PHY, it doesn't. So we need
> to
> do this manually after exiting from DC6. Do the reinit even after
> exiting from DC5, it won't hurt since we only reinit the PHY in case
> it's needed (in case it was disabled to begin with).
> 
> As can be guessed from the bugzilla report leaving the PHY uninited
> will
> lead to a later timeout during the port B specific AUX and DDI_IO
> power
> well enabling.
> 
> Bspec: 21257
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f8da471e81aa..763912c0245c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -843,6 +843,14 @@ static void gen9_dc_off_power_well_enable(struct
> drm_i915_private *dev_priv,
>  
>  	if (IS_GEN9_LP(dev_priv))
>  		bxt_verify_ddi_phy_power_wells(dev_priv);
> +

Minor:

else if (INTEL_GEN(dev_priv) >= 11)

Maybe?

Other than that:
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>


> +	if (IS_ICELAKE(dev_priv))
> +		/*
> +		 * DMC retains HW context only for port A, the other
> combo
> +		 * PHY's HW context for port B is lost after DC
> transitions,
> +		 * so we need to restore it manually.
> +		 */
> +		icl_combo_phys_init(dev_priv);
>  }
>  
>  static void gen9_dc_off_power_well_disable(struct drm_i915_private
> *dev_priv,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/i915/cnl+: Verify combo PHY HW state during PHY uninit

[Imre Deak]

On Fri, Nov 02, 2018 at 11:25:58PM +0200, Souza, Jose wrote:
> On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> > Verify on CNL, ICL that the combo PHY HW state stayed intact after
> > PHY
> > initialization.
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_combo_phy.c | 103
> > ++++++++++++++++++++++++++++++++-
> >  1 file changed, 101 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> > b/drivers/gpu/drm/i915/intel_combo_phy.c
> > index 13184ae5a217..1522e2a25390 100644
> > --- a/drivers/gpu/drm/i915/intel_combo_phy.c
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -52,8 +52,8 @@ static const struct cnl_procmon {
> >   * registers, that's why we call the ICL macros even though the
> > function has CNL
> >   * on its name.
> >   */
> > -static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > -				       enum port port)
> > +static const struct cnl_procmon *
> > +cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum
> > port port)
> >  {
> >  	const struct cnl_procmon *procmon;
> >  	u32 val;
> > @@ -80,6 +80,17 @@ static void cnl_set_procmon_ref_values(struct
> > drm_i915_private *dev_priv,
> >  		break;
> >  	}
> >  
> > +	return procmon;
> > +}
> > +
> > +static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > +				       enum port port)
> > +{
> > +	const struct cnl_procmon *procmon;
> > +	u32 val;
> > +
> > +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> > +
> >  	val = I915_READ(ICL_PORT_COMP_DW1(port));
> >  	val &= ~((0xff << 16) | 0xff);
> >  	val |= procmon->dw1;
> > @@ -89,6 +100,63 @@ static void cnl_set_procmon_ref_values(struct
> > drm_i915_private *dev_priv,
> >  	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> >  }
> >  
> > +static bool check_phy_reg(struct drm_i915_private *dev_priv,
> > +			  enum port port, i915_reg_t reg, u32 mask,
> > +			  u32 expected_val)
> > +{
> > +	u32 val = I915_READ(reg);
> > +
> > +	if ((val & mask) != expected_val) {
> > +		DRM_DEBUG_DRIVER("Port-%c combo PHY reg %08x state
> > mismatch: "
> > +				 "current %08x mask %08x expected
> > %08x\n",
> > +				 port_name(port),
> > +				 reg.reg, val, mask, expected_val);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static bool cnl_verify_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > +					  enum port port)
> > +{
> > +	const struct cnl_procmon *procmon;
> > +	bool ret;
> > +
> > +	procmon = cnl_get_procmon_ref_values(dev_priv, port);
> > +
> > +	ret = check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW1(port),
> > +			    (0xff << 16) | 0xff, procmon->dw1);
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW9(port),
> > +			     -1U, procmon->dw9);
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_COMP_DW10(port),
> > +			     -1U, procmon->dw10);
> > +
> > +	return ret;
> > +}
> > +
> > +static bool cnl_combo_phy_enabled(struct drm_i915_private *dev_priv)
> > +{
> > +	return !(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN) &&
> > +		(I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);
> 
> 
> Minor but would be better add parenthesis in the first part:
> 
> return (!(I915_READ(CHICKEN_MISC_2) & CNL_COMP_PWR_DOWN)) &&
> (I915_READ(CNL_PORT_COMP_DW0) & COMP_INIT);

That's overdoing it imo. Consider that you wouldn't do the same for
!a && b

> 
> > +}
> > +
> > +static bool cnl_combo_phy_verify_state(struct drm_i915_private
> > *dev_priv)
> > +{
> > +	enum port port = PORT_A;
> > +	bool ret;
> > +
> > +	if (!cnl_combo_phy_enabled(dev_priv))
> > +		return false;
> > +
> > +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> > +
> > +	ret &= check_phy_reg(dev_priv, port, CNL_PORT_CL1CM_DW5,
> > +			     CL_POWER_DOWN_ENABLE,
> > CL_POWER_DOWN_ENABLE);
> > +
> > +	return ret;
> > +}
> > +
> >  void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val;
> > @@ -113,11 +181,38 @@ void cnl_combo_phys_uninit(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	u32 val;
> >  
> > +	if (!cnl_combo_phy_verify_state(dev_priv))
> > +		DRM_WARN("Combo PHY HW state changed unexpectedly.\n");
> > +
> >  	val = I915_READ(CHICKEN_MISC_2);
> >  	val |= CNL_COMP_PWR_DOWN;
> >  	I915_WRITE(CHICKEN_MISC_2, val);
> >  }
> >  
> > +static bool icl_combo_phy_enabled(struct drm_i915_private *dev_priv,
> > +				  enum port port)
> > +{
> > +	return !(I915_READ(ICL_PHY_MISC(port)) &
> > +		 ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN) &&
> > +		(I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
> 
> Same in here:
> 
> return (!(I915_READ(ICL_PHY_MISC(port)) &
> ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN)) &&
> (I915_READ(ICL_PORT_COMP_DW0(port)) & COMP_INIT);
> 
> Other than that:
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > +}
> > +
> > +static bool icl_combo_phy_verify_state(struct drm_i915_private
> > *dev_priv,
> > +				       enum port port)
> > +{
> > +	bool ret;
> > +
> > +	if (!icl_combo_phy_enabled(dev_priv, port))
> > +		return false;
> > +
> > +	ret = cnl_verify_procmon_ref_values(dev_priv, port);
> > +
> > +	ret &= check_phy_reg(dev_priv, port, ICL_PORT_CL_DW5(port),
> > +			     CL_POWER_DOWN_ENABLE,
> > CL_POWER_DOWN_ENABLE);
> > +
> > +	return ret;
> > +}
> > +
> >  void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> >  {
> >  	enum port port;
> > @@ -148,6 +243,10 @@ void icl_combo_phys_uninit(struct
> > drm_i915_private *dev_priv)
> >  	for (port = PORT_A; port <= PORT_B; port++) {
> >  		u32 val;
> >  
> > +		if (!icl_combo_phy_verify_state(dev_priv, port))
> > +			DRM_WARN("Port-%c combo PHY HW state changed
> > unexpectedly\n",
> > +				 port_name(port));
> > +
> >  		val = I915_READ(ICL_PHY_MISC(port));
> >  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> >  		I915_WRITE(ICL_PHY_MISC(port), val);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915/icl: Fix combo PHY uninit

[Imre Deak]

On Fri, Nov 02, 2018 at 10:57:19PM +0200, Souza, Jose wrote:
> On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> > BSpec says to clear the comp init HW flag too during combo PHY
> > uninit,
> > so do that. The lack of this could badly interact with the PHY reinit
> > after a DC6/9 transition at least, where (after a follow-up patch
> > fixing
> > the init code) we'd skip the initialization incorrectly due to this
> > flag
> > being set.
> > 
> > BSpec: 21257
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.c
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 6c453366cd24..a7eea8423580 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3668,6 +3668,10 @@ void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  		val = I915_READ(ICL_PHY_MISC(port));
> >  		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> >  		I915_WRITE(ICL_PHY_MISC(port), val);
> > +
> > +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> > +		val &= ~COMP_INIT;
> > +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> 
> As DDIA PHY is the master maybe would be more safe clear it by last?

AFAIK that only means that the slave copies some calibration data from
the master during initialization and the spec doesn't require any order
here either. Perhaps it would still be better to keep the reverse order
in any case, but that should be a separate change as we already disabled
them in this order so far.

> 
> Other than that:
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> 
> >  	}
> >  }
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915/cnl+: Move the combo PHY init/uninit code to a new file

[Imre Deak]

On Fri, Nov 02, 2018 at 11:06:43PM +0200, Souza, Jose wrote:
> On Fri, 2018-11-02 at 20:07 +0200, Imre Deak wrote:
> > Similarly to the GEN9_LP DPIO PHY code keep the CNL+ combo PHY code
> > in a
> > separate file.
> > 
> > No functional change.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile           |   1 +
> >  drivers/gpu/drm/i915/i915_drv.h         |   6 ++
> >  drivers/gpu/drm/i915/intel_combo_phy.c  | 159
> > ++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 127 ++------------------
> > -----
> >  4 files changed, 174 insertions(+), 119 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/intel_combo_phy.c
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 28c7d7884e88..1e7e9513bb10 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -113,6 +113,7 @@ i915-y += intel_audio.o \
> >  	  intel_bios.o \
> >  	  intel_cdclk.o \
> >  	  intel_color.o \
> > +	  intel_combo_phy.o \
> >  	  intel_connector.o \
> >  	  intel_display.o \
> >  	  intel_dpio_phy.o \
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 6157f8128cc5..62882e1ddbee 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3571,6 +3571,12 @@ void vlv_phy_pre_encoder_enable(struct
> > intel_encoder *encoder,
> >  void vlv_phy_reset_lanes(struct intel_encoder *encoder,
> >  			 const struct intel_crtc_state
> > *old_crtc_state);
> >  
> > +/* intel_combo_phy.c */
> > +void icl_combo_phys_init(struct drm_i915_private *dev_priv);
> > +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> > +void cnl_combo_phys_init(struct drm_i915_private *dev_priv);
> > +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv);
> > +
> >  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
> >  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> >  u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c
> > b/drivers/gpu/drm/i915/intel_combo_phy.c
> > new file mode 100644
> > index 000000000000..13184ae5a217
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c
> > @@ -0,0 +1,159 @@
> > +/*
> > + * Copyright © 2018 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom
> > the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> > OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER
> > + * DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#include "intel_drv.h"
> > +
> > +enum {
> > +	PROCMON_0_85V_DOT_0,
> > +	PROCMON_0_95V_DOT_0,
> > +	PROCMON_0_95V_DOT_1,
> > +	PROCMON_1_05V_DOT_0,
> > +	PROCMON_1_05V_DOT_1,
> > +};
> > +
> > +static const struct cnl_procmon {
> > +	u32 dw1, dw9, dw10;
> > +} cnl_procmon_values[] = {
> > +	[PROCMON_0_85V_DOT_0] =
> > +		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> > 0x51914F96, },
> > +	[PROCMON_0_95V_DOT_0] =
> > +		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> > 0x77CA5EAB, },
> > +	[PROCMON_0_95V_DOT_1] =
> > +		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> > 0x8AE871C5, },
> > +	[PROCMON_1_05V_DOT_0] =
> > +		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> > 0x89E46DC1, },
> > +	[PROCMON_1_05V_DOT_1] =
> > +		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> > 0x8AE38FF1, },
> > +};
> > +
> > +/*
> > + * CNL has just one set of registers, while ICL has two sets: one
> > for port A and
> > + * the other for port B. The CNL registers are equivalent to the ICL
> > port A
> > + * registers, that's why we call the ICL macros even though the
> > function has CNL
> > + * on its name.
> > + */
> > +static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > +				       enum port port)
> > +{
> > +	const struct cnl_procmon *procmon;
> > +	u32 val;
> > +
> > +	val = I915_READ(ICL_PORT_COMP_DW3(port));
> > +	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> > +	default:
> > +		MISSING_CASE(val);
> > +		/* fall through */
> > +	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> > +		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> > +		break;
> > +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> > +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> > +		break;
> > +	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> > +		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> > +		break;
> > +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> > +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> > +		break;
> > +	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> > +		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> > +		break;
> > +	}
> > +
> > +	val = I915_READ(ICL_PORT_COMP_DW1(port));
> > +	val &= ~((0xff << 16) | 0xff);
> > +	val |= procmon->dw1;
> > +	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> > +
> > +	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> > +	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> > +}
> > +
> > +void cnl_combo_phys_init(struct drm_i915_private *dev_priv)
> > +{
> > +	u32 val;
> > +
> > +	val = I915_READ(CHICKEN_MISC_2);
> > +	val &= ~CNL_COMP_PWR_DOWN;
> > +	I915_WRITE(CHICKEN_MISC_2, val);
> > +
> > +	/* Dummy PORT_A to get the correct CNL register from the ICL
> > macro */
> > +	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> > +
> > +	val = I915_READ(CNL_PORT_COMP_DW0);
> > +	val |= COMP_INIT;
> > +	I915_WRITE(CNL_PORT_COMP_DW0, val);
> > +
> > +	val = I915_READ(CNL_PORT_CL1CM_DW5);
> > +	val |= CL_POWER_DOWN_ENABLE;
> > +	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> > +}
> > +
> > +void cnl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> > +{
> > +	u32 val;
> > +
> > +	val = I915_READ(CHICKEN_MISC_2);
> > +	val |= CNL_COMP_PWR_DOWN;
> > +	I915_WRITE(CHICKEN_MISC_2, val);
> > +}
> > +
> > +void icl_combo_phys_init(struct drm_i915_private *dev_priv)
> > +{
> > +	enum port port;
> > +
> > +	for (port = PORT_A; port <= PORT_B; port++) {
> > +		u32 val;
> > +
> > +		val = I915_READ(ICL_PHY_MISC(port));
> > +		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > +		I915_WRITE(ICL_PHY_MISC(port), val);
> > +
> > +		cnl_set_procmon_ref_values(dev_priv, port);
> > +
> > +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> > +		val |= COMP_INIT;
> > +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > +
> > +		val = I915_READ(ICL_PORT_CL_DW5(port));
> > +		val |= CL_POWER_DOWN_ENABLE;
> > +		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > +	}
> > +}
> > +
> > +void icl_combo_phys_uninit(struct drm_i915_private *dev_priv)
> > +{
> > +	enum port port;
> > +
> > +	for (port = PORT_A; port <= PORT_B; port++) {
> > +		u32 val;
> > +
> > +		val = I915_READ(ICL_PHY_MISC(port));
> > +		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > +		I915_WRITE(ICL_PHY_MISC(port), val);
> > +
> > +		val = I915_READ(ICL_PORT_COMP_DW0(port));
> > +		val &= ~COMP_INIT;
> > +		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > +	}
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index a7eea8423580..f8da471e81aa 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3436,99 +3436,18 @@ void bxt_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  	usleep_range(10, 30);		/* 10 us delay per Bspec */
> >  }
> >  
> > -enum {
> > -	PROCMON_0_85V_DOT_0,
> > -	PROCMON_0_95V_DOT_0,
> > -	PROCMON_0_95V_DOT_1,
> > -	PROCMON_1_05V_DOT_0,
> > -	PROCMON_1_05V_DOT_1,
> > -};
> > -
> > -static const struct cnl_procmon {
> > -	u32 dw1, dw9, dw10;
> > -} cnl_procmon_values[] = {
> > -	[PROCMON_0_85V_DOT_0] =
> > -		{ .dw1 = 0x00000000, .dw9 = 0x62AB67BB, .dw10 =
> > 0x51914F96, },
> > -	[PROCMON_0_95V_DOT_0] =
> > -		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =
> > 0x77CA5EAB, },
> > -	[PROCMON_0_95V_DOT_1] =
> > -		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =
> > 0x8AE871C5, },
> > -	[PROCMON_1_05V_DOT_0] =
> > -		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =
> > 0x89E46DC1, },
> > -	[PROCMON_1_05V_DOT_1] =
> > -		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =
> > 0x8AE38FF1, },
> > -};
> > -
> > -/*
> > - * CNL has just one set of registers, while ICL has two sets: one
> > for port A and
> > - * the other for port B. The CNL registers are equivalent to the ICL
> > port A
> > - * registers, that's why we call the ICL macros even though the
> > function has CNL
> > - * on its name.
> > - */
> > -static void cnl_set_procmon_ref_values(struct drm_i915_private
> > *dev_priv,
> > -				       enum port port)
> > -{
> > -	const struct cnl_procmon *procmon;
> > -	u32 val;
> > -
> > -	val = I915_READ(ICL_PORT_COMP_DW3(port));
> > -	switch (val & (PROCESS_INFO_MASK | VOLTAGE_INFO_MASK)) {
> > -	default:
> > -		MISSING_CASE(val);
> > -		/* fall through */
> > -	case VOLTAGE_INFO_0_85V | PROCESS_INFO_DOT_0:
> > -		procmon = &cnl_procmon_values[PROCMON_0_85V_DOT_0];
> > -		break;
> > -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_0:
> > -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_0];
> > -		break;
> > -	case VOLTAGE_INFO_0_95V | PROCESS_INFO_DOT_1:
> > -		procmon = &cnl_procmon_values[PROCMON_0_95V_DOT_1];
> > -		break;
> > -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_0:
> > -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_0];
> > -		break;
> > -	case VOLTAGE_INFO_1_05V | PROCESS_INFO_DOT_1:
> > -		procmon = &cnl_procmon_values[PROCMON_1_05V_DOT_1];
> > -		break;
> > -	}
> > -
> > -	val = I915_READ(ICL_PORT_COMP_DW1(port));
> > -	val &= ~((0xff << 16) | 0xff);
> > -	val |= procmon->dw1;
> > -	I915_WRITE(ICL_PORT_COMP_DW1(port), val);
> > -
> > -	I915_WRITE(ICL_PORT_COMP_DW9(port), procmon->dw9);
> > -	I915_WRITE(ICL_PORT_COMP_DW10(port), procmon->dw10);
> > -}
> > -
> >  static void cnl_display_core_init(struct drm_i915_private *dev_priv,
> > bool resume)
> >  {
> >  	struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> >  	struct i915_power_well *well;
> > -	u32 val;
> >  
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >  
> >  	/* 1. Enable PCH Reset Handshake */
> >  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> > -	/* 2. Enable Comp */
> > -	val = I915_READ(CHICKEN_MISC_2);
> > -	val &= ~CNL_COMP_PWR_DOWN;
> > -	I915_WRITE(CHICKEN_MISC_2, val);
> > -
> > -	/* Dummy PORT_A to get the correct CNL register from the ICL
> > macro */
> > -	cnl_set_procmon_ref_values(dev_priv, PORT_A);
> > -
> > -	val = I915_READ(CNL_PORT_COMP_DW0);
> > -	val |= COMP_INIT;
> > -	I915_WRITE(CNL_PORT_COMP_DW0, val);
> > -
> > -	/* 3. */
> > -	val = I915_READ(CNL_PORT_CL1CM_DW5);
> > -	val |= CL_POWER_DOWN_ENABLE;
> > -	I915_WRITE(CNL_PORT_CL1CM_DW5, val);
> > +	/* 2-3. */
> > +	cnl_combo_phys_init(dev_priv);
> >  
> >  	/*
> >  	 * 4. Enable Power Well 1 (PG1).
> > @@ -3553,7 +3472,6 @@ static void cnl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> >  	struct i915_power_well *well;
> > -	u32 val;
> >  
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >  
> > @@ -3577,10 +3495,8 @@ static void cnl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  
> >  	usleep_range(10, 30);		/* 10 us delay per Bspec */
> >  
> > -	/* 5. Disable Comp */
> > -	val = I915_READ(CHICKEN_MISC_2);
> > -	val |= CNL_COMP_PWR_DOWN;
> > -	I915_WRITE(CHICKEN_MISC_2, val);
> > +	/* 5. */
> > +	cnl_combo_phys_uninit(dev_priv);
> >  }
> >  
> >  void icl_display_core_init(struct drm_i915_private *dev_priv,
> > @@ -3588,31 +3504,14 @@ void icl_display_core_init(struct
> > drm_i915_private *dev_priv,
> >  {
> >  	struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> >  	struct i915_power_well *well;
> > -	enum port port;
> > -	u32 val;
> >  
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >  
> >  	/* 1. Enable PCH reset handshake. */
> >  	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> > -	for (port = PORT_A; port <= PORT_B; port++) {
> > -		/* 2. Enable DDI combo PHY comp. */
> > -		val = I915_READ(ICL_PHY_MISC(port));
> > -		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > -		I915_WRITE(ICL_PHY_MISC(port), val);
> > -
> > -		cnl_set_procmon_ref_values(dev_priv, port);
> > -
> > -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> > -		val |= COMP_INIT;
> > -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > -
> > -		/* 3. Set power down enable. */
> 
> If respining this patch, please consider also move the step comments to
> the new functions.

I don't find these comments very useful, I think they just say what the
code already expresses well and you can match that just fine against the
spec too.

> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> 
> > -		val = I915_READ(ICL_PORT_CL_DW5(port));
> > -		val |= CL_POWER_DOWN_ENABLE;
> > -		I915_WRITE(ICL_PORT_CL_DW5(port), val);
> > -	}
> > +	/* 2-3. */
> > +	icl_combo_phys_init(dev_priv);
> >  
> >  	/*
> >  	 * 4. Enable Power Well 1 (PG1).
> > @@ -3640,8 +3539,6 @@ void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	struct i915_power_domains *power_domains = &dev_priv-
> > >power_domains;
> >  	struct i915_power_well *well;
> > -	enum port port;
> > -	u32 val;
> >  
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >  
> > @@ -3663,16 +3560,8 @@ void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  	intel_power_well_disable(dev_priv, well);
> >  	mutex_unlock(&power_domains->lock);
> >  
> > -	/* 5. Disable Comp */
> > -	for (port = PORT_A; port <= PORT_B; port++) {
> > -		val = I915_READ(ICL_PHY_MISC(port));
> > -		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
> > -		I915_WRITE(ICL_PHY_MISC(port), val);
> > -
> > -		val = I915_READ(ICL_PORT_COMP_DW0(port));
> > -		val &= ~COMP_INIT;
> > -		I915_WRITE(ICL_PORT_COMP_DW0(port), val);
> > -	}
> > +	/* 5. */
> > +	icl_combo_phys_uninit(dev_priv);
> >  }
> >  
> >  static void chv_phy_control_init(struct drm_i915_private *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/icl: Fix combo PHY HW context loss

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix combo PHY HW context loss
URL   : https://patchwork.freedesktop.org/series/51970/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5081_full -> Patchwork_10714_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10714_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10714_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10714_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy:
      shard-snb:          SKIP -> PASS +3

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    igt@pm_rpm@universal-planes:
      shard-skl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10714_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-apl:          PASS -> INCOMPLETE (fdo#106886, fdo#103927)

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665, fdo#106887)

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108074)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-glk:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_chv_cursor_fail@pipe-b-128x128-bottom-edge:
      shard-skl:          PASS -> FAIL (fdo#104671)

    igt@kms_color@pipe-b-ctm-negative:
      shard-skl:          PASS -> FAIL (fdo#107361)

    igt@kms_cursor_crc@cursor-128x42-offscreen:
      shard-skl:          NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-64x64-onscreen:
      shard-glk:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#105454, fdo#106509)

    igt@kms_cursor_legacy@pipe-b-torture-move:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled:
      shard-skl:          NOTRUN -> FAIL (fdo#103184)

    igt@kms_flip@flip-vs-dpms-interruptible:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103313, fdo#105345)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#105682) +1

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      shard-skl:          PASS -> FAIL (fdo#107362, fdo#103191)

    igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
      shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#108145) +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_vblank@pipe-b-ts-continuation-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#107773, fdo#104108) +1

    
    ==== Possible fixes ====

    igt@kms_cursor_crc@cursor-128x42-random:
      shard-skl:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-256x256-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS

    igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
      shard-skl:          FAIL (fdo#107815) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@legacy-planes-dpms:
      shard-skl:          INCOMPLETE (fdo#105959, fdo#107807) -> PASS

    
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105345 https://bugs.freedesktop.org/show_bug.cgi?id=105345
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
  fdo#107361 https://bugs.freedesktop.org/show_bug.cgi?id=107361
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5081 -> Patchwork_10714

  CI_DRM_5081: f5e16acf6c85d38756c3efdb77ec6aede55df0ba @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4705: 7983e19ed62ec8db1884f55e07e458a62cc51e37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10714: 162b1a44d002f2bd5df1b4f1b70f5b744d1f5733 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 5/5] drm/i915/icl: Fix port B combo PHY context loss after DC transitions

[Imre Deak]

On Fri, Nov 02, 2018 at 12:22:15PM -0700, Rodrigo Vivi wrote:
> On Fri, Nov 02, 2018 at 08:07:06PM +0200, Imre Deak wrote:
> > On ICL DMC/PCODE retains the HW context only for port A across DC
> > transitions, for the other port B combo PHY, it doesn't. So we need to
> > do this manually after exiting from DC6. Do the reinit even after
> > exiting from DC5, it won't hurt since we only reinit the PHY in case
> > it's needed (in case it was disabled to begin with).
> > 
> > As can be guessed from the bugzilla report leaving the PHY uninited will
> > lead to a later timeout during the port B specific AUX and DDI_IO power
> > well enabling.
> > 
> > Bspec: 21257
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index f8da471e81aa..763912c0245c 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -843,6 +843,14 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> >  
> >  	if (IS_GEN9_LP(dev_priv))
> >  		bxt_verify_ddi_phy_power_wells(dev_priv);
> > +
> > +	if (IS_ICELAKE(dev_priv))
> 
> maybe INTEL_GEN(dev_priv) >= 11?

Ok, will change it.

> 
> 
> with or without:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> 
> > +		/*
> > +		 * DMC retains HW context only for port A, the other combo
> > +		 * PHY's HW context for port B is lost after DC transitions,
> > +		 * so we need to restore it manually.
> > +		 */
> > +		icl_combo_phys_init(dev_priv);
> >  }
> >  
> >  static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> > -- 
> > 2.13.2
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx