drm/i915/icl: Fix HDMI on TypeC static ports

From ICL onwards all the DDI/TypeC ports - even working in HDMI mode -
need to know their corresponding AUX channel, so move the corresponding
helper to a common place.

No functional change.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 45
+++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   | 50 +--------------------------
------------
 3 files changed, 47 insertions(+), 49 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +1 -0

index c9e5bab6861b..c57b701f72a7 100644
@@ -3445,6 +3445,7 @@ bool intel_bios_is_port_hpd_inverted(struct
 drm_i915_private *dev_priv,
 				     enum port port);
 bool intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
 				enum port port);
+enum aux_ch intel_aux_ch(struct drm_i915_private *dev_priv, enum
 port port);
 

/* intel_acpi.c */
#ifdef CONFIG_ACPI

drivers/gpu/drm/i915/intel_bios.c +2 -0

index 1faa494e2bc9..c7682a470c6a 100644
@@ -2159,3 +2159,48 @@ intel_bios_is_lspcon_present(struct
 drm_i915_private *dev_priv,
 
 	return false;
 }
+
+enum aux_ch intel_aux_ch(struct drm_i915_private *dev_priv, enum

port port)

drivers/gpu/drm/i915/intel_bios.c +9 -0

+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[port];
+	enum aux_ch aux_ch;
+
+	if (!info->alternate_aux_channel) {
+		aux_ch = (enum aux_ch) port;
+
+		DRM_DEBUG_KMS("using AUX %c for port %c (platform

default)\n",

drivers/gpu/drm/i915/intel_bios.c +34 -0

+			      aux_ch_name(aux_ch), port_name(port));
+		return aux_ch;
+	}
+
+	switch (info->alternate_aux_channel) {
+	case DP_AUX_A:
+		aux_ch = AUX_CH_A;
+		break;
+	case DP_AUX_B:
+		aux_ch = AUX_CH_B;
+		break;
+	case DP_AUX_C:
+		aux_ch = AUX_CH_C;
+		break;
+	case DP_AUX_D:
+		aux_ch = AUX_CH_D;
+		break;
+	case DP_AUX_E:
+		aux_ch = AUX_CH_E;
+		break;
+	case DP_AUX_F:
+		aux_ch = AUX_CH_F;
+		break;
+	default:
+		MISSING_CASE(info->alternate_aux_channel);
+		aux_ch = AUX_CH_A;
+		break;
+	}
+
+	DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
+		      aux_ch_name(aux_ch), port_name(port));
+
+	return aux_ch;
+}

drivers/gpu/drm/i915/intel_dp.c +0 -48

index 6b37d66194a3..2445897b8f6c 100644
@@ -1503,54 +1503,6 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux,
 struct drm_dp_aux_msg *msg)
 	return ret;
 }
 
-static enum aux_ch intel_aux_ch(struct intel_dp *intel_dp)
-{
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
 >base;
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = encoder->port;
-	const struct ddi_vbt_port_info *info =
-		&dev_priv->vbt.ddi_port_info[port];
-	enum aux_ch aux_ch;
-
-	if (!info->alternate_aux_channel) {
-		aux_ch = (enum aux_ch) port;
-
-		DRM_DEBUG_KMS("using AUX %c for port %c (platform
 default)\n",
-			      aux_ch_name(aux_ch), port_name(port));
-		return aux_ch;
-	}
-
-	switch (info->alternate_aux_channel) {
-	case DP_AUX_A:
-		aux_ch = AUX_CH_A;
-		break;
-	case DP_AUX_B:
-		aux_ch = AUX_CH_B;
-		break;
-	case DP_AUX_C:
-		aux_ch = AUX_CH_C;
-		break;
-	case DP_AUX_D:
-		aux_ch = AUX_CH_D;
-		break;
-	case DP_AUX_E:
-		aux_ch = AUX_CH_E;
-		break;
-	case DP_AUX_F:
-		aux_ch = AUX_CH_F;
-		break;
-	default:
-		MISSING_CASE(info->alternate_aux_channel);
-		aux_ch = AUX_CH_A;
-		break;
-	}
-
-	DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n",
-		      aux_ch_name(aux_ch), port_name(port));
-
-	return aux_ch;
-}
-

static enum intel_display_power_domain intel_aux_power_domain(struct intel_dp *intel_dp) {

drivers/gpu/drm/i915/intel_dp.c +1 -1

@@ -1691,7 +1643,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
 >base;
 
-	intel_dp->aux_ch = intel_aux_ch(intel_dp);
+	intel_dp->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 

if (INTEL_GEN(dev_priv) >= 9) {

From ICL onwards all DDI/TypeC ports - even working in HDMI mode - need
to know their corresponding AUX CH, so move the field to a common struct.

No functional change.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c |  4 +++-
 drivers/gpu/drm/i915/intel_dp.c  | 35 +++++++++++++++++++++++-------
-----
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 27 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +2 -0

index e40a8c97d34b..32a080265d03 100644
@@ -2084,6 +2084,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 static inline enum intel_display_power_domain
 intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
 {
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2096,7 +2098,7 @@ intel_ddi_main_link_aux_domain(struct intel_dp
 *intel_dp)
 	 * Note that PSR is enabled only on Port A even though this

function

drivers/gpu/drm/i915/intel_ddi.c +1 -1

 	 * returns the correct domain for other ports too.
 	 */
-	return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
 					      intel_dp-

Quoted 1 line

aux_power_domain;

}

drivers/gpu/drm/i915/intel_dp.c +1 -0

index 2445897b8f6c..5530c604c694 100644
@@ -1156,6 +1156,7 @@ static uint32_t
 g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp,
 int index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	if (index)
 		return 0;

@@ -1165,7 +1166,7 @@ static uint32_t
 ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	 * like to run at 2MHz.  So, take the cdclk or PCH rawclk value

and

drivers/gpu/drm/i915/intel_dp.c +1 -1

 	 * divide by 2000 and use that
 	 */
-	if (intel_dp->aux_ch == AUX_CH_A)
+	if (dig_port->aux_ch == AUX_CH_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk.hw.cdclk,

2000);

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	else
 		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);

@@ -1174,8 +1175,9 @@ static uint32_t
 ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp,
 int index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 
-	if (intel_dp->aux_ch != AUX_CH_A && HAS_PCH_LPT_H(dev_priv)) {
+	if (dig_port->aux_ch != AUX_CH_A && HAS_PCH_LPT_H(dev_priv)) {
 		/* Workaround for non-ULT HSW */

drivers/gpu/drm/i915/intel_dp.c +3 -1

 		switch (index) {
 		case 0: return 63;

@@ -1506,7 +1508,9 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux,
 struct drm_dp_aux_msg *msg)
 static enum intel_display_power_domain
 intel_aux_power_domain(struct intel_dp *intel_dp)
 {
-	switch (intel_dp->aux_ch) {
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+
+	switch (dig_port->aux_ch) {
 	case AUX_CH_A:
 		return POWER_DOMAIN_AUX_A;

drivers/gpu/drm/i915/intel_dp.c +1 -1

 	case AUX_CH_B:

@@ -1520,7 +1524,7 @@ intel_aux_power_domain(struct intel_dp
 *intel_dp)
 	case AUX_CH_F:
 		return POWER_DOMAIN_AUX_F;
 	default:
-		MISSING_CASE(intel_dp->aux_ch);
+		MISSING_CASE(dig_port->aux_ch);
 		return POWER_DOMAIN_AUX_A;
 	}

}

drivers/gpu/drm/i915/intel_dp.c +2 -1

@@ -1528,7 +1532,8 @@ intel_aux_power_domain(struct intel_dp
 *intel_dp)
 static i915_reg_t g4x_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_B:

@@ -1544,7 +1549,8 @@ static i915_reg_t g4x_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t g4x_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	switch (aux_ch) {
 	case AUX_CH_B:

@@ -1560,7 +1566,8 @@ static i915_reg_t g4x_aux_data_reg(struct
 intel_dp *intel_dp, int index)
 static i915_reg_t ilk_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_A:

@@ -1578,7 +1585,8 @@ static i915_reg_t ilk_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t ilk_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	switch (aux_ch) {
 	case AUX_CH_A:

@@ -1596,7 +1604,8 @@ static i915_reg_t ilk_aux_data_reg(struct
 intel_dp *intel_dp, int index)
 static i915_reg_t skl_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_A:

@@ -1615,7 +1624,8 @@ static i915_reg_t skl_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t skl_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +3 -2

 	switch (aux_ch) {
 	case AUX_CH_A:

@@ -1641,9 +1651,10 @@ static void
 intel_dp_aux_init(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
 >base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &dig_port->base;
 
-	intel_dp->aux_ch = intel_aux_ch(dev_priv, encoder->port);
+	dig_port->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	if (INTEL_GEN(dev_priv) >= 9) {

drivers/gpu/drm/i915/intel_drv.h +0 -1

index 268afb6d2746..a242a118389d 100644
@@ -1109,7 +1109,6 @@ struct intel_dp {
 	bool link_trained;
 	bool has_audio;
 	bool reset_link_params;
-	enum aux_ch aux_ch;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
 	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];

Quoted 204 lines · Imre Deak

From ICL onwards all DDI/TypeC ports - even working in HDMI mode - need
to know their corresponding AUX CH, so move the field to a common struct.

No functional change.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c |  4 +++-
 drivers/gpu/drm/i915/intel_dp.c  | 35 +++++++++++++++++++++++-------
-----
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 27 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +2 -0

index e40a8c97d34b..32a080265d03 100644
@@ -2084,6 +2084,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 static inline enum intel_display_power_domain
 intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
 {
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2096,7 +2098,7 @@ intel_ddi_main_link_aux_domain(struct intel_dp
 *intel_dp)
 	 * Note that PSR is enabled only on Port A even though this

function

drivers/gpu/drm/i915/intel_ddi.c +1 -1

 	 * returns the correct domain for other ports too.
 	 */
-	return intel_dp->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
+	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
 					      intel_dp-

Quoted 1 line

aux_power_domain;

}

drivers/gpu/drm/i915/intel_dp.c +1 -0

index 2445897b8f6c..5530c604c694 100644
@@ -1156,6 +1156,7 @@ static uint32_t
 g4x_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp,
 int index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	if (index)
 		return 0;

@@ -1165,7 +1166,7 @@ static uint32_t
 ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 	 * like to run at 2MHz.  So, take the cdclk or PCH rawclk value

and

drivers/gpu/drm/i915/intel_dp.c +1 -1

 	 * divide by 2000 and use that
 	 */
-	if (intel_dp->aux_ch == AUX_CH_A)
+	if (dig_port->aux_ch == AUX_CH_A)
 		return DIV_ROUND_CLOSEST(dev_priv->cdclk.hw.cdclk,

2000);

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	else
 		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);

@@ -1174,8 +1175,9 @@ static uint32_t
 ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
 static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp,
 int index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 
-	if (intel_dp->aux_ch != AUX_CH_A && HAS_PCH_LPT_H(dev_priv)) {
+	if (dig_port->aux_ch != AUX_CH_A && HAS_PCH_LPT_H(dev_priv)) {
 		/* Workaround for non-ULT HSW */

drivers/gpu/drm/i915/intel_dp.c +3 -1

 		switch (index) {
 		case 0: return 63;

@@ -1506,7 +1508,9 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux,
 struct drm_dp_aux_msg *msg)
 static enum intel_display_power_domain
 intel_aux_power_domain(struct intel_dp *intel_dp)
 {
-	switch (intel_dp->aux_ch) {
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+
+	switch (dig_port->aux_ch) {
 	case AUX_CH_A:
 		return POWER_DOMAIN_AUX_A;

drivers/gpu/drm/i915/intel_dp.c +1 -1

 	case AUX_CH_B:

@@ -1520,7 +1524,7 @@ intel_aux_power_domain(struct intel_dp
 *intel_dp)
 	case AUX_CH_F:
 		return POWER_DOMAIN_AUX_F;
 	default:
-		MISSING_CASE(intel_dp->aux_ch);
+		MISSING_CASE(dig_port->aux_ch);
 		return POWER_DOMAIN_AUX_A;
 	}

}

drivers/gpu/drm/i915/intel_dp.c +2 -1

@@ -1528,7 +1532,8 @@ intel_aux_power_domain(struct intel_dp
 *intel_dp)
 static i915_reg_t g4x_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_B:

@@ -1544,7 +1549,8 @@ static i915_reg_t g4x_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t g4x_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	switch (aux_ch) {
 	case AUX_CH_B:

@@ -1560,7 +1566,8 @@ static i915_reg_t g4x_aux_data_reg(struct
 intel_dp *intel_dp, int index)
 static i915_reg_t ilk_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_A:

@@ -1578,7 +1585,8 @@ static i915_reg_t ilk_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t ilk_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	switch (aux_ch) {
 	case AUX_CH_A:

@@ -1596,7 +1604,8 @@ static i915_reg_t ilk_aux_data_reg(struct
 intel_dp *intel_dp, int index)
 static i915_reg_t skl_aux_ctl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 
 	switch (aux_ch) {

drivers/gpu/drm/i915/intel_dp.c +2 -1

 	case AUX_CH_A:

@@ -1615,7 +1624,8 @@ static i915_reg_t skl_aux_ctl_reg(struct
 intel_dp *intel_dp)
 static i915_reg_t skl_aux_data_reg(struct intel_dp *intel_dp, int
 index)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum aux_ch aux_ch = intel_dp->aux_ch;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum aux_ch aux_ch = dig_port->aux_ch;
 

drivers/gpu/drm/i915/intel_dp.c +3 -2

 	switch (aux_ch) {
 	case AUX_CH_A:

@@ -1641,9 +1651,10 @@ static void
 intel_dp_aux_init(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
 >base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &dig_port->base;
 
-	intel_dp->aux_ch = intel_aux_ch(dev_priv, encoder->port);
+	dig_port->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	if (INTEL_GEN(dev_priv) >= 9) {

drivers/gpu/drm/i915/intel_drv.h +0 -1

index 268afb6d2746..a242a118389d 100644
@@ -1109,7 +1109,6 @@ struct intel_dp {
 	bool link_trained;
 	bool has_audio;
 	bool reset_link_params;
-	enum aux_ch aux_ch;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
 	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];

drivers/gpu/drm/i915/intel_drv.h struct intel_digital_port { · line 1214

Quoted 5 lines

drivers/gpu/drm/i915/intel_drv.h +0 -0

 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];

@@ -1213,6 +1212,7 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;

Please leave a comment here, with the commit message explanation so no one uses it my mistake like using for HDMI in combophy ports.

drivers/gpu/drm/i915/intel_drv.h struct intel_digital_port { · line 1218

Quoted 5 lines

drivers/gpu/drm/i915/intel_drv.h +1 -0

+	enum aux_ch aux_ch;
 	enum intel_display_power_domain ddi_io_power_domain;
 	enum tc_port_type tc_type;
 

From ICL onwards DDI/TypeC ports - even in HDMI static mode - need to know
which AUX CH belongs to them, so initialize aux_ch for those ports too.
For consistency do this for all HDMI ports, not only for DDI/TypeC ones.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c  | 1 +
 drivers/gpu/drm/i915/intel_dp.c   | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -0

index 32a080265d03..3739ef003819 100644
@@ -3852,6 +3852,7 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 			(DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES);
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes =
 intel_ddi_max_lanes(intel_dig_port);
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	switch (port) {
 	case PORT_A:

drivers/gpu/drm/i915/intel_dp.c +1 -1

index 5530c604c694..6645c9faca9a 100644
@@ -1654,7 +1654,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &dig_port->base;
 
-	dig_port->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 
 	if (INTEL_GEN(dev_priv) >= 9) {

@@ -6706,6 +6705,7 @@ bool intel_dp_init(struct drm_i915_private
 *dev_priv,
 	if (port != PORT_A)
 		intel_infoframe_init(intel_dig_port);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;

drivers/gpu/drm/i915/intel_hdmi.c +0 -0

index 129b880bce64..b50c5497048a 100644

drivers/gpu/drm/i915/intel_hdmi.c +0 -0

 	intel_hdmi_init_connector(intel_dig_port, intel_connector);

}

Quoted 61 lines · Imre Deak

From ICL onwards DDI/TypeC ports - even in HDMI static mode - need to know
which AUX CH belongs to them, so initialize aux_ch for those ports too.
For consistency do this for all HDMI ports, not only for DDI/TypeC ones.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c  | 1 +
 drivers/gpu/drm/i915/intel_dp.c   | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -0

index 32a080265d03..3739ef003819 100644
@@ -3852,6 +3852,7 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 			(DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES);
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes =
 intel_ddi_max_lanes(intel_dig_port);
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	switch (port) {
 	case PORT_A:

drivers/gpu/drm/i915/intel_dp.c +1 -1

index 5530c604c694..6645c9faca9a 100644
@@ -1654,7 +1654,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &dig_port->base;
 
-	dig_port->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 
 	if (INTEL_GEN(dev_priv) >= 9) {

@@ -6706,6 +6705,7 @@ bool intel_dp_init(struct drm_i915_private
 *dev_priv,
 	if (port != PORT_A)
 		intel_infoframe_init(intel_dig_port);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;

drivers/gpu/drm/i915/intel_hdmi.c +0 -0

index 129b880bce64..b50c5497048a 100644
@@ -2506,5 +2506,6 @@ void intel_hdmi_init(struct drm_i915_private
 *dev_priv,
 

drivers/gpu/drm/i915/intel_hdmi.c void intel_hdmi_init(struct drm_i915_private · line 2510

Quoted 3 lines

drivers/gpu/drm/i915/intel_hdmi.c +1 -0

 	intel_infoframe_init(intel_dig_port);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);

All ICL ports are DDI ports and aux_ch will not be used in previous gens so I don't see the point to set it here and in intel_dp_init().

Quoted 3 lines

drivers/gpu/drm/i915/intel_hdmi.c +0 -0

 	intel_hdmi_init_connector(intel_dig_port, intel_connector);

}

Quoted 69 lines · Jose

Quoted 68 lines · Imre Deak

From ICL onwards DDI/TypeC ports - even in HDMI static mode - need to
know
which AUX CH belongs to them, so initialize aux_ch for those ports
too.
For consistency do this for all HDMI ports, not only for DDI/TypeC
ones.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c  | 1 +
 drivers/gpu/drm/i915/intel_dp.c   | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -0

index 32a080265d03..3739ef003819 100644
@@ -3852,6 +3852,7 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 			(DDI_BUF_PORT_REVERSAL | DDI_A_4_LANES);
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes =
 intel_ddi_max_lanes(intel_dig_port);
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	switch (port) {
 	case PORT_A:

drivers/gpu/drm/i915/intel_dp.c +0 -1

index 5530c604c694..6645c9faca9a 100644
@@ -1654,7 +1654,6 @@ intel_dp_aux_init(struct intel_dp
 *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &dig_port->base;
 
-	dig_port->aux_ch = intel_aux_ch(dev_priv, encoder->port);
 	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
 

drivers/gpu/drm/i915/intel_dp.c +1 -0

 	if (INTEL_GEN(dev_priv) >= 9) {

@@ -6706,6 +6705,7 @@ bool intel_dp_init(struct drm_i915_private
 *dev_priv,
 	if (port != PORT_A)
 		intel_infoframe_init(intel_dig_port);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;

drivers/gpu/drm/i915/intel_hdmi.c +0 -0

index 129b880bce64..b50c5497048a 100644
@@ -2506,5 +2506,6 @@ void intel_hdmi_init(struct

drm_i915_private
*dev_priv,

intel_infoframe_init(intel_dig_port);

Quoted 5 lines

drivers/gpu/drm/i915/intel_hdmi.c

Quoted 1 line

drivers/gpu/drm/i915/intel_hdmi.c +1 -0

+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);

All ICL ports are DDI ports and aux_ch will not be used in previous gens so I don't see the point to set it here and in intel_dp_init().

aux_ch is used on all GENs for DP ports. As I mentioned in the message I initialize it for HDMI ports too for consistency.

Quoted 3 lines

Quoted 3 lines

intel_hdmi_init_connector(intel_dig_port, intel_connector);
}

From ICL onwards the AUX power domain may change dynamically based on whether a DDI/TypeC port is in thunderbolt or non-thunderbolt mode, so use a helper function instead of a static field to get the current domain.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 73 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 4 files changed, 56 insertions(+), 44 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -1

index 3739ef003819..5bb459011a49 100644
@@ -2099,7 +2099,7 @@ intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
 	 * returns the correct domain for other ports too.
 	 */
 	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
-					      intel_dp->aux_power_domain;
+					      intel_aux_power_domain(dig_port);
 }
 
 static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,

drivers/gpu/drm/i915/intel_display.c +0 -0

index c3cadc09f859..36710a30fb37 100644

drivers/gpu/drm/i915/intel_display.c +19 -0

+	switch (dig_port->aux_ch) {
+	case AUX_CH_A:
+		return POWER_DOMAIN_AUX_A;
+	case AUX_CH_B:
+		return POWER_DOMAIN_AUX_B;
+	case AUX_CH_C:
+		return POWER_DOMAIN_AUX_C;
+	case AUX_CH_D:
+		return POWER_DOMAIN_AUX_D;
+	case AUX_CH_E:
+		return POWER_DOMAIN_AUX_E;
+	case AUX_CH_F:
+		return POWER_DOMAIN_AUX_F;
+	default:
+		MISSING_CASE(dig_port->aux_ch);
+		return POWER_DOMAIN_AUX_A;
+	}
+}
+
 static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 				  struct intel_crtc_state *crtc_state)
 {

drivers/gpu/drm/i915/intel_dp.c +31 -42

index 6645c9faca9a..e6f59ef59be6 100644
@@ -690,7 +690,8 @@ static void pps_lock(struct intel_dp *intel_dp)
 	 * See intel_power_sequencer_reset() why we need
 	 * a power domain reference here.
 	 */
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
 
 	mutex_lock(&dev_priv->pps_mutex);
 }

@@ -701,7 +702,8 @@ static void pps_unlock(struct intel_dp *intel_dp)
 
 	mutex_unlock(&dev_priv->pps_mutex);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
 }
 
 static void

@@ -1505,29 +1507,6 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	return ret;
 }
 
-static enum intel_display_power_domain
-intel_aux_power_domain(struct intel_dp *intel_dp)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
-	switch (dig_port->aux_ch) {
-	case AUX_CH_A:
-		return POWER_DOMAIN_AUX_A;
-	case AUX_CH_B:
-		return POWER_DOMAIN_AUX_B;
-	case AUX_CH_C:
-		return POWER_DOMAIN_AUX_C;
-	case AUX_CH_D:
-		return POWER_DOMAIN_AUX_D;
-	case AUX_CH_E:
-		return POWER_DOMAIN_AUX_E;
-	case AUX_CH_F:
-		return POWER_DOMAIN_AUX_F;
-	default:
-		MISSING_CASE(dig_port->aux_ch);
-		return POWER_DOMAIN_AUX_A;
-	}
-}
 
 static i915_reg_t g4x_aux_ctl_reg(struct intel_dp *intel_dp)
 {

@@ -1654,8 +1633,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &dig_port->base;
 
-	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
-
 	if (INTEL_GEN(dev_priv) >= 9) {
 		intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg;
 		intel_dp->aux_ch_data_reg = skl_aux_data_reg;

@@ -2356,7 +2333,8 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 	if (edp_have_panel_vdd(intel_dp))
 		return need_to_disable;
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
 		      port_name(intel_dig_port->base.port));

@@ -2442,7 +2420,8 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	if ((pp & PANEL_POWER_ON) == 0)
 		intel_dp->panel_power_off_time = ktime_get_boottime();
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 }
 
 static void edp_panel_vdd_work(struct work_struct *__work)

@@ -2555,6 +2534,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
 static void edp_panel_off(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	u32 pp;
 	i915_reg_t pp_ctrl_reg;
 

@@ -2564,10 +2544,10 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 		return;
 
 	DRM_DEBUG_KMS("Turn eDP port %c panel power off\n",
-		      port_name(dp_to_dig_port(intel_dp)->base.port));
+		      port_name(dig_port->base.port));
 
 	WARN(!intel_dp->want_panel_vdd, "Need eDP port %c VDD to turn off panel\n",
-	     port_name(dp_to_dig_port(intel_dp)->base.port));
+	     port_name(dig_port->base.port));
 
 	pp = ironlake_get_pp_control(intel_dp);
 	/* We need to switch off panel power _and_ force vdd, for otherwise some

@@ -2586,7 +2566,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	intel_dp->panel_power_off_time = ktime_get_boottime();
 
 	/* We got a reference when we enabled the VDD. */
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, intel_aux_power_domain(dig_port));
 }
 
 void intel_edp_panel_off(struct intel_dp *intel_dp)

@@ -5069,14 +5049,17 @@ intel_dp_detect(struct drm_connector *connector,
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &dig_port->base;
 	enum drm_connector_status status;
+	enum intel_display_power_domain aux_domain =
+		intel_aux_power_domain(dig_port);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, aux_domain);
 
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))

@@ -5138,7 +5121,7 @@ intel_dp_detect(struct drm_connector *connector,
 		ret = intel_dp_retrain_link(encoder, ctx);
 		if (ret) {
 			intel_display_power_put(dev_priv,
-						intel_dp->aux_power_domain);
+						intel_aux_power_domain(dig_port));
 			return ret;
 		}
 	}

@@ -5162,7 +5145,7 @@ intel_dp_detect(struct drm_connector *connector,
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, aux_domain);
 	return status;
 }
 

@@ -5170,8 +5153,11 @@ static void
 intel_dp_force(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &dig_port->base;
 	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
+	enum intel_display_power_domain aux_domain =
+		intel_aux_power_domain(dig_port);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);

@@ -5180,11 +5166,11 @@ intel_dp_force(struct drm_connector *connector)
 	if (connector->status != connector_status_connected)
 		return;
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, aux_domain);
 
 	intel_dp_set_edid(intel_dp);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, aux_domain);
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)

@@ -5530,6 +5516,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
 static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 

@@ -5543,7 +5530,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 	 * indefinitely.
 	 */
 	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, intel_aux_power_domain(dig_port));
 
 	edp_panel_vdd_schedule_off(intel_dp);
 }

@@ -5641,7 +5628,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		return IRQ_NONE;
 	}
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	if (intel_dp->is_mst) {
 		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {

@@ -5670,7 +5658,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	ret = IRQ_HANDLED;
 
 put_power:
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	return ret;
 }

drivers/gpu/drm/i915/intel_drv.h +2 -1

index a242a118389d..a3d7b93ecddd 100644
@@ -1130,7 +1130,6 @@ struct intel_dp {
 	/* sink or branch descriptor */
 	struct drm_dp_desc desc;
 	struct drm_dp_aux aux;
-	enum intel_display_power_domain aux_power_domain;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;

@@ -1709,6 +1708,8 @@ bool hsw_crtc_state_ips_capable(const struct intel_crtc_state *crtc_state);
 void hsw_enable_ips(const struct intel_crtc_state *crtc_state);
 void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
 enum intel_display_power_domain intel_port_to_power_domain(enum port port);
+enum intel_display_power_domain
+intel_aux_power_domain(struct intel_digital_port *dig_port);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,

Signature · 6 lines

2.13.2

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

Quoted 36 lines · Imre Deak

From ICL onwards the AUX power domain may change dynamically based on whether a DDI/TypeC port is in thunderbolt or non-thunderbolt mode, so use a helper function instead of a static field to get the current domain.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 73 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 4 files changed, 56 insertions(+), 44 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -1

index 3739ef003819..5bb459011a49 100644
@@ -2099,7 +2099,7 @@ intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
 	 * returns the correct domain for other ports too.
 	 */
 	return dig_port->aux_ch == AUX_CH_A ? POWER_DOMAIN_AUX_IO_A :
-					      intel_dp->aux_power_domain;
+					      intel_aux_power_domain(dig_port);
 }
 
 static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,

drivers/gpu/drm/i915/intel_display.c +0 -0

index c3cadc09f859..36710a30fb37 100644
@@ -5949,6 +5949,28 @@ enum intel_display_power_domain intel_port_to_power_domain(enum port port)
 	}
 }
 

drivers/gpu/drm/i915/intel_display.c enum intel_display_power_domain intel_port_to_power_domain(enum port port) · line 5954

Quoted 3 lines

drivers/gpu/drm/i915/intel_display.c +3 -0

+enum intel_display_power_domain
+intel_aux_power_domain(struct intel_digital_port *dig_port)
+{

couldn't we just move aux_power_domain up like yod did for aux_ch? This way we initialize the power domain bassed on the aux channel, and just use it on the other functions. I think this is more future-proof for platforms changing the mapping aux <-> power domain. And avoids passing dev_priv around later just for checking that.

Lucas De Marchi

Quoted 261 lines

drivers/gpu/drm/i915/intel_display.c +19 -0

+	switch (dig_port->aux_ch) {
+	case AUX_CH_A:
+		return POWER_DOMAIN_AUX_A;
+	case AUX_CH_B:
+		return POWER_DOMAIN_AUX_B;
+	case AUX_CH_C:
+		return POWER_DOMAIN_AUX_C;
+	case AUX_CH_D:
+		return POWER_DOMAIN_AUX_D;
+	case AUX_CH_E:
+		return POWER_DOMAIN_AUX_E;
+	case AUX_CH_F:
+		return POWER_DOMAIN_AUX_F;
+	default:
+		MISSING_CASE(dig_port->aux_ch);
+		return POWER_DOMAIN_AUX_A;
+	}
+}
+
 static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 				  struct intel_crtc_state *crtc_state)
 {

drivers/gpu/drm/i915/intel_dp.c +31 -42

index 6645c9faca9a..e6f59ef59be6 100644
@@ -690,7 +690,8 @@ static void pps_lock(struct intel_dp *intel_dp)
 	 * See intel_power_sequencer_reset() why we need
 	 * a power domain reference here.
 	 */
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
 
 	mutex_lock(&dev_priv->pps_mutex);
 }

@@ -701,7 +702,8 @@ static void pps_unlock(struct intel_dp *intel_dp)
 
 	mutex_unlock(&dev_priv->pps_mutex);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(dp_to_dig_port(intel_dp)));
 }
 
 static void

@@ -1505,29 +1507,6 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	return ret;
 }
 
-static enum intel_display_power_domain
-intel_aux_power_domain(struct intel_dp *intel_dp)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
-	switch (dig_port->aux_ch) {
-	case AUX_CH_A:
-		return POWER_DOMAIN_AUX_A;
-	case AUX_CH_B:
-		return POWER_DOMAIN_AUX_B;
-	case AUX_CH_C:
-		return POWER_DOMAIN_AUX_C;
-	case AUX_CH_D:
-		return POWER_DOMAIN_AUX_D;
-	case AUX_CH_E:
-		return POWER_DOMAIN_AUX_E;
-	case AUX_CH_F:
-		return POWER_DOMAIN_AUX_F;
-	default:
-		MISSING_CASE(dig_port->aux_ch);
-		return POWER_DOMAIN_AUX_A;
-	}
-}
 
 static i915_reg_t g4x_aux_ctl_reg(struct intel_dp *intel_dp)
 {

@@ -1654,8 +1633,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &dig_port->base;
 
-	intel_dp->aux_power_domain = intel_aux_power_domain(intel_dp);
-
 	if (INTEL_GEN(dev_priv) >= 9) {
 		intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg;
 		intel_dp->aux_ch_data_reg = skl_aux_data_reg;

@@ -2356,7 +2333,8 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 	if (edp_have_panel_vdd(intel_dp))
 		return need_to_disable;
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
 		      port_name(intel_dig_port->base.port));

@@ -2442,7 +2420,8 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	if ((pp & PANEL_POWER_ON) == 0)
 		intel_dp->panel_power_off_time = ktime_get_boottime();
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 }
 
 static void edp_panel_vdd_work(struct work_struct *__work)

@@ -2555,6 +2534,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
 static void edp_panel_off(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	u32 pp;
 	i915_reg_t pp_ctrl_reg;
 

@@ -2564,10 +2544,10 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 		return;
 
 	DRM_DEBUG_KMS("Turn eDP port %c panel power off\n",
-		      port_name(dp_to_dig_port(intel_dp)->base.port));
+		      port_name(dig_port->base.port));
 
 	WARN(!intel_dp->want_panel_vdd, "Need eDP port %c VDD to turn off panel\n",
-	     port_name(dp_to_dig_port(intel_dp)->base.port));
+	     port_name(dig_port->base.port));
 
 	pp = ironlake_get_pp_control(intel_dp);
 	/* We need to switch off panel power _and_ force vdd, for otherwise some

@@ -2586,7 +2566,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	intel_dp->panel_power_off_time = ktime_get_boottime();
 
 	/* We got a reference when we enabled the VDD. */
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, intel_aux_power_domain(dig_port));
 }
 
 void intel_edp_panel_off(struct intel_dp *intel_dp)

@@ -5069,14 +5049,17 @@ intel_dp_detect(struct drm_connector *connector,
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &dig_port->base;
 	enum drm_connector_status status;
+	enum intel_display_power_domain aux_domain =
+		intel_aux_power_domain(dig_port);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, aux_domain);
 
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))

@@ -5138,7 +5121,7 @@ intel_dp_detect(struct drm_connector *connector,
 		ret = intel_dp_retrain_link(encoder, ctx);
 		if (ret) {
 			intel_display_power_put(dev_priv,
-						intel_dp->aux_power_domain);
+						intel_aux_power_domain(dig_port));
 			return ret;
 		}
 	}

@@ -5162,7 +5145,7 @@ intel_dp_detect(struct drm_connector *connector,
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, aux_domain);
 	return status;
 }
 

@@ -5170,8 +5153,11 @@ static void
 intel_dp_force(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &dig_port->base;
 	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
+	enum intel_display_power_domain aux_domain =
+		intel_aux_power_domain(dig_port);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);

@@ -5180,11 +5166,11 @@ intel_dp_force(struct drm_connector *connector)
 	if (connector->status != connector_status_connected)
 		return;
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, aux_domain);
 
 	intel_dp_set_edid(intel_dp);
 
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv, aux_domain);
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)

@@ -5530,6 +5516,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = {
 static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 

@@ -5543,7 +5530,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 	 * indefinitely.
 	 */
 	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv, intel_aux_power_domain(dig_port));
 
 	edp_panel_vdd_schedule_off(intel_dp);
 }

@@ -5641,7 +5628,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		return IRQ_NONE;
 	}
 
-	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_get(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	if (intel_dp->is_mst) {
 		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {

@@ -5670,7 +5658,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	ret = IRQ_HANDLED;
 
 put_power:
-	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+	intel_display_power_put(dev_priv,
+				intel_aux_power_domain(intel_dig_port));
 
 	return ret;
 }

drivers/gpu/drm/i915/intel_drv.h +2 -1

index a242a118389d..a3d7b93ecddd 100644
@@ -1130,7 +1130,6 @@ struct intel_dp {
 	/* sink or branch descriptor */
 	struct drm_dp_desc desc;
 	struct drm_dp_aux aux;
-	enum intel_display_power_domain aux_power_domain;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;

@@ -1709,6 +1708,8 @@ bool hsw_crtc_state_ips_capable(const struct intel_crtc_state *crtc_state);
 void hsw_enable_ips(const struct intel_crtc_state *crtc_state);
 void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
 enum intel_display_power_domain intel_port_to_power_domain(enum port port);
+enum intel_display_power_domain
+intel_aux_power_domain(struct intel_digital_port *dig_port);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,

Signature · 1 line

2.13.2

Quoted 4 lines

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

For DDI/TypeC ports the AUX power domain needs to be enabled before the port's PLL is enabled, so move the enabling earlier accordingly.

v2:
- Preserve the pre_pll hook for GEN9_LP. (Ville)

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 60 +++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 37 insertions(+), 25 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +3 -8

index 5bb459011a49..9554da06e19a 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

@@ -2120,11 +2118,8 @@ static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |= BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));
-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |= BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));
 
 	return domains;
 }

drivers/gpu/drm/i915/intel_ddi.c +32 -17

+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(dig_port));
+
+	if (IS_GEN9_LP(dev_priv))
+		bxt_ddi_phy_set_lane_optim_mask(encoder,
+					       crtc_state->lane_lat_optim_mask);
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(dig_port));
+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *crtc_state,
 				    const struct drm_connector_state *conn_state)

@@ -2904,9 +2929,6 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
 				 crtc_state->lane_count, is_mst);
 

@@ -3071,9 +3093,6 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
 
 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp));
 }
 
 static void intel_ddi_post_disable_hdmi(struct intel_encoder *encoder,

@@ -3304,15 +3323,6 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
 		intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
 }
 
-static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
-				   const struct intel_crtc_state *pipe_config,
-				   const struct drm_connector_state *conn_state)
-{
-	uint8_t mask = pipe_config->lane_lat_optim_mask;
-
-	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
-}
-
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);

@@ -3828,8 +3838,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
-	if (IS_GEN9_LP(dev_priv))
-		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state, old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state, old_state);
 }
 
 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)

Signature · 6 lines

2.13.2

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

Quoted 42 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before the port's PLL is enabled, so move the enabling earlier accordingly.

v2:
- Preserve the pre_pll hook for GEN9_LP. (Ville)

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 60 +++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 37 insertions(+), 25 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +3 -8

index 5bb459011a49..9554da06e19a 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

@@ -2120,11 +2118,8 @@ static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |= BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));
-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |= BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));
 

drivers/gpu/drm/i915/intel_ddi.c static void intel_ddi_clk_disable(struct intel_encoder *encoder) · line 2888

Quoted 6 lines

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	return domains;
 }

@@ -2891,6 +2886,36 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	}
 }
 

I just realized while merging that my previous patches that are reviewed-by add icl_ddi_pre_pll_enable hook for Gen 11.
Either the below pre_pll_enable should be part of that or I will have to respin those before merging and call it intel_ddi_pre_pll_enable() hook.
https://patchwork.freedesktop.org/patch/258008/

Manasi

Quoted 92 lines

drivers/gpu/drm/i915/intel_ddi.c +32 -17

+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(dig_port));
+
+	if (IS_GEN9_LP(dev_priv))
+		bxt_ddi_phy_set_lane_optim_mask(encoder,
+					       crtc_state->lane_lat_optim_mask);
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(dig_port));
+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state *crtc_state,
 				    const struct drm_connector_state *conn_state)

@@ -2904,9 +2929,6 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
 				 crtc_state->lane_count, is_mst);
 

@@ -3071,9 +3093,6 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
 
 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp));
 }
 
 static void intel_ddi_post_disable_hdmi(struct intel_encoder *encoder,

@@ -3304,15 +3323,6 @@ static void intel_disable_ddi(struct intel_encoder *encoder,
 		intel_disable_ddi_dp(encoder, old_crtc_state, old_conn_state);
 }
 
-static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
-				   const struct intel_crtc_state *pipe_config,
-				   const struct drm_connector_state *conn_state)
-{
-	uint8_t mask = pipe_config->lane_lat_optim_mask;
-
-	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
-}
-
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);

@@ -3828,8 +3838,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
-	if (IS_GEN9_LP(dev_priv))
-		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state, old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state, old_state);
 }
 

Quoted 8 lines

drivers/gpu/drm/i915/intel_display.c +0 -0

 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)

Signature · 6 lines

2.13.2

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

For DDI/TypeC ports the AUX power domain needs to be enabled before the
port's PLL is enabled, so move the enabling earlier accordingly.

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 46 +++++++++++++++++++++++++-
----------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 34 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -3

index 5bb459011a49..7731ca704862 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +2 -5

@@ -2120,11 +2118,8 @@ static u64 intel_ddi_get_power_domains(struct
 intel_encoder *encoder,
 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |=
 BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));
-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |=
 BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));

drivers/gpu/drm/i915/intel_ddi.c +17 -0

 	return domains;
 }

@@ -2891,6 +2886,32 @@ static void intel_ddi_clk_disable(struct
 intel_encoder *encoder)
 	}
 }
 
+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-
 >base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(
 dig_port));
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +3 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(

dig_port));

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state

*crtc_state,
const struct drm_connector_state
*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +0 -3

@@ -2904,9 +2925,6 @@ static void intel_ddi_pre_enable_dp(struct
 intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,

drivers/gpu/drm/i915/intel_ddi.c +0 -3

 				 crtc_state->lane_count, is_mst);

@@ -3071,9 +3089,6 @@ static void intel_ddi_post_disable_dp(struct
 intel_encoder *encoder,
 	intel_display_power_put(dev_priv, dig_port-
 >ddi_io_power_domain);
 
 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));

}

static void intel_ddi_post_disable_hdmi(struct intel_encoder
*encoder,

drivers/gpu/drm/i915/intel_ddi.c +3 -0

@@ -3830,6 +3845,9 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct
 intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state,
 old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state,
 old_state);

}

static void i9xx_pfit_enable(const struct intel_crtc_state
*crtc_state)

Quoted 3 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before the
port's PLL is enabled, so move the enabling earlier accordingly.

Could you just pointed out where did you got this information? I checked in this Bspec pages 20845, 20598 and 21750 and I did not found.

With that:

Reviewed-by José Roberto de Souza <[email protected]>

Quoted 142 lines

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 46 +++++++++++++++++++++++++-
----------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 34 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -3

index 5bb459011a49..7731ca704862 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +2 -5

@@ -2120,11 +2118,8 @@ static u64 intel_ddi_get_power_domains(struct
 intel_encoder *encoder,
 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |=
 BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));
-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |=
 BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));

drivers/gpu/drm/i915/intel_ddi.c +17 -0

 	return domains;
 }

@@ -2891,6 +2886,32 @@ static void intel_ddi_clk_disable(struct
 intel_encoder *encoder)
 	}
 }
 
+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-
 >base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(
 dig_port));
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +3 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(

dig_port));

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 				    const struct intel_crtc_state

*crtc_state,
const struct drm_connector_state
*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +0 -3

@@ -2904,9 +2925,6 @@ static void intel_ddi_pre_enable_dp(struct
 intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,

drivers/gpu/drm/i915/intel_ddi.c +0 -3

 				 crtc_state->lane_count, is_mst);

@@ -3071,9 +3089,6 @@ static void intel_ddi_post_disable_dp(struct
 intel_encoder *encoder,
 	intel_display_power_put(dev_priv, dig_port-
 >ddi_io_power_domain);
 
 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));

}

static void intel_ddi_post_disable_hdmi(struct intel_encoder
*encoder,

drivers/gpu/drm/i915/intel_ddi.c +3 -0

@@ -3830,6 +3845,9 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct
 intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state,
 old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state,
 old_state);

}

Quoted 2 lines

static void i9xx_pfit_enable(const struct intel_crtc_state
*crtc_state)

Quoted 6 lines · Jose

Quoted 4 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before
the
port's PLL is enabled, so move the enabling earlier accordingly.

Quoted 3 lines

Could you just pointed out where did you got this information? I checked in this Bspec pages 20845, 20598 and 21750 and I did not found.

It's at 21750, connect/disconnect flows.

Quoted 150 lines

With that:

Reviewed-by José Roberto de Souza <[email protected]>

Quoted 146 lines

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 46
+++++++++++++++++++++++++-
----------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 34 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -3

index 5bb459011a49..7731ca704862 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port
 *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2120,11 +2118,8 @@ static u64

intel_ddi_get_power_domains(struct
intel_encoder *encoder,

drivers/gpu/drm/i915/intel_ddi.c +0 -4

 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));

drivers/gpu/drm/i915/intel_ddi.c +2 -1

-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));

drivers/gpu/drm/i915/intel_ddi.c +17 -0

 	return domains;
 }

@@ -2891,6 +2886,32 @@ static void intel_ddi_clk_disable(struct
 intel_encoder *encoder)
 	}
 }
 
+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-
 > base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(
 dig_port));
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +3 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(

dig_port));

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder

*encoder,
const struct intel_crtc_state
*crtc_state,
const struct drm_connector_state
*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +0 -3

@@ -2904,9 +2925,6 @@ static void intel_ddi_pre_enable_dp(struct
 intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 				 crtc_state->lane_count, is_mst);

@@ -3071,9 +3089,6 @@ static void

intel_ddi_post_disable_dp(struct intel_encoder *encoder,
intel_display_power_put(dev_priv, dig_port-

Quoted 1 line

ddi_io_power_domain);

drivers/gpu/drm/i915/intel_ddi.c +0 -3

 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp

));
}

static void intel_ddi_post_disable_hdmi(struct intel_encoder
*encoder,

drivers/gpu/drm/i915/intel_ddi.c +3 -0

@@ -3830,6 +3845,9 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct
 intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state,
 old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state,
 old_state);

}

Quoted 2 lines

Quoted 2 lines

static void i9xx_pfit_enable(const struct intel_crtc_state
*crtc_state)

Quoted 11 lines · Imre Deak

Quoted 9 lines · Jose

Quoted 4 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before
the
port's PLL is enabled, so move the enabling earlier accordingly.

Could you just pointed out where did you got this information? I checked in this Bspec pages 20845, 20598 and 21750 and I did not found.

Quoted 1 line

It's at 21750, connect/disconnect flows.

What piece specifically?
The only related that I can find is this one:
"Aux power must be enabled while using Aux channel or the main link (including HDMI). It should be disabled to save power when not using Aux channel or main link."

Quoted 151 lines

Quoted 151 lines

With that:

Reviewed-by José Roberto de Souza <[email protected]>

Quoted 146 lines

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 46
+++++++++++++++++++++++++-
----------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 34 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -3

index 5bb459011a49..7731ca704862 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port
 *dig_port)
 {
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-
 	/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2120,11 +2118,8 @@ static u64

intel_ddi_get_power_domains(struct
intel_encoder *encoder,

drivers/gpu/drm/i915/intel_ddi.c +0 -4

 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for HDMI. */
-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));

drivers/gpu/drm/i915/intel_ddi.c +2 -1

-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));

drivers/gpu/drm/i915/intel_ddi.c +17 -0

 	return domains;
 }

@@ -2891,6 +2886,32 @@ static void intel_ddi_clk_disable(struct
 intel_encoder *encoder)
 	}
 }
 
+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state *crtc_state,
+			 const struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-
 > base);
+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux_domain(
 dig_port));
+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +3 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux_domain(

dig_port));

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder

*encoder,
const struct intel_crtc_state
*crtc_state,
const struct drm_connector_state
*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +0 -3

@@ -2904,9 +2925,6 @@ static void intel_ddi_pre_enable_dp(struct
 intel_encoder *encoder,
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp
 ));
-
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 				 crtc_state->lane_count, is_mst);

@@ -3071,9 +3089,6 @@ static void

intel_ddi_post_disable_dp(struct intel_encoder *encoder,
intel_display_power_put(dev_priv, dig_port-

Quoted 1 line

ddi_io_power_domain);

drivers/gpu/drm/i915/intel_ddi.c +0 -3

 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(intel_dp

));
}

static void intel_ddi_post_disable_hdmi(struct intel_encoder
*encoder,

drivers/gpu/drm/i915/intel_ddi.c +3 -0

@@ -3830,6 +3845,9 @@ void intel_ddi_init(struct drm_i915_private
 *dev_priv, enum port port)
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+
+	intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
+	intel_encoder->post_pll_disable = intel_ddi_post_pll_disable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct
 intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state,
 old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state,
 old_state);

}

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

static void i9xx_pfit_enable(const struct intel_crtc_state
*crtc_state)

Quoted 15 lines · Jose

Quoted 14 lines · Imre Deak

Quoted 12 lines · Jose

Quoted 5 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before
the
port's PLL is enabled, so move the enabling earlier accordingly.

Could you just pointed out where did you got this information? I
checked in this Bspec pages 20845, 20598 and 21750 and I did not
found.

Quoted 8 lines

Quoted 1 line

It's at 21750, connect/disconnect flows.

What piece specifically?
The only related that I can find is this one:
"Aux power must be enabled while using Aux channel or the main link (including HDMI). It should be disabled to save power when not using
Aux channel or main link."

We can do AUX transfers even before enabling the main link, where we enable already AUX power. In fact all the connect flows have:

"Display software issues AUX reads for EDID/DPCD."

So now if we enabled it after PLL programming we'd have the two things
happen in both orders, which I'd rather not do, if not necessary.

Quoted 175 lines

Quoted 175 lines

Quoted 175 lines

With that:

Reviewed-by José Roberto de Souza <[email protected]>

Quoted 171 lines

Cc Paulo Zanoni <[email protected]>

Cc Ville Syrjälä <[email protected]>

Signed-off-by Imre Deak <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     | 46
+++++++++++++++++++++++++-
----------
 drivers/gpu/drm/i915/intel_display.c |  2 ++
 2 files changed, 34 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +1 -3

index 5bb459011a49..7731ca704862 100644
@@ -2082,10 +2082,8 @@ bool intel_ddi_get_hw_state(struct
 intel_encoder *encoder,
 }
 
 static inline enum intel_display_power_domain
-intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
+intel_ddi_main_link_aux_domain(struct intel_digital_port
 *dig_port)
 {
-	struct intel_digital_port *dig_port =
 dp_to_dig_port(intel_dp);
-

/* CNL+ 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

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2120,11 +2118,8 @@ static u64

intel_ddi_get_power_domains(struct
intel_encoder *encoder,

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	domains = BIT_ULL(dig_port->ddi_io_power_domain);
 
 	/* AUX power is only needed for (e)DP mode, not for

HDMI. */

drivers/gpu/drm/i915/intel_ddi.c +0 -4

-	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		struct intel_dp *intel_dp = &dig_port->dp;
-
-		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));

drivers/gpu/drm/i915/intel_ddi.c +2 -1

-	}
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		domains |=

BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	return domains;
 }

@@ -2891,6 +2886,32 @@ static void

intel_ddi_clk_disable(struct
intel_encoder *encoder)

drivers/gpu/drm/i915/intel_ddi.c +3 -0

 	}
 }
 
+static void
+intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+			 const struct intel_crtc_state

*crtc_state,

drivers/gpu/drm/i915/intel_ddi.c +1 -0

+			 const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder-

Quoted 1 line

base.dev);

drivers/gpu/drm/i915/intel_ddi.c +1 -0

+	struct intel_digital_port *dig_port =

enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_get(dev_priv,
+					intel_ddi_main_link_aux

_domain(
dig_port));

drivers/gpu/drm/i915/intel_ddi.c +5 -0

+}
+
+static void
+intel_ddi_post_pll_disable(struct intel_encoder *encoder,
+			   const struct intel_crtc_state

*crtc_state,

drivers/gpu/drm/i915/intel_ddi.c +1 -0

+			   const struct drm_connector_state

*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+{
+	struct drm_i915_private *dev_priv = to_i915(encoder-

Quoted 1 line

base.dev);

drivers/gpu/drm/i915/intel_ddi.c +1 -0

+	struct intel_digital_port *dig_port =

enc_to_dig_port(&encoder-

Quoted 1 line

base);

drivers/gpu/drm/i915/intel_ddi.c +4 -0

+
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		intel_display_power_put(dev_priv,
+					intel_ddi_main_link_aux

_domain(
dig_port));

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+}
+
 static void intel_ddi_pre_enable_dp(struct intel_encoder

*encoder,
const struct
intel_crtc_state
*crtc_state,
const struct
drm_connector_state
*conn_state)

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -2904,9 +2925,6 @@ static void

intel_ddi_pre_enable_dp(struct
intel_encoder *encoder,

drivers/gpu/drm/i915/intel_ddi.c +0 -2

 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-	intel_display_power_get(dev_priv,
-				intel_ddi_main_link_aux_domain(

intel_dp
));

drivers/gpu/drm/i915/intel_ddi.c +0 -1

-
 	intel_dp_set_link_params(intel_dp, crtc_state-

Quoted 1 line

port_clock,

crtc_state->lane_count,
is_mst);

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -3071,9 +3089,6 @@ static void

intel_ddi_post_disable_dp(struct intel_encoder *encoder,
intel_display_power_put(dev_priv, dig_port-

Quoted 1 line

ddi_io_power_domain);

drivers/gpu/drm/i915/intel_ddi.c +0 -3

 	intel_ddi_clk_disable(encoder);
-
-	intel_display_power_put(dev_priv,
-				intel_ddi_main_link_aux_domain(

intel_dp
));
}

static void intel_ddi_post_disable_hdmi(struct intel_encoder
*encoder,

drivers/gpu/drm/i915/intel_ddi.c +0 -0

@@ -3830,6 +3845,9 @@ void intel_ddi_init(struct

drm_i915_private
*dev_priv, enum port port)

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable =

bxt_ddi_pre_pll_enable;

drivers/gpu/drm/i915/intel_ddi.c +2 -0

+
+	intel_encoder->pre_pll_enable =

intel_ddi_pre_pll_enable;

drivers/gpu/drm/i915/intel_ddi.c +1 -0

+	intel_encoder->post_pll_disable =

intel_ddi_post_pll_disable;

drivers/gpu/drm/i915/intel_ddi.c +0 -0

 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
 	intel_encoder->post_disable = intel_ddi_post_disable;

drivers/gpu/drm/i915/intel_display.c +2 -0

index 36710a30fb37..12ba2b923e6b 100644
@@ -5876,6 +5876,8 @@ static void haswell_crtc_disable(struct
 intel_crtc_state *old_crtc_state,
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_unmap_plls_to_ports(crtc, old_crtc_state,
 old_state);
+
+	intel_encoders_post_pll_disable(crtc, old_crtc_state,
 old_state);

}

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

static void i9xx_pfit_enable(const struct intel_crtc_state
*crtc_state)

Quoted 30 lines · Imre Deak

Quoted 15 lines · Jose

Quoted 14 lines · Imre Deak

Quoted 12 lines · Jose

Quoted 5 lines · Imre Deak

For DDI/TypeC ports the AUX power domain needs to be enabled before
the
port's PLL is enabled, so move the enabling earlier accordingly.

Could you just pointed out where did you got this information? I
checked in this Bspec pages 20845, 20598 and 21750 and I did not
found.

Quoted 8 lines

Quoted 1 line

It's at 21750, connect/disconnect flows.

What piece specifically?
The only related that I can find is this one:
"Aux power must be enabled while using Aux channel or the main link (including HDMI). It should be disabled to save power when not using
Aux channel or main link."

We can do AUX transfers even before enabling the main link, where we enable already AUX power. In fact all the connect flows have:

"Display software issues AUX reads for EDID/DPCD."