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

PATCH Superseded Imre Deak 4 participants 19 messages Oct 31, 2018 – Nov 1, 2018

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

Imre Deak

author

2018-10-31 16:27 UTC #0 lore raw

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

v3 of https://patchwork.freedesktop.org/series/51765/ addressing Jose's and Lucas' review comments.

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

Cc Lucas De Marchi <[email protected]>

Cc Manasi Navare <[email protected]>

Imre Deak (8):
drm/i915: Move intel_aux_ch() to intel_bios.c drm/i915: Move aux_ch to intel_digital_port
drm/i915: Init aux_ch for HDMI ports too
drm/i915: Use a helper to get the aux power domain drm/i915: Enable AUX power earlier
drm/i915: Enable AUX power for HDMI DDI/TypeC main link too drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain drm/i915/icl+: Sanitize port to PLL mapping

 drivers/gpu/drm/i915/i915_drv.h         |   6 ++
 drivers/gpu/drm/i915/intel_bios.c       |  45 ++++++++++
 drivers/gpu/drm/i915/intel_ddi.c        |  95 ++++++++++++++------
 drivers/gpu/drm/i915/intel_display.c    |  28 ++++++
 drivers/gpu/drm/i915/intel_dp.c         | 148 +++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h        |   7 +-
 drivers/gpu/drm/i915/intel_hdmi.c       |   1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c |  50 +++++++++--
 8 files changed, 246 insertions(+), 134 deletions(-)

Signature · 5 lines

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

Imre Deak

author

2018-10-31 16:27 UTC #1 lore raw

PATCH v3 1/8 drm/i915: Move intel_aux_ch() to intel_bios.c

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]>

Cc José Roberto de Souza <[email protected]>

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

Reviewed-by José Roberto de Souza <[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 +45 -0

48 lines not on this page · raw message

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

62 lines not on this page · raw message

Signature · 6 lines

2.13.2

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

Imre Deak

author

2018-10-31 16:27 UTC #2 lore raw

PATCH v3 2/8 drm/i915: Move aux_ch to intel_digital_port

In reply to 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.

v3:
- Add code comment about which ports aux_ch is used for. (Jose)

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[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 |  3 ++-
 3 files changed, 28 insertions(+), 14 deletions(-)

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

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

@@ -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
 	 * 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->aux_power_domain;
 }

drivers/gpu/drm/i915/intel_dp.c +23 ~~-12~~

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);
 
 	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
 	 * 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);
 	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 */
 		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;
 	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;
 	}
 }

@@ -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) {
 	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;
 
 	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) {
 	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;
 
 	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) {
 	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;
 
 	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);
 
 	if (INTEL_GEN(dev_priv) >= 9) {

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

index e4eaa40bd5f1..5bc7ba326111 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];

@@ -1213,6 +1212,8 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
+	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
+	enum aux_ch aux_ch;
 	enum intel_display_power_domain ddi_io_power_domain;
 	enum tc_port_type tc_type;

Signature · 6 lines

2.13.2

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

Souza, Jose

Reviewed-by on v3 2/8

2018-10-31 17:11 UTC #3 lore raw

Re: [PATCH v3 2/8] drm/i915: Move aux_ch to intel_digital_port

Quoted 10 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.
v3:
- Add code comment about which ports aux_ch is used for. (Jose)

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

Quoted 200 lines

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[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 |  3 ++-
 3 files changed, 28 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 e4eaa40bd5f1..5bc7ba326111 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];

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

Quoted 8 lines

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

@@ -1213,6 +1212,8 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
+	/* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
+	enum aux_ch aux_ch;
 	enum intel_display_power_domain ddi_io_power_domain;
 	enum tc_port_type tc_type;

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

Imre Deak

author

2018-10-31 16:27 UTC #4 lore raw

PATCH v3 3/8 drm/i915: Init aux_ch for HDMI ports too

In reply to 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]>

Cc José Roberto de Souza <[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);
 
 	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 +1 -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);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
 }

Signature · 6 lines

2.13.2

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

Souza, Jose

2018-10-31 17:14 UTC #5 lore raw

Re: [PATCH v3 3/8] drm/i915: Init aux_ch for HDMI ports too

Quoted 60 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]>

Cc José Roberto de Souza <[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 void intel_hdmi_init(struct drm_i915_private · line 2510

Quoted 5 lines

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

@@ -2506,5 +2506,6 @@ void intel_hdmi_init(struct drm_i915_private
 *dev_priv,
 
 	intel_infoframe_init(intel_dig_port);
 
+	intel_dig_port->aux_ch = intel_aux_ch(dev_priv, port);

Everything else looks good, I'm not just 100% confortable in initializing it here just for consistency, can someone else comment here?

Quoted 2 lines

drivers/gpu/drm/i915/intel_hdmi.c +0 -0
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
}

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

Rodrigo Vivi

maintainer

2018-10-31 17:29 UTC #6 lore raw

Re: [PATCH v3 3/8] drm/i915: Init aux_ch for HDMI ports too

Quoted 63 lines · Jose

Quoted 62 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]>

Cc José Roberto de Souza <[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,

Quoted 7 lines

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);
Everything else looks good, I'm not just 100% confortable in initializing it here just for consistency, can someone else comment here?

The place is fine. My concern with this approach is the impact on some legacy platforms that could be using this aux for something else.

Like... That SKL with edp-to-vga converter plugged on port-E which uses aux from port B because this was unused because it was an HDMI plugged on port B.

What could be the impact in a case like this?

So to avoid regressions in some old frankensteins maybe it is better to add some check for type c no?!

Quoted 7 lines

Quoted 3 lines
drivers/gpu/drm/i915/intel_hdmi.c +0 -0
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
}
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Imre Deak

author

2018-10-31 17:37 UTC #7 lore raw

Re: [PATCH v3 3/8] drm/i915: Init aux_ch for HDMI ports too

Quoted 75 lines · Rodrigo Vivi

Quoted 63 lines · Jose
Quoted 62 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]>
Cc José Roberto de Souza <[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,
 
Quoted 7 lines
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);
Everything else looks good, I'm not just 100% confortable in initializing it here just for consistency, can someone else comment here?
The place is fine. My concern with this approach is the impact on some legacy platforms that could be using this aux for something else.

Quoted 8 lines

Like... That SKL with edp-to-vga converter plugged on port-E which uses aux from port B because this was unused because it was an HDMI plugged on port B.
What could be the impact in a case like this?
So to avoid regressions in some old frankensteins maybe it is better to add some check for type c no?!

We would have a check for type C after this patchset, we would enable AUX on HDMI ports only if that port is a typeC port (which is always false on SKL). I guess that's the config you mean. See the intel_ddi_pre_pll_enable hook I added.

Quoted 8 lines

Quoted 8 lines
Quoted 4 lines
drivers/gpu/drm/i915/intel_hdmi.c +0 -0
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
}
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Rodrigo Vivi

maintainer Reviewed-by

2018-10-31 17:56 UTC #8 lore raw

Re: [PATCH v3 3/8] drm/i915: Init aux_ch for HDMI ports too

Quoted 82 lines · Imre Deak

Quoted 81 lines · Rodrigo Vivi
Quoted 63 lines · Jose
Quoted 62 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]>
Cc José Roberto de Souza <[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,
 
Quoted 7 lines
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);
Everything else looks good, I'm not just 100% confortable in initializing it here just for consistency, can someone else comment here?
The place is fine. My concern with this approach is the impact on some legacy platforms that could be using this aux for something else.
Like... That SKL with edp-to-vga converter plugged on port-E which uses aux from port B because this was unused because it was an HDMI plugged on port B.
What could be the impact in a case like this?

Quoted 7 lines

Quoted 2 lines
So to avoid regressions in some old frankensteins maybe it is better to add some check for type c no?!
We would have a check for type C after this patchset, we would enable AUX on HDMI ports only if that port is a typeC port (which is always false on SKL). I guess that's the config you mean. See the intel_ddi_pre_pll_enable hook I added.

cool, it makes sense.

Also I was looking to the series as a whole thinking about this case that came to my mind and I don't see any possible regression.

Reviewed-by Rodrigo Vivi <[email protected]>

for the series.

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/gpu/drm/i915/intel_hdmi.c +0 -0
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
}

Quoted 8 lines

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

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

Imre Deak

author

2018-10-31 16:27 UTC #9 lore raw

PATCH v3 4/8 drm/i915: Use a helper to get the aux power domain

In reply to 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]>

Cc José Roberto de Souza <[email protected]>

Cc Lucas De Marchi <[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 +22 -0

index c3cadc09f859..36710a30fb37 100644
@@ -5949,6 +5949,28 @@ 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)
+{
+	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~~

193 lines not on this page · raw message

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

index 5bc7ba326111..4e16cda9d94a 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;

@@ -1711,6 +1710,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

Souza, Jose

Reviewed-by on v3 4/8

2018-10-31 17:12 UTC #10 lore raw

Re: [PATCH v3 4/8] drm/i915: Use a helper to get the aux power domain

Quoted 5 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.

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

Quoted 336 lines

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

Cc Lucas De Marchi <[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(di
 g_port);

}

static u64 intel_ddi_get_power_domains(struct intel_encoder
*encoder,

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

index c3cadc09f859..36710a30fb37 100644
@@ -5949,6 +5949,28 @@ 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)
+{
+	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 +2 -1

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(i
 ntel_dp)));
 
 	mutex_lock(&dev_priv->pps_mutex);

}

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

@@ -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(i
 ntel_dp)));
 }

static void

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

@@ -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)

{

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

@@ -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))
 ;

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

 	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)

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

@@ -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));

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

 	pp = ironlake_get_pp_control(intel_dp);
 	/* We need to switch off panel power _and_ force vdd, for

otherwise some

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

@@ -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)

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

@@ -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);

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

 	/* 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));

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

 			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);

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

 	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)

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

@@ -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));

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

 	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))
 ;

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

 	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))
 ;

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

 	return ret;
 }

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

index 5bc7ba326111..4e16cda9d94a 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;

@@ -1711,6 +1710,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,

Quoted 2 lines

struct intel_crtc_state *pipe_config);
void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,

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

Imre Deak

author

2018-10-31 16:27 UTC #11 lore raw

PATCH v3 5/8 drm/i915: Enable AUX power earlier

In reply to 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)
v3:
- Add related BSpec entries to commit log. (Jose)

BSpec: 21750, 22243

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

Cc Manasi Navare <[email protected]>

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

Reviewed-by José Roberto de Souza <[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 +35 ~~-25~~

index 5bb459011a49..54699f0daf9d 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;
 }

@@ -2891,6 +2886,36 @@ 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));
+
+	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

Imre Deak

author

2018-10-31 16:27 UTC #12 lore raw

PATCH v3 6/8 drm/i915: Enable AUX power for HDMI DDI/TypeC main link too

In reply to Imre Deak

DDI/TypeC ports need the AUX power domain for main link functionality even when they operate in HDMI static mode, so enable the power domain for these ports too.

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

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

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

---
 drivers/gpu/drm/i915/intel_ddi.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

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

index 54699f0daf9d..4b6f25113c39 100644
@@ -2103,6 +2103,7 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
 static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
 				       struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port;
 	u64 domains;

@@ -2117,8 +2118,12 @@ static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
 	dig_port = enc_to_dig_port(&encoder->base);
 	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))
+	/*
+	 * 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_port_is_tc(dev_priv, encoder->port))
 		domains |= BIT_ULL(intel_ddi_main_link_aux_domain(dig_port));
 
 	return domains;

@@ -2894,7 +2899,8 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
 	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))
+	if (intel_crtc_has_dp_encoder(crtc_state) ||
+	    intel_port_is_tc(dev_priv, encoder->port))
 		intel_display_power_get(dev_priv,
 					intel_ddi_main_link_aux_domain(dig_port));

@@ -2911,7 +2917,8 @@ intel_ddi_post_pll_disable(struct intel_encoder *encoder,
 	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))
+	if (intel_crtc_has_dp_encoder(crtc_state) ||
+	    intel_port_is_tc(dev_priv, encoder->port))
 		intel_display_power_put(dev_priv,
 					intel_ddi_main_link_aux_domain(dig_port));
 }

Signature · 6 lines

2.13.2

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

Imre Deak

author

2018-10-31 16:27 UTC #13 lore raw

PATCH v3 7/8 drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain

In reply to Imre Deak

Most of the AUX_CH_CTL flags are concerned with DP AUX transfer parameters. As opposed to this the flag specifying the thunderbolt vs. non-thunderbolt mode of the port is not related to AUX transfers at all (rather it's repurposed to enable either TBT or non-TBT PHY HW blocks). The programming has to be done before enabling the corresponding AUX power well, so make it part of the power well code.

v3:
- Use existing enable/disable helpers instead of opencoding. (Jose)
- Fix type of is_tc_tbt to remain a bitfield. (Lucas)
- Add comment describing the is_tc_tbt power well flag. (Lucas)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108548

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

Cc Lucas De Marchi <[email protected]>

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

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

---
 drivers/gpu/drm/i915/i915_drv.h         |  5 ++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 50 +++++++++++++++++++++++++++------
 2 files changed, 47 insertions(+), 8 deletions(-)

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

index c57b701f72a7..6157f8128cc5 100644
@@ -921,6 +921,11 @@ struct i915_power_well_desc {
 			/* The pw is backing the VGA functionality */
 			bool has_vga:1;
 			bool has_fuses:1;
+			/*
+			 * The pw is for an ICL+ TypeC PHY port in
+			 * Thunderbolt mode.
+			 */
+			bool is_tc_tbt:1;
 		} hsw;
 	};
 	const struct i915_power_well_ops *ops;

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

135 lines not on this page · raw message

Signature · 6 lines

2.13.2

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

Imre Deak

author

2018-10-31 16:27 UTC #14 lore raw

PATCH v3 8/8 drm/i915/icl+: Sanitize port to PLL mapping

In reply to Imre Deak

BIOS can leave the PLL to port mapping enabled, even if the corresponding encoder is disabled. Disable the port mapping in this case.

Cc Paulo Zanoni <[email protected]>

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

Cc José Roberto de Souza <[email protected]>

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

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

---
 drivers/gpu/drm/i915/intel_ddi.c     | 23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 3 files changed, 28 insertions(+)

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

index 4b6f25113c39..0575817ac821 100644
@@ -2822,6 +2822,29 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc,
 	}
 }
 
+void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	u32 val = I915_READ(DPCLKA_CFGCR0_ICL);
+	enum port port = encoder->port;
+	bool clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port));
+
+	if (clk_enabled == !!encoder->base.crtc)
+		return;
+
+	/*
+	 * Punt on the case now where clock is disabled, but the encoder is
+	 * enabled, something else is really broken then.
+	 */
+	if (WARN_ON(!clk_enabled))
+		return;
+
+	DRM_NOTE("Port %c is disabled but it has a mapped PLL, unmap it\n",
+		 port_name(port));
+	val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port);
+	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
+}
+
 static void intel_ddi_clk_select(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *crtc_state)
 {

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

index 12ba2b923e6b..2534263ebb41 100644
@@ -15368,6 +15368,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 
 static void intel_sanitize_encoder(struct intel_encoder *encoder)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_connector *connector;
 
 	/* We need to check both for a crtc link (meaning that the

@@ -15409,6 +15410,9 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
 
 	/* notify opregion of the sanitized encoder state */
 	intel_opregion_notify_encoder(encoder, connector && has_active_crtc);
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		icl_sanitize_encoder_pll_mapping(encoder);
 }
 
 void i915_redisable_vga_power_on(struct drm_i915_private *dev_priv)

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

index 4e16cda9d94a..2f1a3258e120 100644
@@ -1520,6 +1520,7 @@ void icl_map_plls_to_ports(struct drm_crtc *crtc,
 void icl_unmap_plls_to_ports(struct drm_crtc *crtc,
 			     struct intel_crtc_state *crtc_state,
 			     struct drm_atomic_state *old_state);
+void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
 
 unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
 				   int color_plane, unsigned int height);

Signature · 6 lines

2.13.2

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

Patchwork

2018-10-31 16:42 UTC #15 lore raw

✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: Fix HDMI on TypeC static ports (rev3)

In reply to Imre Deak

== Series Details ==

Series: drm/i915/icl: Fix HDMI on TypeC static ports (rev3)
URL : https://patchwork.freedesktop.org/series/51765/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
700413b4d745 drm/i915: Move intel_aux_ch() to intel_bios.c
-:49: CHECK:SPACING: No space is necessary after a cast
#49: FILE: drivers/gpu/drm/i915/intel_bios.c:2170:
+		aux_ch = (enum aux_ch) port;

total: 0 errors, 0 warnings, 1 checks, 117 lines checked
b6f44bbdc454 drm/i915: Move aux_ch to intel_digital_port
fc764bcd6aa7 drm/i915: Init aux_ch for HDMI ports too
84fa5ef1a470 drm/i915: Use a helper to get the aux power domain
f2192fbc6a67 drm/i915: Enable AUX power earlier
0c704c7a83d1 drm/i915: Enable AUX power for HDMI DDI/TypeC main link too
c0a2121c9b44 drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain
-:42: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32> #42: FILE: drivers/gpu/drm/i915/i915_drv.h:928:
+ bool is_tc_tbt:1;

total: 0 errors, 1 warnings, 0 checks, 146 lines checked d5042a0d6ff8 drm/i915/icl+: Sanitize port to PLL mapping

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

Patchwork

2018-10-31 16:46 UTC #16 lore raw

✗ Fi.CI.SPARSE: warning for drm/i915/icl: Fix HDMI on TypeC static ports (rev3)

In reply to Imre Deak

== Series Details ==

Series: drm/i915/icl: Fix HDMI on TypeC static ports (rev3)
URL : https://patchwork.freedesktop.org/series/51765/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Move intel_aux_ch() to intel_bios.c
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3699:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3700:16: warning: expression using sizeof(void)

Commit: drm/i915: Move aux_ch to intel_digital_port Okay!

Commit: drm/i915: Init aux_ch for HDMI ports too
Okay!

Commit: drm/i915: Use a helper to get the aux power domain Okay!

Commit: drm/i915: Enable AUX power earlier
Okay!

Commit: drm/i915: Enable AUX power for HDMI DDI/TypeC main link too Okay!

Commit: drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain -drivers/gpu/drm/i915/selftests/../i915_drv.h:3700:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3705:16: warning: expression using sizeof(void)

Commit: drm/i915/icl+: Sanitize port to PLL mapping Okay!

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

Patchwork

2018-10-31 17:00 UTC #17 lore raw

✓ Fi.CI.BAT: success for drm/i915/icl: Fix HDMI on TypeC static ports (rev3)

In reply to Imre Deak

== Series Details ==

Series: drm/i915/icl: Fix HDMI on TypeC static ports (rev3)
URL : https://patchwork.freedesktop.org/series/51765/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5062 -> Patchwork_10672 =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/51765/revisions/3/mbox/

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> INCOMPLETE (fdo#108044)

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

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

    
    ==== Possible fixes ====

    igt@kms_chamelium@common-hpd-after-suspend:
      fi-skl-6700k2:      INCOMPLETE (fdo#104108, fdo#105524, k.org#199541) -> PASS

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

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
    ==== Warnings ====

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

== Participating hosts (49 -> 43) ==

Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-cfl-8109u

== Build changes ==

* Linux: CI_DRM_5062 -> Patchwork_10672

CI_DRM_5062: 3aa71a0d803ee01605f9a3026ddd989a591a73c6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4703: f882a542a3eb24e78e51aa6410a3a67c0efb4e97 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10672: d5042a0d6ff8b636a3d17097ccae13e1de079ab0 @ git://anongit.freedesktop.org/gfx-ci/linux

== Linux commits ==

d5042a0d6ff8 drm/i915/icl+: Sanitize port to PLL mapping c0a2121c9b44 drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain 0c704c7a83d1 drm/i915: Enable AUX power for HDMI DDI/TypeC main link too f2192fbc6a67 drm/i915: Enable AUX power earlier
84fa5ef1a470 drm/i915: Use a helper to get the aux power domain fc764bcd6aa7 drm/i915: Init aux_ch for HDMI ports too b6f44bbdc454 drm/i915: Move aux_ch to intel_digital_port 700413b4d745 drm/i915: Move intel_aux_ch() to intel_bios.c

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10672/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-11-01 00:02 UTC #18 lore raw

✓ Fi.CI.IGT: success for drm/i915/icl: Fix HDMI on TypeC static ports (rev3)

In reply to Imre Deak

== Series Details ==

Series: drm/i915/icl: Fix HDMI on TypeC static ports (rev3)
URL : https://patchwork.freedesktop.org/series/51765/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5062_full -> Patchwork_10672_full =

== Summary - SUCCESS ==

No regressions found.

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@in-order-bsd:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_exec_whisper@normal:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108592)

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-skl:          PASS -> FAIL (fdo#108470, fdo#108228, fdo#107815)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          PASS -> FAIL (fdo#106641)

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

    igt@kms_chv_cursor_fail@pipe-a-128x128-top-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671) +1

    igt@kms_chv_cursor_fail@pipe-c-128x128-bottom-edge:
      shard-apl:          PASS -> DMESG-WARN (fdo#103558, fdo#105602) +9

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +2

    igt@kms_cursor_crc@cursor-size-change:
      shard-skl:          NOTRUN -> FAIL (fdo#103232) +1

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

    igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
      shard-skl:          NOTRUN -> FAIL (fdo#103184)

    igt@kms_flip@basic-plain-flip:
      shard-apl:          PASS -> DMESG-FAIL (fdo#103558, fdo#105602)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#102887, fdo#105363)

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

    igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
      shard-skl:          NOTRUN -> FAIL (fdo#105682)

    igt@kms_pipe_crc_basic@read-crc-pipe-c:
      shard-skl:          NOTRUN -> FAIL (fdo#107362)

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

    igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
      shard-apl:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#108145)

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

    igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
      shard-glk:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@pm_rpm@basic-pci-d3-state:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

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

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-snb:          DMESG-WARN (fdo#107956) -> PASS

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

    igt@kms_color@pipe-b-degamma:
      shard-apl:          FAIL (fdo#104782) -> PASS

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

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-snb:          DMESG-WARN (fdo#102365) -> PASS

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

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

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

    
    ==== Warnings ====

    igt@kms_cursor_crc@cursor-256x256-onscreen:
      shard-snb:          INCOMPLETE (fdo#105411) -> DMESG-WARN (fdo#107469)

    igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
      shard-apl:          FAIL (fdo#108145) -> DMESG-WARN (fdo#103558, fdo#105602)

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

No changes in participating hosts

== Build changes ==

* Linux: CI_DRM_5062 -> Patchwork_10672

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10672/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx