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

[Imre Deak]

On ma, 2016-04-04 at 15:01 +0200, Patrik Jakobsson wrote:
> 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.

Yes, the reversed uninit order is the correct one. I'll mention this
change in the log.

> 
> With above comment fixed,
> Reviewed-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> 
> >  	mutex_unlock(&power_domains->lock);
> >  }
> >  
> > -- 
> > 2.5.0
> > 
>