Re: [PATCH 1/4] drm/i915: add POWER_DOMAIN_PLLS

[Damien Lespiau <damien.lespiau@intel.com>]

On Fri, Jul 04, 2014 at 01:38:33PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> And get/put it when needed. The special thing about this commit is
> that it will now return false in ibx_pch_dpll_get_hw_state() in case
> the power domain is not enabled. This will fix some WARNs we have when
> we run pm_rpm on SNB.
> 
> Testcase: igt/pm_rpm
> Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=80463
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_pm.c      |  1 +
>  4 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8da9985..18d4f9e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2134,6 +2134,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>  		return "VGA";
>  	case POWER_DOMAIN_AUDIO:
>  		return "AUDIO";
> +	case POWER_DOMAIN_PLLS:
> +		return "PLLS";
>  	case POWER_DOMAIN_INIT:
>  		return "INIT";
>  	default:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ac06c0f..1cc1b8f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -129,6 +129,7 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_PORT_OTHER,
>  	POWER_DOMAIN_VGA,
>  	POWER_DOMAIN_AUDIO,
> +	POWER_DOMAIN_PLLS,
>  	POWER_DOMAIN_INIT,
>  
>  	POWER_DOMAIN_NUM,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c12a5da..3d69097 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1837,6 +1837,8 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
>  	}
>  	WARN_ON(pll->on);
>  
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +
>  	DRM_DEBUG_KMS("enabling %s\n", pll->name);
>  	pll->enable(dev_priv, pll);
>  	pll->on = true;
> @@ -1873,6 +1875,8 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
>  	DRM_DEBUG_KMS("disabling %s\n", pll->name);
>  	pll->disable(dev_priv, pll);
>  	pll->on = false;
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
>  static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> @@ -11280,6 +11284,9 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
>  {
>  	uint32_t val;
>  
> +	if (!intel_display_power_enabled(dev_priv, POWER_DOMAIN_PLLS))
> +		return false;
> +
>  	val = I915_READ(PCH_DPLL(pll->id));
>  	hw_state->dpll = val;
>  	hw_state->fp0 = I915_READ(PCH_FP0(pll->id));
> @@ -12845,6 +12852,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		DRM_DEBUG_KMS("%s hw state readout: refcount %i, on %i\n",
>  			      pll->name, pll->refcount, pll->on);
> +
> +		if (pll->refcount)
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  	}
>  
>  	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31ae2b4..cf4c521 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6310,6 +6310,7 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
>  	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_CRT) |			\
> +	BIT(POWER_DOMAIN_PLLS) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  #define HSW_DISPLAY_POWER_DOMAINS (				\
>  	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx