drm/i915: Get power refs in encoder->get_power_domains()

Push getting the reference for the encoders' power domains into the
encoder get_power_domains() hook instead of doing this from the caller.
This way the encoder can store away the corresponding wakerefs.

This fixes the DSI encoder disabling, which didn't release these
power references it acquired during HW state readout.

Note that longtime ownership for the corresponding wakerefs can be thus
acquired / released in two ways. Nevertheless there is always only one
owner for them:

After HW readout (booting/system resume):
- encoder->get_power_domains() acquires
- encoder->disable*() releases

After a modeset (calling intel_atomic_commit()):
- encoder->enable*() acquires
- encoder->disable*() releases

* can be any of the encoder enable/disable hooks.

v2:
- Check that the DSI io_wakerefs are unset both during encoder HW
  readout and enabling. (Chris)

Fixes: 9d37c8e612b47 ("drm/i915: Markup paired operations on display power domains")
Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190407124655.31536-1-imre.deak@intel.com

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 527aafc16e62e0c43cadbe0f786ddcad6d81d386..462936dbeec5cc7867e44ce6fe2bbbed3096a916 100644 (file)
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -327,6 +327,21 @@ static void gen11_dsi_program_esc_clk_div(struct intel_encoder *encoder)
        }
 }
 
+static void get_dsi_io_power_domains(struct drm_i915_private *dev_priv,
+                                    struct intel_dsi *intel_dsi)
+{
+       enum port port;
+
+       for_each_dsi_port(port, intel_dsi->ports) {
+               WARN_ON(intel_dsi->io_wakeref[port]);
+               intel_dsi->io_wakeref[port] =
+                       intel_display_power_get(dev_priv,
+                                               port == PORT_A ?
+                                               POWER_DOMAIN_PORT_DDI_A_IO :
+                                               POWER_DOMAIN_PORT_DDI_B_IO);
+       }
+}
+
 static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
 {
        struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -340,13 +355,7 @@ static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
                I915_WRITE(ICL_DSI_IO_MODECTL(port), tmp);
        }
 
-       for_each_dsi_port(port, intel_dsi->ports) {
-               intel_dsi->io_wakeref[port] =
-                       intel_display_power_get(dev_priv,
-                                               port == PORT_A ?
-                                               POWER_DOMAIN_PORT_DDI_A_IO :
-                                               POWER_DOMAIN_PORT_DDI_B_IO);
-       }
+       get_dsi_io_power_domains(dev_priv, intel_dsi);
 }
 
 static void gen11_dsi_power_up_lanes(struct intel_encoder *encoder)
@@ -1223,20 +1232,11 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
        return 0;
 }
 
-static u64 gen11_dsi_get_power_domains(struct intel_encoder *encoder,
-                                      struct intel_crtc_state *crtc_state)
+static void gen11_dsi_get_power_domains(struct intel_encoder *encoder,
+                                       struct intel_crtc_state *crtc_state)
 {
-       struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
-       u64 domains = 0;
-       enum port port;
-
-       for_each_dsi_port(port, intel_dsi->ports)
-               if (port == PORT_A)
-                       domains |= BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO);
-               else
-                       domains |= BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO);
-
-       return domains;
+       get_dsi_io_power_domains(to_i915(encoder->base.dev),
+                                enc_to_intel_dsi(&encoder->base));
 }
 
 static bool gen11_dsi_get_hw_state(struct intel_encoder *encoder,

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 720de7a3f5e63ffc24bc95025d61a321ff18f830..0ab3a8a4384899f44ac30db764be9108c4926077 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2053,12 +2053,11 @@ intel_ddi_main_link_aux_domain(struct intel_digital_port *dig_port)
                                              intel_aux_power_domain(dig_port);
 }
 
-static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
-                                      struct intel_crtc_state *crtc_state)
+static void 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;
 
        /*
         * TODO: Add support for MST encoders. Atm, the following should never
@@ -2066,10 +2065,10 @@ static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
         * hook.
         */
        if (WARN_ON(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)))
-               return 0;
+               return;
 
        dig_port = enc_to_dig_port(&encoder->base);
-       domains = BIT_ULL(dig_port->ddi_io_power_domain);
+       intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
        /*
         * AUX power is only needed for (e)DP mode, and for HDMI mode on TC
@@ -2077,15 +2076,15 @@ static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder,
         */
        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));
+               intel_display_power_get(dev_priv,
+                                       intel_ddi_main_link_aux_domain(dig_port));
 
        /*
         * VDSC power is needed when DSC is enabled
         */
        if (crtc_state->dsc_params.compression_enable)
-               domains |= BIT_ULL(intel_dsc_power_domain(crtc_state));
-
-       return domains;
+               intel_display_power_get(dev_priv,
+                                       intel_dsc_power_domain(crtc_state));
 }
 
 void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb7f99618f0226d34b2edd3093ffa50aad72bd35..f29a348e8d71ba490a9d8eb6162297e346fcdd33 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16285,8 +16285,6 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
        struct intel_encoder *encoder;
 
        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)
@@ -16300,9 +16298,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
                        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);
+               encoder->get_power_domains(encoder, crtc_state);
        }
 }
 

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 766234a123e058cd2ea3a07f9d21b0d56b987cb5..a38b9cff5cd0e9960f44e23ffdf88875386aaa48 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -272,10 +272,12 @@ struct intel_encoder {
         * be set correctly before calling this function. */
        void (*get_config)(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,
-                                struct intel_crtc_state *crtc_state);
+       /*
+        * Acquires the power domains needed for an active encoder during
+        * hardware state readout.
+        */
+       void (*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