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@xxxxxxxxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx>
>
> > mutex_unlock(&power_domains->lock);
> > }
> >
> > --
> > 2.5.0
> >
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
