From: Imre Deak Date: Thu, 5 Jul 2018 15:26:54 +0300 Subject: drm/i915/ddi: Simplify get_encoder_power_domains() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git-commit: b79ebe74e1c4219e91940b637d40939b0c80c0f2 Patch-mainline: v4.19-rc1 References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166 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ä Cc: Ville Syrjälä Signed-off-by: Imre Deak Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180705122654.17072-1-imre.deak@intel.com Acked-by: Petr Tesarik --- drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++------------ drivers/gpu/drm/i915/intel_display.c | 11 +++++------ 2 files changed, 12 insertions(+), 18 deletions(-) --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2015,24 +2015,19 @@ intel_ddi_main_link_aux_domain(struct in 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)) { --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15726,14 +15726,13 @@ get_encoder_power_domains(struct drm_i91 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);