On Wed, Nov 04, 2015 at 07:24:12PM +0200, Imre Deak wrote:
> From: Damien Lespiau <damien.lespiau@xxxxxxxxx>
>
> 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 <damien.lespiau@xxxxxxxxx>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx>
> ---
> 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",
> --
> 2.1.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
