On Tue, 2017-10-03 at 00:06 -0700, Rodrigo Vivi wrote:
> At cdclk initialization at spec they tell us to always use dvfs
> or avoid change cdclk if it fails.
>
> Here on pll sequence, spec tells us to only use dvfs "if the
> frequency will result in a change to the voltage requirement."
>
> So in order to respect that and avoid necessary interactions with
> PCODE we compare our current needed level with the current set level
> and only perform dvfs sequences when we need to change that level.
>
> v2: - Add missing blank line after declaration block
> - s/need/needs: When adding doc needs sounded better.
>
> Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 10 ++++++++++
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 16 +++++++++-------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index c62d6e752fb7..8111a13079e1 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1545,6 +1545,16 @@ int cnl_dvfs_new_level(int cdclk, int portclk)
> return 2;
> }
>
> +bool cnl_dvfs_needs_change(struct drm_i915_private *dev_priv, int
> level)
> +{
> + int old_level;
I was thinking of naming of this variable. We are fetching the current
level so why not rename this variable as cur_level?
Either way,
Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> +
> + mutex_lock(&dev_priv->rps.hw_lock);
> + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> &old_level);
> + mutex_unlock(&dev_priv->rps.hw_lock);
> + return level != old_level;
> +}
> +
> static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> const struct intel_cdclk_state
> *cdclk_state)
> {
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 0dddbd3a7a97..85c000891439 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1970,9 +1970,10 @@ static void cnl_ddi_pll_enable(struct
> drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll)
> {
> uint32_t val;
> - int ret;
> + int ret = 0;
> int level;
> int cdclk, portclk;
> + bool change_level;
>
> /* 1. Enable DPLL power in DPLL_ENABLE. */
> val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2011,7 +2012,12 @@ static void cnl_ddi_pll_enable(struct
> drm_i915_private *dev_priv,
> * requirement, follow the Display Voltage Frequency
> Switching
> * (DVFS) Sequence Before Frequency Change
> */
> - ret = cnl_dvfs_pre_change(dev_priv);
> + cdclk = dev_priv->cdclk.hw.cdclk;
> + portclk = intel_ddi_port_clock(dev_priv, pll->id);
> + level = cnl_dvfs_new_level(cdclk, portclk);
> + change_level = cnl_dvfs_needs_change(dev_priv, level);
> + if (change_level)
> + ret = cnl_dvfs_pre_change(dev_priv);
>
> /* 6. Enable DPLL in DPLL_ENABLE. */
> val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2031,12 +2037,8 @@ static void cnl_ddi_pll_enable(struct
> drm_i915_private *dev_priv,
> * requirement, follow the Display Voltage Frequency
> Switching
> * (DVFS) Sequence After Frequency Change
> */
> - if (ret == 0) {
> - cdclk = dev_priv->cdclk.hw.cdclk;
> - portclk = intel_ddi_port_clock(dev_priv, pll->id);
> - level = cnl_dvfs_new_level(cdclk, portclk);
> + if (change_level && ret == 0)
> cnl_dvfs_post_change(dev_priv, level);
> - }
>
> /*
> * 9. turn on the clock for the DDI and map the DPLL to the
> DDI
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index dec11d7b15ab..8621110401e3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1326,6 +1326,7 @@ void cnl_uninit_cdclk(struct drm_i915_private
> *dev_priv);
> int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv);
> void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int
> level);
> int cnl_dvfs_new_level(int cdclk, int portclk);
> +bool cnl_dvfs_needs_change(struct drm_i915_private *dev_priv, int
> level);
> void bxt_init_cdclk(struct drm_i915_private *dev_priv);
> void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
--
Mika Kahola - Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
