All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ander Conselvan De Oliveira <conselvan2@gmail.com>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/i915: Remove redundant toggling from the power well sync_hw hooks
Date: Thu, 16 Feb 2017 11:13:22 +0200	[thread overview]
Message-ID: <1487236402.3185.1.camel@gmail.com> (raw)
In-Reply-To: <1487159944-12862-2-git-send-email-imre.deak@intel.com>

On Wed, 2017-02-15 at 13:59 +0200, Imre Deak wrote:
> Doing an explicit enable/disable in the power well sync_hw hook based on
> the power well's reference count is redundant, since by the time these
> hooks are called all the power wells are enabled and have a reference.
> So remove the redundant toggling.
> 
> This is needed by a follow-up patchset that adds power wells which we
> can't enable/disable during power domain initialization and so want to
> preserve their state until modeset init time.
> 
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 52 +++++++-------------------------
> -
>  1 file changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8795679..0f64bc1 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -847,8 +847,6 @@ static void skl_set_power_well(struct drm_i915_private
> *dev_priv,
>  static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> -	hsw_set_power_well(dev_priv, power_well, power_well->count > 0);
> -
>  	/*
>  	 * We're taking over the BIOS, so clear any requests made by it since
>  	 * the driver is in charge now.
> @@ -881,8 +879,6 @@ static bool skl_power_well_enabled(struct drm_i915_private
> *dev_priv,
>  static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				struct i915_power_well *power_well)
>  {
> -	skl_set_power_well(dev_priv, power_well, power_well->count > 0);
> -
>  	/* Clear any request made by BIOS as driver is taking over */
>  	I915_WRITE(HSW_PWR_WELL_BIOS, 0);
>  }
> @@ -917,16 +913,6 @@ static bool bxt_dpio_cmn_power_well_enabled(struct
> drm_i915_private *dev_priv,
>  	return bxt_ddi_phy_is_enabled(dev_priv, power_well->data);
>  }
>  
> -static void bxt_dpio_cmn_power_well_sync_hw(struct drm_i915_private
> *dev_priv,
> -					    struct i915_power_well
> *power_well)
> -{
> -	if (power_well->count > 0)
> -		bxt_dpio_cmn_power_well_enable(dev_priv, power_well);
> -	else
> -		bxt_dpio_cmn_power_well_disable(dev_priv, power_well);
> -}
> -
> -
>  static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_well *power_well;
> @@ -989,13 +975,9 @@ static void gen9_dc_off_power_well_disable(struct
> drm_i915_private *dev_priv,
>  		gen9_enable_dc5(dev_priv);
>  }
>  
> -static void gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
> -					   struct i915_power_well
> *power_well)
> +static void i9xx_power_well_sync_hw_noop(struct drm_i915_private *dev_priv,
> +					 struct i915_power_well *power_well)
>  {
> -	if (power_well->count > 0)
> -		gen9_dc_off_power_well_enable(dev_priv, power_well);
> -	else
> -		gen9_dc_off_power_well_disable(dev_priv, power_well);
>  }
>  
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
> @@ -1045,12 +1027,6 @@ static void vlv_set_power_well(struct drm_i915_private
> *dev_priv,
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -static void vlv_power_well_sync_hw(struct drm_i915_private *dev_priv,
> -				   struct i915_power_well *power_well)
> -{
> -	vlv_set_power_well(dev_priv, power_well, power_well->count > 0);
> -}
> -
>  static void vlv_power_well_enable(struct drm_i915_private *dev_priv,
>  				  struct i915_power_well *power_well)
>  {
> @@ -1661,14 +1637,6 @@ static void chv_set_pipe_power_well(struct
> drm_i915_private *dev_priv,
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
> -					struct i915_power_well *power_well)
> -{
> -	WARN_ON_ONCE(power_well->id != PIPE_A);
> -
> -	chv_set_pipe_power_well(dev_priv, power_well, power_well->count > 0);
> -}
> -
>  static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>  				       struct i915_power_well *power_well)
>  {
> @@ -1914,21 +1882,21 @@ void intel_display_power_put(struct drm_i915_private
> *dev_priv,
>  	BIT_ULL(POWER_DOMAIN_INIT))
>  
>  static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
> -	.sync_hw = i9xx_always_on_power_well_noop,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = i9xx_always_on_power_well_noop,
>  	.disable = i9xx_always_on_power_well_noop,
>  	.is_enabled = i9xx_always_on_power_well_enabled,
>  };
>  
>  static const struct i915_power_well_ops chv_pipe_power_well_ops = {
> -	.sync_hw = chv_pipe_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = chv_pipe_power_well_enable,
>  	.disable = chv_pipe_power_well_disable,
>  	.is_enabled = chv_pipe_power_well_enabled,
>  };
>  
>  static const struct i915_power_well_ops chv_dpio_cmn_power_well_ops = {
> -	.sync_hw = vlv_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = chv_dpio_cmn_power_well_enable,
>  	.disable = chv_dpio_cmn_power_well_disable,
>  	.is_enabled = vlv_power_well_enabled,
> @@ -1958,14 +1926,14 @@ static const struct i915_power_well_ops
> skl_power_well_ops = {
>  };
>  
>  static const struct i915_power_well_ops gen9_dc_off_power_well_ops = {
> -	.sync_hw = gen9_dc_off_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = gen9_dc_off_power_well_enable,
>  	.disable = gen9_dc_off_power_well_disable,
>  	.is_enabled = gen9_dc_off_power_well_enabled,
>  };
>  
>  static const struct i915_power_well_ops bxt_dpio_cmn_power_well_ops = {
> -	.sync_hw = bxt_dpio_cmn_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = bxt_dpio_cmn_power_well_enable,
>  	.disable = bxt_dpio_cmn_power_well_disable,
>  	.is_enabled = bxt_dpio_cmn_power_well_enabled,
> @@ -2000,21 +1968,21 @@ static struct i915_power_well bdw_power_wells[] = {
>  };
>  
>  static const struct i915_power_well_ops vlv_display_power_well_ops = {
> -	.sync_hw = vlv_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = vlv_display_power_well_enable,
>  	.disable = vlv_display_power_well_disable,
>  	.is_enabled = vlv_power_well_enabled,
>  };
>  
>  static const struct i915_power_well_ops vlv_dpio_cmn_power_well_ops = {
> -	.sync_hw = vlv_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = vlv_dpio_cmn_power_well_enable,
>  	.disable = vlv_dpio_cmn_power_well_disable,
>  	.is_enabled = vlv_power_well_enabled,
>  };
>  
>  static const struct i915_power_well_ops vlv_dpio_power_well_ops = {
> -	.sync_hw = vlv_power_well_sync_hw,
> +	.sync_hw = i9xx_power_well_sync_hw_noop,
>  	.enable = vlv_power_well_enable,
>  	.disable = vlv_power_well_disable,
>  	.is_enabled = vlv_power_well_enabled,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-02-16  9:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-15 11:59 [PATCH 0/4] drm/i915: Fix clearing of BIOS power well requests Imre Deak
2017-02-15 11:59 ` [PATCH 1/4] drm/i915: Remove redundant toggling from the power well sync_hw hooks Imre Deak
2017-02-16  9:13   ` Ander Conselvan De Oliveira [this message]
2017-02-15 11:59 ` [PATCH 2/4] drm/i915: Call the sync_hw hook for power wells without a domain Imre Deak
2017-02-16  9:31   ` Ander Conselvan De Oliveira
2017-02-16 14:55     ` Imre Deak
2017-02-15 11:59 ` [PATCH 3/4] drm/i915/gen9: Fix clearing of the BIOS power well request register Imre Deak
2017-02-16  9:43   ` Ander Conselvan De Oliveira
2017-02-16 15:13     ` Imre Deak
2017-02-15 11:59 ` [PATCH 4/4] drm/i915: Preserve the state of power wells not explicitly enabled Imre Deak
2017-02-16 12:18   ` Ander Conselvan De Oliveira
2017-02-16 12:41     ` Imre Deak
2017-02-15 14:52 ` ✓ Fi.CI.BAT: success for drm/i915: Fix clearing of BIOS power well requests Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1487236402.3185.1.camel@gmail.com \
    --to=conselvan2@gmail.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.