[Intel-gfx] [PATCH 0/7] drm/i915/tgl+: Enable DC power states on all eDP ports

[Intel-gfx] [PATCH 0/7] drm/i915/tgl+: Enable DC power states on all eDP ports

[Imre Deak]

The patchset makes sure that the display DC power states are enabled on
all eDP ports; atm these can stay blocked on the TGL+ ports B+. It
also has a few minor related display power domain fixes.

Imre Deak (7):
  drm/i915: Allocate power domain set wakerefs dynamically
  drm/i915: Move the POWER_DOMAIN_AUX_IO_A definition to its logical
    place
  drm/i915/tgl+: Enable display DC power states on all eDP ports
  drm/i915: Add missing AUX_IO_A power domain->well mappings
  drm/i915: Add missing DC_OFF power domain->well mappings
  drm/i915: Factor out function to get/put AUX_IO power for main link
  drm/i915/mtl+: Don't enable the AUX_IO power for non-eDP port main
    links

 drivers/gpu/drm/i915/display/intel_ddi.c      | 91 +++++++++++-------
 .../drm/i915/display/intel_display_power.c    | 95 ++++++++++++++++---
 .../drm/i915/display/intel_display_power.h    | 14 ++-
 .../i915/display/intel_display_power_map.c    | 69 ++++++++++++--
 4 files changed, 213 insertions(+), 56 deletions(-)

-- 
2.37.1

[Intel-gfx] [PATCH 1/7] drm/i915: Allocate power domain set wakerefs dynamically

[Imre Deak]

Since the intel_display_power_domain_set struct, currently its current
size close 1kB, can be allocated on the stack, it's better to allocate
the per-domain wakeref pointer array - used for debugging - within the
struct dynamically, so do this.

The memory freeing is guaranteed by the fact that the acquired domain
references tracked by struct can't be leaked either.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 61 ++++++++++++++++---
 .../drm/i915/display/intel_display_power.h    |  2 +-
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 4c1de91e56ff9..e2da91c2a9638 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -830,19 +830,58 @@ void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+static intel_wakeref_t *
+get_debug_wakerefs(struct drm_i915_private *i915,
+		   struct intel_display_power_domain_set *power_domain_set)
+{
+	if (power_domain_set->wakerefs)
+		return power_domain_set->wakerefs;
+
+	power_domain_set->wakerefs = kcalloc(POWER_DOMAIN_NUM,
+					     sizeof(*power_domain_set->wakerefs),
+					     GFP_KERNEL);
+
+	drm_WARN_ON_ONCE(&i915->drm, !power_domain_set->wakerefs);
+
+	return power_domain_set->wakerefs;
+}
+
+static void
+free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
+{
+	if (power_domain_set->wakerefs &&
+	    bitmap_empty(power_domain_set->mask.bits, POWER_DOMAIN_NUM))
+		kfree(fetch_and_zero(&power_domain_set->wakerefs));
+}
+#else
+static intel_wakeref_t *
+get_debug_wakerefs(struct drm_i915_private *i915,
+		   struct intel_display_power_domain_set *power_domain_set)
+{
+	return NULL;
+}
+
+static void
+free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
+{
+}
+#endif
+
 void
 intel_display_power_get_in_set(struct drm_i915_private *i915,
 			       struct intel_display_power_domain_set *power_domain_set,
 			       enum intel_display_power_domain domain)
 {
 	intel_wakeref_t __maybe_unused wf;
+	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
 
 	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
 
 	wf = intel_display_power_get(i915, domain);
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-	power_domain_set->wakerefs[domain] = wf;
-#endif
+	if (debug_wakerefs)
+		debug_wakerefs[domain] = wf;
+
 	set_bit(domain, power_domain_set->mask.bits);
 }
 
@@ -852,6 +891,7 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
 					  enum intel_display_power_domain domain)
 {
 	intel_wakeref_t wf;
+	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
 
 	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
 
@@ -859,9 +899,9 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
 	if (!wf)
 		return false;
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-	power_domain_set->wakerefs[domain] = wf;
-#endif
+	if (debug_wakerefs)
+		debug_wakerefs[domain] = wf;
+
 	set_bit(domain, power_domain_set->mask.bits);
 
 	return true;
@@ -873,6 +913,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
 				    struct intel_power_domain_mask *mask)
 {
 	enum intel_display_power_domain domain;
+	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
 
 	drm_WARN_ON(&i915->drm,
 		    !bitmap_subset(mask->bits, power_domain_set->mask.bits, POWER_DOMAIN_NUM));
@@ -880,12 +921,14 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
 	for_each_power_domain(domain, mask) {
 		intel_wakeref_t __maybe_unused wf = -1;
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
-		wf = fetch_and_zero(&power_domain_set->wakerefs[domain]);
-#endif
+		if (debug_wakerefs)
+			wf = fetch_and_zero(&debug_wakerefs[domain]);
+
 		intel_display_power_put(i915, domain, wf);
 		clear_bit(domain, power_domain_set->mask.bits);
 	}
+
+	free_empty_debug_wakerefs(power_domain_set);
 }
 
 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 7136ea3f233e9..c847aab7b2f88 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -146,7 +146,7 @@ struct i915_power_domains {
 struct intel_display_power_domain_set {
 	struct intel_power_domain_mask mask;
 #ifdef CONFIG_DRM_I915_DEBUG_RUNTIME_PM
-	intel_wakeref_t wakerefs[POWER_DOMAIN_NUM];
+	intel_wakeref_t *wakerefs;
 #endif
 };
 
-- 
2.37.1

[Intel-gfx] [PATCH 2/7] drm/i915: Move the POWER_DOMAIN_AUX_IO_A definition to its logical place

[Imre Deak]

Move the definition of the AUX_IO_A power domain, requiring only the
corresponding AUX_IO_A power well to be enabled, before all the
AUX_<port> power domains, which require both the AUX_IO_<port> and the
DC_OFF power wells to be enabled.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power.c     | 4 ++--
 drivers/gpu/drm/i915/display/intel_display_power.h     | 5 +++--
 drivers/gpu/drm/i915/display/intel_display_power_map.c | 6 +++---
 3 files changed, 8 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 e2da91c2a9638..da0047359446e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -129,6 +129,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "AUDIO_MMIO";
 	case POWER_DOMAIN_AUDIO_PLAYBACK:
 		return "AUDIO_PLAYBACK";
+	case POWER_DOMAIN_AUX_IO_A:
+		return "AUX_IO_A";
 	case POWER_DOMAIN_AUX_A:
 		return "AUX_A";
 	case POWER_DOMAIN_AUX_B:
@@ -153,8 +155,6 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "AUX_USBC5";
 	case POWER_DOMAIN_AUX_USBC6:
 		return "AUX_USBC6";
-	case POWER_DOMAIN_AUX_IO_A:
-		return "AUX_IO_A";
 	case POWER_DOMAIN_AUX_TBT1:
 		return "AUX_TBT1";
 	case POWER_DOMAIN_AUX_TBT2:
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index c847aab7b2f88..fd68d43bfe322 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -76,6 +76,9 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_VGA,
 	POWER_DOMAIN_AUDIO_MMIO,
 	POWER_DOMAIN_AUDIO_PLAYBACK,
+
+	POWER_DOMAIN_AUX_IO_A,
+
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
@@ -90,8 +93,6 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_AUX_USBC5,
 	POWER_DOMAIN_AUX_USBC6,
 
-	POWER_DOMAIN_AUX_IO_A,
-
 	POWER_DOMAIN_AUX_TBT1,
 	POWER_DOMAIN_AUX_TBT2,
 	POWER_DOMAIN_AUX_TBT3,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index dc04afc6cc8ff..43454022e6a66 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -518,8 +518,8 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
-	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_IO_A,
+	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
@@ -658,8 +658,8 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_e,	POWER_DOMAIN_PORT_DDI_IO_E);
 I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
 
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
-	POWER_DOMAIN_AUX_A,
-	POWER_DOMAIN_AUX_IO_A);
+	POWER_DOMAIN_AUX_IO_A,
+	POWER_DOMAIN_AUX_A);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
-- 
2.37.1

[Intel-gfx] [PATCH 3/7] drm/i915/tgl+: Enable display DC power states on all eDP ports

[Imre Deak]

Starting with TGL eDP is supported on ports B+ (besides port A), so make
sure DC states are not blocked on any such ports. For this add an
AUX_IO_<port> power domain for each port with eDP support. These domains
similarly to AUX_IO_A enable only the AUX_IO_<port> power well for an
enabled port, whereas the existing AUX_<port> domains enable both the
AUX_IO_<port> and the DC_OFF power wells as required by DP AUX transfers.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  8 ++-
 .../drm/i915/display/intel_display_power.c    | 30 +++++++++++
 .../drm/i915/display/intel_display_power.h    |  7 +++
 .../i915/display/intel_display_power_map.c    | 53 +++++++++++++++++--
 4 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index e95bde5cf060e..00b577a5b9a76 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -848,6 +848,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 static enum intel_display_power_domain
 intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 {
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
 	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
 	 * DC states enabled at the same time, while for driver initiated AUX
 	 * transfers we need the same AUX IOs to be powered but with DC states
@@ -860,8 +862,10 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 	 * Note that PSR is enabled only on Port A even though this function
 	 * returns the correct domain for other ports too.
 	 */
-	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
-					      intel_aux_power_domain(dig_port);
+	if (intel_dp_is_edp(&dig_port->dp))
+		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
+	else
+		return intel_aux_power_domain(dig_port);
 }
 
 static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index da0047359446e..ca453518e7fd7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -131,6 +131,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "AUDIO_PLAYBACK";
 	case POWER_DOMAIN_AUX_IO_A:
 		return "AUX_IO_A";
+	case POWER_DOMAIN_AUX_IO_B:
+		return "AUX_IO_B";
+	case POWER_DOMAIN_AUX_IO_C:
+		return "AUX_IO_C";
+	case POWER_DOMAIN_AUX_IO_D:
+		return "AUX_IO_D";
+	case POWER_DOMAIN_AUX_IO_E:
+		return "AUX_IO_E";
+	case POWER_DOMAIN_AUX_IO_F:
+		return "AUX_IO_F";
 	case POWER_DOMAIN_AUX_A:
 		return "AUX_A";
 	case POWER_DOMAIN_AUX_B:
@@ -2332,6 +2342,7 @@ struct intel_ddi_port_domains {
 
 	enum intel_display_power_domain ddi_lanes;
 	enum intel_display_power_domain ddi_io;
+	enum intel_display_power_domain aux_io;
 	enum intel_display_power_domain aux_legacy_usbc;
 	enum intel_display_power_domain aux_tbt;
 };
@@ -2346,6 +2357,7 @@ i9xx_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
+		.aux_io = POWER_DOMAIN_AUX_IO_A,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
 		.aux_tbt = POWER_DOMAIN_INVALID,
 	},
@@ -2361,6 +2373,7 @@ d11_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
+		.aux_io = POWER_DOMAIN_AUX_IO_A,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
 		.aux_tbt = POWER_DOMAIN_INVALID,
 	}, {
@@ -2371,6 +2384,7 @@ d11_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_C,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_C,
+		.aux_io = POWER_DOMAIN_AUX_IO_C,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_C,
 		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
 	},
@@ -2386,6 +2400,7 @@ d12_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
+		.aux_io = POWER_DOMAIN_AUX_IO_A,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
 		.aux_tbt = POWER_DOMAIN_INVALID,
 	}, {
@@ -2396,6 +2411,7 @@ d12_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
+		.aux_io = POWER_DOMAIN_INVALID,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
 		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
 	},
@@ -2411,6 +2427,7 @@ d13_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
+		.aux_io = POWER_DOMAIN_AUX_IO_A,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
 		.aux_tbt = POWER_DOMAIN_INVALID,
 	}, {
@@ -2421,6 +2438,7 @@ d13_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
+		.aux_io = POWER_DOMAIN_INVALID,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
 		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
 	}, {
@@ -2431,6 +2449,7 @@ d13_port_domains[] = {
 
 		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_D,
 		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_D,
+		.aux_io = POWER_DOMAIN_AUX_IO_D,
 		.aux_legacy_usbc = POWER_DOMAIN_AUX_D,
 		.aux_tbt = POWER_DOMAIN_INVALID,
 	},
@@ -2508,6 +2527,17 @@ intel_port_domains_for_aux_ch(struct drm_i915_private *i915, enum aux_ch aux_ch)
 	return NULL;
 }
 
+enum intel_display_power_domain
+intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
+{
+	const struct intel_ddi_port_domains *domains = intel_port_domains_for_aux_ch(i915, aux_ch);
+
+	if (drm_WARN_ON(&i915->drm, !domains) || domains->aux_io == POWER_DOMAIN_INVALID)
+		return POWER_DOMAIN_AUX_IO_A;
+
+	return domains->aux_io + (int)(aux_ch - domains->aux_ch_start);
+}
+
 enum intel_display_power_domain
 intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index fd68d43bfe322..54737e0aec21e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -78,6 +78,11 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_AUDIO_PLAYBACK,
 
 	POWER_DOMAIN_AUX_IO_A,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
+	POWER_DOMAIN_AUX_IO_D,
+	POWER_DOMAIN_AUX_IO_E,
+	POWER_DOMAIN_AUX_IO_F,
 
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
@@ -250,6 +255,8 @@ intel_display_power_ddi_lanes_domain(struct drm_i915_private *i915, enum port po
 enum intel_display_power_domain
 intel_display_power_ddi_io_domain(struct drm_i915_private *i915, enum port port);
 enum intel_display_power_domain
+intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
+enum intel_display_power_domain
 intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
 enum intel_display_power_domain
 intel_display_power_tbt_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index 43454022e6a66..b82c0d0a80c5f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -170,6 +170,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_display,
 	POWER_DOMAIN_VGA,
 	POWER_DOMAIN_AUDIO_MMIO,
 	POWER_DOMAIN_AUDIO_PLAYBACK,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_GMBUS,
@@ -179,6 +181,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
 	POWER_DOMAIN_PORT_DDI_LANES_B,
 	POWER_DOMAIN_PORT_DDI_LANES_C,
 	POWER_DOMAIN_PORT_CRT,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
@@ -186,6 +190,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
 I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_tx_bc_lanes,
 	POWER_DOMAIN_PORT_DDI_LANES_B,
 	POWER_DOMAIN_PORT_DDI_LANES_C,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
@@ -243,6 +249,9 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
 	POWER_DOMAIN_VGA,
 	POWER_DOMAIN_AUDIO_MMIO,
 	POWER_DOMAIN_AUDIO_PLAYBACK,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
+	POWER_DOMAIN_AUX_IO_D,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_AUX_D,
@@ -252,12 +261,15 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
 I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_bc,
 	POWER_DOMAIN_PORT_DDI_LANES_B,
 	POWER_DOMAIN_PORT_DDI_LANES_C,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_d,
 	POWER_DOMAIN_PORT_DDI_LANES_D,
+	POWER_DOMAIN_AUX_IO_D,
 	POWER_DOMAIN_AUX_D,
 	POWER_DOMAIN_INIT);
 
@@ -305,6 +317,9 @@ static const struct i915_power_well_desc_list chv_power_wells[] = {
 	POWER_DOMAIN_VGA, \
 	POWER_DOMAIN_AUDIO_MMIO, \
 	POWER_DOMAIN_AUDIO_PLAYBACK, \
+	POWER_DOMAIN_AUX_IO_B, \
+	POWER_DOMAIN_AUX_IO_C, \
+	POWER_DOMAIN_AUX_IO_D, \
 	POWER_DOMAIN_AUX_B, \
 	POWER_DOMAIN_AUX_C, \
 	POWER_DOMAIN_AUX_D
@@ -407,6 +422,8 @@ static const struct i915_power_well_desc_list skl_power_wells[] = {
 	POWER_DOMAIN_VGA, \
 	POWER_DOMAIN_AUDIO_MMIO, \
 	POWER_DOMAIN_AUDIO_PLAYBACK, \
+	POWER_DOMAIN_AUX_IO_B, \
+	POWER_DOMAIN_AUX_IO_C, \
 	POWER_DOMAIN_AUX_B, \
 	POWER_DOMAIN_AUX_C
 
@@ -430,6 +447,8 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
 I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_bc,
 	POWER_DOMAIN_PORT_DDI_LANES_B,
 	POWER_DOMAIN_PORT_DDI_LANES_C,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
@@ -483,6 +502,8 @@ static const struct i915_power_well_desc_list bxt_power_wells[] = {
 	POWER_DOMAIN_VGA, \
 	POWER_DOMAIN_AUDIO_MMIO, \
 	POWER_DOMAIN_AUDIO_PLAYBACK, \
+	POWER_DOMAIN_AUX_IO_B, \
+	POWER_DOMAIN_AUX_IO_C, \
 	POWER_DOMAIN_AUX_B, \
 	POWER_DOMAIN_AUX_C
 
@@ -509,11 +530,13 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_b,
 	POWER_DOMAIN_PORT_DDI_LANES_B,
+	POWER_DOMAIN_AUX_IO_B,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
 	POWER_DOMAIN_PORT_DDI_LANES_C,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
 
@@ -523,10 +546,12 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
+	POWER_DOMAIN_AUX_IO_B,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_aux_c,
+	POWER_DOMAIN_AUX_IO_C,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_INIT);
 
@@ -617,6 +642,11 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_pw_4,
 	POWER_DOMAIN_VGA, \
 	POWER_DOMAIN_AUDIO_MMIO, \
 	POWER_DOMAIN_AUDIO_PLAYBACK, \
+	POWER_DOMAIN_AUX_IO_B, \
+	POWER_DOMAIN_AUX_IO_C, \
+	POWER_DOMAIN_AUX_IO_D, \
+	POWER_DOMAIN_AUX_IO_E, \
+	POWER_DOMAIN_AUX_IO_F, \
 	POWER_DOMAIN_AUX_B, \
 	POWER_DOMAIN_AUX_C, \
 	POWER_DOMAIN_AUX_D, \
@@ -660,11 +690,21 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
 	POWER_DOMAIN_AUX_IO_A,
 	POWER_DOMAIN_AUX_A);
-I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
-I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
-I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
-I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,		POWER_DOMAIN_AUX_E);
-I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,		POWER_DOMAIN_AUX_F);
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
+	POWER_DOMAIN_AUX_IO_B,
+	POWER_DOMAIN_AUX_B);
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
+	POWER_DOMAIN_AUX_IO_C,
+	POWER_DOMAIN_AUX_C);
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
+	POWER_DOMAIN_AUX_IO_D,
+	POWER_DOMAIN_AUX_D);
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
+	POWER_DOMAIN_AUX_IO_E,
+	POWER_DOMAIN_AUX_E);
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
+	POWER_DOMAIN_AUX_IO_F,
+	POWER_DOMAIN_AUX_F);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt1,	POWER_DOMAIN_AUX_TBT1);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt2,	POWER_DOMAIN_AUX_TBT2);
 I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt3,	POWER_DOMAIN_AUX_TBT3);
@@ -1215,6 +1255,9 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_a,
 	POWER_DOMAIN_PORT_DDI_LANES_TC4, \
 	POWER_DOMAIN_VGA, \
 	POWER_DOMAIN_AUDIO_PLAYBACK, \
+	POWER_DOMAIN_AUX_IO_C, \
+	POWER_DOMAIN_AUX_IO_D, \
+	POWER_DOMAIN_AUX_IO_E, \
 	POWER_DOMAIN_AUX_C, \
 	POWER_DOMAIN_AUX_D, \
 	POWER_DOMAIN_AUX_E, \
-- 
2.37.1

[Intel-gfx] [PATCH 4/7] drm/i915: Add missing AUX_IO_A power domain->well mappings

[Imre Deak]

BXT and GLK were missing the AUX_IO_A power domain -> PHY A common power
well mapping, add these now. This didn't cause a problem as the
AUX_IO_A and DDI_LANES_A power domains are acquired together.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power_map.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index b82c0d0a80c5f..aa9d1ae9e8a26 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -441,6 +441,7 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dc_off,
 
 I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
 	POWER_DOMAIN_PORT_DDI_LANES_A,
+	POWER_DOMAIN_AUX_IO_A,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_INIT);
 
@@ -525,6 +526,7 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_ddi_io_c,	POWER_DOMAIN_PORT_DDI_IO_C);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
 	POWER_DOMAIN_PORT_DDI_LANES_A,
+	POWER_DOMAIN_AUX_IO_A,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_INIT);
 
-- 
2.37.1

[Intel-gfx] [PATCH 5/7] drm/i915: Add missing DC_OFF power domain->well mappings

[Imre Deak]

Add the missing DC_OFF power domain -> DC_OFF power well mappings on all
platforms. This didn't cause a problem as the DC_OFF power domain is
only used on JSL, where the mapping was already correct.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power_map.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index aa9d1ae9e8a26..f5d66ca85b19b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -333,6 +333,7 @@ I915_DECL_PW_DOMAINS(skl_pwdoms_dc_off,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_GT_IRQ,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(skl_pwdoms_ddi_io_a_e,
@@ -437,6 +438,7 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dc_off,
 	POWER_DOMAIN_GMBUS,
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_GT_IRQ,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
@@ -518,6 +520,7 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dc_off,
 	POWER_DOMAIN_GMBUS,
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_GT_IRQ,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(glk_pwdoms_ddi_io_a,	POWER_DOMAIN_PORT_DDI_IO_A);
@@ -858,6 +861,7 @@ I915_DECL_PW_DOMAINS(tgl_pwdoms_dc_off,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_MODESET,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(tgl_pwdoms_ddi_io_tc1,	POWER_DOMAIN_PORT_DDI_IO_TC1);
@@ -1054,6 +1058,7 @@ I915_DECL_PW_DOMAINS(rkl_pwdoms_dc_off,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_MODESET,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 static const struct i915_power_well_desc rkl_power_wells_main[] = {
@@ -1136,6 +1141,7 @@ I915_DECL_PW_DOMAINS(dg1_pwdoms_dc_off,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_MODESET,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(dg1_pwdoms_pw_2,
@@ -1300,6 +1306,7 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_dc_off,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_MODESET,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 static const struct i915_power_well_desc xelpd_power_wells_main[] = {
@@ -1421,6 +1428,7 @@ I915_DECL_PW_DOMAINS(xelpdp_pwdoms_dc_off,
 	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_AUX_A,
 	POWER_DOMAIN_AUX_B,
+	POWER_DOMAIN_DC_OFF,
 	POWER_DOMAIN_INIT);
 
 I915_DECL_PW_DOMAINS(xelpdp_pwdoms_aux_tc1,
-- 
2.37.1

[Intel-gfx] [PATCH 6/7] drm/i915: Factor out function to get/put AUX_IO power for main link

[Imre Deak]

Factor out functions to get/put the AUX_IO power domain for the main
link on DDI ports.

While at it clarify the corresponding code comment.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 84 ++++++++++++++----------
 1 file changed, 51 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 00b577a5b9a76..7453772d2073d 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -846,26 +846,63 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 }
 
 static enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
+intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port,
+			       const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
 
-	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
+	/*
+	 * ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
 	 * DC states enabled at the same time, while for driver initiated AUX
 	 * transfers we need the same AUX IOs to be powered but with DC states
-	 * disabled. Accordingly use the AUX power domain here which leaves DC
-	 * states enabled.
-	 * However, for non-A AUX ports the corresponding non-EDP transcoders
-	 * would have already enabled power well 2 and DC_OFF. This means we can
-	 * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
-	 * specific AUX_IO reference without powering up any extra wells.
-	 * Note that PSR is enabled only on Port A even though this function
-	 * returns the correct domain for other ports too.
+	 * disabled. Accordingly use the AUX_IO_<port> power domain here which
+	 * leaves DC states enabled.
+	 *
+	 * Before MTL all DP ports and HDMI ports on TypeC PHYs also require
+	 * AUX IO to be enabled, but all these require DC_OFF to be enabled as
+	 * well, so we can acquire a wider AUX_<port> power domain reference
+	 * instead of a specific AUX_IO_<port> reference without powering up any
+	 * extra wells.
 	 */
 	if (intel_dp_is_edp(&dig_port->dp))
 		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
-	else
+	else if (intel_crtc_has_dp_encoder(crtc_state) ||
+		 intel_phy_is_tc(i915, phy))
 		return intel_aux_power_domain(dig_port);
+	else
+		return POWER_DOMAIN_INVALID;
+}
+
+static void
+get_aux_power_for_main_link(struct intel_digital_port *dig_port,
+			    const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	enum intel_display_power_domain domain =
+		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
+
+	drm_WARN_ON(&i915->drm, dig_port->aux_wakeref);
+
+	if (domain == POWER_DOMAIN_INVALID)
+		return;
+
+	dig_port->aux_wakeref = intel_display_power_get(i915, domain);
+}
+
+static void
+put_aux_power_for_main_link(struct intel_digital_port *dig_port,
+			    const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	intel_wakeref_t wf = fetch_and_zero(&dig_port->aux_wakeref);
+	enum intel_display_power_domain domain =
+		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
+
+	if (!wf)
+		return;
+
+	intel_display_power_put(i915, domain, wf);
 }
 
 static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
@@ -873,7 +910,6 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port;
-	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
 
 	/*
 	 * TODO: Add support for MST encoders. Atm, the following should never
@@ -892,17 +928,7 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
 								   dig_port->ddi_io_power_domain);
 	}
 
-	/*
-	 * AUX power is only needed for (e)DP mode, and for HDMI mode on TC
-	 * ports.
-	 */
-	if (intel_crtc_has_dp_encoder(crtc_state) ||
-	    intel_phy_is_tc(dev_priv, phy)) {
-		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
-		dig_port->aux_wakeref =
-			intel_display_power_get(dev_priv,
-						intel_ddi_main_link_aux_domain(dig_port));
-	}
+	get_aux_power_for_main_link(dig_port, crtc_state);
 }
 
 void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
@@ -2741,10 +2767,7 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
 		intel_ddi_post_disable_dp(state, encoder, old_crtc_state,
 					  old_conn_state);
 
-	if (intel_crtc_has_dp_encoder(old_crtc_state) || is_tc_port)
-		intel_display_power_put(dev_priv,
-					intel_ddi_main_link_aux_domain(dig_port),
-					fetch_and_zero(&dig_port->aux_wakeref));
+	put_aux_power_for_main_link(dig_port, old_crtc_state);
 
 	if (is_tc_port)
 		intel_tc_port_put_link(dig_port);
@@ -3065,12 +3088,7 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
 	if (is_tc_port)
 		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
 
-	if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port) {
-		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
-		dig_port->aux_wakeref =
-			intel_display_power_get(dev_priv,
-						intel_ddi_main_link_aux_domain(dig_port));
-	}
+	get_aux_power_for_main_link(dig_port, crtc_state);
 
 	if (is_tc_port && !intel_tc_port_in_tbt_alt_mode(dig_port))
 		/*
-- 
2.37.1

[Intel-gfx] [PATCH 7/7] drm/i915/mtl+: Don't enable the AUX_IO power for non-eDP port main links

[Imre Deak]

MTL+ requires the AUX_IO power for the main link only on eDP, so don't
enable it in other cases.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 7453772d2073d..8be52ff06e999 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -867,8 +867,9 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port,
 	 */
 	if (intel_dp_is_edp(&dig_port->dp))
 		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
-	else if (intel_crtc_has_dp_encoder(crtc_state) ||
-		 intel_phy_is_tc(i915, phy))
+	else if (DISPLAY_VER(i915) < 14 &&
+		 (intel_crtc_has_dp_encoder(crtc_state) ||
+		  intel_phy_is_tc(i915, phy)))
 		return intel_aux_power_domain(dig_port);
 	else
 		return POWER_DOMAIN_INVALID;
-- 
2.37.1

Re: [Intel-gfx] [PATCH 3/7] drm/i915/tgl+: Enable display DC power states on all eDP ports

[Ville Syrjälä]

On Wed, Nov 02, 2022 at 07:15:26PM +0200, Imre Deak wrote:
> Starting with TGL eDP is supported on ports B+ (besides port A), so make
> sure DC states are not blocked on any such ports. For this add an
> AUX_IO_<port> power domain for each port with eDP support. These domains
> similarly to AUX_IO_A enable only the AUX_IO_<port> power well for an
> enabled port, whereas the existing AUX_<port> domains enable both the
> AUX_IO_<port> and the DC_OFF power wells as required by DP AUX transfers.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 ++-
>  .../drm/i915/display/intel_display_power.c    | 30 +++++++++++
>  .../drm/i915/display/intel_display_power.h    |  7 +++
>  .../i915/display/intel_display_power_map.c    | 53 +++++++++++++++++--
>  4 files changed, 91 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index e95bde5cf060e..00b577a5b9a76 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -848,6 +848,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  static enum intel_display_power_domain
>  intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
>  {
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +
>  	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
>  	 * DC states enabled at the same time, while for driver initiated AUX
>  	 * transfers we need the same AUX IOs to be powered but with DC states
> @@ -860,8 +862,10 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
>  	 * Note that PSR is enabled only on Port A even though this function
>  	 * returns the correct domain for other ports too.
>  	 */
> -	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> -					      intel_aux_power_domain(dig_port);
> +	if (intel_dp_is_edp(&dig_port->dp))
> +		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> +	else
> +		return intel_aux_power_domain(dig_port);

This seems to have two distinct changes in it:
1) define more AUX_IO domains
2) don't use AUX_IO for external DP and instead use the full AUX domain

So seems like this needs to be split. Also why do we need 2)?

>  }
>  
>  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index da0047359446e..ca453518e7fd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -131,6 +131,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "AUDIO_PLAYBACK";
>  	case POWER_DOMAIN_AUX_IO_A:
>  		return "AUX_IO_A";
> +	case POWER_DOMAIN_AUX_IO_B:
> +		return "AUX_IO_B";
> +	case POWER_DOMAIN_AUX_IO_C:
> +		return "AUX_IO_C";
> +	case POWER_DOMAIN_AUX_IO_D:
> +		return "AUX_IO_D";
> +	case POWER_DOMAIN_AUX_IO_E:
> +		return "AUX_IO_E";
> +	case POWER_DOMAIN_AUX_IO_F:
> +		return "AUX_IO_F";
>  	case POWER_DOMAIN_AUX_A:
>  		return "AUX_A";
>  	case POWER_DOMAIN_AUX_B:
> @@ -2332,6 +2342,7 @@ struct intel_ddi_port_domains {
>  
>  	enum intel_display_power_domain ddi_lanes;
>  	enum intel_display_power_domain ddi_io;
> +	enum intel_display_power_domain aux_io;
>  	enum intel_display_power_domain aux_legacy_usbc;
>  	enum intel_display_power_domain aux_tbt;
>  };
> @@ -2346,6 +2357,7 @@ i9xx_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> +		.aux_io = POWER_DOMAIN_AUX_IO_A,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
>  		.aux_tbt = POWER_DOMAIN_INVALID,
>  	},
> @@ -2361,6 +2373,7 @@ d11_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> +		.aux_io = POWER_DOMAIN_AUX_IO_A,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
>  		.aux_tbt = POWER_DOMAIN_INVALID,
>  	}, {
> @@ -2371,6 +2384,7 @@ d11_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_C,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_C,
> +		.aux_io = POWER_DOMAIN_AUX_IO_C,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_C,
>  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
>  	},
> @@ -2386,6 +2400,7 @@ d12_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> +		.aux_io = POWER_DOMAIN_AUX_IO_A,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
>  		.aux_tbt = POWER_DOMAIN_INVALID,
>  	}, {
> @@ -2396,6 +2411,7 @@ d12_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> +		.aux_io = POWER_DOMAIN_INVALID,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
>  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
>  	},
> @@ -2411,6 +2427,7 @@ d13_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> +		.aux_io = POWER_DOMAIN_AUX_IO_A,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
>  		.aux_tbt = POWER_DOMAIN_INVALID,
>  	}, {
> @@ -2421,6 +2438,7 @@ d13_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> +		.aux_io = POWER_DOMAIN_INVALID,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
>  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
>  	}, {
> @@ -2431,6 +2449,7 @@ d13_port_domains[] = {
>  
>  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_D,
>  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_D,
> +		.aux_io = POWER_DOMAIN_AUX_IO_D,
>  		.aux_legacy_usbc = POWER_DOMAIN_AUX_D,
>  		.aux_tbt = POWER_DOMAIN_INVALID,
>  	},
> @@ -2508,6 +2527,17 @@ intel_port_domains_for_aux_ch(struct drm_i915_private *i915, enum aux_ch aux_ch)
>  	return NULL;
>  }
>  
> +enum intel_display_power_domain
> +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> +{
> +	const struct intel_ddi_port_domains *domains = intel_port_domains_for_aux_ch(i915, aux_ch);
> +
> +	if (drm_WARN_ON(&i915->drm, !domains) || domains->aux_io == POWER_DOMAIN_INVALID)
> +		return POWER_DOMAIN_AUX_IO_A;a

The POWER_DOMAIN_INVALID things is for TC ports I guess? Shouldn't
this return the full AUX domain for the specific port for those?

Not that you should ever have eDP on them I suppose, so given
what you had in that earlier function I guess this should never
happen.

> +
> +	return domains->aux_io + (int)(aux_ch - domains->aux_ch_start);
> +}
> +
>  enum intel_display_power_domain
>  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index fd68d43bfe322..54737e0aec21e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -78,6 +78,11 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_AUDIO_PLAYBACK,
>  
>  	POWER_DOMAIN_AUX_IO_A,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
> +	POWER_DOMAIN_AUX_IO_D,
> +	POWER_DOMAIN_AUX_IO_E,
> +	POWER_DOMAIN_AUX_IO_F,
>  
>  	POWER_DOMAIN_AUX_A,
>  	POWER_DOMAIN_AUX_B,
> @@ -250,6 +255,8 @@ intel_display_power_ddi_lanes_domain(struct drm_i915_private *i915, enum port po
>  enum intel_display_power_domain
>  intel_display_power_ddi_io_domain(struct drm_i915_private *i915, enum port port);
>  enum intel_display_power_domain
> +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> +enum intel_display_power_domain
>  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
>  enum intel_display_power_domain
>  intel_display_power_tbt_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> index 43454022e6a66..b82c0d0a80c5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> @@ -170,6 +170,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_display,
>  	POWER_DOMAIN_VGA,
>  	POWER_DOMAIN_AUDIO_MMIO,
>  	POWER_DOMAIN_AUDIO_PLAYBACK,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_GMBUS,
> @@ -179,6 +181,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
>  	POWER_DOMAIN_PORT_DDI_LANES_B,
>  	POWER_DOMAIN_PORT_DDI_LANES_C,
>  	POWER_DOMAIN_PORT_CRT,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
> @@ -186,6 +190,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
>  I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_tx_bc_lanes,
>  	POWER_DOMAIN_PORT_DDI_LANES_B,
>  	POWER_DOMAIN_PORT_DDI_LANES_C,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
> @@ -243,6 +249,9 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
>  	POWER_DOMAIN_VGA,
>  	POWER_DOMAIN_AUDIO_MMIO,
>  	POWER_DOMAIN_AUDIO_PLAYBACK,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
> +	POWER_DOMAIN_AUX_IO_D,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_AUX_D,
> @@ -252,12 +261,15 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
>  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_bc,
>  	POWER_DOMAIN_PORT_DDI_LANES_B,
>  	POWER_DOMAIN_PORT_DDI_LANES_C,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
>  
>  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_d,
>  	POWER_DOMAIN_PORT_DDI_LANES_D,
> +	POWER_DOMAIN_AUX_IO_D,
>  	POWER_DOMAIN_AUX_D,
>  	POWER_DOMAIN_INIT);
>  
> @@ -305,6 +317,9 @@ static const struct i915_power_well_desc_list chv_power_wells[] = {
>  	POWER_DOMAIN_VGA, \
>  	POWER_DOMAIN_AUDIO_MMIO, \
>  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> +	POWER_DOMAIN_AUX_IO_B, \
> +	POWER_DOMAIN_AUX_IO_C, \
> +	POWER_DOMAIN_AUX_IO_D, \
>  	POWER_DOMAIN_AUX_B, \
>  	POWER_DOMAIN_AUX_C, \
>  	POWER_DOMAIN_AUX_D
> @@ -407,6 +422,8 @@ static const struct i915_power_well_desc_list skl_power_wells[] = {
>  	POWER_DOMAIN_VGA, \
>  	POWER_DOMAIN_AUDIO_MMIO, \
>  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> +	POWER_DOMAIN_AUX_IO_B, \
> +	POWER_DOMAIN_AUX_IO_C, \
>  	POWER_DOMAIN_AUX_B, \
>  	POWER_DOMAIN_AUX_C
>  
> @@ -430,6 +447,8 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
>  I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_bc,
>  	POWER_DOMAIN_PORT_DDI_LANES_B,
>  	POWER_DOMAIN_PORT_DDI_LANES_C,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
> @@ -483,6 +502,8 @@ static const struct i915_power_well_desc_list bxt_power_wells[] = {
>  	POWER_DOMAIN_VGA, \
>  	POWER_DOMAIN_AUDIO_MMIO, \
>  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> +	POWER_DOMAIN_AUX_IO_B, \
> +	POWER_DOMAIN_AUX_IO_C, \
>  	POWER_DOMAIN_AUX_B, \
>  	POWER_DOMAIN_AUX_C
>  
> @@ -509,11 +530,13 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
>  
>  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_b,
>  	POWER_DOMAIN_PORT_DDI_LANES_B,
> +	POWER_DOMAIN_AUX_IO_B,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_INIT);
>  
>  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
>  	POWER_DOMAIN_PORT_DDI_LANES_C,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
>  
> @@ -523,10 +546,12 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
>  	POWER_DOMAIN_INIT);
>  
>  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
> +	POWER_DOMAIN_AUX_IO_B,
>  	POWER_DOMAIN_AUX_B,
>  	POWER_DOMAIN_INIT);
>  
>  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_c,
> +	POWER_DOMAIN_AUX_IO_C,
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_INIT);
>  
> @@ -617,6 +642,11 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_pw_4,
>  	POWER_DOMAIN_VGA, \
>  	POWER_DOMAIN_AUDIO_MMIO, \
>  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> +	POWER_DOMAIN_AUX_IO_B, \
> +	POWER_DOMAIN_AUX_IO_C, \
> +	POWER_DOMAIN_AUX_IO_D, \
> +	POWER_DOMAIN_AUX_IO_E, \
> +	POWER_DOMAIN_AUX_IO_F, \
>  	POWER_DOMAIN_AUX_B, \
>  	POWER_DOMAIN_AUX_C, \
>  	POWER_DOMAIN_AUX_D, \
> @@ -660,11 +690,21 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
>  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
>  	POWER_DOMAIN_AUX_IO_A,
>  	POWER_DOMAIN_AUX_A);
> -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
> -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
> -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
> -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,		POWER_DOMAIN_AUX_E);
> -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,		POWER_DOMAIN_AUX_F);
> +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
> +	POWER_DOMAIN_AUX_IO_B,
> +	POWER_DOMAIN_AUX_B);
> +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
> +	POWER_DOMAIN_AUX_IO_C,
> +	POWER_DOMAIN_AUX_C);
> +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
> +	POWER_DOMAIN_AUX_IO_D,
> +	POWER_DOMAIN_AUX_D);
> +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
> +	POWER_DOMAIN_AUX_IO_E,
> +	POWER_DOMAIN_AUX_E);
> +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
> +	POWER_DOMAIN_AUX_IO_F,
> +	POWER_DOMAIN_AUX_F);
>  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt1,	POWER_DOMAIN_AUX_TBT1);
>  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt2,	POWER_DOMAIN_AUX_TBT2);
>  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt3,	POWER_DOMAIN_AUX_TBT3);
> @@ -1215,6 +1255,9 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_a,
>  	POWER_DOMAIN_PORT_DDI_LANES_TC4, \
>  	POWER_DOMAIN_VGA, \
>  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> +	POWER_DOMAIN_AUX_IO_C, \
> +	POWER_DOMAIN_AUX_IO_D, \
> +	POWER_DOMAIN_AUX_IO_E, \
>  	POWER_DOMAIN_AUX_C, \
>  	POWER_DOMAIN_AUX_D, \
>  	POWER_DOMAIN_AUX_E, \
> -- 
> 2.37.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/7] drm/i915/tgl+: Enable display DC power states on all eDP ports

[Imre Deak]

On Wed, Nov 02, 2022 at 07:35:17PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 02, 2022 at 07:15:26PM +0200, Imre Deak wrote:
> > Starting with TGL eDP is supported on ports B+ (besides port A), so make
> > sure DC states are not blocked on any such ports. For this add an
> > AUX_IO_<port> power domain for each port with eDP support. These domains
> > similarly to AUX_IO_A enable only the AUX_IO_<port> power well for an
> > enabled port, whereas the existing AUX_<port> domains enable both the
> > AUX_IO_<port> and the DC_OFF power wells as required by DP AUX transfers.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 ++-
> >  .../drm/i915/display/intel_display_power.c    | 30 +++++++++++
> >  .../drm/i915/display/intel_display_power.h    |  7 +++
> >  .../i915/display/intel_display_power_map.c    | 53 +++++++++++++++++--
> >  4 files changed, 91 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index e95bde5cf060e..00b577a5b9a76 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -848,6 +848,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> >  static enum intel_display_power_domain
> >  intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> >  {
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +
> >  	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> >  	 * DC states enabled at the same time, while for driver initiated AUX
> >  	 * transfers we need the same AUX IOs to be powered but with DC states
> > @@ -860,8 +862,10 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> >  	 * Note that PSR is enabled only on Port A even though this function
> >  	 * returns the correct domain for other ports too.
> >  	 */
> > -	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > -					      intel_aux_power_domain(dig_port);
> > +	if (intel_dp_is_edp(&dig_port->dp))
> > +		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> > +	else
> > +		return intel_aux_power_domain(dig_port);
> 
> This seems to have two distinct changes in it:
> 1) define more AUX_IO domains
> 2) don't use AUX_IO for external DP and instead use the full AUX domain

By 2) you mean on port A full AUX is used for external DP, then yes.

> So seems like this needs to be split.

Ok, can add a patch with only a

	if (is_edp(port) && port == A)
		return AUX_IO_A;
	else
		return AUX_A;

change followed by this one, if that's what you meant.

> Also why do we need 2)?

The DC5/6 states only make sense with PSR enabled. Even though we enable
these atm for external outputs enabled on port A as well (for instance
DG2), the firmware will not actually enter the DC power states in this
case. Since for TypeC ports the AUX domain/power well also depends on
the TBT vs. non-TBT mode, the easiest way to select between the AUX_IO
vs. AUX domain was eDP vs. non-eDP (see also below).

> 
> >  }
> >  
> >  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index da0047359446e..ca453518e7fd7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -131,6 +131,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> >  		return "AUDIO_PLAYBACK";
> >  	case POWER_DOMAIN_AUX_IO_A:
> >  		return "AUX_IO_A";
> > +	case POWER_DOMAIN_AUX_IO_B:
> > +		return "AUX_IO_B";
> > +	case POWER_DOMAIN_AUX_IO_C:
> > +		return "AUX_IO_C";
> > +	case POWER_DOMAIN_AUX_IO_D:
> > +		return "AUX_IO_D";
> > +	case POWER_DOMAIN_AUX_IO_E:
> > +		return "AUX_IO_E";
> > +	case POWER_DOMAIN_AUX_IO_F:
> > +		return "AUX_IO_F";
> >  	case POWER_DOMAIN_AUX_A:
> >  		return "AUX_A";
> >  	case POWER_DOMAIN_AUX_B:
> > @@ -2332,6 +2342,7 @@ struct intel_ddi_port_domains {
> >  
> >  	enum intel_display_power_domain ddi_lanes;
> >  	enum intel_display_power_domain ddi_io;
> > +	enum intel_display_power_domain aux_io;
> >  	enum intel_display_power_domain aux_legacy_usbc;
> >  	enum intel_display_power_domain aux_tbt;
> >  };
> > @@ -2346,6 +2357,7 @@ i9xx_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> >  		.aux_tbt = POWER_DOMAIN_INVALID,
> >  	},
> > @@ -2361,6 +2373,7 @@ d11_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> >  		.aux_tbt = POWER_DOMAIN_INVALID,
> >  	}, {
> > @@ -2371,6 +2384,7 @@ d11_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_C,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_C,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_C,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_C,
> >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> >  	},
> > @@ -2386,6 +2400,7 @@ d12_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> >  		.aux_tbt = POWER_DOMAIN_INVALID,
> >  	}, {
> > @@ -2396,6 +2411,7 @@ d12_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > +		.aux_io = POWER_DOMAIN_INVALID,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> >  	},
> > @@ -2411,6 +2427,7 @@ d13_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> >  		.aux_tbt = POWER_DOMAIN_INVALID,
> >  	}, {
> > @@ -2421,6 +2438,7 @@ d13_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > +		.aux_io = POWER_DOMAIN_INVALID,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> >  	}, {
> > @@ -2431,6 +2449,7 @@ d13_port_domains[] = {
> >  
> >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_D,
> >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_D,
> > +		.aux_io = POWER_DOMAIN_AUX_IO_D,
> >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_D,
> >  		.aux_tbt = POWER_DOMAIN_INVALID,
> >  	},
> > @@ -2508,6 +2527,17 @@ intel_port_domains_for_aux_ch(struct drm_i915_private *i915, enum aux_ch aux_ch)
> >  	return NULL;
> >  }
> >  
> > +enum intel_display_power_domain
> > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > +{
> > +	const struct intel_ddi_port_domains *domains = intel_port_domains_for_aux_ch(i915, aux_ch);
> > +
> > +	if (drm_WARN_ON(&i915->drm, !domains) || domains->aux_io == POWER_DOMAIN_INVALID)
> > +		return POWER_DOMAIN_AUX_IO_A;
> 
> The POWER_DOMAIN_INVALID things is for TC ports I guess? 

Yes.

> Shouldn't this return the full AUX domain for the specific port for those?

At this level it's not well-defined, as that depends on the TBT vs.
non-TBT mode as well and as you point out they shouldn't be used.

> Not that you should ever have eDP on them I suppose, so given
> what you had in that earlier function I guess this should never
> happen.

Right.

I guess we could assume that the AUX_IO domain will be not required in
TBT mode (vs. the full AUX domain) and also define/return from here new
AUX_IO_USBC[1-6] domains which wouldn't explicitly disable DC states.
Right now only non-TC ports support eDP, so these domains wouldn't be
used, hence I opted for not defining them.

> > +
> > +	return domains->aux_io + (int)(aux_ch - domains->aux_ch_start);
> > +}
> > +
> >  enum intel_display_power_domain
> >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index fd68d43bfe322..54737e0aec21e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -78,6 +78,11 @@ enum intel_display_power_domain {
> >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> >  
> >  	POWER_DOMAIN_AUX_IO_A,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> > +	POWER_DOMAIN_AUX_IO_D,
> > +	POWER_DOMAIN_AUX_IO_E,
> > +	POWER_DOMAIN_AUX_IO_F,
> >  
> >  	POWER_DOMAIN_AUX_A,
> >  	POWER_DOMAIN_AUX_B,
> > @@ -250,6 +255,8 @@ intel_display_power_ddi_lanes_domain(struct drm_i915_private *i915, enum port po
> >  enum intel_display_power_domain
> >  intel_display_power_ddi_io_domain(struct drm_i915_private *i915, enum port port);
> >  enum intel_display_power_domain
> > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > +enum intel_display_power_domain
> >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> >  enum intel_display_power_domain
> >  intel_display_power_tbt_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > index 43454022e6a66..b82c0d0a80c5f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > @@ -170,6 +170,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_display,
> >  	POWER_DOMAIN_VGA,
> >  	POWER_DOMAIN_AUDIO_MMIO,
> >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_GMBUS,
> > @@ -179,6 +181,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> >  	POWER_DOMAIN_PORT_CRT,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> > @@ -186,6 +190,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> >  I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_tx_bc_lanes,
> >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> > @@ -243,6 +249,9 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> >  	POWER_DOMAIN_VGA,
> >  	POWER_DOMAIN_AUDIO_MMIO,
> >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> > +	POWER_DOMAIN_AUX_IO_D,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_AUX_D,
> > @@ -252,12 +261,15 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_bc,
> >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> >  
> >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_d,
> >  	POWER_DOMAIN_PORT_DDI_LANES_D,
> > +	POWER_DOMAIN_AUX_IO_D,
> >  	POWER_DOMAIN_AUX_D,
> >  	POWER_DOMAIN_INIT);
> >  
> > @@ -305,6 +317,9 @@ static const struct i915_power_well_desc_list chv_power_wells[] = {
> >  	POWER_DOMAIN_VGA, \
> >  	POWER_DOMAIN_AUDIO_MMIO, \
> >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > +	POWER_DOMAIN_AUX_IO_B, \
> > +	POWER_DOMAIN_AUX_IO_C, \
> > +	POWER_DOMAIN_AUX_IO_D, \
> >  	POWER_DOMAIN_AUX_B, \
> >  	POWER_DOMAIN_AUX_C, \
> >  	POWER_DOMAIN_AUX_D
> > @@ -407,6 +422,8 @@ static const struct i915_power_well_desc_list skl_power_wells[] = {
> >  	POWER_DOMAIN_VGA, \
> >  	POWER_DOMAIN_AUDIO_MMIO, \
> >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > +	POWER_DOMAIN_AUX_IO_B, \
> > +	POWER_DOMAIN_AUX_IO_C, \
> >  	POWER_DOMAIN_AUX_B, \
> >  	POWER_DOMAIN_AUX_C
> >  
> > @@ -430,6 +447,8 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
> >  I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_bc,
> >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> > @@ -483,6 +502,8 @@ static const struct i915_power_well_desc_list bxt_power_wells[] = {
> >  	POWER_DOMAIN_VGA, \
> >  	POWER_DOMAIN_AUDIO_MMIO, \
> >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > +	POWER_DOMAIN_AUX_IO_B, \
> > +	POWER_DOMAIN_AUX_IO_C, \
> >  	POWER_DOMAIN_AUX_B, \
> >  	POWER_DOMAIN_AUX_C
> >  
> > @@ -509,11 +530,13 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
> >  
> >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_b,
> >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > +	POWER_DOMAIN_AUX_IO_B,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_INIT);
> >  
> >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
> >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> >  
> > @@ -523,10 +546,12 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
> >  	POWER_DOMAIN_INIT);
> >  
> >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
> > +	POWER_DOMAIN_AUX_IO_B,
> >  	POWER_DOMAIN_AUX_B,
> >  	POWER_DOMAIN_INIT);
> >  
> >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_c,
> > +	POWER_DOMAIN_AUX_IO_C,
> >  	POWER_DOMAIN_AUX_C,
> >  	POWER_DOMAIN_INIT);
> >  
> > @@ -617,6 +642,11 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_pw_4,
> >  	POWER_DOMAIN_VGA, \
> >  	POWER_DOMAIN_AUDIO_MMIO, \
> >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > +	POWER_DOMAIN_AUX_IO_B, \
> > +	POWER_DOMAIN_AUX_IO_C, \
> > +	POWER_DOMAIN_AUX_IO_D, \
> > +	POWER_DOMAIN_AUX_IO_E, \
> > +	POWER_DOMAIN_AUX_IO_F, \
> >  	POWER_DOMAIN_AUX_B, \
> >  	POWER_DOMAIN_AUX_C, \
> >  	POWER_DOMAIN_AUX_D, \
> > @@ -660,11 +690,21 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
> >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
> >  	POWER_DOMAIN_AUX_IO_A,
> >  	POWER_DOMAIN_AUX_A);
> > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
> > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
> > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
> > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,		POWER_DOMAIN_AUX_E);
> > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,		POWER_DOMAIN_AUX_F);
> > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
> > +	POWER_DOMAIN_AUX_IO_B,
> > +	POWER_DOMAIN_AUX_B);
> > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
> > +	POWER_DOMAIN_AUX_IO_C,
> > +	POWER_DOMAIN_AUX_C);
> > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
> > +	POWER_DOMAIN_AUX_IO_D,
> > +	POWER_DOMAIN_AUX_D);
> > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
> > +	POWER_DOMAIN_AUX_IO_E,
> > +	POWER_DOMAIN_AUX_E);
> > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
> > +	POWER_DOMAIN_AUX_IO_F,
> > +	POWER_DOMAIN_AUX_F);
> >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt1,	POWER_DOMAIN_AUX_TBT1);
> >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt2,	POWER_DOMAIN_AUX_TBT2);
> >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt3,	POWER_DOMAIN_AUX_TBT3);
> > @@ -1215,6 +1255,9 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_a,
> >  	POWER_DOMAIN_PORT_DDI_LANES_TC4, \
> >  	POWER_DOMAIN_VGA, \
> >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > +	POWER_DOMAIN_AUX_IO_C, \
> > +	POWER_DOMAIN_AUX_IO_D, \
> > +	POWER_DOMAIN_AUX_IO_E, \
> >  	POWER_DOMAIN_AUX_C, \
> >  	POWER_DOMAIN_AUX_D, \
> >  	POWER_DOMAIN_AUX_E, \
> > -- 
> > 2.37.1
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/7] drm/i915: Allocate power domain set wakerefs dynamically

[Jani Nikula]

On Wed, 02 Nov 2022, Imre Deak <imre.deak@intel.com> wrote:
> Since the intel_display_power_domain_set struct, currently its current
> size close 1kB, can be allocated on the stack, it's better to allocate
> the per-domain wakeref pointer array - used for debugging - within the
> struct dynamically, so do this.
>
> The memory freeing is guaranteed by the fact that the acquired domain
> references tracked by struct can't be leaked either.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 61 ++++++++++++++++---
>  .../drm/i915/display/intel_display_power.h    |  2 +-
>  2 files changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 4c1de91e56ff9..e2da91c2a9638 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -830,19 +830,58 @@ void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> +static intel_wakeref_t *
> +get_debug_wakerefs(struct drm_i915_private *i915,
> +		   struct intel_display_power_domain_set *power_domain_set)
> +{
> +	if (power_domain_set->wakerefs)
> +		return power_domain_set->wakerefs;
> +
> +	power_domain_set->wakerefs = kcalloc(POWER_DOMAIN_NUM,
> +					     sizeof(*power_domain_set->wakerefs),
> +					     GFP_KERNEL);
> +
> +	drm_WARN_ON_ONCE(&i915->drm, !power_domain_set->wakerefs);

The rule of thumb is not to warn or log on allocation failures.

> +
> +	return power_domain_set->wakerefs;
> +}
> +
> +static void
> +free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
> +{
> +	if (power_domain_set->wakerefs &&
> +	    bitmap_empty(power_domain_set->mask.bits, POWER_DOMAIN_NUM))
> +		kfree(fetch_and_zero(&power_domain_set->wakerefs));

FWIW, I'm really not happy about fetch_and_zero() or its use anywhere. I
kind of get the point, but the impression of any kind of atomicity the
naming gives is totally misleading. And it's our own thing in
i915_utils.h, and not a global thing like the name suggests.

> +}
> +#else
> +static intel_wakeref_t *
> +get_debug_wakerefs(struct drm_i915_private *i915,
> +		   struct intel_display_power_domain_set *power_domain_set)
> +{
> +	return NULL;
> +}
> +
> +static void
> +free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
> +{
> +}
> +#endif

get/free is an odd pairing of names.

> +
>  void
>  intel_display_power_get_in_set(struct drm_i915_private *i915,
>  			       struct intel_display_power_domain_set *power_domain_set,
>  			       enum intel_display_power_domain domain)
>  {
>  	intel_wakeref_t __maybe_unused wf;
> +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
>  
>  	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
>  
>  	wf = intel_display_power_get(i915, domain);
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> -	power_domain_set->wakerefs[domain] = wf;
> -#endif
> +	if (debug_wakerefs)
> +		debug_wakerefs[domain] = wf;
> +

If you abstracted setting the debug wakeref for a domain, it could
handle the allocation internally without any of the local vars etc.
here. And it would only be a single line in the entire function.

>  	set_bit(domain, power_domain_set->mask.bits);
>  }
>  
> @@ -852,6 +891,7 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
>  					  enum intel_display_power_domain domain)
>  {
>  	intel_wakeref_t wf;
> +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
>  
>  	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
>  
> @@ -859,9 +899,9 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
>  	if (!wf)
>  		return false;
>  
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> -	power_domain_set->wakerefs[domain] = wf;
> -#endif
> +	if (debug_wakerefs)
> +		debug_wakerefs[domain] = wf;
> +
>  	set_bit(domain, power_domain_set->mask.bits);
>  
>  	return true;
> @@ -873,6 +913,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
>  				    struct intel_power_domain_mask *mask)
>  {
>  	enum intel_display_power_domain domain;
> +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
>  
>  	drm_WARN_ON(&i915->drm,
>  		    !bitmap_subset(mask->bits, power_domain_set->mask.bits, POWER_DOMAIN_NUM));
> @@ -880,12 +921,14 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
>  	for_each_power_domain(domain, mask) {
>  		intel_wakeref_t __maybe_unused wf = -1;
>  
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> -		wf = fetch_and_zero(&power_domain_set->wakerefs[domain]);
> -#endif
> +		if (debug_wakerefs)
> +			wf = fetch_and_zero(&debug_wakerefs[domain]);
> +
>  		intel_display_power_put(i915, domain, wf);
>  		clear_bit(domain, power_domain_set->mask.bits);
>  	}
> +
> +	free_empty_debug_wakerefs(power_domain_set);
>  }
>  
>  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 7136ea3f233e9..c847aab7b2f88 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -146,7 +146,7 @@ struct i915_power_domains {
>  struct intel_display_power_domain_set {
>  	struct intel_power_domain_mask mask;
>  #ifdef CONFIG_DRM_I915_DEBUG_RUNTIME_PM
> -	intel_wakeref_t wakerefs[POWER_DOMAIN_NUM];
> +	intel_wakeref_t *wakerefs;
>  #endif
>  };

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Factor out function to get/put AUX_IO power for main link

[Jani Nikula]

On Wed, 02 Nov 2022, Imre Deak <imre.deak@intel.com> wrote:
> Factor out functions to get/put the AUX_IO power domain for the main
> link on DDI ports.
>
> While at it clarify the corresponding code comment.
>
> No functional change.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 84 ++++++++++++++----------
>  1 file changed, 51 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 00b577a5b9a76..7453772d2073d 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -846,26 +846,63 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  }
>  
>  static enum intel_display_power_domain
> -intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> +intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port,
> +			       const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
>  
> -	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> +	/*
> +	 * ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
>  	 * DC states enabled at the same time, while for driver initiated AUX
>  	 * transfers we need the same AUX IOs to be powered but with DC states
> -	 * disabled. Accordingly use the AUX power domain here which leaves DC
> -	 * states enabled.
> -	 * However, for non-A AUX ports the corresponding non-EDP transcoders
> -	 * would have already enabled power well 2 and DC_OFF. This means we can
> -	 * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
> -	 * specific AUX_IO reference without powering up any extra wells.
> -	 * Note that PSR is enabled only on Port A even though this function
> -	 * returns the correct domain for other ports too.
> +	 * disabled. Accordingly use the AUX_IO_<port> power domain here which
> +	 * leaves DC states enabled.
> +	 *
> +	 * Before MTL all DP ports and HDMI ports on TypeC PHYs also require
> +	 * AUX IO to be enabled, but all these require DC_OFF to be enabled as
> +	 * well, so we can acquire a wider AUX_<port> power domain reference
> +	 * instead of a specific AUX_IO_<port> reference without powering up any
> +	 * extra wells.
>  	 */
>  	if (intel_dp_is_edp(&dig_port->dp))
>  		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> -	else
> +	else if (intel_crtc_has_dp_encoder(crtc_state) ||
> +		 intel_phy_is_tc(i915, phy))
>  		return intel_aux_power_domain(dig_port);
> +	else
> +		return POWER_DOMAIN_INVALID;
> +}
> +
> +static void
> +get_aux_power_for_main_link(struct intel_digital_port *dig_port,
> +			    const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	enum intel_display_power_domain domain =
> +		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
> +
> +	drm_WARN_ON(&i915->drm, dig_port->aux_wakeref);
> +
> +	if (domain == POWER_DOMAIN_INVALID)
> +		return;
> +
> +	dig_port->aux_wakeref = intel_display_power_get(i915, domain);
> +}
> +
> +static void
> +put_aux_power_for_main_link(struct intel_digital_port *dig_port,
> +			    const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +	intel_wakeref_t wf = fetch_and_zero(&dig_port->aux_wakeref);
> +	enum intel_display_power_domain domain =
> +		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
> +
> +	if (!wf)
> +		return;
> +
> +	intel_display_power_put(i915, domain, wf);
>  }

Nitpick, I think foo_get()/foo_put() are much more common in the driver
than get_foo()/put_foo().

>  
>  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> @@ -873,7 +910,6 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port;
> -	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>  
>  	/*
>  	 * TODO: Add support for MST encoders. Atm, the following should never
> @@ -892,17 +928,7 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
>  								   dig_port->ddi_io_power_domain);
>  	}
>  
> -	/*
> -	 * AUX power is only needed for (e)DP mode, and for HDMI mode on TC
> -	 * ports.
> -	 */
> -	if (intel_crtc_has_dp_encoder(crtc_state) ||
> -	    intel_phy_is_tc(dev_priv, phy)) {
> -		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
> -		dig_port->aux_wakeref =
> -			intel_display_power_get(dev_priv,
> -						intel_ddi_main_link_aux_domain(dig_port));
> -	}
> +	get_aux_power_for_main_link(dig_port, crtc_state);
>  }
>  
>  void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
> @@ -2741,10 +2767,7 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
>  		intel_ddi_post_disable_dp(state, encoder, old_crtc_state,
>  					  old_conn_state);
>  
> -	if (intel_crtc_has_dp_encoder(old_crtc_state) || is_tc_port)
> -		intel_display_power_put(dev_priv,
> -					intel_ddi_main_link_aux_domain(dig_port),
> -					fetch_and_zero(&dig_port->aux_wakeref));
> +	put_aux_power_for_main_link(dig_port, old_crtc_state);
>  
>  	if (is_tc_port)
>  		intel_tc_port_put_link(dig_port);
> @@ -3065,12 +3088,7 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
>  	if (is_tc_port)
>  		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
>  
> -	if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port) {
> -		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
> -		dig_port->aux_wakeref =
> -			intel_display_power_get(dev_priv,
> -						intel_ddi_main_link_aux_domain(dig_port));
> -	}
> +	get_aux_power_for_main_link(dig_port, crtc_state);
>  
>  	if (is_tc_port && !intel_tc_port_in_tbt_alt_mode(dig_port))
>  		/*

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 3/7] drm/i915/tgl+: Enable display DC power states on all eDP ports

[Ville Syrjälä]

On Wed, Nov 02, 2022 at 08:34:48PM +0200, Imre Deak wrote:
> On Wed, Nov 02, 2022 at 07:35:17PM +0200, Ville Syrjälä wrote:
> > On Wed, Nov 02, 2022 at 07:15:26PM +0200, Imre Deak wrote:
> > > Starting with TGL eDP is supported on ports B+ (besides port A), so make
> > > sure DC states are not blocked on any such ports. For this add an
> > > AUX_IO_<port> power domain for each port with eDP support. These domains
> > > similarly to AUX_IO_A enable only the AUX_IO_<port> power well for an
> > > enabled port, whereas the existing AUX_<port> domains enable both the
> > > AUX_IO_<port> and the DC_OFF power wells as required by DP AUX transfers.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 ++-
> > >  .../drm/i915/display/intel_display_power.c    | 30 +++++++++++
> > >  .../drm/i915/display/intel_display_power.h    |  7 +++
> > >  .../i915/display/intel_display_power_map.c    | 53 +++++++++++++++++--
> > >  4 files changed, 91 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index e95bde5cf060e..00b577a5b9a76 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -848,6 +848,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> > >  static enum intel_display_power_domain
> > >  intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> > >  {
> > > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > > +
> > >  	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> > >  	 * DC states enabled at the same time, while for driver initiated AUX
> > >  	 * transfers we need the same AUX IOs to be powered but with DC states
> > > @@ -860,8 +862,10 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> > >  	 * Note that PSR is enabled only on Port A even though this function
> > >  	 * returns the correct domain for other ports too.
> > >  	 */
> > > -	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > -					      intel_aux_power_domain(dig_port);
> > > +	if (intel_dp_is_edp(&dig_port->dp))
> > > +		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> > > +	else
> > > +		return intel_aux_power_domain(dig_port);
> > 
> > This seems to have two distinct changes in it:
> > 1) define more AUX_IO domains
> > 2) don't use AUX_IO for external DP and instead use the full AUX domain
> 
> By 2) you mean on port A full AUX is used for external DP, then yes.
> 
> > So seems like this needs to be split.
> 
> Ok, can add a patch with only a
> 
> 	if (is_edp(port) && port == A)
> 		return AUX_IO_A;
> 	else
> 		return AUX_A;
> 
> change followed by this one, if that's what you meant.
> 
> > Also why do we need 2)?
> 
> The DC5/6 states only make sense with PSR enabled. Even though we enable
> these atm for external outputs enabled on port A as well (for instance
> DG2), the firmware will not actually enter the DC power states in this
> case. Since for TypeC ports the AUX domain/power well also depends on
> the TBT vs. non-TBT mode, the easiest way to select between the AUX_IO
> vs. AUX domain was eDP vs. non-eDP (see also below).

OK, so you're saying for combo ports we don't actually need
it and the current code works fine. So from the pure hw pov
the sufficient thing would be more or less just this?

if (is_tc_port)
	return aux_domain;
else
	return aux_io_domain;

is_edp() I consider to be a software defined concept.
It doesn't really change how the hardware actually
works. Which is why I don't really like using it for
anything. I've noticed people have a bad habit of 
trying to use it incorrectly in places where eg. a
specific port is what we actually want to check.

> 
> > 
> > >  }
> > >  
> > >  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index da0047359446e..ca453518e7fd7 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -131,6 +131,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > >  		return "AUDIO_PLAYBACK";
> > >  	case POWER_DOMAIN_AUX_IO_A:
> > >  		return "AUX_IO_A";
> > > +	case POWER_DOMAIN_AUX_IO_B:
> > > +		return "AUX_IO_B";
> > > +	case POWER_DOMAIN_AUX_IO_C:
> > > +		return "AUX_IO_C";
> > > +	case POWER_DOMAIN_AUX_IO_D:
> > > +		return "AUX_IO_D";
> > > +	case POWER_DOMAIN_AUX_IO_E:
> > > +		return "AUX_IO_E";
> > > +	case POWER_DOMAIN_AUX_IO_F:
> > > +		return "AUX_IO_F";
> > >  	case POWER_DOMAIN_AUX_A:
> > >  		return "AUX_A";
> > >  	case POWER_DOMAIN_AUX_B:
> > > @@ -2332,6 +2342,7 @@ struct intel_ddi_port_domains {
> > >  
> > >  	enum intel_display_power_domain ddi_lanes;
> > >  	enum intel_display_power_domain ddi_io;
> > > +	enum intel_display_power_domain aux_io;
> > >  	enum intel_display_power_domain aux_legacy_usbc;
> > >  	enum intel_display_power_domain aux_tbt;
> > >  };
> > > @@ -2346,6 +2357,7 @@ i9xx_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > >  	},
> > > @@ -2361,6 +2373,7 @@ d11_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > >  	}, {
> > > @@ -2371,6 +2384,7 @@ d11_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_C,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_C,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_C,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_C,
> > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > >  	},
> > > @@ -2386,6 +2400,7 @@ d12_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > >  	}, {
> > > @@ -2396,6 +2411,7 @@ d12_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > > +		.aux_io = POWER_DOMAIN_INVALID,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > >  	},
> > > @@ -2411,6 +2427,7 @@ d13_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > >  	}, {
> > > @@ -2421,6 +2438,7 @@ d13_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > > +		.aux_io = POWER_DOMAIN_INVALID,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > >  	}, {
> > > @@ -2431,6 +2449,7 @@ d13_port_domains[] = {
> > >  
> > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_D,
> > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_D,
> > > +		.aux_io = POWER_DOMAIN_AUX_IO_D,
> > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_D,
> > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > >  	},
> > > @@ -2508,6 +2527,17 @@ intel_port_domains_for_aux_ch(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > >  	return NULL;
> > >  }
> > >  
> > > +enum intel_display_power_domain
> > > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > > +{
> > > +	const struct intel_ddi_port_domains *domains = intel_port_domains_for_aux_ch(i915, aux_ch);
> > > +
> > > +	if (drm_WARN_ON(&i915->drm, !domains) || domains->aux_io == POWER_DOMAIN_INVALID)
> > > +		return POWER_DOMAIN_AUX_IO_A;
> > 
> > The POWER_DOMAIN_INVALID things is for TC ports I guess? 
> 
> Yes.
> 
> > Shouldn't this return the full AUX domain for the specific port for those?
> 
> At this level it's not well-defined, as that depends on the TBT vs.
> non-TBT mode as well and as you point out they shouldn't be used.
> 
> > Not that you should ever have eDP on them I suppose, so given
> > what you had in that earlier function I guess this should never
> > happen.
> 
> Right.
> 
> I guess we could assume that the AUX_IO domain will be not required in
> TBT mode (vs. the full AUX domain) and also define/return from here new
> AUX_IO_USBC[1-6] domains which wouldn't explicitly disable DC states.
> Right now only non-TC ports support eDP, so these domains wouldn't be
> used, hence I opted for not defining them.

If this thing doesn't make sense for the type-c ports then
we should probably just not use it for those. So just extending
the WARN (assuming we want one) to cover that case is perhaps
as well want to do.

> 
> > > +
> > > +	return domains->aux_io + (int)(aux_ch - domains->aux_ch_start);
> > > +}
> > > +
> > >  enum intel_display_power_domain
> > >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > >  {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index fd68d43bfe322..54737e0aec21e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > @@ -78,6 +78,11 @@ enum intel_display_power_domain {
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > >  
> > >  	POWER_DOMAIN_AUX_IO_A,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > > +	POWER_DOMAIN_AUX_IO_D,
> > > +	POWER_DOMAIN_AUX_IO_E,
> > > +	POWER_DOMAIN_AUX_IO_F,
> > >  
> > >  	POWER_DOMAIN_AUX_A,
> > >  	POWER_DOMAIN_AUX_B,
> > > @@ -250,6 +255,8 @@ intel_display_power_ddi_lanes_domain(struct drm_i915_private *i915, enum port po
> > >  enum intel_display_power_domain
> > >  intel_display_power_ddi_io_domain(struct drm_i915_private *i915, enum port port);
> > >  enum intel_display_power_domain
> > > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > > +enum intel_display_power_domain
> > >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > >  enum intel_display_power_domain
> > >  intel_display_power_tbt_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > index 43454022e6a66..b82c0d0a80c5f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > @@ -170,6 +170,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_display,
> > >  	POWER_DOMAIN_VGA,
> > >  	POWER_DOMAIN_AUDIO_MMIO,
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_GMBUS,
> > > @@ -179,6 +181,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > >  	POWER_DOMAIN_PORT_CRT,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > > @@ -186,6 +190,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> > >  I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_tx_bc_lanes,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > > @@ -243,6 +249,9 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> > >  	POWER_DOMAIN_VGA,
> > >  	POWER_DOMAIN_AUDIO_MMIO,
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > > +	POWER_DOMAIN_AUX_IO_D,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_AUX_D,
> > > @@ -252,12 +261,15 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> > >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_bc,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_d,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_D,
> > > +	POWER_DOMAIN_AUX_IO_D,
> > >  	POWER_DOMAIN_AUX_D,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > > @@ -305,6 +317,9 @@ static const struct i915_power_well_desc_list chv_power_wells[] = {
> > >  	POWER_DOMAIN_VGA, \
> > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > +	POWER_DOMAIN_AUX_IO_B, \
> > > +	POWER_DOMAIN_AUX_IO_C, \
> > > +	POWER_DOMAIN_AUX_IO_D, \
> > >  	POWER_DOMAIN_AUX_B, \
> > >  	POWER_DOMAIN_AUX_C, \
> > >  	POWER_DOMAIN_AUX_D
> > > @@ -407,6 +422,8 @@ static const struct i915_power_well_desc_list skl_power_wells[] = {
> > >  	POWER_DOMAIN_VGA, \
> > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > +	POWER_DOMAIN_AUX_IO_B, \
> > > +	POWER_DOMAIN_AUX_IO_C, \
> > >  	POWER_DOMAIN_AUX_B, \
> > >  	POWER_DOMAIN_AUX_C
> > >  
> > > @@ -430,6 +447,8 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
> > >  I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_bc,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > > @@ -483,6 +502,8 @@ static const struct i915_power_well_desc_list bxt_power_wells[] = {
> > >  	POWER_DOMAIN_VGA, \
> > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > +	POWER_DOMAIN_AUX_IO_B, \
> > > +	POWER_DOMAIN_AUX_IO_C, \
> > >  	POWER_DOMAIN_AUX_B, \
> > >  	POWER_DOMAIN_AUX_C
> > >  
> > > @@ -509,11 +530,13 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
> > >  
> > >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_b,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > > @@ -523,10 +546,12 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > >  	POWER_DOMAIN_AUX_B,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_c,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > >  	POWER_DOMAIN_AUX_C,
> > >  	POWER_DOMAIN_INIT);
> > >  
> > > @@ -617,6 +642,11 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_pw_4,
> > >  	POWER_DOMAIN_VGA, \
> > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > +	POWER_DOMAIN_AUX_IO_B, \
> > > +	POWER_DOMAIN_AUX_IO_C, \
> > > +	POWER_DOMAIN_AUX_IO_D, \
> > > +	POWER_DOMAIN_AUX_IO_E, \
> > > +	POWER_DOMAIN_AUX_IO_F, \
> > >  	POWER_DOMAIN_AUX_B, \
> > >  	POWER_DOMAIN_AUX_C, \
> > >  	POWER_DOMAIN_AUX_D, \
> > > @@ -660,11 +690,21 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
> > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
> > >  	POWER_DOMAIN_AUX_IO_A,
> > >  	POWER_DOMAIN_AUX_A);
> > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
> > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
> > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
> > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,		POWER_DOMAIN_AUX_E);
> > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,		POWER_DOMAIN_AUX_F);
> > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
> > > +	POWER_DOMAIN_AUX_IO_B,
> > > +	POWER_DOMAIN_AUX_B);
> > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
> > > +	POWER_DOMAIN_AUX_IO_C,
> > > +	POWER_DOMAIN_AUX_C);
> > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
> > > +	POWER_DOMAIN_AUX_IO_D,
> > > +	POWER_DOMAIN_AUX_D);
> > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
> > > +	POWER_DOMAIN_AUX_IO_E,
> > > +	POWER_DOMAIN_AUX_E);
> > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
> > > +	POWER_DOMAIN_AUX_IO_F,
> > > +	POWER_DOMAIN_AUX_F);
> > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt1,	POWER_DOMAIN_AUX_TBT1);
> > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt2,	POWER_DOMAIN_AUX_TBT2);
> > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt3,	POWER_DOMAIN_AUX_TBT3);
> > > @@ -1215,6 +1255,9 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_a,
> > >  	POWER_DOMAIN_PORT_DDI_LANES_TC4, \
> > >  	POWER_DOMAIN_VGA, \
> > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > +	POWER_DOMAIN_AUX_IO_C, \
> > > +	POWER_DOMAIN_AUX_IO_D, \
> > > +	POWER_DOMAIN_AUX_IO_E, \
> > >  	POWER_DOMAIN_AUX_C, \
> > >  	POWER_DOMAIN_AUX_D, \
> > >  	POWER_DOMAIN_AUX_E, \
> > > -- 
> > > 2.37.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tgl+: Enable DC power states on all eDP ports

[Patchwork]

== Series Details ==

Series: drm/i915/tgl+: Enable DC power states on all eDP ports
URL   : https://patchwork.freedesktop.org/series/110433/
State : warning

== Summary ==

Error: dim checkpatch failed
41570281b352 drm/i915: Allocate power domain set wakerefs dynamically
087d66bbc539 drm/i915: Move the POWER_DOMAIN_AUX_IO_A definition to its logical place
1db7e301decc drm/i915/tgl+: Enable display DC power states on all eDP ports
-:321: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#321: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:694:
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
+	POWER_DOMAIN_AUX_IO_B,

-:324: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#324: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:697:
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
+	POWER_DOMAIN_AUX_IO_C,

-:327: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#327: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:700:
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
+	POWER_DOMAIN_AUX_IO_D,

-:330: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#330: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:703:
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
+	POWER_DOMAIN_AUX_IO_E,

-:333: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#333: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:706:
+I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
+	POWER_DOMAIN_AUX_IO_F,

total: 0 errors, 0 warnings, 5 checks, 287 lines checked
b4adc58a2c87 drm/i915: Add missing AUX_IO_A power domain->well mappings
0138b898c33c drm/i915: Add missing DC_OFF power domain->well mappings
054682760ffe drm/i915: Factor out function to get/put AUX_IO power for main link
687f4ddf8cbb drm/i915/mtl+: Don't enable the AUX_IO power for non-eDP port main links

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/tgl+: Enable DC power states on all eDP ports

[Patchwork]

== Series Details ==

Series: drm/i915/tgl+: Enable DC power states on all eDP ports
URL   : https://patchwork.freedesktop.org/series/110433/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/tgl+: Enable DC power states on all eDP ports

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4896 bytes --]

== Series Details ==

Series: drm/i915/tgl+: Enable DC power states on all eDP ports
URL   : https://patchwork.freedesktop.org/series/110433/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12332 -> Patchwork_110433v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (40 -> 28)
------------------------------

  Missing    (12): bat-dg2-8 bat-adlm-1 fi-icl-u2 bat-dg2-9 bat-adlp-6 bat-adlp-4 bat-adln-1 bat-rplp-1 bat-rpls-1 bat-rpls-2 bat-dg2-11 bat-jsl-1 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-bdw-5557u:       [PASS][1] -> [INCOMPLETE][2] ([i915#146])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3@smem.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-bdw-5557u/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-bxt-dsi:         [PASS][3] -> [DMESG-FAIL][4] ([i915#5334])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-bxt-dsi/igt@i915_selftest@live@gt_heartbeat.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-bxt-dsi/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-g3258:       NOTRUN -> [SKIP][5] ([fdo#109271] / [fdo#111827])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-hsw-g3258/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [PASS][6] -> [FAIL][7] ([i915#6298])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [DMESG-FAIL][8] ([i915#5334]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@hangcheck:
    - fi-adl-ddr5:        [DMESG-WARN][10] ([i915#5591]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-adl-ddr5/igt@i915_selftest@live@hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-adl-ddr5/igt@i915_selftest@live@hangcheck.html
    - fi-hsw-g3258:       [INCOMPLETE][12] ([i915#3303] / [i915#4785]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298


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

  * Linux: CI_DRM_12332 -> Patchwork_110433v1

  CI-20190529: 20190529
  CI_DRM_12332: 601b2ef606e4b83d5518aa6a5011bb2b1c5954d9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7038: 5389b3f3b9b75df6bd8506e4aa3da357fd0c0ab1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_110433v1: 601b2ef606e4b83d5518aa6a5011bb2b1c5954d9 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

7e1ef3aa39ae drm/i915/mtl+: Don't enable the AUX_IO power for non-eDP port main links
29a2429eab20 drm/i915: Factor out function to get/put AUX_IO power for main link
d30ea721324a drm/i915: Add missing DC_OFF power domain->well mappings
89d3dd4acda8 drm/i915: Add missing AUX_IO_A power domain->well mappings
30f3b3a2f716 drm/i915/tgl+: Enable display DC power states on all eDP ports
651a184460d8 drm/i915: Move the POWER_DOMAIN_AUX_IO_A definition to its logical place
cbc6d34b9261 drm/i915: Allocate power domain set wakerefs dynamically

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/index.html

[-- Attachment #2: Type: text/html, Size: 5768 bytes --]

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/tgl+: Enable DC power states on all eDP ports

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 29899 bytes --]

== Series Details ==

Series: drm/i915/tgl+: Enable DC power states on all eDP ports
URL   : https://patchwork.freedesktop.org/series/110433/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12332_full -> Patchwork_110433v1_full
====================================================

Summary
-------

  **FAILURE**

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

  

Participating hosts (9 -> 9)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_suspend@debugfs-reader:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl6/igt@i915_suspend@debugfs-reader.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/igt@i915_suspend@debugfs-reader.html

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - shard-skl:          ([PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26], [PASS][27]) -> ([PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [FAIL][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50], [PASS][51], [PASS][52]) ([i915#5032])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl9/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl9/boot.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl9/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl7/boot.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl7/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl7/boot.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl6/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl6/boot.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl6/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl5/boot.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl5/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl5/boot.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl4/boot.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl4/boot.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl4/boot.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/boot.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/boot.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/boot.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl1/boot.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl1/boot.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl1/boot.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl10/boot.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl10/boot.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl10/boot.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl10/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/boot.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/boot.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/boot.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl7/boot.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl7/boot.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl7/boot.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/boot.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/boot.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/boot.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/boot.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl5/boot.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl5/boot.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl5/boot.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl5/boot.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/boot.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/boot.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/boot.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/boot.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/boot.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/boot.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/boot.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/boot.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl10/boot.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl10/boot.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl10/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         [PASS][53] -> [SKIP][54] ([i915#658])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@feature_discovery@psr2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@feature_discovery@psr2.html

  * igt@gem_exec_balancer@parallel-keep-in-fence:
    - shard-iclb:         [PASS][55] -> [SKIP][56] ([i915#4525])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@gem_exec_balancer@parallel-keep-in-fence.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb8/igt@gem_exec_balancer@parallel-keep-in-fence.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-glk:          [PASS][57] -> [FAIL][58] ([i915#2846])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-glk8/igt@gem_exec_fair@basic-deadline.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-glk7/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][59] -> [FAIL][60] ([i915#2842])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-glk7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-glk1/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_lmem_swapping@basic:
    - shard-skl:          NOTRUN -> [SKIP][61] ([fdo#109271] / [i915#4613]) +2 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@parallel-random:
    - shard-apl:          NOTRUN -> [SKIP][62] ([fdo#109271] / [i915#4613]) +2 similar issues
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl2/igt@gem_lmem_swapping@parallel-random.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [INCOMPLETE][63] ([i915#7248])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl1/igt@gem_pread@exhaustion.html

  * igt@gem_softpin@evict-single-offset:
    - shard-apl:          NOTRUN -> [FAIL][64] ([i915#4171])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl2/igt@gem_softpin@evict-single-offset.html

  * igt@gem_tiled_wb:
    - shard-skl:          NOTRUN -> [TIMEOUT][65] ([i915#6990])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/igt@gem_tiled_wb.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][66] -> [FAIL][67] ([i915#3989] / [i915#454])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb8/igt@i915_pm_dc@dc6-psr.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb3/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-skl:          [PASS][68] -> [DMESG-FAIL][69] ([i915#5334])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl7/igt@i915_selftest@live@gt_heartbeat.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-c-edp-1:
    - shard-skl:          [PASS][70] -> [FAIL][71] ([i915#2521])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl5/igt@kms_async_flips@alternate-sync-async-flip@pipe-c-edp-1.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/igt@kms_async_flips@alternate-sync-async-flip@pipe-c-edp-1.html

  * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][72] ([fdo#109271] / [i915#3886]) +5 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl8/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@hdmi-edid-change-during-suspend:
    - shard-apl:          NOTRUN -> [SKIP][73] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl8/igt@kms_chamelium@hdmi-edid-change-during-suspend.html

  * igt@kms_color_chamelium@gamma:
    - shard-skl:          NOTRUN -> [SKIP][74] ([fdo#109271] / [fdo#111827])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@kms_color_chamelium@gamma.html

  * igt@kms_cursor_crc@cursor-sliding-32x32:
    - shard-apl:          NOTRUN -> [SKIP][75] ([fdo#109271]) +63 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl2/igt@kms_cursor_crc@cursor-sliding-32x32.html

  * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy:
    - shard-skl:          NOTRUN -> [FAIL][76] ([i915#2346])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor@toggle:
    - shard-iclb:         [PASS][77] -> [FAIL][78] ([i915#2346]) +3 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb5/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html

  * igt@kms_flip@flip-vs-expired-vblank@c-edp1:
    - shard-skl:          [PASS][79] -> [FAIL][80] ([i915#79]) +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html

  * igt@kms_flip@plain-flip-fb-recreate@a-edp1:
    - shard-skl:          [PASS][81] -> [FAIL][82] ([i915#2122]) +2 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl4/igt@kms_flip@plain-flip-fb-recreate@a-edp1.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/igt@kms_flip@plain-flip-fb-recreate@a-edp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][83] ([i915#3555]) +1 similar issue
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][84] ([i915#2672]) +3 similar issues
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-downscaling@pipe-a-valid-mode:
    - shard-iclb:         NOTRUN -> [SKIP][85] ([i915#2587] / [i915#2672]) +2 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-downscaling@pipe-a-valid-mode.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-skl:          NOTRUN -> [SKIP][86] ([fdo#109271]) +59 similar issues
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_plane_alpha_blend@alpha-basic@pipe-a-dp-1:
    - shard-apl:          NOTRUN -> [FAIL][87] ([i915#4573]) +2 similar issues
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl2/igt@kms_plane_alpha_blend@alpha-basic@pipe-a-dp-1.html

  * igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2:
    - shard-glk:          [PASS][88] -> [DMESG-WARN][89] ([i915#118])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-glk9/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-glk3/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-sf:
    - shard-skl:          NOTRUN -> [SKIP][90] ([fdo#109271] / [i915#658]) +1 similar issue
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@kms_psr2_sf@cursor-plane-move-continuous-sf.html

  * igt@kms_psr2_su@page_flip-p010@pipe-b-edp-1:
    - shard-iclb:         NOTRUN -> [FAIL][91] ([i915#5939]) +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb2/igt@kms_psr2_su@page_flip-p010@pipe-b-edp-1.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][92] -> [SKIP][93] ([fdo#109441]) +2 similar issues
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@kms_psr@psr2_cursor_blt.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-skl:          NOTRUN -> [SKIP][94] ([fdo#109271] / [i915#2437])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@perf_pmu@rc6-suspend:
    - shard-skl:          [PASS][95] -> [INCOMPLETE][96] ([i915#7252])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/igt@perf_pmu@rc6-suspend.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl10/igt@perf_pmu@rc6-suspend.html

  * igt@sysfs_clients@split-10:
    - shard-skl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#2994])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl1/igt@sysfs_clients@split-10.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-tglb:         [FAIL][98] ([i915#2842]) -> [PASS][99]
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-tglb3/igt@gem_exec_fair@basic-none-share@rcs0.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-tglb6/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-iclb:         [FAIL][100] ([i915#2842]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb5/igt@gem_exec_fair@basic-pace@vecs0.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb7/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_mmap_offset@bad-object:
    - shard-skl:          [DMESG-WARN][102] ([i915#1982]) -> [PASS][103] +3 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl4/igt@gem_mmap_offset@bad-object.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/igt@gem_mmap_offset@bad-object.html

  * igt@i915_pm_rc6_residency@rc6-idle@vcs0:
    - shard-skl:          [WARN][104] ([i915#1804]) -> [PASS][105]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl7/igt@i915_pm_rc6_residency@rc6-idle@vcs0.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/igt@i915_pm_rc6_residency@rc6-idle@vcs0.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - shard-iclb:         [FAIL][106] -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb3/igt@i915_pm_rpm@basic-pci-d3-state.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rps@engine-order:
    - shard-apl:          [FAIL][108] ([i915#6537]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl8/igt@i915_pm_rps@engine-order.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl1/igt@i915_pm_rps@engine-order.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1:
    - shard-skl:          [FAIL][110] ([i915#2122]) -> [PASS][111] +1 similar issue
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl5/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl7/igt@kms_flip@flip-vs-absolute-wf_vblank@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
    - shard-skl:          [FAIL][112] ([i915#79]) -> [PASS][113] +2 similar issues
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html

  * igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-1:
    - shard-glk:          [DMESG-FAIL][114] ([i915#118]) -> [PASS][115]
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-glk9/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-1.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-glk3/igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-1.html

  * igt@kms_plane_lowres@tiling-y@pipe-c-hdmi-a-1:
    - shard-glk:          [FAIL][116] ([i915#7307]) -> [PASS][117] +1 similar issue
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-glk9/igt@kms_plane_lowres@tiling-y@pipe-c-hdmi-a-1.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-glk3/igt@kms_plane_lowres@tiling-y@pipe-c-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1:
    - shard-iclb:         [SKIP][118] ([i915#5176]) -> [PASS][119] +2 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html

  * igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1:
    - shard-iclb:         [SKIP][120] ([i915#5235]) -> [PASS][121] +2 similar issues
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html

  * igt@kms_psr_stress_test@invalidate-primary-flip-overlay:
    - shard-iclb:         [SKIP][122] ([i915#5519]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb5/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb7/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][124] ([i915#180]) -> [PASS][125] +3 similar issues
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl2/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl1/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][126] ([i915#1542]) -> [PASS][127]
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl9/igt@perf@polling.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl9/igt@perf@polling.html

  * igt@prime_mmap_coherency@ioctl-errors:
    - shard-skl:          [INCOMPLETE][128] ([i915#2295]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/igt@prime_mmap_coherency@ioctl-errors.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl4/igt@prime_mmap_coherency@ioctl-errors.html

  * igt@syncobj_wait@wait-all-for-submit-snapshot:
    - shard-skl:          [FAIL][130] ([i915#7109]) -> [PASS][131]
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl10/igt@syncobj_wait@wait-all-for-submit-snapshot.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl10/igt@syncobj_wait@wait-all-for-submit-snapshot.html

  
#### Warnings ####

  * igt@gem_exec_balancer@parallel-ordering:
    - shard-iclb:         [SKIP][132] ([i915#4525]) -> [FAIL][133] ([i915#6117])
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb3/igt@gem_exec_balancer@parallel-ordering.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@gem_exec_balancer@parallel-ordering.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-skl:          [TIMEOUT][134] ([i915#7248]) -> [INCOMPLETE][135] ([i915#7248])
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-skl3/igt@gem_pwrite@basic-exhaustion.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-skl6/igt@gem_pwrite@basic-exhaustion.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][136] ([i915#588]) -> [SKIP][137] ([i915#658])
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf:
    - shard-iclb:         [SKIP][138] ([i915#2920]) -> [SKIP][139] ([i915#658])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb1/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
    - shard-iclb:         [SKIP][140] ([i915#658]) -> [SKIP][141] ([i915#2920])
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb8/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
    - shard-iclb:         [SKIP][142] ([fdo#111068] / [i915#658]) -> [SKIP][143] ([i915#2920])
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-iclb1/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-iclb2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][144], [FAIL][145], [FAIL][146], [FAIL][147], [FAIL][148], [FAIL][149]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312]) -> ([FAIL][150], [FAIL][151]) ([i915#3002] / [i915#4312])
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl2/igt@runner@aborted.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl3/igt@runner@aborted.html
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl3/igt@runner@aborted.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl8/igt@runner@aborted.html
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl1/igt@runner@aborted.html
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12332/shard-apl6/igt@runner@aborted.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl7/igt@runner@aborted.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110433v1/shard-apl8/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1804]: https://gitlab.freedesktop.org/drm/intel/issues/1804
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4171]: https://gitlab.freedesktop.org/drm/intel/issues/4171
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5032]: https://gitlab.freedesktop.org/drm/intel/issues/5032
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#5939]: https://gitlab.freedesktop.org/drm/intel/issues/5939
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6537]: https://gitlab.freedesktop.org/drm/intel/issues/6537
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6990]: https://gitlab.freedesktop.org/drm/intel/issues/6990
  [i915#7109]: https://gitlab.freedesktop.org/drm/intel/issues/7109
  [i915#7248]: https://gitlab.freedesktop.org/drm/intel/issues/7248
  [i915#7252]: https://gitlab.freedesktop.org/drm/intel/issues/7252
  [i915#7307]: https://gitlab.freedesktop.org/drm/intel/issues/7307
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79


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

  * Linux: CI_DRM_12332 -> Patchwork_110433v1

  CI-20190529: 20190529
  CI_DRM_12332: 601b2ef606e4b83d5518aa6a5011bb2b1c5954d9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7038: 5389b3f3b9b75df6bd8506e4aa3da357fd0c0ab1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_110433v1: 601b2ef606e4b83d5518aa6a5011bb2b1c5954d9 @ 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_110433v1/index.html

[-- Attachment #2: Type: text/html, Size: 34320 bytes --]

Re: [Intel-gfx] [PATCH 3/7] drm/i915/tgl+: Enable display DC power states on all eDP ports

[Imre Deak]

On Wed, Nov 02, 2022 at 09:07:19PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 02, 2022 at 08:34:48PM +0200, Imre Deak wrote:
> > On Wed, Nov 02, 2022 at 07:35:17PM +0200, Ville Syrjälä wrote:
> > > On Wed, Nov 02, 2022 at 07:15:26PM +0200, Imre Deak wrote:
> > > > Starting with TGL eDP is supported on ports B+ (besides port A), so make
> > > > sure DC states are not blocked on any such ports. For this add an
> > > > AUX_IO_<port> power domain for each port with eDP support. These domains
> > > > similarly to AUX_IO_A enable only the AUX_IO_<port> power well for an
> > > > enabled port, whereas the existing AUX_<port> domains enable both the
> > > > AUX_IO_<port> and the DC_OFF power wells as required by DP AUX transfers.
> > > > 
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_ddi.c      |  8 ++-
> > > >  .../drm/i915/display/intel_display_power.c    | 30 +++++++++++
> > > >  .../drm/i915/display/intel_display_power.h    |  7 +++
> > > >  .../i915/display/intel_display_power_map.c    | 53 +++++++++++++++++--
> > > >  4 files changed, 91 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index e95bde5cf060e..00b577a5b9a76 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -848,6 +848,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> > > >  static enum intel_display_power_domain
> > > >  intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> > > >  {
> > > > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > > > +
> > > >  	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> > > >  	 * DC states enabled at the same time, while for driver initiated AUX
> > > >  	 * transfers we need the same AUX IOs to be powered but with DC states
> > > > @@ -860,8 +862,10 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> > > >  	 * Note that PSR is enabled only on Port A even though this function
> > > >  	 * returns the correct domain for other ports too.
> > > >  	 */
> > > > -	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
> > > > -					      intel_aux_power_domain(dig_port);
> > > > +	if (intel_dp_is_edp(&dig_port->dp))
> > > > +		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> > > > +	else
> > > > +		return intel_aux_power_domain(dig_port);
> > > 
> > > This seems to have two distinct changes in it:
> > > 1) define more AUX_IO domains
> > > 2) don't use AUX_IO for external DP and instead use the full AUX domain
> > 
> > By 2) you mean on port A full AUX is used for external DP, then yes.
> > 
> > > So seems like this needs to be split.
> > 
> > Ok, can add a patch with only a
> > 
> > 	if (is_edp(port) && port == A)
> > 		return AUX_IO_A;
> > 	else
> > 		return AUX_A;
> > 
> > change followed by this one, if that's what you meant.
> > 
> > > Also why do we need 2)?
> > 
> > The DC5/6 states only make sense with PSR enabled. Even though we enable
> > these atm for external outputs enabled on port A as well (for instance
> > DG2), the firmware will not actually enter the DC power states in this
> > case. Since for TypeC ports the AUX domain/power well also depends on
> > the TBT vs. non-TBT mode, the easiest way to select between the AUX_IO
> > vs. AUX domain was eDP vs. non-eDP (see also below).
> 
> OK, so you're saying for combo ports we don't actually need it and the
> current code works fine. So from the pure hw pov the sufficient thing
> would be more or less just this?
> 
> if (is_tc_port)
> 	return aux_domain;
> else
> 	return aux_io_domain;
>
> is_edp() I consider to be a software defined concept.
> It doesn't really change how the hardware actually
> works. Which is why I don't really like using it for
> anything. I've noticed people have a bad habit of 
> trying to use it incorrectly in places where eg. a
> specific port is what we actually want to check.

Yes, atm checking the port alone would be enough, but on MTL+ only eDP
needs AUX (for PSR) and external outputs don't need it on any port, see
patch 7.

> > > >  }
> > > >  
> > > >  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > index da0047359446e..ca453518e7fd7 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > @@ -131,6 +131,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> > > >  		return "AUDIO_PLAYBACK";
> > > >  	case POWER_DOMAIN_AUX_IO_A:
> > > >  		return "AUX_IO_A";
> > > > +	case POWER_DOMAIN_AUX_IO_B:
> > > > +		return "AUX_IO_B";
> > > > +	case POWER_DOMAIN_AUX_IO_C:
> > > > +		return "AUX_IO_C";
> > > > +	case POWER_DOMAIN_AUX_IO_D:
> > > > +		return "AUX_IO_D";
> > > > +	case POWER_DOMAIN_AUX_IO_E:
> > > > +		return "AUX_IO_E";
> > > > +	case POWER_DOMAIN_AUX_IO_F:
> > > > +		return "AUX_IO_F";
> > > >  	case POWER_DOMAIN_AUX_A:
> > > >  		return "AUX_A";
> > > >  	case POWER_DOMAIN_AUX_B:
> > > > @@ -2332,6 +2342,7 @@ struct intel_ddi_port_domains {
> > > >  
> > > >  	enum intel_display_power_domain ddi_lanes;
> > > >  	enum intel_display_power_domain ddi_io;
> > > > +	enum intel_display_power_domain aux_io;
> > > >  	enum intel_display_power_domain aux_legacy_usbc;
> > > >  	enum intel_display_power_domain aux_tbt;
> > > >  };
> > > > @@ -2346,6 +2357,7 @@ i9xx_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > > >  	},
> > > > @@ -2361,6 +2373,7 @@ d11_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > > >  	}, {
> > > > @@ -2371,6 +2384,7 @@ d11_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_C,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_C,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_C,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_C,
> > > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > > >  	},
> > > > @@ -2386,6 +2400,7 @@ d12_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > > >  	}, {
> > > > @@ -2396,6 +2411,7 @@ d12_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > > > +		.aux_io = POWER_DOMAIN_INVALID,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> > > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > > >  	},
> > > > @@ -2411,6 +2427,7 @@ d13_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_A,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_A,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_A,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_A,
> > > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > > >  	}, {
> > > > @@ -2421,6 +2438,7 @@ d13_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_TC1,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_TC1,
> > > > +		.aux_io = POWER_DOMAIN_INVALID,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_USBC1,
> > > >  		.aux_tbt = POWER_DOMAIN_AUX_TBT1,
> > > >  	}, {
> > > > @@ -2431,6 +2449,7 @@ d13_port_domains[] = {
> > > >  
> > > >  		.ddi_lanes = POWER_DOMAIN_PORT_DDI_LANES_D,
> > > >  		.ddi_io = POWER_DOMAIN_PORT_DDI_IO_D,
> > > > +		.aux_io = POWER_DOMAIN_AUX_IO_D,
> > > >  		.aux_legacy_usbc = POWER_DOMAIN_AUX_D,
> > > >  		.aux_tbt = POWER_DOMAIN_INVALID,
> > > >  	},
> > > > @@ -2508,6 +2527,17 @@ intel_port_domains_for_aux_ch(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > > >  	return NULL;
> > > >  }
> > > >  
> > > > +enum intel_display_power_domain
> > > > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > > > +{
> > > > +	const struct intel_ddi_port_domains *domains = intel_port_domains_for_aux_ch(i915, aux_ch);
> > > > +
> > > > +	if (drm_WARN_ON(&i915->drm, !domains) || domains->aux_io == POWER_DOMAIN_INVALID)
> > > > +		return POWER_DOMAIN_AUX_IO_A;
> > > 
> > > The POWER_DOMAIN_INVALID things is for TC ports I guess? 
> > 
> > Yes.
> > 
> > > Shouldn't this return the full AUX domain for the specific port for those?
> > 
> > At this level it's not well-defined, as that depends on the TBT vs.
> > non-TBT mode as well and as you point out they shouldn't be used.
> > 
> > > Not that you should ever have eDP on them I suppose, so given
> > > what you had in that earlier function I guess this should never
> > > happen.
> > 
> > Right.
> > 
> > I guess we could assume that the AUX_IO domain will be not required in
> > TBT mode (vs. the full AUX domain) and also define/return from here new
> > AUX_IO_USBC[1-6] domains which wouldn't explicitly disable DC states.
> > Right now only non-TC ports support eDP, so these domains wouldn't be
> > used, hence I opted for not defining them.
> 
> If this thing doesn't make sense for the type-c ports then
> we should probably just not use it for those. So just extending
> the WARN (assuming we want one) to cover that case is perhaps
> as well want to do.
> 
> > 
> > > > +
> > > > +	return domains->aux_io + (int)(aux_ch - domains->aux_ch_start);
> > > > +}
> > > > +
> > > >  enum intel_display_power_domain
> > > >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch)
> > > >  {
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > index fd68d43bfe322..54737e0aec21e 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > @@ -78,6 +78,11 @@ enum intel_display_power_domain {
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > > >  
> > > >  	POWER_DOMAIN_AUX_IO_A,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > > +	POWER_DOMAIN_AUX_IO_D,
> > > > +	POWER_DOMAIN_AUX_IO_E,
> > > > +	POWER_DOMAIN_AUX_IO_F,
> > > >  
> > > >  	POWER_DOMAIN_AUX_A,
> > > >  	POWER_DOMAIN_AUX_B,
> > > > @@ -250,6 +255,8 @@ intel_display_power_ddi_lanes_domain(struct drm_i915_private *i915, enum port po
> > > >  enum intel_display_power_domain
> > > >  intel_display_power_ddi_io_domain(struct drm_i915_private *i915, enum port port);
> > > >  enum intel_display_power_domain
> > > > +intel_display_power_aux_io_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > > > +enum intel_display_power_domain
> > > >  intel_display_power_legacy_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > > >  enum intel_display_power_domain
> > > >  intel_display_power_tbt_aux_domain(struct drm_i915_private *i915, enum aux_ch aux_ch);
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > > index 43454022e6a66..b82c0d0a80c5f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > > > @@ -170,6 +170,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_display,
> > > >  	POWER_DOMAIN_VGA,
> > > >  	POWER_DOMAIN_AUDIO_MMIO,
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_GMBUS,
> > > > @@ -179,6 +181,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > >  	POWER_DOMAIN_PORT_CRT,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > > @@ -186,6 +190,8 @@ I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_cmn_bc,
> > > >  I915_DECL_PW_DOMAINS(vlv_pwdoms_dpio_tx_bc_lanes,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > > @@ -243,6 +249,9 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> > > >  	POWER_DOMAIN_VGA,
> > > >  	POWER_DOMAIN_AUDIO_MMIO,
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > > +	POWER_DOMAIN_AUX_IO_D,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_AUX_D,
> > > > @@ -252,12 +261,15 @@ I915_DECL_PW_DOMAINS(chv_pwdoms_display,
> > > >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_bc,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > >  I915_DECL_PW_DOMAINS(chv_pwdoms_dpio_cmn_d,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_D,
> > > > +	POWER_DOMAIN_AUX_IO_D,
> > > >  	POWER_DOMAIN_AUX_D,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > > @@ -305,6 +317,9 @@ static const struct i915_power_well_desc_list chv_power_wells[] = {
> > > >  	POWER_DOMAIN_VGA, \
> > > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > > +	POWER_DOMAIN_AUX_IO_B, \
> > > > +	POWER_DOMAIN_AUX_IO_C, \
> > > > +	POWER_DOMAIN_AUX_IO_D, \
> > > >  	POWER_DOMAIN_AUX_B, \
> > > >  	POWER_DOMAIN_AUX_C, \
> > > >  	POWER_DOMAIN_AUX_D
> > > > @@ -407,6 +422,8 @@ static const struct i915_power_well_desc_list skl_power_wells[] = {
> > > >  	POWER_DOMAIN_VGA, \
> > > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > > +	POWER_DOMAIN_AUX_IO_B, \
> > > > +	POWER_DOMAIN_AUX_IO_C, \
> > > >  	POWER_DOMAIN_AUX_B, \
> > > >  	POWER_DOMAIN_AUX_C
> > > >  
> > > > @@ -430,6 +447,8 @@ I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_a,
> > > >  I915_DECL_PW_DOMAINS(bxt_pwdoms_dpio_cmn_bc,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > > @@ -483,6 +502,8 @@ static const struct i915_power_well_desc_list bxt_power_wells[] = {
> > > >  	POWER_DOMAIN_VGA, \
> > > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > > +	POWER_DOMAIN_AUX_IO_B, \
> > > > +	POWER_DOMAIN_AUX_IO_C, \
> > > >  	POWER_DOMAIN_AUX_B, \
> > > >  	POWER_DOMAIN_AUX_C
> > > >  
> > > > @@ -509,11 +530,13 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_a,
> > > >  
> > > >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_b,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_B,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > >  I915_DECL_PW_DOMAINS(glk_pwdoms_dpio_cmn_c,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_C,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > > @@ -523,10 +546,12 @@ I915_DECL_PW_DOMAINS(glk_pwdoms_aux_a,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_b,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > >  	POWER_DOMAIN_AUX_B,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > >  I915_DECL_PW_DOMAINS(glk_pwdoms_aux_c,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > >  	POWER_DOMAIN_AUX_C,
> > > >  	POWER_DOMAIN_INIT);
> > > >  
> > > > @@ -617,6 +642,11 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_pw_4,
> > > >  	POWER_DOMAIN_VGA, \
> > > >  	POWER_DOMAIN_AUDIO_MMIO, \
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > > +	POWER_DOMAIN_AUX_IO_B, \
> > > > +	POWER_DOMAIN_AUX_IO_C, \
> > > > +	POWER_DOMAIN_AUX_IO_D, \
> > > > +	POWER_DOMAIN_AUX_IO_E, \
> > > > +	POWER_DOMAIN_AUX_IO_F, \
> > > >  	POWER_DOMAIN_AUX_B, \
> > > >  	POWER_DOMAIN_AUX_C, \
> > > >  	POWER_DOMAIN_AUX_D, \
> > > > @@ -660,11 +690,21 @@ I915_DECL_PW_DOMAINS(icl_pwdoms_ddi_io_f,	POWER_DOMAIN_PORT_DDI_IO_F);
> > > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_a,
> > > >  	POWER_DOMAIN_AUX_IO_A,
> > > >  	POWER_DOMAIN_AUX_A);
> > > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,		POWER_DOMAIN_AUX_B);
> > > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,		POWER_DOMAIN_AUX_C);
> > > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,		POWER_DOMAIN_AUX_D);
> > > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,		POWER_DOMAIN_AUX_E);
> > > > -I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,		POWER_DOMAIN_AUX_F);
> > > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_b,
> > > > +	POWER_DOMAIN_AUX_IO_B,
> > > > +	POWER_DOMAIN_AUX_B);
> > > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_c,
> > > > +	POWER_DOMAIN_AUX_IO_C,
> > > > +	POWER_DOMAIN_AUX_C);
> > > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_d,
> > > > +	POWER_DOMAIN_AUX_IO_D,
> > > > +	POWER_DOMAIN_AUX_D);
> > > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_e,
> > > > +	POWER_DOMAIN_AUX_IO_E,
> > > > +	POWER_DOMAIN_AUX_E);
> > > > +I915_DECL_PW_DOMAINS(icl_pwdoms_aux_f,
> > > > +	POWER_DOMAIN_AUX_IO_F,
> > > > +	POWER_DOMAIN_AUX_F);
> > > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt1,	POWER_DOMAIN_AUX_TBT1);
> > > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt2,	POWER_DOMAIN_AUX_TBT2);
> > > >  I915_DECL_PW_DOMAINS(icl_pwdoms_aux_tbt3,	POWER_DOMAIN_AUX_TBT3);
> > > > @@ -1215,6 +1255,9 @@ I915_DECL_PW_DOMAINS(xelpd_pwdoms_pw_a,
> > > >  	POWER_DOMAIN_PORT_DDI_LANES_TC4, \
> > > >  	POWER_DOMAIN_VGA, \
> > > >  	POWER_DOMAIN_AUDIO_PLAYBACK, \
> > > > +	POWER_DOMAIN_AUX_IO_C, \
> > > > +	POWER_DOMAIN_AUX_IO_D, \
> > > > +	POWER_DOMAIN_AUX_IO_E, \
> > > >  	POWER_DOMAIN_AUX_C, \
> > > >  	POWER_DOMAIN_AUX_D, \
> > > >  	POWER_DOMAIN_AUX_E, \
> > > > -- 
> > > > 2.37.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/7] drm/i915: Allocate power domain set wakerefs dynamically

[Imre Deak]

On Wed, Nov 02, 2022 at 08:57:21PM +0200, Jani Nikula wrote:
> On Wed, 02 Nov 2022, Imre Deak <imre.deak@intel.com> wrote:
> > Since the intel_display_power_domain_set struct, currently its current
> > size close 1kB, can be allocated on the stack, it's better to allocate
> > the per-domain wakeref pointer array - used for debugging - within the
> > struct dynamically, so do this.
> >
> > The memory freeing is guaranteed by the fact that the acquired domain
> > references tracked by struct can't be leaked either.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_power.c    | 61 ++++++++++++++++---
> >  .../drm/i915/display/intel_display_power.h    |  2 +-
> >  2 files changed, 53 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 4c1de91e56ff9..e2da91c2a9638 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -830,19 +830,58 @@ void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
> >  }
> >  #endif
> >  
> > +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> > +static intel_wakeref_t *
> > +get_debug_wakerefs(struct drm_i915_private *i915,
> > +		   struct intel_display_power_domain_set *power_domain_set)
> > +{
> > +	if (power_domain_set->wakerefs)
> > +		return power_domain_set->wakerefs;
> > +
> > +	power_domain_set->wakerefs = kcalloc(POWER_DOMAIN_NUM,
> > +					     sizeof(*power_domain_set->wakerefs),
> > +					     GFP_KERNEL);
> > +
> > +	drm_WARN_ON_ONCE(&i915->drm, !power_domain_set->wakerefs);
> 
> The rule of thumb is not to warn or log on allocation failures.

Ok, will drop this.

> 
> > +
> > +	return power_domain_set->wakerefs;
> > +}
> > +
> > +static void
> > +free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
> > +{
> > +	if (power_domain_set->wakerefs &&
> > +	    bitmap_empty(power_domain_set->mask.bits, POWER_DOMAIN_NUM))
> > +		kfree(fetch_and_zero(&power_domain_set->wakerefs));
> 
> FWIW, I'm really not happy about fetch_and_zero() or its use anywhere. I
> kind of get the point, but the impression of any kind of atomicity the
> naming gives is totally misleading. And it's our own thing in
> i915_utils.h, and not a global thing like the name suggests.

Ok, here zeroing the pointer separately is clearer, will do that. For
intel_wakeref_t pointers fetch_and_zero() denotes that the ownership of
the reference is moved, which should never happen with a simple copy to
another pointer. Imo for that the use of it is justified.

> > +}
> > +#else
> > +static intel_wakeref_t *
> > +get_debug_wakerefs(struct drm_i915_private *i915,
> > +		   struct intel_display_power_domain_set *power_domain_set)
> > +{
> > +	return NULL;
> > +}
> > +
> > +static void
> > +free_empty_debug_wakerefs(struct intel_display_power_domain_set *power_domain_set)
> > +{
> > +}
> > +#endif
> 
> get/free is an odd pairing of names.
> 
> > +
> >  void
> >  intel_display_power_get_in_set(struct drm_i915_private *i915,
> >  			       struct intel_display_power_domain_set *power_domain_set,
> >  			       enum intel_display_power_domain domain)
> >  {
> >  	intel_wakeref_t __maybe_unused wf;
> > +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
> >  
> >  	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
> >  
> >  	wf = intel_display_power_get(i915, domain);
> > -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> > -	power_domain_set->wakerefs[domain] = wf;
> > -#endif
> > +	if (debug_wakerefs)
> > +		debug_wakerefs[domain] = wf;
> > +
> 
> If you abstracted setting the debug wakeref for a domain, it could
> handle the allocation internally without any of the local vars etc.
> here. And it would only be a single line in the entire function.

Things here could be simplified by abstracting adding both the domain
and the wakeref to power_domain_set, can do that (calling
add_domain_to_set() here and remove_domain_from_set() in the *_put()
functions).

> 
> >  	set_bit(domain, power_domain_set->mask.bits);
> >  }
> >  
> > @@ -852,6 +891,7 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
> >  					  enum intel_display_power_domain domain)
> >  {
> >  	intel_wakeref_t wf;
> > +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
> >  
> >  	drm_WARN_ON(&i915->drm, test_bit(domain, power_domain_set->mask.bits));
> >  
> > @@ -859,9 +899,9 @@ intel_display_power_get_in_set_if_enabled(struct drm_i915_private *i915,
> >  	if (!wf)
> >  		return false;
> >  
> > -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> > -	power_domain_set->wakerefs[domain] = wf;
> > -#endif
> > +	if (debug_wakerefs)
> > +		debug_wakerefs[domain] = wf;
> > +
> >  	set_bit(domain, power_domain_set->mask.bits);
> >  
> >  	return true;
> > @@ -873,6 +913,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
> >  				    struct intel_power_domain_mask *mask)
> >  {
> >  	enum intel_display_power_domain domain;
> > +	intel_wakeref_t *debug_wakerefs = get_debug_wakerefs(i915, power_domain_set);
> >  
> >  	drm_WARN_ON(&i915->drm,
> >  		    !bitmap_subset(mask->bits, power_domain_set->mask.bits, POWER_DOMAIN_NUM));
> > @@ -880,12 +921,14 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915,
> >  	for_each_power_domain(domain, mask) {
> >  		intel_wakeref_t __maybe_unused wf = -1;
> >  
> > -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> > -		wf = fetch_and_zero(&power_domain_set->wakerefs[domain]);
> > -#endif
> > +		if (debug_wakerefs)
> > +			wf = fetch_and_zero(&debug_wakerefs[domain]);
> > +
> >  		intel_display_power_put(i915, domain, wf);
> >  		clear_bit(domain, power_domain_set->mask.bits);
> >  	}
> > +
> > +	free_empty_debug_wakerefs(power_domain_set);
> >  }
> >  
> >  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 7136ea3f233e9..c847aab7b2f88 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -146,7 +146,7 @@ struct i915_power_domains {
> >  struct intel_display_power_domain_set {
> >  	struct intel_power_domain_mask mask;
> >  #ifdef CONFIG_DRM_I915_DEBUG_RUNTIME_PM
> > -	intel_wakeref_t wakerefs[POWER_DOMAIN_NUM];
> > +	intel_wakeref_t *wakerefs;
> >  #endif
> >  };
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 6/7] drm/i915: Factor out function to get/put AUX_IO power for main link

[Imre Deak]

On Wed, Nov 02, 2022 at 09:02:41PM +0200, Jani Nikula wrote:
> On Wed, 02 Nov 2022, Imre Deak <imre.deak@intel.com> wrote:
> > Factor out functions to get/put the AUX_IO power domain for the main
> > link on DDI ports.
> >
> > While at it clarify the corresponding code comment.
> >
> > No functional change.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 84 ++++++++++++++----------
> >  1 file changed, 51 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 00b577a5b9a76..7453772d2073d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -846,26 +846,63 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> >  }
> >  
> >  static enum intel_display_power_domain
> > -intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
> > +intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port,
> > +			       const struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> >  
> > -	/* ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> > +	/*
> > +	 * ICL+ HW requires corresponding AUX IOs to be powered up for PSR with
> >  	 * DC states enabled at the same time, while for driver initiated AUX
> >  	 * transfers we need the same AUX IOs to be powered but with DC states
> > -	 * disabled. Accordingly use the AUX power domain here which leaves DC
> > -	 * states enabled.
> > -	 * However, for non-A AUX ports the corresponding non-EDP transcoders
> > -	 * would have already enabled power well 2 and DC_OFF. This means we can
> > -	 * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
> > -	 * specific AUX_IO reference without powering up any extra wells.
> > -	 * Note that PSR is enabled only on Port A even though this function
> > -	 * returns the correct domain for other ports too.
> > +	 * disabled. Accordingly use the AUX_IO_<port> power domain here which
> > +	 * leaves DC states enabled.
> > +	 *
> > +	 * Before MTL all DP ports and HDMI ports on TypeC PHYs also require
> > +	 * AUX IO to be enabled, but all these require DC_OFF to be enabled as
> > +	 * well, so we can acquire a wider AUX_<port> power domain reference
> > +	 * instead of a specific AUX_IO_<port> reference without powering up any
> > +	 * extra wells.
> >  	 */
> >  	if (intel_dp_is_edp(&dig_port->dp))
> >  		return intel_display_power_aux_io_domain(i915, dig_port->aux_ch);
> > -	else
> > +	else if (intel_crtc_has_dp_encoder(crtc_state) ||
> > +		 intel_phy_is_tc(i915, phy))
> >  		return intel_aux_power_domain(dig_port);
> > +	else
> > +		return POWER_DOMAIN_INVALID;
> > +}
> > +
> > +static void
> > +get_aux_power_for_main_link(struct intel_digital_port *dig_port,
> > +			    const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	enum intel_display_power_domain domain =
> > +		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
> > +
> > +	drm_WARN_ON(&i915->drm, dig_port->aux_wakeref);
> > +
> > +	if (domain == POWER_DOMAIN_INVALID)
> > +		return;
> > +
> > +	dig_port->aux_wakeref = intel_display_power_get(i915, domain);
> > +}
> > +
> > +static void
> > +put_aux_power_for_main_link(struct intel_digital_port *dig_port,
> > +			    const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +	intel_wakeref_t wf = fetch_and_zero(&dig_port->aux_wakeref);
> > +	enum intel_display_power_domain domain =
> > +		intel_ddi_main_link_aux_domain(dig_port, crtc_state);
> > +
> > +	if (!wf)
> > +		return;
> > +
> > +	intel_display_power_put(i915, domain, wf);
> >  }
> 
> Nitpick, I think foo_get()/foo_put() are much more common in the driver
> than get_foo()/put_foo().

Ok, can rename these.

> >  static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> > @@ -873,7 +910,6 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_digital_port *dig_port;
> > -	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >  
> >  	/*
> >  	 * TODO: Add support for MST encoders. Atm, the following should never
> > @@ -892,17 +928,7 @@ static void intel_ddi_get_power_domains(struct intel_encoder *encoder,
> >  								   dig_port->ddi_io_power_domain);
> >  	}
> >  
> > -	/*
> > -	 * AUX power is only needed for (e)DP mode, and for HDMI mode on TC
> > -	 * ports.
> > -	 */
> > -	if (intel_crtc_has_dp_encoder(crtc_state) ||
> > -	    intel_phy_is_tc(dev_priv, phy)) {
> > -		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
> > -		dig_port->aux_wakeref =
> > -			intel_display_power_get(dev_priv,
> > -						intel_ddi_main_link_aux_domain(dig_port));
> > -	}
> > +	get_aux_power_for_main_link(dig_port, crtc_state);
> >  }
> >  
> >  void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
> > @@ -2741,10 +2767,7 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
> >  		intel_ddi_post_disable_dp(state, encoder, old_crtc_state,
> >  					  old_conn_state);
> >  
> > -	if (intel_crtc_has_dp_encoder(old_crtc_state) || is_tc_port)
> > -		intel_display_power_put(dev_priv,
> > -					intel_ddi_main_link_aux_domain(dig_port),
> > -					fetch_and_zero(&dig_port->aux_wakeref));
> > +	put_aux_power_for_main_link(dig_port, old_crtc_state);
> >  
> >  	if (is_tc_port)
> >  		intel_tc_port_put_link(dig_port);
> > @@ -3065,12 +3088,7 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
> >  	if (is_tc_port)
> >  		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> >  
> > -	if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port) {
> > -		drm_WARN_ON(&dev_priv->drm, dig_port->aux_wakeref);
> > -		dig_port->aux_wakeref =
> > -			intel_display_power_get(dev_priv,
> > -						intel_ddi_main_link_aux_domain(dig_port));
> > -	}
> > +	get_aux_power_for_main_link(dig_port, crtc_state);
> >  
> >  	if (is_tc_port && !intel_tc_port_in_tbt_alt_mode(dig_port))
> >  		/*
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center