[Intel-gfx] [PATCH] drm/i915/skl: SKL CDCLK change on modeset tracking VCO

Ville Syrjälä ville.syrjala at linux.intel.com
Wed Dec 9 12:53:47 PST 2015


On Tue, Dec 08, 2015 at 04:15:05PM -0800, clinton.a.taylor at intel.com wrote:
> From: Clint Taylor <clinton.a.taylor at intel.com>
> 
> Track VCO frequency of SKL instead of the boot CDCLK and allow modeset
> to set cdclk based on the max required pixel clock based on VCO
> selected.
> 
> Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    2 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |    2 +-
>  drivers/gpu/drm/i915/intel_display.c |   83 +++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_dp.c      |    9 +++-
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  5 files changed, 81 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f1a8a53..3ff5abd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1777,7 +1777,7 @@ struct drm_i915_private {
>  	int num_fence_regs; /* 8 on pre-965, 16 otherwise */
>  
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
> -	unsigned int skl_boot_cdclk;
> +	unsigned int skl_vco_freq;
>  	unsigned int cdclk_freq, max_cdclk_freq;
>  	unsigned int max_dotclk_freq;
>  	unsigned int hpll_freq;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5d20c64..b787d02 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3019,7 +3019,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  		int cdclk_freq;
>  
>  		cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> -		dev_priv->skl_boot_cdclk = cdclk_freq;
> +		dev_priv->skl_vco_freq = skl_cdclk_get_vco(cdclk_freq);
>  		if (skl_sanitize_cdclk(dev_priv))
>  			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2e8d1a8..852dd08 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5671,7 +5671,7 @@ static unsigned int skl_cdclk_decimal(unsigned int freq)
>  	return (freq - 1000) / 500;
>  }
>  
> -static unsigned int skl_cdclk_get_vco(unsigned int freq)
> +unsigned int skl_cdclk_get_vco(unsigned int freq)
>  {
>  	unsigned int i;
>  
> @@ -5829,17 +5829,17 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  
>  void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	unsigned int required_vco;
> -
>  	/* DPLL0 not enabled (happens on early BIOS versions) */
>  	if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) {
>  		/* enable DPLL0 */
> -		required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk);
> -		skl_dpll0_enable(dev_priv, required_vco);
> +		if (dev_priv->skl_vco_freq != 8640) {
> +			dev_priv->skl_vco_freq = 8100;
> +		}
> +		skl_dpll0_enable(dev_priv, dev_priv->skl_vco_freq);
>  	}
>  
>  	/* set CDCLK to the frequency the BIOS chose */
> -	skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk);
> +	skl_set_cdclk(dev_priv, (dev_priv->skl_vco_freq == 8100) ? 337500 : 308570 );
>  
>  	/* enable DBUF power */
>  	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> @@ -5855,7 +5855,7 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  {
>  	uint32_t lcpll1 = I915_READ(LCPLL1_CTL);
>  	uint32_t cdctl = I915_READ(CDCLK_CTL);
> -	int freq = dev_priv->skl_boot_cdclk;
> +	int freq = dev_priv->cdclk_freq;
>  
>  	/*
>  	 * check if the pre-os intialized the display
> @@ -5879,11 +5879,7 @@ int skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  		/* All well; nothing to sanitize */
>  		return false;
>  sanitize:
> -	/*
> -	 * As of now initialize with max cdclk till
> -	 * we get dynamic cdclk support
> -	 * */
> -	dev_priv->skl_boot_cdclk = dev_priv->max_cdclk_freq;
> +
>  	skl_init_cdclk(dev_priv);
>  
>  	/* we did have to sanitize */
> @@ -9805,6 +9801,64 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
>  	broadwell_set_cdclk(dev, req_cdclk);
>  }
>  
> +static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	int max_pixclk = ilk_max_pixel_rate(state);
> +	int cdclk;
> +	
> +	/*
> +	* FIXME should also account for plane ratio
> +	* once 64bpp pixel formats are supported.
> +	*/
> +
> +	if (dev_priv->skl_vco_freq == 8640) {
> +		/* vco 8640 */
> +		if (max_pixclk > 540000)
> +			cdclk = 617140;
> +		else if (max_pixclk > 432000)
> +			cdclk = 540000;
> +		else if (max_pixclk > 308570)
> +			cdclk = 432000;
> +		else
> +			cdclk = 308570;
> +	}
> +	else {
> +		/* VCO 8100 */
> +		if (max_pixclk > 540000)
> +			cdclk = 675000;
> +		else if (max_pixclk > 450000)
> +			cdclk = 540000;
> +		else if (max_pixclk > 337500)
> +			cdclk = 450000;
> +		else
> +			cdclk = 337500;
> +	}
> +
> +	/*
> +	 * FIXME move the cdclk caclulation to
> +	 * compute_config() so we can fail gracegully.
> +	 */
> +	if (cdclk > dev_priv->max_cdclk_freq) {
> +		DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> +			  cdclk, dev_priv->max_cdclk_freq);
> +		cdclk = dev_priv->max_cdclk_freq;
> +	}
> +
> +	to_intel_atomic_state(state)->cdclk = cdclk;
> +
> +	return 0;
> +}
> +
> +static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> +{
> +	struct drm_device *dev = old_state->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
> +
> +	skl_set_cdclk(dev_priv, req_cdclk);
> +}
> +
>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)
>  {
> @@ -14945,6 +14999,11 @@ static void intel_init_display(struct drm_device *dev)
>  			broxton_modeset_commit_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			broxton_modeset_calc_cdclk;
> +	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> +		dev_priv->display.modeset_commit_cdclk =
> +			skl_modeset_commit_cdclk;
> +		dev_priv->display.modeset_calc_cdclk =
> +			skl_modeset_calc_cdclk;
>  	}
>  
>  	switch (INTEL_INFO(dev)->gen) {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f335c92..e87adcb 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1220,9 +1220,10 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
>  }
>  
>  static void
> -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config)
> +skl_edp_set_pll_config(struct drm_i915_private *dev_priv, struct intel_crtc_state *pipe_config)
>  {
>  	u32 ctrl1;
> +	u32 vco = 8100;
>  
>  	memset(&pipe_config->dpll_hw_state, 0,
>  	       sizeof(pipe_config->dpll_hw_state));
> @@ -1255,13 +1256,17 @@ skl_edp_set_pll_config(struct intel_crtc_state *pipe_config)
>  	case 108000:
>  		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
>  					      SKL_DPLL0);
> +		vco = 8640;
>  		break;
>  	case 216000:
>  		ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160,
>  					      SKL_DPLL0);
> +		vco = 8640;
>  		break;
>  
>  	}
> +
> +	dev_priv->skl_vco_freq = vco;

I think we would need to include this also in the pipe_config, compute
that in intel_dp_compute_config() based on the required link frequency,
and then use that in the calc_cdclk function. That way we'll compute the
right cdclk, and force a modeset across all pipes if it needs to change.

One extra complication is that in theory we'll need to account for all
the pipe's computed vco frequency. For non-eDP I think we'll want the
compute_config to set the pipe_config earget vco to 0, and we'll take
that to mean "don't care". Then the cdclk computation can first walk
all the crtc states (like ilk_max_pixel_rate()) to find the actual
requested vco (and if that's 0, then we can keep using the current
vco), aborting if there's a conflict between two states. We can then
stuff the result into the top level intel_atomic_state for later use
like we do with the cdclk.

intel_modeset_checks() will then have to check if dev_priv->skl_vco_freq
differs from the one we computed so that we'll also force a modeset in
case cdclk remains the same but the vco must be changed (needed since
540MHz can be done with either vco).

Then once we actually set the cdclk we also update
dev_priv->skl_vco_freq with whatever we calculated. Actually we should
handke it same way we handle cdclk, and actually read it out from DPLL0.

>  	pipe_config->dpll_hw_state.ctrl1 = ctrl1;
>  }
>  
> @@ -1642,7 +1647,7 @@ found:
>  	}
>  
>  	if ((IS_SKYLAKE(dev)  || IS_KABYLAKE(dev)) && is_edp(intel_dp))
> -		skl_edp_set_pll_config(pipe_config);
> +		skl_edp_set_pll_config(dev_priv, pipe_config);
>  	else if (IS_BROXTON(dev))
>  		/* handled in ddi */;
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8963a8a..554d587 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1182,6 +1182,7 @@ void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>  void skl_init_cdclk(struct drm_i915_private *dev_priv);
>  int skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
>  void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
> +unsigned int skl_cdclk_get_vco(unsigned int freq);
>  void skl_enable_dc6(struct drm_i915_private *dev_priv);
>  void skl_disable_dc6(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC


More information about the Intel-gfx mailing list