On ke, 2015-11-04 at 19:24 +0200, Imre Deak wrote: > From: Damien Lespiau <[email protected]> > > Before this patch, we used the intel_display_power_{get,put} > functions > to make sure the PW1 and Misc I/O power wells were enabled all the > time while LCPLL was enabled. We called a get() at > intel_ddi_pll_init() when we discovered that LCPLL was enabled, then > we would call put/get at skl_{un,}init_cdclk(). > > The problem is that skl_uninit_cdclk() is indirectly called by > intel_runtime_suspend(). So it will only release its power well > _after_ we already decided to runtime suspend. But since we only > decide to runtime suspend after all power wells and refcounts are > released, that basically means we will never decide to runtime > suspend. > > So what this patch does to fix that problem is move the PW1 + Misc > I/O > power well handling out of the runtime PM mechanism: instead of > calling intel_display_power_{get_put} - functions that touch the > refcount -, we'll call the low level intel_power_well_{en,dis}able, > which don't change the refcount. This way, it is now possible for the > refcount to actually reach zero, and we'll now start runtime > suspending/resuming. > > v2 (from Paulo): > - Write a commit message since the original patch left it empty. > - Rebase after the intel_power_well_{en,dis}able rename. > - Use lookup_power_well() instead of hardcoded indexes. > > Testcase: igt/pm_rpm/rte (and every other rpm test) > Signed-off-by: Damien Lespiau <[email protected]> > Reviewed-by: Paulo Zanoni <[email protected]> > Signed-off-by: Paulo Zanoni <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92211 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92605 > --- > drivers/gpu/drm/i915/intel_ddi.c | 4 ++-- > drivers/gpu/drm/i915/intel_display.c | 5 +++-- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 29 > +++++++++++++++++++++++++++++ > 4 files changed, 36 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index b164122..cacc406 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2951,8 +2951,8 @@ void intel_ddi_pll_init(struct drm_device *dev) > dev_priv->skl_boot_cdclk = cdclk_freq; > if (skl_sanitize_cdclk(dev_priv)) > DRM_DEBUG_KMS("Sanitized cdclk programmed by > pre-os\n"); > - else > - intel_display_power_get(dev_priv, > POWER_DOMAIN_PLLS); > + if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) > + DRM_ERROR("LCPLL1 is disabled\n"); > } else if (IS_BROXTON(dev)) { > broxton_init_cdclk(dev); > broxton_ddi_phy_init(dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 7b3cfb6..ef2ebcd 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5702,7 +5702,8 @@ void skl_uninit_cdclk(struct drm_i915_private > *dev_priv) > DRM_ERROR("Couldn't disable DPLL0\n"); > } > > - intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > + /* disable PG1 and Misc I/O */ > + skl_pw1_misc_io_fini(dev_priv); > } > > void skl_init_cdclk(struct drm_i915_private *dev_priv) > @@ -5715,7 +5716,7 @@ void skl_init_cdclk(struct drm_i915_private > *dev_priv) > I915_WRITE(HSW_NDE_RSTWRN_OPT, val | > RESET_PCH_HANDSHAKE_ENABLE); > > /* enable PG1 and Misc I/O */ > - intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS); > + skl_pw1_misc_io_init(dev_priv); > > /* DPLL0 not enabled (happens on early BIOS versions) */ > if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 5bc744a..3d9d1d3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1381,6 +1381,8 @@ void intel_psr_single_frame_update(struct > drm_device *dev, > 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); > +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); > > bool intel_display_power_is_enabled(struct drm_i915_private > *dev_priv, > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 868c0f2..16691a3 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1783,6 +1783,35 @@ 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)) > + 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)) > + 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", _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
