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.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
