[Intel-gfx] [PATCH v2 08/16] drm/i915/skl: Unexport skl_pw1_misc_io_init

[Patrik Jakobsson]

On Mon, Apr 04, 2016 at 03:42:57PM +0300, Imre Deak wrote:
> On Broxton we need to enable/disable power well 1 during the init/unit display
> sequence similarly to Skylake/Kabylake. The code for this will be added in a
> follow-up patch, but to prepare for that unexport skl_pw1_misc_io_init(). It's
> a simple function called only from a single place and having it inlined in the
> Skylake display core init/unit functions will make it easier to compare it
> with its Broxton counterpart.
> 
> No functional change.
> 
> v2:
> - Fix incorrect enable vs. disable power well call in
>   skl_display_core_uninit() (Patrik)
> 
> CC: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h        |  2 --
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 49 ++++++++++++---------------------
>  2 files changed, 18 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9255b56..8ba2ac3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1460,8 +1460,6 @@ int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>  void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
> -void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
> -void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
>  const char *
>  intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b16315e..8d401bb 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1921,34 +1921,6 @@ static struct i915_power_well skl_power_wells[] = {
>  	},
>  };
>  
> -void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv)
> -{
> -	struct i915_power_well *well;
> -
> -	if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)))
> -		return;
> -
> -	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> -	intel_power_well_enable(dev_priv, well);
> -
> -	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> -	intel_power_well_enable(dev_priv, well);
> -}
> -
> -void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv)
> -{
> -	struct i915_power_well *well;
> -
> -	if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)))
> -		return;
> -
> -	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> -	intel_power_well_disable(dev_priv, well);
> -
> -	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> -	intel_power_well_disable(dev_priv, well);
> -}
> -
>  static struct i915_power_well bxt_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -2139,9 +2111,10 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
>  }
>  
>  static void skl_display_core_init(struct drm_i915_private *dev_priv,
> -				  bool resume)
> +				   bool resume)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *well;
>  	uint32_t val;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> @@ -2152,7 +2125,13 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
> -	skl_pw1_misc_io_init(dev_priv);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_enable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_enable(dev_priv, well);
> +
>  	mutex_unlock(&power_domains->lock);
>  
>  	if (!resume)
> @@ -2167,6 +2146,7 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *well;
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> @@ -2174,8 +2154,15 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  
>  	/* The spec doesn't call for removing the reset handshake flag */
>  	/* disable PG1 and Misc I/O */
> +
>  	mutex_lock(&power_domains->lock);
> -	skl_pw1_misc_io_fini(dev_priv);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_MISC_IO);
> +	intel_power_well_disable(dev_priv, well);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_disable(dev_priv, well);
> +

I see you've flipped the order here. Probably for the better since I'm guessing
the old behaviour was by accident and not by design, but perhaps we should
mention this in the commit.

With above comment fixed,
Reviewed-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>

>  	mutex_unlock(&power_domains->lock);
>  }
>  
> -- 
> 2.5.0
> 

-- 
---
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027