[PATCH 1/2] drm/i915/ddi: Get AUX power domain for DP main link too

* [PATCH 1/2] drm/i915/ddi: Get AUX power domain for DP main link too
@ 2018-06-21 15:58 Imre Deak
  2018-06-21 15:58 ` [PATCH 2/2] drm/i915/cnl: Don't get separate AUX power domain ref for DP PSR Imre Deak
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Imre Deak @ 2018-06-21 15:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni

So far we got an AUX power domain reference only for the duration of DP
AUX transfers. However, the followings suggest that we also need these
for main link functionality:
- The specification doesn't state whether it's needed or not for main
  link functionality, but suggests that these power wells need to be
  enabled already during display core initialization (Sequences to
  Initialize Display).
- For PSR we need to keep the AUX power well enabled.
- On ICL combo PHY ports (non-TC) the AUX power well is needed for
  link training too: while the port is enabled with a DP link training
  test pattern trying to toggle the AUX power well will time out.
- On ICL MG PHY ports (TC) the AUX power well is needed also for main
  link functionality (both in DP and HDMI modes).
- Windows enables these power wells both for main and AUX lane
  functionality.

Based on the above take an AUX power reference for main link
functionality too. This makes a difference only on GEN10+ (GLK+)
platforms, where we have separate port specific AUX power wells.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 33 +++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 044fe1fb9872..b09cb6969bbb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1983,15 +1983,36 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	return ret;
 }
 
-static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder)
+static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
+				       struct intel_crtc_state *crtc_state)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	enum pipe pipe;
+	u64 domains;
 
-	if (intel_ddi_get_hw_state(encoder, &pipe))
-		return BIT_ULL(dig_port->ddi_io_power_domain);
+	if (!intel_ddi_get_hw_state(encoder, &pipe))
+		return 0;
 
-	return 0;
+	domains = BIT_ULL(dig_port->ddi_io_power_domain);
+	if (!crtc_state)
+		return domains;
+
+	/*
+	 * TODO: Add support for MST encoders. Atm, the following should never
+	 * happen since this function will be called only for the primary
+	 * encoder which doesn't have its own pipe/crtc_state.
+	 */
+	if (WARN_ON(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)))
+		return domains;
+
+	/* AUX power is only needed for (e)DP mode, not for HDMI. */
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
+		struct intel_dp *intel_dp = &dig_port->dp;
+
+		domains |= BIT_ULL(intel_dp->aux_power_domain);
+	}
+
+	return domains;
 }
 
 void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state)
@@ -2631,6 +2652,8 @@ 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_dp->aux_power_domain);
+
 	intel_dp_set_link_params(intel_dp, crtc_state->port_clock,
 				 crtc_state->lane_count, is_mst);
 
@@ -2775,6 +2798,8 @@ 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_dp->aux_power_domain);
 }
 
 static void intel_ddi_post_disable_hdmi(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2c8fef3ede54..d9fefcec4b1a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15676,11 +15676,21 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
 	for_each_intel_encoder(&dev_priv->drm, encoder) {
 		u64 get_domains;
 		enum intel_display_power_domain domain;
+		struct intel_crtc_state *crtc_state;
 
 		if (!encoder->get_power_domains)
 			continue;
 
-		get_domains = encoder->get_power_domains(encoder);
+		/*
+		 * In case of a dangling encoder without a crtc we still need
+		 * to get power domain refs. Such encoders will be disabled
+		 * dropping these references.
+		 */
+		crtc_state = encoder->base.crtc ?
+			     to_intel_crtc_state(encoder->base.crtc->state) :
+			     NULL;
+
+		get_domains = encoder->get_power_domains(encoder, crtc_state);
 		for_each_power_domain(domain, get_domains)
 			intel_display_power_get(dev_priv, domain);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0c3ac0eafde0..7174429d930a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -254,7 +254,8 @@ struct intel_encoder {
 			   struct intel_crtc_state *pipe_config);
 	/* Returns a mask of power domains that need to be referenced as part
 	 * of the hardware state readout code. */
-	u64 (*get_power_domains)(struct intel_encoder *encoder);
+	u64 (*get_power_domains)(struct intel_encoder *encoder,
+				 struct intel_crtc_state *crtc_state);
 	/*
 	 * Called during system suspend after all pending requests for the
 	 * encoder are flushed (for example for DP AUX transactions) and
-- 
2.13.2

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

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