Skip to contentSkip to conversation

drm/i915/icl: Fix combo PHY HW context loss

PATCH Superseded Imre Deak 4 participants 23 messages Nov 2, 2018 – Nov 5, 2018

Subsystems (2)
DRM drivers Intel i915

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Imre Deak

author
#0 lore raw

cover drm/i915/icl: Fix combo PHY HW context loss

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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>

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
Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#1 lore raw

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

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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +4 -0
index 6c453366cd24..a7eea8423580 100644
@@ -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);
 	}
 }
Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/5
#2 lore raw

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

Quoted 25 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
index 6c453366cd24..a7eea8423580 100644

drivers/gpu/drm/i915/intel_runtime_pm.c void icl_display_core_uninit(struct · line 3675

Quoted 8 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +4 -0
@@ -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 <[email protected]>

drivers/gpu/drm/i915/intel_runtime_pm.c void icl_display_core_uninit(struct · line 3677

Quoted 3 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
 	}
 }

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#3 lore raw

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

Quoted 20 lines · Jose
Quoted 19 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++
 1 file changed, 4 insertions(+)
Quoted 16 lines
Quoted 6 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
index 6c453366cd24..a7eea8423580 100644

drivers/gpu/drm/i915/intel_runtime_pm.c void icl_display_core_uninit(struct · line 3675

Quoted 8 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +4 -0
@@ -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.

Quoted 8 lines

Other than that:

Reviewed-by José Roberto de Souza <[email protected]>
Quoted 3 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
 	}
 }

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#4 lore raw

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

In reply to 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ä <[email protected]>
Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 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
drivers/gpu/drm/i915/Makefile +1 -0
index 28c7d7884e88..1e7e9513bb10 100644
@@ -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 \
drivers/gpu/drm/i915/i915_drv.h +6 -0
index 6157f8128cc5..62882e1ddbee 100644
@@ -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,
drivers/gpu/drm/i915/intel_combo_phy.c new file +159 -0

159 lines not on this page · raw message

drivers/gpu/drm/i915/intel_runtime_pm.c +8 -119

179 lines not on this page · raw message

Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
maintainer Reviewed-by on v1 2/5
#5 lore raw

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

Quoted 66 lines · 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ä <[email protected]>
Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 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
drivers/gpu/drm/i915/Makefile +1 -0
index 28c7d7884e88..1e7e9513bb10 100644
@@ -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 \
drivers/gpu/drm/i915/i915_drv.h +6 -0
index 6157f8128cc5..62882e1ddbee 100644
@@ -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,
drivers/gpu/drm/i915/intel_combo_phy.c new file +12 -0
new file mode 100644
index 000000000000..13184ae5a217
@@ -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

drivers/gpu/drm/i915/intel_combo_phy.c line 22

Quoted 10 lines
drivers/gpu/drm/i915/intel_combo_phy.c new file +10 -0
+ * 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 <[email protected]>
Quoted 329 lines
drivers/gpu/drm/i915/intel_combo_phy.c new file +137 -0
+
+#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);
+	}
+}
drivers/gpu/drm/i915/intel_runtime_pm.c +8 -119
index a7eea8423580..f8da471e81aa 100644
@@ -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)
Signature · 1 line
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reviewed-by on v1 2/5
#6 lore raw

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

In reply to Imre Deak
Quoted 406 lines · 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ä <[email protected]>
Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 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
drivers/gpu/drm/i915/Makefile +1 -0
index 28c7d7884e88..1e7e9513bb10 100644
@@ -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 \
drivers/gpu/drm/i915/i915_drv.h +6 -0
index 6157f8128cc5..62882e1ddbee 100644
@@ -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,

drivers/gpu/drm/i915/intel_combo_phy.c new file +4 -0
new file mode 100644
index 000000000000..13184ae5a217
@@ -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.
+ */

drivers/gpu/drm/i915/intel_combo_phy.c new file +16 -0
+
+#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, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_0_95V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =

0x77CA5EAB, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_0_95V_DOT_1] =
+		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =

0x8AE871C5, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_1_05V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =

0x89E46DC1, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_1_05V_DOT_1] =
+		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =

0x8AE38FF1, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +4 -0
+};
+
+/*
+ * 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,

drivers/gpu/drm/i915/intel_combo_phy.c new file +45 -0
+				       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 */

drivers/gpu/drm/i915/intel_combo_phy.c new file +59 -0
+	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);
+	}
+}
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -25
index a7eea8423580..f8da471e81aa 100644
@@ -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,

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -36
-				       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-

Quoted 1 line

power_domains;

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -7
 	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 */

drivers/gpu/drm/i915/intel_runtime_pm.c +2 -11
-	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;
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -4
 	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,

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -5
@@ -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));

drivers/gpu/drm/i915/intel_runtime_pm.c void icl_display_core_init(struct · line 3613

Quoted 10 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -10
-		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 <[email protected]>
Quoted 39 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -4
-		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);
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -2
 	/*
 	 * 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;
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -10
 	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);
 }
Quoted 1 line

static void chv_phy_control_init(struct drm_i915_private *dev_priv)

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#7 lore raw

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

Quoted 404 lines · Jose
Quoted 403 lines · 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ä <[email protected]>
Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 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
drivers/gpu/drm/i915/Makefile +1 -0
index 28c7d7884e88..1e7e9513bb10 100644
@@ -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 \
drivers/gpu/drm/i915/i915_drv.h +6 -0
index 6157f8128cc5..62882e1ddbee 100644
@@ -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,

drivers/gpu/drm/i915/intel_combo_phy.c new file +4 -0
new file mode 100644
index 000000000000..13184ae5a217
@@ -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.
+ */

drivers/gpu/drm/i915/intel_combo_phy.c new file +16 -0
+
+#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, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_0_95V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x86E172C7, .dw10 =

0x77CA5EAB, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_0_95V_DOT_1] =
+		{ .dw1 = 0x00000000, .dw9 = 0x93F87FE1, .dw10 =

0x8AE871C5, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_1_05V_DOT_0] =
+		{ .dw1 = 0x00000000, .dw9 = 0x98FA82DD, .dw10 =

0x89E46DC1, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +2 -0
+	[PROCMON_1_05V_DOT_1] =
+		{ .dw1 = 0x00440000, .dw9 = 0x9A00AB25, .dw10 =

0x8AE38FF1, },

drivers/gpu/drm/i915/intel_combo_phy.c new file +4 -0
+};
+
+/*
+ * 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,

drivers/gpu/drm/i915/intel_combo_phy.c new file +45 -0
+				       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 */

drivers/gpu/drm/i915/intel_combo_phy.c new file +59 -0
+	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);
+	}
+}
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -25
index a7eea8423580..f8da471e81aa 100644
@@ -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,

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -36
-				       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-

Quoted 1 line

power_domains;

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -7
 	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 */

drivers/gpu/drm/i915/intel_runtime_pm.c +2 -11
-	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;
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -4
 	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,

drivers/gpu/drm/i915/intel_runtime_pm.c +0 -2
@@ -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));
Quoted 16 lines

drivers/gpu/drm/i915/intel_runtime_pm.c void icl_display_core_init(struct drm_i915_private *dev_priv, · line 3611

Quoted 13 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -13
-	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.

Quoted 43 lines
Reviewed-by José Roberto de Souza <[email protected]>
Quoted 39 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -4
-		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);
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -2
 	/*
 	 * 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;
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -10
 	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);
 }
Quoted 1 line
Quoted 1 line

static void chv_phy_control_init(struct drm_i915_private *dev_priv)

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#8 lore raw

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

In reply to Imre Deak

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

Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103 ++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_combo_phy.c +101 -2
index 13184ae5a217..1522e2a25390 100644
@@ -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);
Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 3/5
#9 lore raw

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

Quoted 90 lines · Imre Deak

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

Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103
++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
index 13184ae5a217..1522e2a25390 100644
@@ -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,

drivers/gpu/drm/i915/intel_combo_phy.c +2 -1
-				       enum port port)
+static const struct cnl_procmon *
+cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum

port port)
{

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
 	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));
drivers/gpu/drm/i915/intel_combo_phy.c +8 -0
 	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: "

drivers/gpu/drm/i915/intel_combo_phy.c +1 -0
+				 "current %08x mask %08x expected

%08x\n",

drivers/gpu/drm/i915/intel_combo_phy.c +9 -0
+				 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,

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
+					  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);

drivers/gpu/drm/i915/intel_combo_phy.c static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv, · line 141

Quoted 10 lines
drivers/gpu/drm/i915/intel_combo_phy.c +10 -0
+	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);

Quoted 32 lines
drivers/gpu/drm/i915/intel_combo_phy.c +3 -0
+}
+
+static bool cnl_combo_phy_verify_state(struct drm_i915_private

*dev_priv)

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
+{
+	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);

drivers/gpu/drm/i915/intel_combo_phy.c +7 -0
+
+	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);

drivers/gpu/drm/i915/intel_combo_phy.c void cnl_combo_phys_uninit(struct · line 198

Quoted 10 lines
drivers/gpu/drm/i915/intel_combo_phy.c +6 -0
 	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 <[email protected]>
Quoted 34 lines
drivers/gpu/drm/i915/intel_combo_phy.c +18 -0
+}
+
+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) {

drivers/gpu/drm/i915/intel_combo_phy.c +4 -0
 	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));

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_uninit(struct drm_i915_private *dev_priv) · line 252

Quoted 2 lines
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#10 lore raw

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

Quoted 105 lines · Jose
Quoted 90 lines · Imre Deak

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

Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103
++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
index 13184ae5a217..1522e2a25390 100644
@@ -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,

drivers/gpu/drm/i915/intel_combo_phy.c +2 -1
-				       enum port port)
+static const struct cnl_procmon *
+cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum

port port)
{

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
 	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));
drivers/gpu/drm/i915/intel_combo_phy.c +8 -0
 	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: "

drivers/gpu/drm/i915/intel_combo_phy.c +1 -0
+				 "current %08x mask %08x expected

%08x\n",

drivers/gpu/drm/i915/intel_combo_phy.c +9 -0
+				 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,

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
+					  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);

drivers/gpu/drm/i915/intel_combo_phy.c static void cnl_set_procmon_ref_values(struct drm_i915_private *dev_priv, · line 141

Quoted 10 lines
drivers/gpu/drm/i915/intel_combo_phy.c +10 -0
+	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:

Quoted 2 lines

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

Quoted 81 lines
Quoted 33 lines
drivers/gpu/drm/i915/intel_combo_phy.c +3 -0
+}
+
+static bool cnl_combo_phy_verify_state(struct drm_i915_private

*dev_priv)

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
+{
+	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);

drivers/gpu/drm/i915/intel_combo_phy.c +7 -0
+
+	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);

drivers/gpu/drm/i915/intel_combo_phy.c void cnl_combo_phys_uninit(struct · line 198

Quoted 10 lines
drivers/gpu/drm/i915/intel_combo_phy.c +6 -0
 	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 <[email protected]>
Quoted 28 lines
drivers/gpu/drm/i915/intel_combo_phy.c +18 -0
+}
+
+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) {

drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 	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;
Quoted 8 lines

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_uninit(struct drm_i915_private *dev_priv) · line 252

Quoted 8 lines
drivers/gpu/drm/i915/intel_combo_phy.c +4 -0
+		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));
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#11 lore raw

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

In reply to Imre Deak
Quoted 166 lines · Imre Deak

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

Cc Paulo Zanoni <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 103
++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
index 13184ae5a217..1522e2a25390 100644
@@ -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,

drivers/gpu/drm/i915/intel_combo_phy.c +2 -1
-				       enum port port)
+static const struct cnl_procmon *
+cnl_get_procmon_ref_values(struct drm_i915_private *dev_priv, enum

port port)
{

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
 	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));
drivers/gpu/drm/i915/intel_combo_phy.c +8 -0
 	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: "

drivers/gpu/drm/i915/intel_combo_phy.c +1 -0
+				 "current %08x mask %08x expected

%08x\n",

drivers/gpu/drm/i915/intel_combo_phy.c +9 -0
+				 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,

drivers/gpu/drm/i915/intel_combo_phy.c +24 -0
+					  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)

drivers/gpu/drm/i915/intel_combo_phy.c +11 -0
+{
+	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);

drivers/gpu/drm/i915/intel_combo_phy.c +31 -0
+
+	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) {

drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 	enum port port;

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_uninit(struct · line 250

Quoted 8 lines
drivers/gpu/drm/i915/intel_combo_phy.c +3 -0
@@ -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"

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_uninit(struct drm_i915_private *dev_priv) · line 252

Quoted 4 lines
drivers/gpu/drm/i915/intel_combo_phy.c +1 -0
+
 		val = I915_READ(ICL_PHY_MISC(port));
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 		val |= ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#12 lore raw

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

In reply to 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/i915/intel_combo_phy.c +6 -0
index 1522e2a25390..7b911702f834 100644
@@ -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);
Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
maintainer Reviewed-by on v1 4/5
#13 lore raw

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

Quoted 21 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
index 1522e2a25390..7b911702f834 100644

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_init(struct drm_i915_private *dev_priv) · line 225

Quoted 6 lines
drivers/gpu/drm/i915/intel_combo_phy.c +3 -0
@@ -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 <[email protected]>
Quoted 9 lines
drivers/gpu/drm/i915/intel_combo_phy.c +3 -0
+			continue;
+		}
+
 		val = I915_READ(ICL_PHY_MISC(port));
 		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);
Signature · 1 line
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reviewed-by on v1 4/5
#14 lore raw

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

In reply to Imre Deak
Quoted 10 lines · 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 <[email protected]>
Cc: Ville Syrjälä <[email protected]>

Reviewed-by José Roberto de Souza <[email protected]>
Quoted 25 lines
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_combo_phy.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/i915/intel_combo_phy.c +6 -0
index 1522e2a25390..7b911702f834 100644
@@ -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));

drivers/gpu/drm/i915/intel_combo_phy.c void icl_combo_phys_init(struct drm_i915_private *dev_priv) · line 231

Quoted 2 lines
drivers/gpu/drm/i915/intel_combo_phy.c +0 -0
 		val &= ~ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN;
 		I915_WRITE(ICL_PHY_MISC(port), val);

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#15 lore raw

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

In reply to 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +8 -0
index f8da471e81aa..763912c0245c 100644
@@ -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,
Signature · 6 lines
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
maintainer Reviewed-by on v1 5/5
#16 lore raw

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

Quoted 26 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
index f8da471e81aa..763912c0245c 100644

drivers/gpu/drm/i915/intel_runtime_pm.c static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv, · line 847

Quoted 5 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -0
@@ -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 <[email protected]>
Quoted 12 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +6 -0
+		/*
+		 * 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,
Signature · 1 line
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

author
#17 lore raw

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

Quoted 28 lines · Rodrigo Vivi
Quoted 27 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
index f8da471e81aa..763912c0245c 100644
@@ -843,6 +843,14 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
Quoted 6 lines

drivers/gpu/drm/i915/intel_runtime_pm.c static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv, · line 847

Quoted 4 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +2 -0
 	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.

Quoted 16 lines

with or without:

Reviewed-by Rodrigo Vivi <[email protected]>
Quoted 8 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +6 -0
+		/*
+		 * 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);
 }
Quoted 4 lines
Quoted 4 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
 static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
Signature · 1 line
2.13.2

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reviewed-by on v1 5/5
#18 lore raw

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

In reply to Imre Deak
Quoted 30 lines · 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 <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc José Roberto de Souza <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Imre Deak <[email protected]>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/gpu/drm/i915/intel_runtime_pm.c +0 -0
index f8da471e81aa..763912c0245c 100644

drivers/gpu/drm/i915/intel_runtime_pm.c static void gen9_dc_off_power_well_enable(struct · line 847

Quoted 5 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +1 -0
@@ -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 <[email protected]>
Quoted 12 lines
drivers/gpu/drm/i915/intel_runtime_pm.c +3 -0
+	if (IS_ICELAKE(dev_priv))
+		/*
+		 * DMC retains HW context only for port A, the other

combo

drivers/gpu/drm/i915/intel_runtime_pm.c +1 -0
+		 * PHY's HW context for port B is lost after DC

transitions,

drivers/gpu/drm/i915/intel_runtime_pm.c +3 -0
+		 * 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
Quoted 1 line

*dev_priv,

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#19 lore raw

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

In reply to Imre Deak
== Series Details ==

== 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 <[email protected]'
#19: 
Cc: Rodrigo Vivi <[email protected]

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#20 lore raw

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

In reply to Imre Deak
== Series Details ==

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#21 lore raw

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

In reply to Imre Deak
== Series Details ==

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#22 lore raw

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

In reply to Imre Deak
== Series Details ==

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx