Re: [PATCH 08/21] drm/i915: Keep track of preferred cdclk vco frequency on SKL

[Imre Deak <imre.deak@intel.com>]

On pe, 2016-05-13 at 23:41 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that skl_vco_freq tracks the actual DPLL0 vco frequency, we'll need
> something that keeps track of which vco frequency we want to use in case
> the current vco is 0. This would be important across supend/resume since
> we'll disable DPLL0 around those parts.
> 
> We'll also update our idea of max cdclk/dotclock when the preferred
> vco changes. That could happen if out initial guess was wrong, and
> later eDP would force us to change it. One issue here could be that
> changing the max dotclock could cause our mode list to change during
> next time the displays get probed. But I don't see a good way to avoid
> that, except perhaps by allowing either vco frequency to be used as
> needed. But the docs suggest that such usage wasn't really inteded.
> 
> Also need to make sure we don't update our max_cdclk value before we
> have a preferred vco value, which means moving that to happen after
> the cdclk sanitation.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/intel_display.c  | 48 +++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  5 ++++
>  drivers/gpu/drm/i915/intel_drv.h      |  1 +
>  4 files changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b319da970c8a..46a22732088e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1821,6 +1821,7 @@ struct drm_i915_private {
>  
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
>  	unsigned int skl_vco_freq;
> +	unsigned int skl_preferred_vco_freq;
>  	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
>  	unsigned int max_dotclk_freq;
>  	unsigned int rawclk_freq;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cd2809179042..107a7799bdde 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5265,21 +5265,34 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  		return max_cdclk_freq*90/100;
>  }
>  
> +static int skl_calc_cdclk(int max_pixclk, int vco);
> +
>  static void intel_update_max_cdclk(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
>  		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> +		int max_cdclk, vco;
> +
> +		vco = dev_priv->skl_preferred_vco_freq;
> +		WARN_ON(vco != 8100 && vco != 8640);
>  
> +		/*
> +		 * Use the lower (vco 8640) cdclk values as a
> +		 * first guess. skl_calc_cdclk() will correct it
> +		 * if the preferred vco is 8100 instead.
> +		 */
>  		if (limit == SKL_DFSM_CDCLK_LIMIT_675)
> -			dev_priv->max_cdclk_freq = 675000;
> +			max_cdclk = 617140;
>  		else if (limit == SKL_DFSM_CDCLK_LIMIT_540)
> -			dev_priv->max_cdclk_freq = 540000;
> +			max_cdclk = 540000;
>  		else if (limit == SKL_DFSM_CDCLK_LIMIT_450)
> -			dev_priv->max_cdclk_freq = 450000;
> +			max_cdclk = 432000;
>  		else
> -			dev_priv->max_cdclk_freq = 337500;
> +			max_cdclk = 308570;
> +
> +		dev_priv->max_cdclk_freq = skl_calc_cdclk(max_cdclk, vco);
>  	} else if (IS_BROXTON(dev)) {
>  		dev_priv->max_cdclk_freq = 624000;
>  	} else if (IS_BROADWELL(dev))  {
> @@ -5336,9 +5349,6 @@ static void intel_update_cdclk(struct drm_device *dev)
>  	 */
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> -
> -	if (dev_priv->max_cdclk_freq == 0)
> -		intel_update_max_cdclk(dev);
>  }
>  
>  /* convert from kHz to .1 fixpoint MHz with -1MHz offset */
> @@ -5587,12 +5597,24 @@ skl_dpll0_update(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv, int vco)
> +{
> +	bool changed = dev_priv->skl_preferred_vco_freq != vco;
> +
> +	dev_priv->skl_preferred_vco_freq = vco;
> +
> +	if (changed)
> +		intel_update_max_cdclk(dev_priv->dev);
> +}
> +
>  static void
>  skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>  {
>  	int min_cdclk = skl_calc_cdclk(0, vco);
>  	u32 val;
>  
> +	WARN_ON(vco != 8100 && vco != 8640);
> +
>  	/* select the minimum CDCLK before enabling DPLL 0 */
>  	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_cdclk);
>  	I915_WRITE(CDCLK_CTL, val);
> @@ -5628,6 +5650,9 @@ skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>  		DRM_ERROR("DPLL0 not locked\n");
>  
>  	dev_priv->skl_vco_freq = vco;
> +
> +	/* We'll want to keep using the current vco from now on. */
> +	skl_set_preferred_cdclk_vco(dev_priv, vco);
>  }
>  
>  static void
> @@ -5744,7 +5769,9 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  		int cdclk, vco;
>  
>  		/* set CDCLK to the lowest frequency, Modeset follows */
> -		vco = 8100;
> +		vco = dev_priv->skl_preferred_vco_freq;
> +		if (vco == 0)
> +			vco = 8100;
>  		cdclk = skl_calc_cdclk(0, vco);
>  
>  		skl_set_cdclk(dev_priv, cdclk, vco);
> @@ -13359,6 +13386,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>  	if (dev_priv->display.modeset_calc_cdclk) {
>  		if (!intel_state->cdclk_pll_vco)
>  			intel_state->cdclk_pll_vco = dev_priv->skl_vco_freq;
> +		if (!intel_state->cdclk_pll_vco)
> +			intel_state->cdclk_pll_vco = dev_priv->skl_preferred_vco_freq;
>  
>  		ret = dev_priv->display.modeset_calc_cdclk(state);
>  		if (ret < 0)
> @@ -15525,6 +15554,9 @@ void intel_modeset_init(struct drm_device *dev)
>  
>  	intel_shared_dpll_init(dev);
>  
> +	if (dev_priv->max_cdclk_freq == 0)
> +		intel_update_max_cdclk(dev);
> +
>  	/* Just disable it once at startup */
>  	i915_disable_vga(dev);
>  	intel_setup_outputs(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 5391ab66b64d..34ec149fde85 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1635,6 +1635,11 @@ static void intel_ddi_pll_init(struct drm_device *dev)
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
>  		if (skl_sanitize_cdclk(dev_priv))
>  			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
> +
> +		/* We'll want to keep using the current vco from now on */
> +		if (dev_priv->skl_vco_freq != 0)
> +			skl_set_preferred_cdclk_vco(dev_priv,
> +						    dev_priv->skl_vco_freq);
>  	} else if (!IS_BROXTON(dev_priv)) {
>  		/*
>  		 * The LCPLL register should be turned on by the BIOS. For now
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c7cb9829547e..8f48a32e991b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1145,6 +1145,7 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv);
>  void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
>  
>  /* intel_display.c */
> +void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv, int vco);
>  void intel_update_rawclk(struct drm_i915_private *dev_priv);
>  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
>  		      const char *name, u32 reg, int ref_freq);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx