[PATCH] drm/i915/ddi: Simplify get_encoder_power_domains()

* [PATCH] drm/i915/ddi: Simplify get_encoder_power_domains()
@ 2018-07-05 12:26 Imre Deak
  2018-07-05 12:44 ` Ville Syrjälä
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Imre Deak @ 2018-07-05 12:26 UTC (permalink / raw)
  To: intel-gfx

We can simplify the encoder's get_power_domains() hook by calling it
only if the encoder is active. That way the hook can return its power
domains unconditionally without checking the active state by calling
encoder::get_hw_state(). This get_hw_state() query is in fact
redundant since it's already done by intel_modeset_readout_hw_state()
setting the encoder's crtc or leaving it NULL accordingly. Let's use
this fact to decide if the encoder is active.

While at it clarify the comment in intel_ddi_get_power_domains() about
primary vs. fake MST encoders and make sure we never do an incorrect
encoder->dig_port cast for fake MST encoders.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++------------
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0319825b725b..6b74eee5a3ac 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2005,24 +2005,19 @@ intel_ddi_main_link_aux_domain(struct intel_dp *intel_dp)
 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;
+	struct intel_digital_port *dig_port;
 	u64 domains;
 
-	if (!intel_ddi_get_hw_state(encoder, &pipe))
-		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.
+	 * happen since fake-MST encoders don't set their get_power_domains()
+	 * hook.
 	 */
 	if (WARN_ON(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)))
-		return domains;
+		return 0;
+
+	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)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 681e0710a467..a61dbdf2f612 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15705,14 +15705,13 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
 			continue;
 
 		/*
-		 * For MST and inactive encoders we don't have a crtc state.
-		 * FIXME: no need to call get_power_domains in such cases, it
-		 * will always return 0.
+		 * MST-primary and inactive encoders don't have a crtc state
+		 * and neither of these require any power domain references.
 		 */
-		crtc_state = encoder->base.crtc ?
-			     to_intel_crtc_state(encoder->base.crtc->state) :
-			     NULL;
+		if (!encoder->base.crtc)
+			continue;
 
+		crtc_state = to_intel_crtc_state(encoder->base.crtc->state);
 		get_domains = encoder->get_power_domains(encoder, crtc_state);
 		for_each_power_domain(domain, get_domains)
 			intel_display_power_get(dev_priv, domain);
-- 
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] 5+ messages in thread