From: Imre Deak <imre.deak@intel.com>
Date: Thu, 21 Jun 2018 21:44:49 +0300
Subject: drm/i915/ddi: Get AUX power domain for DP main link too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 525280552b21722d2dfb48a7020dcd56f9e2023c
Patch-mainline: v4.19-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166
So far we got an AUX power domain reference only for the duration of DP
AUX transfers. However, the following suggests 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.
For PSR we still need to distinguish between port A and the other
ports, since on port A DC states must stay enabled for main link
functionality, but DC states must be disabled for driver initiated
AUX transfers. So re-use the corresponding helper from intel_psr.c.
Since we take now a reference for main link functionality on all DP
ports we can forgo taking the separate power ref for PSR functionality.
v2:
- Make sure DC states stay enabled when taking the ref on port A.
(Ville)
v3: (Ville)
- Fix comment about logic for encoders without a crtc state and
add FIXME note for a simplification to avoid calling get_power_domains
in such cases.
- Use intel_crtc_has_dp_encoder() instead !intel_crtc_has_type(HDMI).
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>
[Clarified code comments in intel_ddi_main_link_aux_domain() and
intel_ddi_get_power_domains() (Imre)]
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180621184449.26634-1-imre.deak@intel.com
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 54 +++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_display.c | 12 ++++++-
drivers/gpu/drm/i915/intel_drv.h | 3 +
drivers/gpu/drm/i915/intel_psr.c | 41 ------------------------
drivers/gpu/drm/i915/intel_runtime_pm.c | 1
5 files changed, 64 insertions(+), 47 deletions(-)
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1992,15 +1992,55 @@ out:
return ret;
}
-static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder)
+static inline enum intel_display_power_domain
+intel_ddi_main_link_aux_domain(struct intel_dp *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
+ * disabled. Accordingly use the AUX power domain here which leaves DC
+ * states enabled.
+ * However, for non-A AUX ports the corresponding non-EDP transcoders
+ * would have already enabled power well 2 and DC_OFF. This means we can
+ * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
+ * specific AUX_IO reference without powering up any extra wells.
+ * 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 :
+ intel_dp->aux_power_domain;
+}
+
+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_dp_encoder(crtc_state)) {
+ struct intel_dp *intel_dp = &dig_port->dp;
+
+ domains |= BIT_ULL(intel_ddi_main_link_aux_domain(intel_dp));
+ }
+
+ return domains;
}
void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state)
@@ -2640,6 +2680,9 @@ static void intel_ddi_pre_enable_dp(stru
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);
@@ -2784,6 +2827,9 @@ static void intel_ddi_post_disable_dp(st
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,
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15723,11 +15723,21 @@ get_encoder_power_domains(struct drm_i91
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);
+ /*
+ * 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.
+ */
+ 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);
}
--- 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
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,43 +56,6 @@
#include "intel_drv.h"
#include "i915_drv.h"
-static inline enum intel_display_power_domain
-psr_aux_domain(struct intel_dp *intel_dp)
-{
- /* CNL HW requires corresponding AUX IOs to be powered up for PSR.
- * However, for non-A AUX ports the corresponding non-EDP transcoders
- * would have already enabled power well 2 and DC_OFF. This means we can
- * acquire a wider POWER_DOMAIN_AUX_{B,C,D,F} reference instead of a
- * specific AUX_IO reference without powering up any extra wells.
- * 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 :
- intel_dp->aux_power_domain;
-}
-
-static void psr_aux_io_power_get(struct intel_dp *intel_dp)
-{
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
-
- if (INTEL_GEN(dev_priv) < 10)
- return;
-
- intel_display_power_get(dev_priv, psr_aux_domain(intel_dp));
-}
-
-static void psr_aux_io_power_put(struct intel_dp *intel_dp)
-{
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
-
- if (INTEL_GEN(dev_priv) < 10)
- return;
-
- intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
-}
-
void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
{
u32 debug_mask, mask;
@@ -595,8 +558,6 @@ static void hsw_psr_enable_source(struct
struct drm_i915_private *dev_priv = to_i915(dev);
enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- psr_aux_io_power_get(intel_dp);
-
/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
* use hardcoded values PSR AUX transactions
*/
@@ -717,8 +678,6 @@ static void hsw_psr_disable(struct intel
else
WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
}
-
- psr_aux_io_power_put(intel_dp);
}
/**
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1824,6 +1824,7 @@ void intel_display_power_put(struct drm_
BIT_ULL(POWER_DOMAIN_INIT))
#define GLK_DISPLAY_AUX_A_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_A) | \
+ BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \
BIT_ULL(POWER_DOMAIN_INIT))
#define GLK_DISPLAY_AUX_B_POWER_DOMAINS ( \
BIT_ULL(POWER_DOMAIN_AUX_B) | \