On Thu, May 09, 2019 at 09:19:50AM +0300, Imre Deak wrote: > We don't need the AUX power for the whole duration of the detect, only > when we're doing AUX transfers. The AUX transfer function takes its own > reference on the AUX power domain already. The two places during detect > which access display core registers (not specific to a > pipe/port/transcoder) only need the power domain that is required for > that access. That power domain is equivalent to the device global power > domain on most platforms (enabled whenever we hold a runtime PM > reference) except on CHV/VLV where it's equivalent to the display power > well. > > Add a new power domain that reflects the above, and use this at the two > spots accessing registers. With that we can avoid taking the AUX > reference for the whole duration of the detect function. > > Put the domains asynchronously to avoid the unneeded on-off-on toggling. > > Cc: Ville Syrjala <ville.syrj...@linux.intel.com> > Signed-off-by: Imre Deak <imre.d...@intel.com> > --- > drivers/gpu/drm/i915/intel_display.h | 1 + > drivers/gpu/drm/i915/intel_dp.c | 32 +++++++++++++++++-------- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++++ > 3 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.h > b/drivers/gpu/drm/i915/intel_display.h > index 1b6f5a71184d..7f3fafdfbd5f 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -220,6 +220,7 @@ enum aux_ch { > #define aux_ch_name(a) ((a) + 'A') > > enum intel_display_power_domain { > + POWER_DOMAIN_DISPLAY_CORE, > POWER_DOMAIN_PIPE_A, > POWER_DOMAIN_PIPE_B, > POWER_DOMAIN_PIPE_C, > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 1e011998e9d5..24cca12a9a3e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -216,15 +216,21 @@ static int intel_dp_get_fia_supported_lane_count(struct > intel_dp *intel_dp) > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); > enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port); > + intel_wakeref_t wakeref; > u32 lane_info; > > if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC) > return 4; > > + wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DISPLAY_CORE); > + > lane_info = (I915_READ(PORT_TX_DFLEXDPSP) & > DP_LANE_ASSIGNMENT_MASK(tc_port)) >> > DP_LANE_ASSIGNMENT_SHIFT(tc_port); > > + intel_display_power_put_async(dev_priv, POWER_DOMAIN_DISPLAY_CORE, > + wakeref); > +
Future idea: maybe we want a with_something() {} for this sort of thing? > switch (lane_info) { > default: > MISSING_CASE(lane_info); Unrelated but this thing here looks like a hand rolled hweight(). And for some reason it's using decimal for bitmasks. > @@ -5294,7 +5300,7 @@ static bool icl_digital_port_connected(struct > intel_encoder *encoder) > * > * Return %true if port is connected, %false otherwise. > */ > -bool intel_digital_port_connected(struct intel_encoder *encoder) > +static bool __intel_digital_port_connected(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > @@ -5324,6 +5330,20 @@ bool intel_digital_port_connected(struct intel_encoder > *encoder) > return false; > } > > +bool intel_digital_port_connected(struct intel_encoder *encoder) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + intel_wakeref_t wakeref; > + bool res; bool is_connected perhaps? > + > + wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DISPLAY_CORE); > + res = __intel_digital_port_connected(encoder); > + intel_display_power_put_async(dev_priv, POWER_DOMAIN_DISPLAY_CORE, > + wakeref); > + > + return res; > +} > + > static struct edid * > intel_dp_get_edid(struct intel_dp *intel_dp) > { > @@ -5377,16 +5397,11 @@ intel_dp_detect(struct drm_connector *connector, > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > struct intel_encoder *encoder = &dig_port->base; > enum drm_connector_status status; > - enum intel_display_power_domain aux_domain = > - intel_aux_power_domain(dig_port); > - intel_wakeref_t wakeref; > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, connector->name); > > WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex)); > > - wakeref = intel_display_power_get(dev_priv, aux_domain); > - > /* Can't disconnect eDP */ > if (intel_dp_is_edp(intel_dp)) > status = edp_detect(intel_dp); > @@ -5450,10 +5465,8 @@ intel_dp_detect(struct drm_connector *connector, > int ret; > > ret = intel_dp_retrain_link(encoder, ctx); > - if (ret) { > - intel_display_power_put(dev_priv, aux_domain, wakeref); > + if (ret) > return ret; > - } > } > > /* > @@ -5475,7 +5488,6 @@ intel_dp_detect(struct drm_connector *connector, > if (status != connector_status_connected && !intel_dp->is_mst) > intel_dp_unset_edid(intel_dp); > > - intel_display_power_put(dev_priv, aux_domain, wakeref); > return status; > } > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 829d483d27e9..618dd42f1b2d 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -406,6 +406,8 @@ const char * > intel_display_power_domain_str(enum intel_display_power_domain domain) > { > switch (domain) { > + case POWER_DOMAIN_DISPLAY_CORE: > + return "DISPLAY_CORE"; > case POWER_DOMAIN_PIPE_A: > return "PIPE_A"; > case POWER_DOMAIN_PIPE_B: > @@ -2383,6 +2385,7 @@ void intel_display_power_put(struct drm_i915_private > *dev_priv, > BIT_ULL(POWER_DOMAIN_INIT)) > > #define VLV_DISPLAY_POWER_DOMAINS ( \ > + BIT_ULL(POWER_DOMAIN_DISPLAY_CORE) | \ > BIT_ULL(POWER_DOMAIN_PIPE_A) | \ > BIT_ULL(POWER_DOMAIN_PIPE_B) | \ > BIT_ULL(POWER_DOMAIN_PIPE_A_PANEL_FITTER) | \ > @@ -2429,6 +2432,7 @@ void intel_display_power_put(struct drm_i915_private > *dev_priv, > BIT_ULL(POWER_DOMAIN_INIT)) > > #define CHV_DISPLAY_POWER_DOMAINS ( \ > + BIT_ULL(POWER_DOMAIN_DISPLAY_CORE) | \ > BIT_ULL(POWER_DOMAIN_PIPE_A) | \ > BIT_ULL(POWER_DOMAIN_PIPE_B) | \ > BIT_ULL(POWER_DOMAIN_PIPE_C) | \ > -- > 2.17.1 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx