All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
@ 2020-04-13 16:45 José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 2/9] drm/i915/display: Add intel_legacy_aux_to_power_domain() José Roberto de Souza
                   ` (11 more replies)
  0 siblings, 12 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: You-Sheng Yang

Moving the code to return the digital port of the aux channel also
removing the intel_phy_is_tc() to make it generic.
digital_port will be needed in icl_tc_phy_aux_power_well_enable()
so adding it as a parameter to icl_tc_port_assert_ref_held().

While at at removing the duplicated call to icl_tc_phy_aux_ch() in
icl_tc_port_assert_ref_held().

v2:
- fixed build when DRM_I915_DEBUG_RUNTIME_PM is not set
- moved to before hsw_wait_for_power_well_enable() as it will be
needed by hsw_wait_for_power_well_enable() in a future patch

v4:
- fixed action of if (!dig_port), continue instead of return

Cc: You-Sheng Yang <vicamo@gmail.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 69 ++++++++++---------
 1 file changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 433e5a81dd4d..814e223836db 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -282,6 +282,33 @@ static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv,
 		gen8_irq_power_well_pre_disable(dev_priv, irq_pipe_mask);
 }
 
+static struct intel_digital_port *
+aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
+		       enum aux_ch aux_ch)
+{
+	struct intel_digital_port *dig_port = NULL;
+	struct intel_encoder *encoder;
+
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		/* We'll check the MST primary port */
+		if (encoder->type == INTEL_OUTPUT_DP_MST)
+			continue;
+
+		dig_port = enc_to_dig_port(encoder);
+		if (!dig_port)
+			continue;
+
+		if (dig_port->aux_ch != aux_ch) {
+			dig_port = NULL;
+			continue;
+		}
+
+		break;
+	}
+
+	return dig_port;
+}
+
 static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
@@ -501,41 +528,14 @@ static int power_well_async_ref_count(struct drm_i915_private *dev_priv,
 }
 
 static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
-					struct i915_power_well *power_well)
+					struct i915_power_well *power_well,
+					struct intel_digital_port *dig_port)
 {
-	enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
-	struct intel_digital_port *dig_port = NULL;
-	struct intel_encoder *encoder;
-
 	/* Bypass the check if all references are released asynchronously */
 	if (power_well_async_ref_count(dev_priv, power_well) ==
 	    power_well->count)
 		return;
 
-	aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
-
-	for_each_intel_encoder(&dev_priv->drm, encoder) {
-		enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
-
-		if (!intel_phy_is_tc(dev_priv, phy))
-			continue;
-
-		/* We'll check the MST primary port */
-		if (encoder->type == INTEL_OUTPUT_DP_MST)
-			continue;
-
-		dig_port = enc_to_dig_port(encoder);
-		if (drm_WARN_ON(&dev_priv->drm, !dig_port))
-			continue;
-
-		if (dig_port->aux_ch != aux_ch) {
-			dig_port = NULL;
-			continue;
-		}
-
-		break;
-	}
-
 	if (drm_WARN_ON(&dev_priv->drm, !dig_port))
 		return;
 
@@ -545,7 +545,8 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
 #else
 
 static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
-					struct i915_power_well *power_well)
+					struct i915_power_well *power_well,
+					struct intel_digital_port *dig_port)
 {
 }
 
@@ -558,9 +559,10 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 				 struct i915_power_well *power_well)
 {
 	enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
+	struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
 	u32 val;
 
-	icl_tc_port_assert_ref_held(dev_priv, power_well);
+	icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
 
 	val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch));
 	val &= ~DP_AUX_CH_CTL_TBT_IO;
@@ -588,7 +590,10 @@ static void
 icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
 				  struct i915_power_well *power_well)
 {
-	icl_tc_port_assert_ref_held(dev_priv, power_well);
+	enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
+	struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
+
+	icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
 
 	hsw_power_well_disable(dev_priv, power_well);
 }
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 2/9] drm/i915/display: Add intel_legacy_aux_to_power_domain()
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 3/9] drm/i915/display: Split hsw_power_well_enable() into two José Roberto de Souza
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng

This is a similar function to intel_aux_power_domain() but it do not
care about TBT ports, this will be needed by ICL TC sequences.

v2:
- renamed to intel_legacy_aux_to_power_domain()

Cc: Imre Deak <imre.deak@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++++++--
 drivers/gpu/drm/i915/display/intel_display.h |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 70ec301fe6e3..a95960b71001 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7291,7 +7291,17 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
 		}
 	}
 
-	switch (dig_port->aux_ch) {
+	return intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+}
+
+/*
+ * Converts aux_ch to power_domain without caring about TBT ports for that use
+ * intel_aux_power_domain()
+ */
+enum intel_display_power_domain
+intel_legacy_aux_to_power_domain(enum aux_ch aux_ch)
+{
+	switch (aux_ch) {
 	case AUX_CH_A:
 		return POWER_DOMAIN_AUX_A;
 	case AUX_CH_B:
@@ -7307,7 +7317,7 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
 	case AUX_CH_G:
 		return POWER_DOMAIN_AUX_G;
 	default:
-		MISSING_CASE(dig_port->aux_ch);
+		MISSING_CASE(aux_ch);
 		return POWER_DOMAIN_AUX_A;
 	}
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index cc7f287804d7..8d872ed0de36 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -583,6 +583,8 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
 enum intel_display_power_domain intel_port_to_power_domain(enum port port);
 enum intel_display_power_domain
 intel_aux_power_domain(struct intel_digital_port *dig_port);
+enum intel_display_power_domain
+intel_legacy_aux_to_power_domain(enum aux_ch aux_ch);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 3/9] drm/i915/display: Split hsw_power_well_enable() into two
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 2/9] drm/i915/display: Add intel_legacy_aux_to_power_domain() José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences José Roberto de Souza
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx

This is a preparation for ICL TC cold exit sequences.

v2:
- renamed new functions to hsw_power_well_enable_prepare()/complete()

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
---
 .../drm/i915/display/intel_display_power.c    | 39 +++++++++++++++----
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 814e223836db..5bef7dda4a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -380,16 +380,16 @@ static void gen9_wait_for_power_well_fuses(struct drm_i915_private *dev_priv,
 					  SKL_FUSE_PG_DIST_STATUS(pg), 1));
 }
 
-static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
-				  struct i915_power_well *power_well)
+static void hsw_power_well_enable_prepare(struct drm_i915_private *dev_priv,
+					  struct i915_power_well *power_well)
 {
 	const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
 	int pw_idx = power_well->desc->hsw.idx;
-	bool wait_fuses = power_well->desc->hsw.has_fuses;
-	enum skl_power_gate uninitialized_var(pg);
 	u32 val;
 
-	if (wait_fuses) {
+	if (power_well->desc->hsw.has_fuses) {
+		enum skl_power_gate pg;
+
 		pg = INTEL_GEN(dev_priv) >= 11 ? ICL_PW_CTL_IDX_TO_PG(pw_idx) :
 						 SKL_PW_CTL_IDX_TO_PG(pw_idx);
 		/*
@@ -406,25 +406,46 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
 	val = intel_de_read(dev_priv, regs->driver);
 	intel_de_write(dev_priv, regs->driver,
 		       val | HSW_PWR_WELL_CTL_REQ(pw_idx));
+}
+
+static void hsw_power_well_enable_complete(struct drm_i915_private *dev_priv,
+					   struct i915_power_well *power_well)
+{
+	int pw_idx = power_well->desc->hsw.idx;
+
 	hsw_wait_for_power_well_enable(dev_priv, power_well);
 
 	/* Display WA #1178: cnl */
 	if (IS_CANNONLAKE(dev_priv) &&
 	    pw_idx >= GLK_PW_CTL_IDX_AUX_B &&
 	    pw_idx <= CNL_PW_CTL_IDX_AUX_F) {
+		u32 val;
+
 		val = intel_de_read(dev_priv, CNL_AUX_ANAOVRD1(pw_idx));
 		val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
 		intel_de_write(dev_priv, CNL_AUX_ANAOVRD1(pw_idx), val);
 	}
 
-	if (wait_fuses)
+	if (power_well->desc->hsw.has_fuses) {
+		enum skl_power_gate pg;
+
+		pg = INTEL_GEN(dev_priv) >= 11 ? ICL_PW_CTL_IDX_TO_PG(pw_idx) :
+						 SKL_PW_CTL_IDX_TO_PG(pw_idx);
 		gen9_wait_for_power_well_fuses(dev_priv, pg);
+	}
 
 	hsw_power_well_post_enable(dev_priv,
 				   power_well->desc->hsw.irq_pipe_mask,
 				   power_well->desc->hsw.has_vga);
 }
 
+static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
+				  struct i915_power_well *power_well)
+{
+	hsw_power_well_enable_prepare(dev_priv, power_well);
+	hsw_power_well_enable_complete(dev_priv, power_well);
+}
+
 static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
@@ -570,7 +591,11 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 		val |= DP_AUX_CH_CTL_TBT_IO;
 	intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val);
 
-	hsw_power_well_enable(dev_priv, power_well);
+	hsw_power_well_enable_prepare(dev_priv, power_well);
+
+	/* TODO ICL TC cold handling */
+
+	hsw_power_well_enable_complete(dev_priv, power_well);
 
 	if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) {
 		enum tc_port tc_port;
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 2/9] drm/i915/display: Add intel_legacy_aux_to_power_domain() José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 3/9] drm/i915/display: Split hsw_power_well_enable() into two José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-14 17:31   ` Imre Deak
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 5/9] drm/i915/tc: Skip ref held check for TC legacy aux power wells José Roberto de Souza
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng

This is required for legacy/static TC ports as IOM is not aware of
the connection and will not trigger the TC cold exit.

Just request PCODE to exit TCCOLD is not enough as it could enter
again before driver makes use of the port, to prevent it BSpec states
that aux powerwell should be held.

So here embedding the TC cold exit sequence into ICL aux enable,
it will enable aux and then request TC cold to exit.

The TC cold block(exit and aux hold) and unblock was added to some
exported TC functions for the others and to access PHY registers,
callers should enable and keep aux powerwell enabled during access.

Also adding TC cold check and warnig in tc_port_load_fia_params() as
at this point of the driver initialization we can't request power
wells, if we get this warning we will need to figure out how to handle
it.

v2:
- moved ICL TC cold exit function to intel_display_power
- using dig_port->tc_legacy_port to only execute sequences for legacy
ports, hopefully VBTs will have this right
- fixed check to call _hsw_power_well_continue_enable()
- calling _hsw_power_well_continue_enable() unconditionally in
icl_tc_phy_aux_power_well_enable(), if needed we will surpress timeout
warnings of TC legacy ports
- only blocking TC cold around fia access

v3:
- added timeout of 5msec to not loop forever if
sandybridge_pcode_write_timeout() keeps returning -EAGAIN
returning -EAGAIN in in icl_tc_cold_exit()
- removed leftover tc_cold_wakeref
- added one msec sleep when PCODE returns -EAGAIN

v4:
- removed 5msec timeout, instead giving 1msec to whoever is using
PCODE to finish it up to 3 times
- added a comment about turn TC cold exit failure as a error in future

BSpec: 21750
Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
Cc: Imre Deak <imre.deak@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 25 +++++++-
 drivers/gpu/drm/i915/display/intel_tc.c       | 64 +++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h               |  1 +
 3 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 5bef7dda4a7a..e2dddaf1051b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -575,6 +575,28 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
 
 #define TGL_AUX_PW_TO_TC_PORT(pw_idx)	((pw_idx) - TGL_PW_CTL_IDX_AUX_TC1)
 
+static void icl_tc_cold_exit(struct drm_i915_private *i915)
+{
+	int ret, tries = 0;
+
+	while (1) {
+		ret = sandybridge_pcode_write_timeout(i915,
+						      ICL_PCODE_EXIT_TCCOLD,
+						      0, 250, 1);
+		if (ret != -EAGAIN || ++tries == 3)
+			break;
+		msleep(1);
+	}
+
+	/* Spec states that TC cold exit can take up to 1ms to complete */
+	if (!ret)
+		msleep(1);
+
+	/* TODO: turn failure into a error as soon i915 CI updates ICL IFWI */
+	drm_dbg_kms(&i915->drm, "TC cold block %s\n", ret ? "failed" :
+		    "succeeded");
+}
+
 static void
 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 				 struct i915_power_well *power_well)
@@ -593,7 +615,8 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 
 	hsw_power_well_enable_prepare(dev_priv, power_well);
 
-	/* TODO ICL TC cold handling */
+	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
+		icl_tc_cold_exit(dev_priv);
 
 	hsw_power_well_enable_complete(dev_priv, power_well);
 
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 275618bedf32..0cf33d4d21c3 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -34,6 +34,7 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
 	if (INTEL_INFO(i915)->display.has_modular_fia) {
 		modular_fia = intel_uncore_read(&i915->uncore,
 						PORT_TX_DFLEXDPSP(FIA1));
+		drm_WARN_ON(&i915->drm, modular_fia == 0xffffffff);
 		modular_fia &= MODULAR_FIA_MASK;
 	} else {
 		modular_fia = 0;
@@ -52,6 +53,37 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
 	}
 }
 
+static intel_wakeref_t
+tc_cold_block(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum intel_display_power_domain domain;
+
+	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
+		return 0;
+
+	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	return intel_display_power_get(i915, domain);
+}
+
+static void
+tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum intel_display_power_domain domain;
+
+	/*
+	 * wakeref == -1, means some error happened saving save_depot_stack but
+	 * power should still be put down and 0 is a invalid save_depot_stack
+	 * id so can be used to skip it for non TC legacy ports.
+	 */
+	if (wakeref == 0)
+		return;
+
+	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	intel_display_power_put_async(i915, domain, wakeref);
+}
+
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
@@ -420,9 +452,14 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
 	enum tc_port_mode old_tc_mode = dig_port->tc_mode;
 
 	intel_display_power_flush_work(i915);
-	drm_WARN_ON(&i915->drm,
-		    intel_display_power_is_enabled(i915,
-					intel_aux_power_domain(dig_port)));
+	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port) {
+		enum intel_display_power_domain aux_domain;
+		bool aux_powered;
+
+		aux_domain = intel_aux_power_domain(dig_port);
+		aux_powered = intel_display_power_is_enabled(i915, aux_domain);
+		drm_WARN_ON(&i915->drm, aux_powered);
+	}
 
 	icl_tc_phy_disconnect(dig_port);
 	icl_tc_phy_connect(dig_port, required_lanes);
@@ -445,9 +482,11 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	struct intel_encoder *encoder = &dig_port->base;
+	intel_wakeref_t tc_cold_wref;
 	int active_links = 0;
 
 	mutex_lock(&dig_port->tc_lock);
+	tc_cold_wref = tc_cold_block(dig_port);
 
 	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
 	if (dig_port->dp.is_mst)
@@ -473,6 +512,7 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
 		    dig_port->tc_port_name,
 		    tc_port_mode_name(dig_port->tc_mode));
 
+	tc_cold_unblock(dig_port, tc_cold_wref);
 	mutex_unlock(&dig_port->tc_lock);
 }
 
@@ -494,10 +534,15 @@ static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
 bool intel_tc_port_connected(struct intel_digital_port *dig_port)
 {
 	bool is_connected;
+	intel_wakeref_t tc_cold_wref;
 
 	intel_tc_port_lock(dig_port);
+	tc_cold_wref = tc_cold_block(dig_port);
+
 	is_connected = tc_port_live_status_mask(dig_port) &
 		       BIT(dig_port->tc_mode);
+
+	tc_cold_unblock(dig_port, tc_cold_wref);
 	intel_tc_port_unlock(dig_port);
 
 	return is_connected;
@@ -513,9 +558,16 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
 
 	mutex_lock(&dig_port->tc_lock);
 
-	if (!dig_port->tc_link_refcount &&
-	    intel_tc_port_needs_reset(dig_port))
-		intel_tc_port_reset_mode(dig_port, required_lanes);
+	if (!dig_port->tc_link_refcount) {
+		intel_wakeref_t tc_cold_wref;
+
+		tc_cold_wref = tc_cold_block(dig_port);
+
+		if (intel_tc_port_needs_reset(dig_port))
+			intel_tc_port_reset_mode(dig_port, required_lanes);
+
+		tc_cold_unblock(dig_port, tc_cold_wref);
+	}
 
 	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
 	dig_port->tc_lock_wakeref = wakeref;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0b39b9abf8a4..e4667add70b0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9108,6 +9108,7 @@ enum {
 #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
 #define   GEN6_PCODE_READ_D_COMP		0x10
 #define   GEN6_PCODE_WRITE_D_COMP		0x11
+#define   ICL_PCODE_EXIT_TCCOLD			0x12
 #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
 #define   DISPLAY_IPS_CONTROL			0x19
             /* See also IPS_CTL */
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 5/9] drm/i915/tc: Skip ref held check for TC legacy aux power wells
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (2 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences José Roberto de Souza
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx

As part of ICL TC cold exit sequences we need to request aux power
well before lock the access to TC ports, so skiping the
intel_tc_port_ref_held() check for TC legacy ports.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index e2dddaf1051b..50bed2d1dd13 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -560,6 +560,9 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
 	if (drm_WARN_ON(&dev_priv->drm, !dig_port))
 		return;
 
+	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
+		return;
+
 	drm_WARN_ON(&dev_priv->drm, !intel_tc_port_ref_held(dig_port));
 }
 
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (3 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 5/9] drm/i915/tc: Skip ref held check for TC legacy aux power wells José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-14 17:39   ` Imre Deak
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 7/9] drm/i915/tc: Catch TC users accessing FIA registers without enable aux José Roberto de Souza
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Kai-Heng Feng

TC ports can enter in TCCOLD to save power and is required to request
to PCODE to exit this state before use or read to TC registers.

For TGL there is a new MBOX command to do that with a parameter to ask
PCODE to exit and block TCCOLD entry or unblock TCCOLD entry.

So adding a new power domain to reuse the refcount and only allow
TC cold when all TC ports are not in use.

v2:
- fixed missing case in intel_display_power_domain_str()
- moved tgl_tc_cold_request to intel_display_power.c
- renamed TGL_TC_COLD_OFF to TGL_TC_COLD_OFF_POWER_DOMAINS
- added all TC and TBT aux power domains to
TGL_TC_COLD_OFF_POWER_DOMAINS

v3:
- added one msec sleep when PCODE returns -EAGAIN
- added timeout of 5msec to not loop forever if
sandybridge_pcode_write_timeout() keeps returning -EAGAIN

v4:
- Made failure to block or unblock TC cold a error
- removed 5msec timeout, intead giving PCODE 1msec by up 3 times to
recover from the internal error

BSpec: 49294
Cc: Imre Deak <imre.deak@intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 107 ++++++++++++++++++
 .../drm/i915/display/intel_display_power.h    |   1 +
 drivers/gpu/drm/i915/display/intel_tc.c       |  17 ++-
 drivers/gpu/drm/i915/i915_reg.h               |   4 +
 4 files changed, 126 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 50bed2d1dd13..00de926aaccf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -151,6 +151,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "GT_IRQ";
 	case POWER_DOMAIN_DPLL_DC_OFF:
 		return "DPLL_DC_OFF";
+	case POWER_DOMAIN_TC_COLD_OFF:
+		return "TC_COLD_OFF";
 	default:
 		MISSING_CASE(domain);
 		return "?";
@@ -2861,6 +2863,21 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS (	\
 	BIT_ULL(POWER_DOMAIN_AUX_I_TBT))
 
+#define TGL_TC_COLD_OFF_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_AUX_D)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_E)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_F)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_G)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_H)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_I)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_D_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_E_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_F_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_G_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_H_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_AUX_I_TBT)	|	\
+	BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
+
 static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
 	.sync_hw = i9xx_power_well_sync_hw_noop,
 	.enable = i9xx_always_on_power_well_noop,
@@ -3963,6 +3980,90 @@ static const struct i915_power_well_desc ehl_power_wells[] = {
 	},
 };
 
+static void
+tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
+{
+	u8 tries = 0;
+	int ret;
+
+	while (1) {
+		u32 low_val = 0, high_val;
+
+		if (block)
+			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ;
+		else
+			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
+
+		/*
+		 * Spec states that we should timeout the request after 200us
+		 * but the function below will timeout after 500us
+		 */
+		ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD, &low_val,
+					     &high_val);
+		if (ret == 0) {
+			if (block &&
+			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
+				ret = -EIO;
+			else
+				break;
+		}
+
+		if (++tries == 3)
+			break;
+
+		msleep(1);
+	}
+
+	if (ret)
+		drm_err(&i915->drm, "TC cold %sblock failed\n",
+			block ? "" : "un");
+	else
+		drm_dbg_kms(&i915->drm, "TC cold %sblock succeeded\n",
+			    block ? "" : "un");
+}
+
+static void
+tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
+				  struct i915_power_well *power_well)
+{
+	tgl_tc_cold_request(i915, true);
+}
+
+static void
+tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915,
+				   struct i915_power_well *power_well)
+{
+	tgl_tc_cold_request(i915, false);
+}
+
+static void
+tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915,
+				   struct i915_power_well *power_well)
+{
+	if (power_well->count > 0)
+		tgl_tc_cold_off_power_well_enable(i915, power_well);
+	else
+		tgl_tc_cold_off_power_well_disable(i915, power_well);
+}
+
+static bool
+tgl_tc_cold_off_power_well_is_enabled(struct drm_i915_private *dev_priv,
+				      struct i915_power_well *power_well)
+{
+	/*
+	 * Not the correctly implementation but there is no way to just read it
+	 * from PCODE, so returning count to avoid state mismatch errors
+	 */
+	return power_well->count;
+}
+
+static const struct i915_power_well_ops tgl_tc_cold_off_ops = {
+	.sync_hw = tgl_tc_cold_off_power_well_sync_hw,
+	.enable = tgl_tc_cold_off_power_well_enable,
+	.disable = tgl_tc_cold_off_power_well_disable,
+	.is_enabled = tgl_tc_cold_off_power_well_is_enabled,
+};
+
 static const struct i915_power_well_desc tgl_power_wells[] = {
 	{
 		.name = "always-on",
@@ -4290,6 +4391,12 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
 			.hsw.irq_pipe_mask = BIT(PIPE_D),
 		},
 	},
+	{
+		.name = "TC cold off",
+		.domains = TGL_TC_COLD_OFF_POWER_DOMAINS,
+		.ops = &tgl_tc_cold_off_ops,
+		.id = DISP_PW_ID_NONE,
+	},
 };
 
 static int
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index da64a5edae7a..070457e7b948 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -76,6 +76,7 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_GT_IRQ,
 	POWER_DOMAIN_DPLL_DC_OFF,
+	POWER_DOMAIN_TC_COLD_OFF,
 	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 0cf33d4d21c3..521a94c63640 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -53,16 +53,27 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
 	}
 }
 
+static enum intel_display_power_domain
+tc_cold_get_power_domain(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+	if (INTEL_GEN(i915) == 11)
+		return intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	else
+		return POWER_DOMAIN_TC_COLD_OFF;
+}
+
 static intel_wakeref_t
 tc_cold_block(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
 	enum intel_display_power_domain domain;
 
-	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
+	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
 		return 0;
 
-	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	domain = tc_cold_get_power_domain(dig_port);
 	return intel_display_power_get(i915, domain);
 }
 
@@ -80,7 +91,7 @@ tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
 	if (wakeref == 0)
 		return;
 
-	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
+	domain = tc_cold_get_power_domain(dig_port);
 	intel_display_power_put_async(i915, domain, wakeref);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e4667add70b0..39f281fe6d6c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9111,6 +9111,10 @@ enum {
 #define   ICL_PCODE_EXIT_TCCOLD			0x12
 #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
 #define   DISPLAY_IPS_CONTROL			0x19
+#define   TGL_PCODE_TCCOLD			0x26
+#define     TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED	REG_BIT(0)
+#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ	0
+#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ	REG_BIT(0)
             /* See also IPS_CTL */
 #define     IPS_PCODE_CONTROL			(1 << 30)
 #define   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 7/9] drm/i915/tc: Catch TC users accessing FIA registers without enable aux
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (4 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout José Roberto de Souza
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx

As described in "drm/i915/tc/icl: Implement TC cold sequences" users
of TC functions should held aux power well during access to avoid
read garbage due HW in TC cold state.

v3:
- renamed is_tc_cold_blocked() to assert_tc_cold_blocked()
- restored the removed 0xffffffff checks

Reviewed-by: Imre Deak <imre.deak@intel.com>
Tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_tc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 521a94c63640..d3bd5e798fbc 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -95,6 +95,20 @@ tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
 	intel_display_power_put_async(i915, domain, wakeref);
 }
 
+static void
+assert_tc_cold_blocked(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	bool enabled;
+
+	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
+		return;
+
+	enabled = intel_display_power_is_enabled(i915,
+						 tc_cold_get_power_domain(dig_port));
+	drm_WARN_ON(&i915->drm, !enabled);
+}
+
 u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
@@ -105,6 +119,7 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
 				      PORT_TX_DFLEXDPSP(dig_port->tc_phy_fia));
 
 	drm_WARN_ON(&i915->drm, lane_mask == 0xffffffff);
+	assert_tc_cold_blocked(dig_port);
 
 	lane_mask &= DP_LANE_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx);
 	return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
@@ -120,6 +135,7 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
 				     PORT_TX_DFLEXPA1(dig_port->tc_phy_fia));
 
 	drm_WARN_ON(&i915->drm, pin_mask == 0xffffffff);
+	assert_tc_cold_blocked(dig_port);
 
 	return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >>
 	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
@@ -134,6 +150,8 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 	if (dig_port->tc_mode != TC_PORT_DP_ALT)
 		return 4;
 
+	assert_tc_cold_blocked(dig_port);
+
 	lane_mask = 0;
 	with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref)
 		lane_mask = intel_tc_port_get_lane_mask(dig_port);
@@ -166,6 +184,8 @@ void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port,
 	drm_WARN_ON(&i915->drm,
 		    lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
 
+	assert_tc_cold_blocked(dig_port);
+
 	val = intel_uncore_read(uncore,
 				PORT_TX_DFLEXDPMLE1(dig_port->tc_phy_fia));
 	val &= ~DFLEXDPMLE1_DPMLETC_MASK(dig_port->tc_phy_fia_idx);
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (5 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 7/9] drm/i915/tc: Catch TC users accessing FIA registers without enable aux José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-14 17:47   ` Imre Deak
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure José Roberto de Souza
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx

This is a expected timeout of static TC ports not conneceted, so
not throwing warnings that would taint CI.

v3:
- moved checks to tc_phy_aux_timeout_expected()

v4:
- moved and add comments to tc_phy_aux_timeout_expected()

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 56 +++++++++++++------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 00de926aaccf..2d2125d1534b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -284,6 +284,21 @@ static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv,
 		gen8_irq_power_well_pre_disable(dev_priv, irq_pipe_mask);
 }
 
+#define ICL_AUX_PW_TO_CH(pw_idx)	\
+	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
+
+#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
+	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
+
+static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
+				     struct i915_power_well *power_well)
+{
+	int pw_idx = power_well->desc->hsw.idx;
+
+	return power_well->desc->hsw.is_tc_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
+						 ICL_AUX_PW_TO_CH(pw_idx);
+}
+
 static struct intel_digital_port *
 aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
 		       enum aux_ch aux_ch)
@@ -311,6 +326,27 @@ aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
 	return dig_port;
 }
 
+static bool tc_phy_aux_timeout_expected(struct drm_i915_private *dev_priv,
+					struct i915_power_well *power_well)
+{
+	/* An AUX timeout is expected if the TBT DP tunnel is down. */
+	if (power_well->desc->hsw.is_tc_tbt)
+		return true;
+
+	/*
+	 * An AUX timeout is expected because we enable TC legacy port aux
+	 * to hold port out of TC cold
+	 */
+	if (INTEL_GEN(dev_priv) == 11) {
+		enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
+		struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
+
+		return dig_port->tc_legacy_port;
+	}
+
+	return false;
+}
+
 static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
@@ -323,8 +359,9 @@ static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
 		drm_dbg_kms(&dev_priv->drm, "%s power well enable timeout\n",
 			    power_well->desc->name);
 
-		/* An AUX timeout is expected if the TBT DP tunnel is down. */
-		drm_WARN_ON(&dev_priv->drm, !power_well->desc->hsw.is_tc_tbt);
+		drm_WARN_ON(&dev_priv->drm,
+			    !tc_phy_aux_timeout_expected(dev_priv, power_well));
+
 	}
 }
 
@@ -520,21 +557,6 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
 	hsw_wait_for_power_well_disable(dev_priv, power_well);
 }
 
-#define ICL_AUX_PW_TO_CH(pw_idx)	\
-	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
-
-#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
-	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
-
-static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
-				     struct i915_power_well *power_well)
-{
-	int pw_idx = power_well->desc->hsw.idx;
-
-	return power_well->desc->hsw.is_tc_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
-						 ICL_AUX_PW_TO_CH(pw_idx);
-}
-
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
 
 static u64 async_put_domains_mask(struct i915_power_domains *power_domains);
-- 
2.26.0

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

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

* [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (6 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout José Roberto de Souza
@ 2020-04-13 16:45 ` José Roberto de Souza
  2020-04-14 18:29   ` Imre Deak
  2020-04-14 20:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch Patchwork
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: José Roberto de Souza @ 2020-04-13 16:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

The intel_display_power_put_async() used in TC cold sequences made
easy to hit the missing deinitialization of driver in case of load
failure as seen in the stack trace bellow.

intel_modeset_driver_remove_noirq() had to be removed from
i915_driver_modeset_remove_noirq() as those are different
initialialition steps with IRQ in between then.

[drm:__intel_engine_init_ctx_wa [i915]] Initialized 3 context workarounds on rcs'0
[drm:__i915_inject_probe_error [i915]] Injecting failure -19 at checkpoint 36 [__uc_init:294]
[drm:i915_hdcp_component_unbind [i915]] I915 HDCP comp unbind
[drm:edp_panel_vdd_off_sync [i915]] Turning [ENCODER:275:DDI A] VDD off
[drm:edp_panel_vdd_off_sync [i915]] PP_STATUS: 0x00000000 PP_CONTROL: 0x00000060
[drm:intel_power_well_disable [i915]] disabling AUX A
general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted: G     U            5.6.0-CI-Patchwork_17226+ #1
Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
Workqueue: events_unbound intel_display_power_put_async_work [i915]
RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41 a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03 <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
FS:  0000000000000000(0000) GS:ffff88849ff80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005634fa8ed670 CR3: 0000000005610004 CR4: 0000000000760ee0
PKRU: 55555554
Call Trace:
 release_async_put_domains+0x9b/0x110 [i915]
 intel_display_power_put_async_work+0x91/0xf0 [i915]
 process_one_work+0x260/0x600
 ? worker_thread+0xc9/0x380
 worker_thread+0x37/0x380
 ? process_one_work+0x600/0x600
 kthread+0x119/0x130
 ? kthread_park+0x80/0x80
 ret_from_fork+0x24/0x50
Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]
---[ end trace b402d1b4060f8b97 ]---
BUG: sleeping function called from invalid context at kernel/sched/completion.c:99
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1142, name: kworker/u16:20
INFO: lockdep is turned off.
Preemption disabled at:
[<0000000000000000>] 0x0
CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted: G     UD           5.6.0-CI-Patchwork_17226+ #1
Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
Workqueue: events_unbound intel_display_power_put_async_work [i915]
Call Trace:
 dump_stack+0x71/0x9b
 ___might_sleep+0x178/0x260
 wait_for_completion+0x37/0x1a0
 virt_efi_query_variable_info+0x161/0x1b0
 efi_query_variable_store+0xb3/0x1a0
 ? efivar_entry_set_safe+0x19c/0x220
 efivar_entry_set_safe+0x19c/0x220
 ? efi_pstore_write+0x10b/0x150
 ? efi_pstore_write+0xa0/0x150
 efi_pstore_write+0x10b/0x150
 pstore_dump+0x123/0x340
 kmsg_dump+0x87/0x1b0
 oops_end+0x3e/0x90
 do_general_protection+0x1c3/0x2f0
 general_protection+0x2d/0x40
RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41 a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03 <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
 release_async_put_domains+0x9b/0x110 [i915]
 intel_display_power_put_async_work+0x91/0xf0 [i915]
 process_one_work+0x260/0x600
 ? worker_thread+0xc9/0x380
 worker_thread+0x37/0x380
 ? process_one_work+0x600/0x600
 kthread+0x119/0x130
 ? kthread_park+0x80/0x80
 ret_from_fork+0x24/0x50
------------[ cut here ]------------
WARNING: CPU: 3 PID: 1142 at kernel/rcu/tree_plugin.h:293 rcu_note_context_switch+0x87/0x650
Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7a3b4b98572..9727689e264b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -248,8 +248,11 @@ static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
 	return 0;
 
 cleanup_vga_client:
+	intel_csr_ucode_fini(i915);
+	intel_power_domains_driver_remove(i915);
 	intel_vga_unregister(i915);
 out:
+	intel_bios_driver_remove(i915);
 	return ret;
 }
 
@@ -308,13 +311,13 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915)
 /* part #2: call after irq uninstall */
 static void i915_driver_modeset_remove_noirq(struct drm_i915_private *i915)
 {
-	intel_modeset_driver_remove_noirq(i915);
+	intel_csr_ucode_fini(i915);
 
-	intel_bios_driver_remove(i915);
+	intel_power_domains_driver_remove(i915);
 
 	intel_vga_unregister(i915);
 
-	intel_csr_ucode_fini(i915);
+	intel_bios_driver_remove(i915);
 }
 
 static void intel_init_dpio(struct drm_i915_private *dev_priv)
@@ -994,7 +997,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 out_cleanup_irq:
 	intel_irq_uninstall(i915);
 out_cleanup_modeset:
-	/* FIXME */
+	i915_driver_modeset_remove_noirq(i915);
 out_cleanup_hw:
 	i915_driver_hw_remove(i915);
 	intel_memory_regions_driver_release(i915);
@@ -1031,13 +1034,13 @@ void i915_driver_remove(struct drm_i915_private *i915)
 
 	intel_irq_uninstall(i915);
 
+	intel_modeset_driver_remove_noirq(i915);
+
 	i915_driver_modeset_remove_noirq(i915);
 
 	i915_reset_error_state(i915);
 	i915_gem_driver_remove(i915);
 
-	intel_power_domains_driver_remove(i915);
-
 	i915_driver_hw_remove(i915);
 
 	enable_rpm_wakeref_asserts(&i915->runtime_pm);
-- 
2.26.0

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

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

* Re: [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences José Roberto de Souza
@ 2020-04-14 17:31   ` Imre Deak
  0 siblings, 0 replies; 21+ messages in thread
From: Imre Deak @ 2020-04-14 17:31 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Cooper Chiou, intel-gfx, Kai-Heng Feng

On Mon, Apr 13, 2020 at 09:45:10AM -0700, José Roberto de Souza wrote:
> This is required for legacy/static TC ports as IOM is not aware of
> the connection and will not trigger the TC cold exit.
> 
> Just request PCODE to exit TCCOLD is not enough as it could enter
> again before driver makes use of the port, to prevent it BSpec states
> that aux powerwell should be held.
> 
> So here embedding the TC cold exit sequence into ICL aux enable,
> it will enable aux and then request TC cold to exit.
> 
> The TC cold block(exit and aux hold) and unblock was added to some
> exported TC functions for the others and to access PHY registers,
> callers should enable and keep aux powerwell enabled during access.
> 
> Also adding TC cold check and warnig in tc_port_load_fia_params() as
> at this point of the driver initialization we can't request power
> wells, if we get this warning we will need to figure out how to handle
> it.
> 
> v2:
> - moved ICL TC cold exit function to intel_display_power
> - using dig_port->tc_legacy_port to only execute sequences for legacy
> ports, hopefully VBTs will have this right
> - fixed check to call _hsw_power_well_continue_enable()
> - calling _hsw_power_well_continue_enable() unconditionally in
> icl_tc_phy_aux_power_well_enable(), if needed we will surpress timeout
> warnings of TC legacy ports
> - only blocking TC cold around fia access
> 
> v3:
> - added timeout of 5msec to not loop forever if
> sandybridge_pcode_write_timeout() keeps returning -EAGAIN
> returning -EAGAIN in in icl_tc_cold_exit()
> - removed leftover tc_cold_wakeref
> - added one msec sleep when PCODE returns -EAGAIN
> 
> v4:
> - removed 5msec timeout, instead giving 1msec to whoever is using
> PCODE to finish it up to 3 times
> - added a comment about turn TC cold exit failure as a error in future
> 
> BSpec: 21750
> Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1296
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  .../drm/i915/display/intel_display_power.c    | 25 +++++++-
>  drivers/gpu/drm/i915/display/intel_tc.c       | 64 +++++++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>  3 files changed, 83 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 5bef7dda4a7a..e2dddaf1051b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -575,6 +575,28 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
>  
>  #define TGL_AUX_PW_TO_TC_PORT(pw_idx)	((pw_idx) - TGL_PW_CTL_IDX_AUX_TC1)
>  
> +static void icl_tc_cold_exit(struct drm_i915_private *i915)
> +{
> +	int ret, tries = 0;
> +
> +	while (1) {
> +		ret = sandybridge_pcode_write_timeout(i915,
> +						      ICL_PCODE_EXIT_TCCOLD,
> +						      0, 250, 1);
> +		if (ret != -EAGAIN || ++tries == 3)
> +			break;
> +		msleep(1);
> +	}
> +
> +	/* Spec states that TC cold exit can take up to 1ms to complete */
> +	if (!ret)
> +		msleep(1);
> +
> +	/* TODO: turn failure into a error as soon i915 CI updates ICL IFWI */
> +	drm_dbg_kms(&i915->drm, "TC cold block %s\n", ret ? "failed" :
> +		    "succeeded");
> +}
> +
>  static void
>  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  				 struct i915_power_well *power_well)
> @@ -593,7 +615,8 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  
>  	hsw_power_well_enable_prepare(dev_priv, power_well);
>  
> -	/* TODO ICL TC cold handling */
> +	if (INTEL_GEN(dev_priv) == 11 && dig_port->tc_legacy_port)
> +		icl_tc_cold_exit(dev_priv);
>  
>  	hsw_power_well_enable_complete(dev_priv, power_well);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 275618bedf32..0cf33d4d21c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -34,6 +34,7 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
>  	if (INTEL_INFO(i915)->display.has_modular_fia) {
>  		modular_fia = intel_uncore_read(&i915->uncore,
>  						PORT_TX_DFLEXDPSP(FIA1));
> +		drm_WARN_ON(&i915->drm, modular_fia == 0xffffffff);
>  		modular_fia &= MODULAR_FIA_MASK;
>  	} else {
>  		modular_fia = 0;
> @@ -52,6 +53,37 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
>  	}
>  }
>  
> +static intel_wakeref_t
> +tc_cold_block(struct intel_digital_port *dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum intel_display_power_domain domain;
> +
> +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> +		return 0;
> +
> +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	return intel_display_power_get(i915, domain);
> +}
> +
> +static void
> +tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum intel_display_power_domain domain;
> +
> +	/*
> +	 * wakeref == -1, means some error happened saving save_depot_stack but
> +	 * power should still be put down and 0 is a invalid save_depot_stack
> +	 * id so can be used to skip it for non TC legacy ports.
> +	 */
> +	if (wakeref == 0)
> +		return;
> +
> +	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	intel_display_power_put_async(i915, domain, wakeref);
> +}
> +
>  u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> @@ -420,9 +452,14 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
>  	enum tc_port_mode old_tc_mode = dig_port->tc_mode;
>  
>  	intel_display_power_flush_work(i915);
> -	drm_WARN_ON(&i915->drm,
> -		    intel_display_power_is_enabled(i915,
> -					intel_aux_power_domain(dig_port)));
> +	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port) {
> +		enum intel_display_power_domain aux_domain;
> +		bool aux_powered;
> +
> +		aux_domain = intel_aux_power_domain(dig_port);
> +		aux_powered = intel_display_power_is_enabled(i915, aux_domain);
> +		drm_WARN_ON(&i915->drm, aux_powered);
> +	}
>  
>  	icl_tc_phy_disconnect(dig_port);
>  	icl_tc_phy_connect(dig_port, required_lanes);
> @@ -445,9 +482,11 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	struct intel_encoder *encoder = &dig_port->base;
> +	intel_wakeref_t tc_cold_wref;
>  	int active_links = 0;
>  
>  	mutex_lock(&dig_port->tc_lock);
> +	tc_cold_wref = tc_cold_block(dig_port);
>  
>  	dig_port->tc_mode = intel_tc_port_get_current_mode(dig_port);
>  	if (dig_port->dp.is_mst)
> @@ -473,6 +512,7 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
>  		    dig_port->tc_port_name,
>  		    tc_port_mode_name(dig_port->tc_mode));
>  
> +	tc_cold_unblock(dig_port, tc_cold_wref);
>  	mutex_unlock(&dig_port->tc_lock);
>  }
>  
> @@ -494,10 +534,15 @@ static bool intel_tc_port_needs_reset(struct intel_digital_port *dig_port)
>  bool intel_tc_port_connected(struct intel_digital_port *dig_port)
>  {
>  	bool is_connected;
> +	intel_wakeref_t tc_cold_wref;
>  
>  	intel_tc_port_lock(dig_port);
> +	tc_cold_wref = tc_cold_block(dig_port);
> +
>  	is_connected = tc_port_live_status_mask(dig_port) &
>  		       BIT(dig_port->tc_mode);
> +
> +	tc_cold_unblock(dig_port, tc_cold_wref);
>  	intel_tc_port_unlock(dig_port);
>  
>  	return is_connected;
> @@ -513,9 +558,16 @@ static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
>  
>  	mutex_lock(&dig_port->tc_lock);
>  
> -	if (!dig_port->tc_link_refcount &&
> -	    intel_tc_port_needs_reset(dig_port))
> -		intel_tc_port_reset_mode(dig_port, required_lanes);
> +	if (!dig_port->tc_link_refcount) {
> +		intel_wakeref_t tc_cold_wref;
> +
> +		tc_cold_wref = tc_cold_block(dig_port);
> +
> +		if (intel_tc_port_needs_reset(dig_port))
> +			intel_tc_port_reset_mode(dig_port, required_lanes);
> +
> +		tc_cold_unblock(dig_port, tc_cold_wref);
> +	}
>  
>  	drm_WARN_ON(&i915->drm, dig_port->tc_lock_wakeref);
>  	dig_port->tc_lock_wakeref = wakeref;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0b39b9abf8a4..e4667add70b0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9108,6 +9108,7 @@ enum {
>  #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
>  #define   GEN6_PCODE_READ_D_COMP		0x10
>  #define   GEN6_PCODE_WRITE_D_COMP		0x11
> +#define   ICL_PCODE_EXIT_TCCOLD			0x12
>  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
>  #define   DISPLAY_IPS_CONTROL			0x19
>              /* See also IPS_CTL */
> -- 
> 2.26.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences José Roberto de Souza
@ 2020-04-14 17:39   ` Imre Deak
  2020-04-14 17:41     ` Souza, Jose
  0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-14 17:39 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Cooper Chiou, intel-gfx, Kai-Heng Feng

On Mon, Apr 13, 2020 at 09:45:12AM -0700, José Roberto de Souza wrote:
> TC ports can enter in TCCOLD to save power and is required to request
> to PCODE to exit this state before use or read to TC registers.
> 
> For TGL there is a new MBOX command to do that with a parameter to ask
> PCODE to exit and block TCCOLD entry or unblock TCCOLD entry.
> 
> So adding a new power domain to reuse the refcount and only allow
> TC cold when all TC ports are not in use.
> 
> v2:
> - fixed missing case in intel_display_power_domain_str()
> - moved tgl_tc_cold_request to intel_display_power.c
> - renamed TGL_TC_COLD_OFF to TGL_TC_COLD_OFF_POWER_DOMAINS
> - added all TC and TBT aux power domains to
> TGL_TC_COLD_OFF_POWER_DOMAINS
> 
> v3:
> - added one msec sleep when PCODE returns -EAGAIN
> - added timeout of 5msec to not loop forever if
> sandybridge_pcode_write_timeout() keeps returning -EAGAIN
> 
> v4:
> - Made failure to block or unblock TC cold a error
> - removed 5msec timeout, intead giving PCODE 1msec by up 3 times to
> recover from the internal error
> 
> BSpec: 49294
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 107 ++++++++++++++++++
>  .../drm/i915/display/intel_display_power.h    |   1 +
>  drivers/gpu/drm/i915/display/intel_tc.c       |  17 ++-
>  drivers/gpu/drm/i915/i915_reg.h               |   4 +
>  4 files changed, 126 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 50bed2d1dd13..00de926aaccf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -151,6 +151,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "GT_IRQ";
>  	case POWER_DOMAIN_DPLL_DC_OFF:
>  		return "DPLL_DC_OFF";
> +	case POWER_DOMAIN_TC_COLD_OFF:
> +		return "TC_COLD_OFF";
>  	default:
>  		MISSING_CASE(domain);
>  		return "?";
> @@ -2861,6 +2863,21 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>  #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS (	\
>  	BIT_ULL(POWER_DOMAIN_AUX_I_TBT))
>  
> +#define TGL_TC_COLD_OFF_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_AUX_D)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_E)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_F)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_G)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_H)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_I)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_D_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_E_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_F_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_G_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_H_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_AUX_I_TBT)	|	\
> +	BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
> +
>  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
>  	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = i9xx_always_on_power_well_noop,
> @@ -3963,6 +3980,90 @@ static const struct i915_power_well_desc ehl_power_wells[] = {
>  	},
>  };
>  
> +static void
> +tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
> +{
> +	u8 tries = 0;
> +	int ret;
> +
> +	while (1) {
> +		u32 low_val = 0, high_val;
> +
> +		if (block)
> +			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ;
> +		else
> +			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
> +
> +		/*
> +		 * Spec states that we should timeout the request after 200us
> +		 * but the function below will timeout after 500us
> +		 */
> +		ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD, &low_val,
> +					     &high_val);
> +		if (ret == 0) {
> +			if (block &&
> +			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> +				ret = -EIO;
> +			else
> +				break;
> +		}
> +
> +		if (++tries == 3)
> +			break;
> +
> +		msleep(1);

Why did this go back to msleep unconditionally? Doing that only for
ret == -EAGAIN made sense to me, since that is only the case where you'd
want to avoid busy looping.

> +	}
> +
> +	if (ret)
> +		drm_err(&i915->drm, "TC cold %sblock failed\n",
> +			block ? "" : "un");
> +	else
> +		drm_dbg_kms(&i915->drm, "TC cold %sblock succeeded\n",
> +			    block ? "" : "un");
> +}
> +
> +static void
> +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
> +				  struct i915_power_well *power_well)
> +{
> +	tgl_tc_cold_request(i915, true);
> +}
> +
> +static void
> +tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915,
> +				   struct i915_power_well *power_well)
> +{
> +	tgl_tc_cold_request(i915, false);
> +}
> +
> +static void
> +tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915,
> +				   struct i915_power_well *power_well)
> +{
> +	if (power_well->count > 0)
> +		tgl_tc_cold_off_power_well_enable(i915, power_well);
> +	else
> +		tgl_tc_cold_off_power_well_disable(i915, power_well);
> +}
> +
> +static bool
> +tgl_tc_cold_off_power_well_is_enabled(struct drm_i915_private *dev_priv,
> +				      struct i915_power_well *power_well)
> +{
> +	/*
> +	 * Not the correctly implementation but there is no way to just read it
> +	 * from PCODE, so returning count to avoid state mismatch errors
> +	 */
> +	return power_well->count;
> +}
> +
> +static const struct i915_power_well_ops tgl_tc_cold_off_ops = {
> +	.sync_hw = tgl_tc_cold_off_power_well_sync_hw,
> +	.enable = tgl_tc_cold_off_power_well_enable,
> +	.disable = tgl_tc_cold_off_power_well_disable,
> +	.is_enabled = tgl_tc_cold_off_power_well_is_enabled,
> +};
> +
>  static const struct i915_power_well_desc tgl_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -4290,6 +4391,12 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
>  			.hsw.irq_pipe_mask = BIT(PIPE_D),
>  		},
>  	},
> +	{
> +		.name = "TC cold off",
> +		.domains = TGL_TC_COLD_OFF_POWER_DOMAINS,
> +		.ops = &tgl_tc_cold_off_ops,
> +		.id = DISP_PW_ID_NONE,
> +	},
>  };
>  
>  static int
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index da64a5edae7a..070457e7b948 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -76,6 +76,7 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_MODESET,
>  	POWER_DOMAIN_GT_IRQ,
>  	POWER_DOMAIN_DPLL_DC_OFF,
> +	POWER_DOMAIN_TC_COLD_OFF,
>  	POWER_DOMAIN_INIT,
>  
>  	POWER_DOMAIN_NUM,
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index 0cf33d4d21c3..521a94c63640 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -53,16 +53,27 @@ tc_port_load_fia_params(struct drm_i915_private *i915,
>  	}
>  }
>  
> +static enum intel_display_power_domain
> +tc_cold_get_power_domain(struct intel_digital_port *dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +
> +	if (INTEL_GEN(i915) == 11)
> +		return intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	else
> +		return POWER_DOMAIN_TC_COLD_OFF;
> +}
> +
>  static intel_wakeref_t
>  tc_cold_block(struct intel_digital_port *dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>  	enum intel_display_power_domain domain;
>  
> -	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> +	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
>  		return 0;
>  
> -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	domain = tc_cold_get_power_domain(dig_port);
>  	return intel_display_power_get(i915, domain);
>  }
>  
> @@ -80,7 +91,7 @@ tc_cold_unblock(struct intel_digital_port *dig_port, intel_wakeref_t wakeref)
>  	if (wakeref == 0)
>  		return;
>  
> -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> +	domain = tc_cold_get_power_domain(dig_port);
>  	intel_display_power_put_async(i915, domain, wakeref);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e4667add70b0..39f281fe6d6c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9111,6 +9111,10 @@ enum {
>  #define   ICL_PCODE_EXIT_TCCOLD			0x12
>  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
>  #define   DISPLAY_IPS_CONTROL			0x19
> +#define   TGL_PCODE_TCCOLD			0x26
> +#define     TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED	REG_BIT(0)
> +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ	0
> +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ	REG_BIT(0)
>              /* See also IPS_CTL */
>  #define     IPS_PCODE_CONTROL			(1 << 30)
>  #define   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
> -- 
> 2.26.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences
  2020-04-14 17:39   ` Imre Deak
@ 2020-04-14 17:41     ` Souza, Jose
  2020-04-14 19:02       ` Imre Deak
  0 siblings, 1 reply; 21+ messages in thread
From: Souza, Jose @ 2020-04-14 17:41 UTC (permalink / raw)
  To: Deak, Imre; +Cc: Chiou, Cooper, intel-gfx, kai.heng.feng

On Tue, 2020-04-14 at 20:39 +0300, Imre Deak wrote:
> On Mon, Apr 13, 2020 at 09:45:12AM -0700, José Roberto de Souza
> wrote:
> > TC ports can enter in TCCOLD to save power and is required to
> > request
> > to PCODE to exit this state before use or read to TC registers.
> > 
> > For TGL there is a new MBOX command to do that with a parameter to
> > ask
> > PCODE to exit and block TCCOLD entry or unblock TCCOLD entry.
> > 
> > So adding a new power domain to reuse the refcount and only allow
> > TC cold when all TC ports are not in use.
> > 
> > v2:
> > - fixed missing case in intel_display_power_domain_str()
> > - moved tgl_tc_cold_request to intel_display_power.c
> > - renamed TGL_TC_COLD_OFF to TGL_TC_COLD_OFF_POWER_DOMAINS
> > - added all TC and TBT aux power domains to
> > TGL_TC_COLD_OFF_POWER_DOMAINS
> > 
> > v3:
> > - added one msec sleep when PCODE returns -EAGAIN
> > - added timeout of 5msec to not loop forever if
> > sandybridge_pcode_write_timeout() keeps returning -EAGAIN
> > 
> > v4:
> > - Made failure to block or unblock TC cold a error
> > - removed 5msec timeout, intead giving PCODE 1msec by up 3 times to
> > recover from the internal error
> > 
> > BSpec: 49294
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 107
> > ++++++++++++++++++
> >  .../drm/i915/display/intel_display_power.h    |   1 +
> >  drivers/gpu/drm/i915/display/intel_tc.c       |  17 ++-
> >  drivers/gpu/drm/i915/i915_reg.h               |   4 +
> >  4 files changed, 126 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 50bed2d1dd13..00de926aaccf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -151,6 +151,8 @@ intel_display_power_domain_str(enum
> > intel_display_power_domain domain)
> >  		return "GT_IRQ";
> >  	case POWER_DOMAIN_DPLL_DC_OFF:
> >  		return "DPLL_DC_OFF";
> > +	case POWER_DOMAIN_TC_COLD_OFF:
> > +		return "TC_COLD_OFF";
> >  	default:
> >  		MISSING_CASE(domain);
> >  		return "?";
> > @@ -2861,6 +2863,21 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> >  #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS (	\
> >  	BIT_ULL(POWER_DOMAIN_AUX_I_TBT))
> >  
> > +#define TGL_TC_COLD_OFF_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_AUX_D)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_E)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_F)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_G)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_H)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_I)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_D_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_E_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_F_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_G_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_H_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_AUX_I_TBT)	|	\
> > +	BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
> > +
> >  static const struct i915_power_well_ops
> > i9xx_always_on_power_well_ops = {
> >  	.sync_hw = i9xx_power_well_sync_hw_noop,
> >  	.enable = i9xx_always_on_power_well_noop,
> > @@ -3963,6 +3980,90 @@ static const struct i915_power_well_desc
> > ehl_power_wells[] = {
> >  	},
> >  };
> >  
> > +static void
> > +tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
> > +{
> > +	u8 tries = 0;
> > +	int ret;
> > +
> > +	while (1) {
> > +		u32 low_val = 0, high_val;
> > +
> > +		if (block)
> > +			high_val =
> > TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ;
> > +		else
> > +			high_val =
> > TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
> > +
> > +		/*
> > +		 * Spec states that we should timeout the request after
> > 200us
> > +		 * but the function below will timeout after 500us
> > +		 */
> > +		ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD,
> > &low_val,
> > +					     &high_val);
> > +		if (ret == 0) {
> > +			if (block &&
> > +			    (low_val &
> > TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> > +				ret = -EIO;
> > +			else
> > +				break;
> > +		}
> > +
> > +		if (++tries == 3)
> > +			break;
> > +
> > +		msleep(1);
> 
> Why did this go back to msleep unconditionally? Doing that only for
> ret == -EAGAIN made sense to me, since that is only the case where
> you'd
> want to avoid busy looping.

Mostly thinking about give PCODE some time to recover it selft when it
returns low_val == TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED

> 
> > +	}
> > +
> > +	if (ret)
> > +		drm_err(&i915->drm, "TC cold %sblock failed\n",
> > +			block ? "" : "un");
> > +	else
> > +		drm_dbg_kms(&i915->drm, "TC cold %sblock succeeded\n",
> > +			    block ? "" : "un");
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
> > +				  struct i915_power_well *power_well)
> > +{
> > +	tgl_tc_cold_request(i915, true);
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915,
> > +				   struct i915_power_well *power_well)
> > +{
> > +	tgl_tc_cold_request(i915, false);
> > +}
> > +
> > +static void
> > +tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915,
> > +				   struct i915_power_well *power_well)
> > +{
> > +	if (power_well->count > 0)
> > +		tgl_tc_cold_off_power_well_enable(i915, power_well);
> > +	else
> > +		tgl_tc_cold_off_power_well_disable(i915, power_well);
> > +}
> > +
> > +static bool
> > +tgl_tc_cold_off_power_well_is_enabled(struct drm_i915_private
> > *dev_priv,
> > +				      struct i915_power_well
> > *power_well)
> > +{
> > +	/*
> > +	 * Not the correctly implementation but there is no way to just
> > read it
> > +	 * from PCODE, so returning count to avoid state mismatch
> > errors
> > +	 */
> > +	return power_well->count;
> > +}
> > +
> > +static const struct i915_power_well_ops tgl_tc_cold_off_ops = {
> > +	.sync_hw = tgl_tc_cold_off_power_well_sync_hw,
> > +	.enable = tgl_tc_cold_off_power_well_enable,
> > +	.disable = tgl_tc_cold_off_power_well_disable,
> > +	.is_enabled = tgl_tc_cold_off_power_well_is_enabled,
> > +};
> > +
> >  static const struct i915_power_well_desc tgl_power_wells[] = {
> >  	{
> >  		.name = "always-on",
> > @@ -4290,6 +4391,12 @@ static const struct i915_power_well_desc
> > tgl_power_wells[] = {
> >  			.hsw.irq_pipe_mask = BIT(PIPE_D),
> >  		},
> >  	},
> > +	{
> > +		.name = "TC cold off",
> > +		.domains = TGL_TC_COLD_OFF_POWER_DOMAINS,
> > +		.ops = &tgl_tc_cold_off_ops,
> > +		.id = DISP_PW_ID_NONE,
> > +	},
> >  };
> >  
> >  static int
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index da64a5edae7a..070457e7b948 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -76,6 +76,7 @@ enum intel_display_power_domain {
> >  	POWER_DOMAIN_MODESET,
> >  	POWER_DOMAIN_GT_IRQ,
> >  	POWER_DOMAIN_DPLL_DC_OFF,
> > +	POWER_DOMAIN_TC_COLD_OFF,
> >  	POWER_DOMAIN_INIT,
> >  
> >  	POWER_DOMAIN_NUM,
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > b/drivers/gpu/drm/i915/display/intel_tc.c
> > index 0cf33d4d21c3..521a94c63640 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -53,16 +53,27 @@ tc_port_load_fia_params(struct drm_i915_private
> > *i915,
> >  	}
> >  }
> >  
> > +static enum intel_display_power_domain
> > +tc_cold_get_power_domain(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> > +
> > +	if (INTEL_GEN(i915) == 11)
> > +		return intel_legacy_aux_to_power_domain(dig_port-
> > >aux_ch);
> > +	else
> > +		return POWER_DOMAIN_TC_COLD_OFF;
> > +}
> > +
> >  static intel_wakeref_t
> >  tc_cold_block(struct intel_digital_port *dig_port)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port-
> > >base.base.dev);
> >  	enum intel_display_power_domain domain;
> >  
> > -	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> > +	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
> >  		return 0;
> >  
> > -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > +	domain = tc_cold_get_power_domain(dig_port);
> >  	return intel_display_power_get(i915, domain);
> >  }
> >  
> > @@ -80,7 +91,7 @@ tc_cold_unblock(struct intel_digital_port
> > *dig_port, intel_wakeref_t wakeref)
> >  	if (wakeref == 0)
> >  		return;
> >  
> > -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > +	domain = tc_cold_get_power_domain(dig_port);
> >  	intel_display_power_put_async(i915, domain, wakeref);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index e4667add70b0..39f281fe6d6c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9111,6 +9111,10 @@ enum {
> >  #define   ICL_PCODE_EXIT_TCCOLD			0x12
> >  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> >  #define   DISPLAY_IPS_CONTROL			0x19
> > +#define   TGL_PCODE_TCCOLD			0x26
> > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED	REG_BIT
> > (0)
> > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ	0
> > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ	REG_BIT
> > (0)
> >              /* See also IPS_CTL */
> >  #define     IPS_PCODE_CONTROL			(1 << 30)
> >  #define   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
> > -- 
> > 2.26.0
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout José Roberto de Souza
@ 2020-04-14 17:47   ` Imre Deak
  2020-04-14 18:05     ` Souza, Jose
  0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-14 17:47 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Mon, Apr 13, 2020 at 09:45:14AM -0700, José Roberto de Souza wrote:
> This is a expected timeout of static TC ports not conneceted, so
> not throwing warnings that would taint CI.
> 
> v3:
> - moved checks to tc_phy_aux_timeout_expected()
> 
> v4:
> - moved and add comments to tc_phy_aux_timeout_expected()
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 56 +++++++++++++------
>  1 file changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 00de926aaccf..2d2125d1534b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -284,6 +284,21 @@ static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv,
>  		gen8_irq_power_well_pre_disable(dev_priv, irq_pipe_mask);
>  }
>  
> +#define ICL_AUX_PW_TO_CH(pw_idx)	\
> +	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
> +
> +#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
> +	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
> +
> +static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
> +				     struct i915_power_well *power_well)
> +{
> +	int pw_idx = power_well->desc->hsw.idx;
> +
> +	return power_well->desc->hsw.is_tc_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
> +						 ICL_AUX_PW_TO_CH(pw_idx);
> +}
> +
>  static struct intel_digital_port *
>  aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
>  		       enum aux_ch aux_ch)
> @@ -311,6 +326,27 @@ aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
>  	return dig_port;
>  }
>  
> +static bool tc_phy_aux_timeout_expected(struct drm_i915_private *dev_priv,
> +					struct i915_power_well *power_well)
> +{
> +	/* An AUX timeout is expected if the TBT DP tunnel is down. */
> +	if (power_well->desc->hsw.is_tc_tbt)
> +		return true;
> +
> +	/*
> +	 * An AUX timeout is expected because we enable TC legacy port aux
> +	 * to hold port out of TC cold
> +	 */
> +	if (INTEL_GEN(dev_priv) == 11) {

You missed my comment on the previous version, should be:
 	if (GEN==11 && power_well->desc->ops == &icl_tc_phy_aux_power_well_ops) {

> +		enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> +		struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
> +
> +		return dig_port->tc_legacy_port;
> +	}
> +
> +	return false;
> +}
> +
>  static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> @@ -323,8 +359,9 @@ static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
>  		drm_dbg_kms(&dev_priv->drm, "%s power well enable timeout\n",
>  			    power_well->desc->name);
>  
> -		/* An AUX timeout is expected if the TBT DP tunnel is down. */
> -		drm_WARN_ON(&dev_priv->drm, !power_well->desc->hsw.is_tc_tbt);
> +		drm_WARN_ON(&dev_priv->drm,
> +			    !tc_phy_aux_timeout_expected(dev_priv, power_well));
> +
>  	}
>  }
>  
> @@ -520,21 +557,6 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
>  	hsw_wait_for_power_well_disable(dev_priv, power_well);
>  }
>  
> -#define ICL_AUX_PW_TO_CH(pw_idx)	\
> -	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
> -
> -#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
> -	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
> -
> -static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
> -				     struct i915_power_well *power_well)
> -{
> -	int pw_idx = power_well->desc->hsw.idx;
> -
> -	return power_well->desc->hsw.is_tc_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
> -						 ICL_AUX_PW_TO_CH(pw_idx);
> -}
> -
>  #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>  
>  static u64 async_put_domains_mask(struct i915_power_domains *power_domains);
> -- 
> 2.26.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout
  2020-04-14 17:47   ` Imre Deak
@ 2020-04-14 18:05     ` Souza, Jose
  0 siblings, 0 replies; 21+ messages in thread
From: Souza, Jose @ 2020-04-14 18:05 UTC (permalink / raw)
  To: Deak, Imre; +Cc: intel-gfx

On Tue, 2020-04-14 at 20:47 +0300, Imre Deak wrote:
> On Mon, Apr 13, 2020 at 09:45:14AM -0700, José Roberto de Souza
> wrote:
> > This is a expected timeout of static TC ports not conneceted, so
> > not throwing warnings that would taint CI.
> > 
> > v3:
> > - moved checks to tc_phy_aux_timeout_expected()
> > 
> > v4:
> > - moved and add comments to tc_phy_aux_timeout_expected()
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 56 +++++++++++++
> > ------
> >  1 file changed, 39 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 00de926aaccf..2d2125d1534b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -284,6 +284,21 @@ static void hsw_power_well_pre_disable(struct
> > drm_i915_private *dev_priv,
> >  		gen8_irq_power_well_pre_disable(dev_priv,
> > irq_pipe_mask);
> >  }
> >  
> > +#define ICL_AUX_PW_TO_CH(pw_idx)	\
> > +	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
> > +
> > +#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
> > +	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
> > +
> > +static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private
> > *dev_priv,
> > +				     struct i915_power_well
> > *power_well)
> > +{
> > +	int pw_idx = power_well->desc->hsw.idx;
> > +
> > +	return power_well->desc->hsw.is_tc_tbt ?
> > ICL_TBT_AUX_PW_TO_CH(pw_idx) :
> > +						 ICL_AUX_PW_TO_CH(pw_id
> > x);
> > +}
> > +
> >  static struct intel_digital_port *
> >  aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
> >  		       enum aux_ch aux_ch)
> > @@ -311,6 +326,27 @@ aux_ch_to_digital_port(struct drm_i915_private
> > *dev_priv,
> >  	return dig_port;
> >  }
> >  
> > +static bool tc_phy_aux_timeout_expected(struct drm_i915_private
> > *dev_priv,
> > +					struct i915_power_well
> > *power_well)
> > +{
> > +	/* An AUX timeout is expected if the TBT DP tunnel is down. */
> > +	if (power_well->desc->hsw.is_tc_tbt)
> > +		return true;
> > +
> > +	/*
> > +	 * An AUX timeout is expected because we enable TC legacy port
> > aux
> > +	 * to hold port out of TC cold
> > +	 */
> > +	if (INTEL_GEN(dev_priv) == 11) {
> 
> You missed my comment on the previous version, should be:
>  	if (GEN==11 && power_well->desc->ops ==
> &icl_tc_phy_aux_power_well_ops) {

Sorry, fixing it.

> 
> > +		enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv,
> > power_well);
> > +		struct intel_digital_port *dig_port =
> > aux_ch_to_digital_port(dev_priv, aux_ch);
> > +
> > +		return dig_port->tc_legacy_port;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  static void hsw_wait_for_power_well_enable(struct drm_i915_private
> > *dev_priv,
> >  					   struct i915_power_well
> > *power_well)
> >  {
> > @@ -323,8 +359,9 @@ static void
> > hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
> >  		drm_dbg_kms(&dev_priv->drm, "%s power well enable
> > timeout\n",
> >  			    power_well->desc->name);
> >  
> > -		/* An AUX timeout is expected if the TBT DP tunnel is
> > down. */
> > -		drm_WARN_ON(&dev_priv->drm, !power_well->desc-
> > >hsw.is_tc_tbt);
> > +		drm_WARN_ON(&dev_priv->drm,
> > +			    !tc_phy_aux_timeout_expected(dev_priv,
> > power_well));
> > +
> >  	}
> >  }
> >  
> > @@ -520,21 +557,6 @@ icl_combo_phy_aux_power_well_disable(struct
> > drm_i915_private *dev_priv,
> >  	hsw_wait_for_power_well_disable(dev_priv, power_well);
> >  }
> >  
> > -#define ICL_AUX_PW_TO_CH(pw_idx)	\
> > -	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
> > -
> > -#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
> > -	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
> > -
> > -static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private
> > *dev_priv,
> > -				     struct i915_power_well
> > *power_well)
> > -{
> > -	int pw_idx = power_well->desc->hsw.idx;
> > -
> > -	return power_well->desc->hsw.is_tc_tbt ?
> > ICL_TBT_AUX_PW_TO_CH(pw_idx) :
> > -						 ICL_AUX_PW_TO_CH(pw_id
> > x);
> > -}
> > -
> >  #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> >  
> >  static u64 async_put_domains_mask(struct i915_power_domains
> > *power_domains);
> > -- 
> > 2.26.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure José Roberto de Souza
@ 2020-04-14 18:29   ` Imre Deak
  2020-04-14 20:50     ` Souza, Jose
  0 siblings, 1 reply; 21+ messages in thread
From: Imre Deak @ 2020-04-14 18:29 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: Jani Nikula, intel-gfx, Chris Wilson

On Mon, Apr 13, 2020 at 09:45:15AM -0700, José Roberto de Souza wrote:
> The intel_display_power_put_async() used in TC cold sequences made
> easy to hit the missing deinitialization of driver in case of load
> failure as seen in the stack trace bellow.
> 
> intel_modeset_driver_remove_noirq() had to be removed from
> i915_driver_modeset_remove_noirq() as those are different
> initialialition steps with IRQ in between then.

Looks ok with some comments below, but this fix should be applied before
the rest of the changes in the patchset, could you resend it separately?

Is it needed in -fixes too?

> [drm:__intel_engine_init_ctx_wa [i915]] Initialized 3 context workarounds on rcs'0
> [drm:__i915_inject_probe_error [i915]] Injecting failure -19 at checkpoint 36 [__uc_init:294]
> [drm:i915_hdcp_component_unbind [i915]] I915 HDCP comp unbind
> [drm:edp_panel_vdd_off_sync [i915]] Turning [ENCODER:275:DDI A] VDD off
> [drm:edp_panel_vdd_off_sync [i915]] PP_STATUS: 0x00000000 PP_CONTROL: 0x00000060
> [drm:intel_power_well_disable [i915]] disabling AUX A
> general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted: G     U            5.6.0-CI-Patchwork_17226+ #1
> Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
> Workqueue: events_unbound intel_display_power_put_async_work [i915]
> RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
> Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41 a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03 <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
> RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
> RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
> RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
> R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
> FS:  0000000000000000(0000) GS:ffff88849ff80000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005634fa8ed670 CR3: 0000000005610004 CR4: 0000000000760ee0
> PKRU: 55555554
> Call Trace:
>  release_async_put_domains+0x9b/0x110 [i915]
>  intel_display_power_put_async_work+0x91/0xf0 [i915]
>  process_one_work+0x260/0x600
>  ? worker_thread+0xc9/0x380
>  worker_thread+0x37/0x380
>  ? process_one_work+0x600/0x600
>  kthread+0x119/0x130
>  ? kthread_park+0x80/0x80
>  ret_from_fork+0x24/0x50
> Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]
> ---[ end trace b402d1b4060f8b97 ]---
> BUG: sleeping function called from invalid context at kernel/sched/completion.c:99
> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1142, name: kworker/u16:20
> INFO: lockdep is turned off.
> Preemption disabled at:
> [<0000000000000000>] 0x0
> CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted: G     UD           5.6.0-CI-Patchwork_17226+ #1
> Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
> Workqueue: events_unbound intel_display_power_put_async_work [i915]
> Call Trace:
>  dump_stack+0x71/0x9b
>  ___might_sleep+0x178/0x260
>  wait_for_completion+0x37/0x1a0
>  virt_efi_query_variable_info+0x161/0x1b0
>  efi_query_variable_store+0xb3/0x1a0
>  ? efivar_entry_set_safe+0x19c/0x220
>  efivar_entry_set_safe+0x19c/0x220
>  ? efi_pstore_write+0x10b/0x150
>  ? efi_pstore_write+0xa0/0x150
>  efi_pstore_write+0x10b/0x150
>  pstore_dump+0x123/0x340
>  kmsg_dump+0x87/0x1b0
>  oops_end+0x3e/0x90
>  do_general_protection+0x1c3/0x2f0
>  general_protection+0x2d/0x40
> RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
> Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41 a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03 <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
> RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
> RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
> RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
> R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
>  release_async_put_domains+0x9b/0x110 [i915]
>  intel_display_power_put_async_work+0x91/0xf0 [i915]
>  process_one_work+0x260/0x600
>  ? worker_thread+0xc9/0x380
>  worker_thread+0x37/0x380
>  ? process_one_work+0x600/0x600
>  kthread+0x119/0x130
>  ? kthread_park+0x80/0x80
>  ret_from_fork+0x24/0x50
> ------------[ cut here ]------------
> WARNING: CPU: 3 PID: 1142 at kernel/rcu/tree_plugin.h:293 rcu_note_context_switch+0x87/0x650
> Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7a3b4b98572..9727689e264b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -248,8 +248,11 @@ static int i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
>  	return 0;
>  
>  cleanup_vga_client:
> +	intel_csr_ucode_fini(i915);
> +	intel_power_domains_driver_remove(i915);
>  	intel_vga_unregister(i915);
>  out:
> +	intel_bios_driver_remove(i915);

It's also called now incorrectly, needs its own label.

>  	return ret;
>  }
>  
> @@ -308,13 +311,13 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915)
>  /* part #2: call after irq uninstall */
>  static void i915_driver_modeset_remove_noirq(struct drm_i915_private *i915)
>  {
> -	intel_modeset_driver_remove_noirq(i915);
> +	intel_csr_ucode_fini(i915);
>  
> -	intel_bios_driver_remove(i915);
> +	intel_power_domains_driver_remove(i915);
>  
>  	intel_vga_unregister(i915);
>  
> -	intel_csr_ucode_fini(i915);
> +	intel_bios_driver_remove(i915);
>  }
>  
>  static void intel_init_dpio(struct drm_i915_private *dev_priv)
> @@ -994,7 +997,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  out_cleanup_irq:
>  	intel_irq_uninstall(i915);
>  out_cleanup_modeset:
> -	/* FIXME */
> +	i915_driver_modeset_remove_noirq(i915);
>  out_cleanup_hw:
>  	i915_driver_hw_remove(i915);
>  	intel_memory_regions_driver_release(i915);
> @@ -1031,13 +1034,13 @@ void i915_driver_remove(struct drm_i915_private *i915)
>  
>  	intel_irq_uninstall(i915);
>  
> +	intel_modeset_driver_remove_noirq(i915);
> +
>  	i915_driver_modeset_remove_noirq(i915);
>  
>  	i915_reset_error_state(i915);
>  	i915_gem_driver_remove(i915);
>  
> -	intel_power_domains_driver_remove(i915);
> -

Reordering the above wrt. i915_gem_driver_remove() makes it not symmetric
with i915_gem_init(), which happens after intel_power_domains_init_hw().
GEM may use now some power domain after we deinited power domains.

>  	i915_driver_hw_remove(i915);
>  
>  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> -- 
> 2.26.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences
  2020-04-14 17:41     ` Souza, Jose
@ 2020-04-14 19:02       ` Imre Deak
  0 siblings, 0 replies; 21+ messages in thread
From: Imre Deak @ 2020-04-14 19:02 UTC (permalink / raw)
  To: Souza, Jose; +Cc: Chiou, Cooper, intel-gfx, kai.heng.feng

On Tue, Apr 14, 2020 at 08:41:50PM +0300, Souza, Jose wrote:
> On Tue, 2020-04-14 at 20:39 +0300, Imre Deak wrote:
> > On Mon, Apr 13, 2020 at 09:45:12AM -0700, José Roberto de Souza
> > wrote:
> > > TC ports can enter in TCCOLD to save power and is required to
> > > request
> > > to PCODE to exit this state before use or read to TC registers.
> > > 
> > > For TGL there is a new MBOX command to do that with a parameter to
> > > ask
> > > PCODE to exit and block TCCOLD entry or unblock TCCOLD entry.
> > > 
> > > So adding a new power domain to reuse the refcount and only allow
> > > TC cold when all TC ports are not in use.
> > > 
> > > v2:
> > > - fixed missing case in intel_display_power_domain_str()
> > > - moved tgl_tc_cold_request to intel_display_power.c
> > > - renamed TGL_TC_COLD_OFF to TGL_TC_COLD_OFF_POWER_DOMAINS
> > > - added all TC and TBT aux power domains to
> > > TGL_TC_COLD_OFF_POWER_DOMAINS
> > > 
> > > v3:
> > > - added one msec sleep when PCODE returns -EAGAIN
> > > - added timeout of 5msec to not loop forever if
> > > sandybridge_pcode_write_timeout() keeps returning -EAGAIN
> > > 
> > > v4:
> > > - Made failure to block or unblock TC cold a error
> > > - removed 5msec timeout, intead giving PCODE 1msec by up 3 times to
> > > recover from the internal error
> > > 
> > > BSpec: 49294
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_power.c    | 107
> > > ++++++++++++++++++
> > >  .../drm/i915/display/intel_display_power.h    |   1 +
> > >  drivers/gpu/drm/i915/display/intel_tc.c       |  17 ++-
> > >  drivers/gpu/drm/i915/i915_reg.h               |   4 +
> > >  4 files changed, 126 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 50bed2d1dd13..00de926aaccf 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -151,6 +151,8 @@ intel_display_power_domain_str(enum
> > > intel_display_power_domain domain)
> > >  		return "GT_IRQ";
> > >  	case POWER_DOMAIN_DPLL_DC_OFF:
> > >  		return "DPLL_DC_OFF";
> > > +	case POWER_DOMAIN_TC_COLD_OFF:
> > > +		return "TC_COLD_OFF";
> > >  	default:
> > >  		MISSING_CASE(domain);
> > >  		return "?";
> > > @@ -2861,6 +2863,21 @@ void intel_display_power_put(struct
> > > drm_i915_private *dev_priv,
> > >  #define TGL_AUX_I_TBT6_IO_POWER_DOMAINS (	\
> > >  	BIT_ULL(POWER_DOMAIN_AUX_I_TBT))
> > >  
> > > +#define TGL_TC_COLD_OFF_POWER_DOMAINS (		\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_D)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_E)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_F)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_G)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_H)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_I)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_D_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_E_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_F_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_G_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_H_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_AUX_I_TBT)	|	\
> > > +	BIT_ULL(POWER_DOMAIN_TC_COLD_OFF))
> > > +
> > >  static const struct i915_power_well_ops
> > > i9xx_always_on_power_well_ops = {
> > >  	.sync_hw = i9xx_power_well_sync_hw_noop,
> > >  	.enable = i9xx_always_on_power_well_noop,
> > > @@ -3963,6 +3980,90 @@ static const struct i915_power_well_desc
> > > ehl_power_wells[] = {
> > >  	},
> > >  };
> > >  
> > > +static void
> > > +tgl_tc_cold_request(struct drm_i915_private *i915, bool block)
> > > +{
> > > +	u8 tries = 0;
> > > +	int ret;
> > > +
> > > +	while (1) {
> > > +		u32 low_val = 0, high_val;
> > > +
> > > +		if (block)
> > > +			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ;
> > > +		else
> > > +			high_val = TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ;
> > > +
> > > +		/*
> > > +		 * Spec states that we should timeout the request after
> > > 200us
> > > +		 * but the function below will timeout after 500us
> > > +		 */
> > > +		ret = sandybridge_pcode_read(i915, TGL_PCODE_TCCOLD,
> > > &low_val,
> > > +					     &high_val);
> > > +		if (ret == 0) {
> > > +			if (block &&
> > > +			    (low_val &
> > > TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> > > +				ret = -EIO;
> > > +			else
> > > +				break;
> > > +		}
> > > +
> > > +		if (++tries == 3)
> > > +			break;
> > > +
> > > +		msleep(1);
> > 
> > Why did this go back to msleep unconditionally? Doing that only for
> > ret == -EAGAIN made sense to me, since that is only the case where
> > you'd
> > want to avoid busy looping.
> 
> Mostly thinking about give PCODE some time to recover it selft when it
> returns low_val == TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED

The spec says to retry the request in that case immediately. Please put
some comment at least here what the msleep() is for.

> > > +
> > > +	if (ret)
> > > +		drm_err(&i915->drm, "TC cold %sblock failed\n",
> > > +			block ? "" : "un");

Could've printed the error code too.

Reviewed-by: Imre Deak <imre.deak@intel.com>

> > > +	else
> > > +		drm_dbg_kms(&i915->drm, "TC cold %sblock succeeded\n",
> > > +			    block ? "" : "un");
> > > +}
> > > +
> > > +static void
> > > +tgl_tc_cold_off_power_well_enable(struct drm_i915_private *i915,
> > > +				  struct i915_power_well *power_well)
> > > +{
> > > +	tgl_tc_cold_request(i915, true);
> > > +}
> > > +
> > > +static void
> > > +tgl_tc_cold_off_power_well_disable(struct drm_i915_private *i915,
> > > +				   struct i915_power_well *power_well)
> > > +{
> > > +	tgl_tc_cold_request(i915, false);
> > > +}
> > > +
> > > +static void
> > > +tgl_tc_cold_off_power_well_sync_hw(struct drm_i915_private *i915,
> > > +				   struct i915_power_well *power_well)
> > > +{
> > > +	if (power_well->count > 0)
> > > +		tgl_tc_cold_off_power_well_enable(i915, power_well);
> > > +	else
> > > +		tgl_tc_cold_off_power_well_disable(i915, power_well);
> > > +}
> > > +
> > > +static bool
> > > +tgl_tc_cold_off_power_well_is_enabled(struct drm_i915_private
> > > *dev_priv,
> > > +				      struct i915_power_well
> > > *power_well)
> > > +{
> > > +	/*
> > > +	 * Not the correctly implementation but there is no way to just
> > > read it
> > > +	 * from PCODE, so returning count to avoid state mismatch
> > > errors
> > > +	 */
> > > +	return power_well->count;
> > > +}
> > > +
> > > +static const struct i915_power_well_ops tgl_tc_cold_off_ops = {
> > > +	.sync_hw = tgl_tc_cold_off_power_well_sync_hw,
> > > +	.enable = tgl_tc_cold_off_power_well_enable,
> > > +	.disable = tgl_tc_cold_off_power_well_disable,
> > > +	.is_enabled = tgl_tc_cold_off_power_well_is_enabled,
> > > +};
> > > +
> > >  static const struct i915_power_well_desc tgl_power_wells[] = {
> > >  	{
> > >  		.name = "always-on",
> > > @@ -4290,6 +4391,12 @@ static const struct i915_power_well_desc
> > > tgl_power_wells[] = {
> > >  			.hsw.irq_pipe_mask = BIT(PIPE_D),
> > >  		},
> > >  	},
> > > +	{
> > > +		.name = "TC cold off",
> > > +		.domains = TGL_TC_COLD_OFF_POWER_DOMAINS,
> > > +		.ops = &tgl_tc_cold_off_ops,
> > > +		.id = DISP_PW_ID_NONE,
> > > +	},
> > >  };
> > >  
> > >  static int
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index da64a5edae7a..070457e7b948 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > @@ -76,6 +76,7 @@ enum intel_display_power_domain {
> > >  	POWER_DOMAIN_MODESET,
> > >  	POWER_DOMAIN_GT_IRQ,
> > >  	POWER_DOMAIN_DPLL_DC_OFF,
> > > +	POWER_DOMAIN_TC_COLD_OFF,
> > >  	POWER_DOMAIN_INIT,
> > >  
> > >  	POWER_DOMAIN_NUM,
> > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > > b/drivers/gpu/drm/i915/display/intel_tc.c
> > > index 0cf33d4d21c3..521a94c63640 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > > @@ -53,16 +53,27 @@ tc_port_load_fia_params(struct drm_i915_private
> > > *i915,
> > >  	}
> > >  }
> > >  
> > > +static enum intel_display_power_domain
> > > +tc_cold_get_power_domain(struct intel_digital_port *dig_port)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(dig_port-
> > > >base.base.dev);
> > > +
> > > +	if (INTEL_GEN(i915) == 11)
> > > +		return intel_legacy_aux_to_power_domain(dig_port-
> > > >aux_ch);
> > > +	else
> > > +		return POWER_DOMAIN_TC_COLD_OFF;
> > > +}
> > > +
> > >  static intel_wakeref_t
> > >  tc_cold_block(struct intel_digital_port *dig_port)
> > >  {
> > >  	struct drm_i915_private *i915 = to_i915(dig_port-
> > > >base.base.dev);
> > >  	enum intel_display_power_domain domain;
> > >  
> > > -	if (INTEL_GEN(i915) != 11 || !dig_port->tc_legacy_port)
> > > +	if (INTEL_GEN(i915) == 11 && !dig_port->tc_legacy_port)
> > >  		return 0;
> > >  
> > > -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > > +	domain = tc_cold_get_power_domain(dig_port);
> > >  	return intel_display_power_get(i915, domain);
> > >  }
> > >  
> > > @@ -80,7 +91,7 @@ tc_cold_unblock(struct intel_digital_port
> > > *dig_port, intel_wakeref_t wakeref)
> > >  	if (wakeref == 0)
> > >  		return;
> > >  
> > > -	domain = intel_legacy_aux_to_power_domain(dig_port->aux_ch);
> > > +	domain = tc_cold_get_power_domain(dig_port);
> > >  	intel_display_power_put_async(i915, domain, wakeref);
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index e4667add70b0..39f281fe6d6c 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9111,6 +9111,10 @@ enum {
> > >  #define   ICL_PCODE_EXIT_TCCOLD			0x12
> > >  #define   HSW_PCODE_DE_WRITE_FREQ_REQ		0x17
> > >  #define   DISPLAY_IPS_CONTROL			0x19
> > > +#define   TGL_PCODE_TCCOLD			0x26
> > > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED	REG_BIT
> > > (0)
> > > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_BLOCK_REQ	0
> > > +#define     TGL_PCODE_EXIT_TCCOLD_DATA_H_UNBLOCK_REQ	REG_BIT
> > > (0)
> > >              /* See also IPS_CTL */
> > >  #define     IPS_PCODE_CONTROL			(1 << 30)
> > >  #define   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL	0x1A
> > > -- 
> > > 2.26.0
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (7 preceding siblings ...)
  2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure José Roberto de Souza
@ 2020-04-14 20:01 ` Patchwork
  2020-04-14 20:21 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-14 20:01 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
URL   : https://patchwork.freedesktop.org/series/75886/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
07f7e1eaedcd drm/i915/display: Move out code to return the digital_port of the aux ch
0c650cdb3e24 drm/i915/display: Add intel_legacy_aux_to_power_domain()
13beb443d72c drm/i915/display: Split hsw_power_well_enable() into two
53d8424945c4 drm/i915/tc/icl: Implement TC cold sequences
-:76: WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst
#76: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:588:
+		msleep(1);

-:81: WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst
#81: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:593:
+		msleep(1);

total: 0 errors, 2 warnings, 0 checks, 157 lines checked
4d5beb240164 drm/i915/tc: Skip ref held check for TC legacy aux power wells
87ba4c5c772a drm/i915/tc/tgl: Implement TC cold sequences
-:112: WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst
#112: FILE: drivers/gpu/drm/i915/display/intel_display_power.c:4014:
+		msleep(1);

total: 0 errors, 1 warnings, 0 checks, 185 lines checked
1de4d7a9096c drm/i915/tc: Catch TC users accessing FIA registers without enable aux
073806ccfac9 drm/i915/tc: Do not warn when aux power well of static TC ports timeout
167a931dadfe drm/i915: Add missing deinitialization cases of load failure
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#17: 
[drm:__intel_engine_init_ctx_wa [i915]] Initialized 3 context workarounds on rcs'0

total: 0 errors, 1 warnings, 0 checks, 50 lines checked

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

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

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (8 preceding siblings ...)
  2020-04-14 20:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch Patchwork
@ 2020-04-14 20:21 ` Patchwork
  2020-04-14 20:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-04-15 12:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-14 20:21 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
URL   : https://patchwork.freedesktop.org/series/75886/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
/home/cidrm/kernel/Documentation/gpu/i915.rst:610: WARNING: duplicate label gpu/i915:layout, other instance in /home/cidrm/kernel/Documentation/gpu/i915.rst

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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (9 preceding siblings ...)
  2020-04-14 20:21 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
@ 2020-04-14 20:25 ` Patchwork
  2020-04-15 12:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-14 20:25 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
URL   : https://patchwork.freedesktop.org/series/75886/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8298 -> Patchwork_17288
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@ring_submission:
    - fi-bwr-2160:        [PASS][1] -> [INCOMPLETE][2] ([i915#489])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/fi-bwr-2160/igt@i915_selftest@live@ring_submission.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/fi-bwr-2160/igt@i915_selftest@live@ring_submission.html

  
  [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489


Participating hosts (48 -> 43)
------------------------------

  Missing    (5): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7560u fi-byt-clapper 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8298 -> Patchwork_17288

  CI-20190529: 20190529
  CI_DRM_8298: 17f82f0c2857d0b442adbdb62eb44b61d0f5b775 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5589: 31962324ac86f029e2841e56e97c42cf9d572956 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17288: 167a931dadfee92ceaf749345422501df3c8c4e3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

167a931dadfe drm/i915: Add missing deinitialization cases of load failure
073806ccfac9 drm/i915/tc: Do not warn when aux power well of static TC ports timeout
1de4d7a9096c drm/i915/tc: Catch TC users accessing FIA registers without enable aux
87ba4c5c772a drm/i915/tc/tgl: Implement TC cold sequences
4d5beb240164 drm/i915/tc: Skip ref held check for TC legacy aux power wells
53d8424945c4 drm/i915/tc/icl: Implement TC cold sequences
13beb443d72c drm/i915/display: Split hsw_power_well_enable() into two
0c650cdb3e24 drm/i915/display: Add intel_legacy_aux_to_power_domain()
07f7e1eaedcd drm/i915/display: Move out code to return the digital_port of the aux ch

== Logs ==

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

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

* Re: [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure
  2020-04-14 18:29   ` Imre Deak
@ 2020-04-14 20:50     ` Souza, Jose
  0 siblings, 0 replies; 21+ messages in thread
From: Souza, Jose @ 2020-04-14 20:50 UTC (permalink / raw)
  To: Deak, Imre; +Cc: Nikula, Jani, intel-gfx, chris

On Tue, 2020-04-14 at 21:29 +0300, Imre Deak wrote:
> On Mon, Apr 13, 2020 at 09:45:15AM -0700, José Roberto de Souza
> wrote:
> > The intel_display_power_put_async() used in TC cold sequences made
> > easy to hit the missing deinitialization of driver in case of load
> > failure as seen in the stack trace bellow.
> > 
> > intel_modeset_driver_remove_noirq() had to be removed from
> > i915_driver_modeset_remove_noirq() as those are different
> > initialialition steps with IRQ in between then.
> 
> Looks ok with some comments below, but this fix should be applied
> before
> the rest of the changes in the patchset, could you resend it
> separately?

Sure

> 
> Is it needed in -fixes too?
> 
> > [drm:__intel_engine_init_ctx_wa [i915]] Initialized 3 context
> > workarounds on rcs'0
> > [drm:__i915_inject_probe_error [i915]] Injecting failure -19 at
> > checkpoint 36 [__uc_init:294]
> > [drm:i915_hdcp_component_unbind [i915]] I915 HDCP comp unbind
> > [drm:edp_panel_vdd_off_sync [i915]] Turning [ENCODER:275:DDI A] VDD
> > off
> > [drm:edp_panel_vdd_off_sync [i915]] PP_STATUS: 0x00000000
> > PP_CONTROL: 0x00000060
> > [drm:intel_power_well_disable [i915]] disabling AUX A
> > general protection fault, probably for non-canonical address
> > 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
> > CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted:
> > G     U            5.6.0-CI-Patchwork_17226+ #1
> > Hardware name: Intel Corporation Tiger Lake Client
> > Platform/TigerLake U DDR4 SODIMM RVP, BIOS
> > TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
> > Workqueue: events_unbound intel_display_power_put_async_work [i915]
> > RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
> > Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41
> > a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03
> > <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
> > RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
> > RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
> > RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
> > RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
> > R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
> > FS:  0000000000000000(0000) GS:ffff88849ff80000(0000)
> > knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00005634fa8ed670 CR3: 0000000005610004 CR4: 0000000000760ee0
> > PKRU: 55555554
> > Call Trace:
> >  release_async_put_domains+0x9b/0x110 [i915]
> >  intel_display_power_put_async_work+0x91/0xf0 [i915]
> >  process_one_work+0x260/0x600
> >  ? worker_thread+0xc9/0x380
> >  worker_thread+0x37/0x380
> >  ? process_one_work+0x600/0x600
> >  kthread+0x119/0x130
> >  ? kthread_park+0x80/0x80
> >  ret_from_fork+0x24/0x50
> > Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp
> > x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul
> > cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel
> > snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm
> > pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]
> > ---[ end trace b402d1b4060f8b97 ]---
> > BUG: sleeping function called from invalid context at
> > kernel/sched/completion.c:99
> > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1142, name:
> > kworker/u16:20
> > INFO: lockdep is turned off.
> > Preemption disabled at:
> > [<0000000000000000>] 0x0
> > CPU: 3 PID: 1142 Comm: kworker/u16:20 Tainted:
> > G     UD           5.6.0-CI-Patchwork_17226+ #1
> > Hardware name: Intel Corporation Tiger Lake Client
> > Platform/TigerLake U DDR4 SODIMM RVP, BIOS
> > TGLSFWI1.R00.2457.A16.1912270059 12/27/2019
> > Workqueue: events_unbound intel_display_power_put_async_work [i915]
> > Call Trace:
> >  dump_stack+0x71/0x9b
> >  ___might_sleep+0x178/0x260
> >  wait_for_completion+0x37/0x1a0
> >  virt_efi_query_variable_info+0x161/0x1b0
> >  efi_query_variable_store+0xb3/0x1a0
> >  ? efivar_entry_set_safe+0x19c/0x220
> >  efivar_entry_set_safe+0x19c/0x220
> >  ? efi_pstore_write+0x10b/0x150
> >  ? efi_pstore_write+0xa0/0x150
> >  efi_pstore_write+0x10b/0x150
> >  pstore_dump+0x123/0x340
> >  kmsg_dump+0x87/0x1b0
> >  oops_end+0x3e/0x90
> >  do_general_protection+0x1c3/0x2f0
> >  general_protection+0x2d/0x40
> > RIP: 0010:__intel_display_power_put_domain+0xa5/0x180 [i915]
> > Code: 48 85 c0 78 54 44 89 e1 41 bd 01 00 00 00 49 c7 c4 80 44 41
> > a0 49 d3 e5 eb 0d 48 83 eb 10 48 3b 9d 08 ad 00 00 78 32 48 8b 03
> > <4c> 85 68 10 74 ea 8b 53 08 85 d2 74 2d 83 ea 01 85 d2 89 53 08 75
> > RSP: 0018:ffffc9000061fdb0 EFLAGS: 00010206
> > RAX: 6b6b6b6b6b6b6b6b RBX: ffff8884948f5df0 RCX: 000000000000003d
> > RDX: 0000000080000001 RSI: 0000000000000000 RDI: 0000000000000000
> > RBP: ffff888479be0000 R08: ffff88849a180920 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0414480
> > R13: 2000000000000000 R14: ffff888479beb320 R15: 2000000000000000
> >  release_async_put_domains+0x9b/0x110 [i915]
> >  intel_display_power_put_async_work+0x91/0xf0 [i915]
> >  process_one_work+0x260/0x600
> >  ? worker_thread+0xc9/0x380
> >  worker_thread+0x37/0x380
> >  ? process_one_work+0x600/0x600
> >  kthread+0x119/0x130
> >  ? kthread_park+0x80/0x80
> >  ret_from_fork+0x24/0x50
> > ------------[ cut here ]------------
> > WARNING: CPU: 3 PID: 1142 at kernel/rcu/tree_plugin.h:293
> > rcu_note_context_switch+0x87/0x650
> > Modules linked in: i915(+) vgem snd_hda_codec_hdmi mei_hdcp
> > x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul
> > cdc_ether usbnet mii snd_intel_dspcfg ghash_clmulni_intel
> > snd_hda_codec snd_hwdep snd_hda_core e1000e ptp mei_me snd_pcm
> > pps_core mei intel_lpss_pci prime_numbers [last unloaded: i915]
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index a7a3b4b98572..9727689e264b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -248,8 +248,11 @@ static int
> > i915_driver_modeset_probe_noirq(struct drm_i915_private *i915)
> >  	return 0;
> >  
> >  cleanup_vga_client:
> > +	intel_csr_ucode_fini(i915);
> > +	intel_power_domains_driver_remove(i915);
> >  	intel_vga_unregister(i915);
> >  out:
> > +	intel_bios_driver_remove(i915);
> 
> It's also called now incorrectly, needs its own label.

Thanks.
Easier just return instead of goto in case of error in
drm_vblank_init().

> 
> >  	return ret;
> >  }
> >  
> > @@ -308,13 +311,13 @@ static void i915_driver_modeset_remove(struct
> > drm_i915_private *i915)
> >  /* part #2: call after irq uninstall */
> >  static void i915_driver_modeset_remove_noirq(struct
> > drm_i915_private *i915)
> >  {
> > -	intel_modeset_driver_remove_noirq(i915);
> > +	intel_csr_ucode_fini(i915);
> >  
> > -	intel_bios_driver_remove(i915);
> > +	intel_power_domains_driver_remove(i915);
> >  
> >  	intel_vga_unregister(i915);
> >  
> > -	intel_csr_ucode_fini(i915);
> > +	intel_bios_driver_remove(i915);
> >  }
> >  
> >  static void intel_init_dpio(struct drm_i915_private *dev_priv)
> > @@ -994,7 +997,7 @@ int i915_driver_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> >  out_cleanup_irq:
> >  	intel_irq_uninstall(i915);
> >  out_cleanup_modeset:
> > -	/* FIXME */
> > +	i915_driver_modeset_remove_noirq(i915);
> >  out_cleanup_hw:
> >  	i915_driver_hw_remove(i915);
> >  	intel_memory_regions_driver_release(i915);
> > @@ -1031,13 +1034,13 @@ void i915_driver_remove(struct
> > drm_i915_private *i915)
> >  
> >  	intel_irq_uninstall(i915);
> >  
> > +	intel_modeset_driver_remove_noirq(i915);
> > +
> >  	i915_driver_modeset_remove_noirq(i915);
> >  
> >  	i915_reset_error_state(i915);
> >  	i915_gem_driver_remove(i915);
> >  
> > -	intel_power_domains_driver_remove(i915);
> > -
> 
> Reordering the above wrt. i915_gem_driver_remove() makes it not
> symmetric
> with i915_gem_init(), which happens after
> intel_power_domains_init_hw().
> GEM may use now some power domain after we deinited power domains.

Hum so moving i915_gem_driver_remove() before
i915_driver_modeset_remove_noirq() will fix it and will match the error
handling in i915_driver_modeset_probe()

void i915_driver_remove(struct drm_i915_private *i915)
{
	disable_rpm_wakeref_asserts(&i915->runtime_pm);

	i915_driver_unregister(i915);

	/* Flush any external code that still may be under the RCU lock
*/
	synchronize_rcu();

	i915_gem_suspend(i915);

	drm_atomic_helper_shutdown(&i915->drm);

	intel_gvt_driver_remove(i915);

	i915_driver_modeset_remove(i915);

	intel_irq_uninstall(i915);

	intel_modeset_driver_remove_noirq(i915);

	i915_gem_driver_remove(i915);

	i915_driver_modeset_remove_noirq(i915);

	i915_reset_error_state(i915);

	i915_driver_hw_remove(i915);

	enable_rpm_wakeref_asserts(&i915->runtime_pm);
}

Will run some tests before send it separated.

Thanks for the comments.

> 
> >  	i915_driver_hw_remove(i915);
> >  
> >  	enable_rpm_wakeref_asserts(&i915->runtime_pm);
> > -- 
> > 2.26.0
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
  2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
                   ` (10 preceding siblings ...)
  2020-04-14 20:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-04-15 12:05 ` Patchwork
  11 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-04-15 12:05 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch
URL   : https://patchwork.freedesktop.org/series/75886/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8298_full -> Patchwork_17288_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2] ([i915#69])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl8/igt@i915_pm_backlight@fade_with_suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl2/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-snb:          [PASS][3] -> [FAIL][4] ([i915#1066])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-snb2/igt@i915_pm_rc6_residency@rc6-idle.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-snb4/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@i915_suspend@forcewake:
    - shard-kbl:          [PASS][5] -> [DMESG-WARN][6] ([i915#180]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl2/igt@i915_suspend@forcewake.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl2/igt@i915_suspend@forcewake.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen:
    - shard-apl:          [PASS][7] -> [FAIL][8] ([i915#54] / [i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-apl1/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-apl1/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([i915#34])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl10/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl9/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][11] -> [DMESG-WARN][12] ([i915#180] / [i915#93] / [i915#95])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108145] / [i915#265])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb6/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_setmode@basic:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([i915#31])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl7/igt@kms_setmode@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl10/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_exec_params@invalid-bsd-ring:
    - shard-iclb:         [SKIP][19] ([fdo#109276]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb7/igt@gem_exec_params@invalid-bsd-ring.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb1/igt@gem_exec_params@invalid-bsd-ring.html

  * {igt@gem_wait@write-wait@all}:
    - shard-skl:          [FAIL][21] ([i915#1676]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl2/igt@gem_wait@write-wait@all.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl4/igt@gem_wait@write-wait@all.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [DMESG-WARN][23] ([i915#716]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-apl3/igt@gen9_exec_parse@allowed-all.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-apl3/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_module_load@reload-with-fault-injection:
    - shard-kbl:          [INCOMPLETE][25] ([i915#1423]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl7/igt@i915_module_load@reload-with-fault-injection.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl4/igt@i915_module_load@reload-with-fault-injection.html
    - shard-apl:          [INCOMPLETE][27] ([i915#1423]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-apl3/igt@i915_module_load@reload-with-fault-injection.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-apl4/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rpm@dpms-lpsp:
    - shard-iclb:         [SKIP][29] ([i915#1316] / [i915#579]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb2/igt@i915_pm_rpm@dpms-lpsp.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb5/igt@i915_pm_rpm@dpms-lpsp.html
    - shard-tglb:         [SKIP][31] ([i915#1316] / [i915#579]) -> [PASS][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-tglb5/igt@i915_pm_rpm@dpms-lpsp.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-tglb2/igt@i915_pm_rpm@dpms-lpsp.html

  * igt@i915_pm_rpm@sysfs-read:
    - shard-glk:          [SKIP][33] ([fdo#109271]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-glk2/igt@i915_pm_rpm@sysfs-read.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-glk4/igt@i915_pm_rpm@sysfs-read.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][35] ([i915#79]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][37] ([i915#1188]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl10/igt@kms_hdr@bpc-switch-dpms.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [DMESG-WARN][39] ([i915#180]) -> [PASS][40] +2 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][43] ([i915#31]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl4/igt@kms_setmode@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl6/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm:
    - shard-skl:          [SKIP][45] ([fdo#109271]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl4/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl9/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html
    - shard-tglb:         [SKIP][47] ([fdo#112015]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-tglb5/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-tglb2/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html
    - shard-iclb:         [SKIP][49] ([fdo#109278]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb2/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb5/igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-apl4/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-apl4/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * {igt@sysfs_heartbeat_interval@mixed@vcs0}:
    - shard-skl:          [INCOMPLETE][53] ([i915#198]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-skl7/igt@sysfs_heartbeat_interval@mixed@vcs0.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-skl10/igt@sysfs_heartbeat_interval@mixed@vcs0.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][55] ([i915#658]) -> [SKIP][56] ([i915#588])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb4/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_rpm@debugfs-forcewake-user:
    - shard-snb:          [SKIP][57] ([fdo#109271]) -> [INCOMPLETE][58] ([i915#82])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-snb1/igt@i915_pm_rpm@debugfs-forcewake-user.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-snb5/igt@i915_pm_rpm@debugfs-forcewake-user.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [DMESG-FAIL][59] ([i915#180] / [i915#95]) -> [FAIL][60] ([i915#93] / [i915#95])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-apl:          [FAIL][61] ([fdo#108145] / [i915#265]) -> [FAIL][62] ([fdo#108145] / [i915#265] / [i915#95])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-apl1/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          [FAIL][63] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95]) -> [FAIL][64] ([fdo#108145] / [i915#265])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-kbl2/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-kbl2/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][65] ([fdo#109642] / [fdo#111068]) -> [FAIL][66] ([i915#608])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8298/shard-iclb4/igt@kms_psr2_su@page_flip.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17288/shard-iclb2/igt@kms_psr2_su@page_flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112015]: https://bugs.freedesktop.org/show_bug.cgi?id=112015
  [i915#1066]: https://gitlab.freedesktop.org/drm/intel/issues/1066
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1316]: https://gitlab.freedesktop.org/drm/intel/issues/1316
  [i915#1423]: https://gitlab.freedesktop.org/drm/intel/issues/1423
  [i915#1676]: https://gitlab.freedesktop.org/drm/intel/issues/1676
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8298 -> Patchwork_17288

  CI-20190529: 20190529
  CI_DRM_8298: 17f82f0c2857d0b442adbdb62eb44b61d0f5b775 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5589: 31962324ac86f029e2841e56e97c42cf9d572956 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17288: 167a931dadfee92ceaf749345422501df3c8c4e3 @ 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_17288/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2020-04-15 12:05 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 16:45 [Intel-gfx] [PATCH v4 1/9] drm/i915/display: Move out code to return the digital_port of the aux ch José Roberto de Souza
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 2/9] drm/i915/display: Add intel_legacy_aux_to_power_domain() José Roberto de Souza
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 3/9] drm/i915/display: Split hsw_power_well_enable() into two José Roberto de Souza
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 4/9] drm/i915/tc/icl: Implement TC cold sequences José Roberto de Souza
2020-04-14 17:31   ` Imre Deak
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 5/9] drm/i915/tc: Skip ref held check for TC legacy aux power wells José Roberto de Souza
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 6/9] drm/i915/tc/tgl: Implement TC cold sequences José Roberto de Souza
2020-04-14 17:39   ` Imre Deak
2020-04-14 17:41     ` Souza, Jose
2020-04-14 19:02       ` Imre Deak
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 7/9] drm/i915/tc: Catch TC users accessing FIA registers without enable aux José Roberto de Souza
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 8/9] drm/i915/tc: Do not warn when aux power well of static TC ports timeout José Roberto de Souza
2020-04-14 17:47   ` Imre Deak
2020-04-14 18:05     ` Souza, Jose
2020-04-13 16:45 ` [Intel-gfx] [PATCH v4 9/9] drm/i915: Add missing deinitialization cases of load failure José Roberto de Souza
2020-04-14 18:29   ` Imre Deak
2020-04-14 20:50     ` Souza, Jose
2020-04-14 20:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v4,1/9] drm/i915/display: Move out code to return the digital_port of the aux ch Patchwork
2020-04-14 20:21 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-04-14 20:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-04-15 12:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.