| Message ID | 1459773777-10701-1-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@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@linux.intel.com> > mutex_unlock(&power_domains->lock); > } > > -- > 2.5.0 >
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@linux.intel.com> > > Signed-off-by: Imre Deak <imre.deak@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@linux.intel.com> > > > mutex_unlock(&power_domains->lock); > > } > > > > -- > > 2.5.0 > > >
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); + mutex_unlock(&power_domains->lock); }
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@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@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(-)