On Fri, 2016-05-13 at 23:41 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Make bxt_set_cdclk() more readable by looking at current vs. target
> DE PLL vco to determine if the DE PLL needs disabling and/or enabling.
> We can also calculate the CD2X divider simply as (vco/cdclk) instead of
> depending on magic numbers.
>
> The magic numbers are still needed though, but only to map the supported
> cdclk frequencies to corresponding DE PLL frequencies.
>
> Note that w'll now program CDCLK_CTL correctly even for the bypass case.
> Actually the CD2X divider should not matter in that case since the
> hardware will bypass it too, but the "decimal" part should matter (if we
> want to do gmbus/aux with the bypass enabled).
Well, what the bypass can be used for is not defined by the spec
(neither is the decimal value for the bypass freq).
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++------------------
> 1 file changed, 63 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4542c1f5012f..14bc14c7827b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5358,6 +5358,30 @@ static int skl_cdclk_decimal(int cdclk)
> return DIV_ROUND_CLOSEST(cdclk - 1000, 500);
> }
>
> +static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> +{
> + int ratio;
> +
> + if (cdclk == dev_priv->cdclk_pll.ref)
> + return 0;
> +
> + switch (cdclk) {
> + default:
> + MISSING_CASE(cdclk);
> + case 144000:
> + case 288000:
> + case 384000:
> + case 576000:
> + ratio = 60;
> + break;
> + case 624000:
> + ratio = 65;
> + break;
> + }
> +
> + return dev_priv->cdclk_pll.ref * ratio;
> +}
We could also check in bxt_de_pll_update() that the VCO is always
either 1152 or 1248MHz.
The patch makes things easier to follow and closer to SKL:
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> +
> static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> {
> I915_WRITE(BXT_DE_PLL_ENABLE, 0);
> @@ -5369,13 +5393,14 @@ static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> dev_priv->cdclk_pll.vco = 0;
> }
>
> -static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, u32 ratio)
> +static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> {
> + int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk_pll.ref);
> u32 val;
>
> val = I915_READ(BXT_DE_PLL_CTL);
> val &= ~BXT_DE_PLL_RATIO_MASK;
> - val |= ratio;
> + val |= BXT_DE_PLL_RATIO(ratio);
> I915_WRITE(BXT_DE_PLL_CTL, val);
>
> I915_WRITE(BXT_DE_PLL_ENABLE, BXT_DE_PLL_PLL_ENABLE);
> @@ -5384,54 +5409,42 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, u32 ratio)
> if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) != 0, 1))
> DRM_ERROR("timeout waiting for DE PLL lock\n");
>
> - dev_priv->cdclk_pll.vco = ratio * dev_priv->cdclk_pll.ref;
> + dev_priv->cdclk_pll.vco = vco;
> }
>
> static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> {
> - uint32_t divider;
> - uint32_t ratio;
> - uint32_t current_cdclk;
> - int ret;
> + u32 val, divider;
> + int vco, ret;
>
> - /* frequency = 19.2MHz * ratio / 2 / div{1,1.5,2,4} */
> - switch (cdclk) {
> - case 144000:
> + vco = bxt_de_pll_vco(dev_priv, cdclk);
> +
> + DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz (VCO %d kHz)\n", cdclk, vco);
> +
> + /* cdclk = vco / 2 / div{1,1.5,2,4} */
> + switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
> + case 8:
> divider = BXT_CDCLK_CD2X_DIV_SEL_4;
> - ratio = BXT_DE_PLL_RATIO(60);
> break;
> - case 288000:
> + case 4:
> divider = BXT_CDCLK_CD2X_DIV_SEL_2;
> - ratio = BXT_DE_PLL_RATIO(60);
> break;
> - case 384000:
> + case 3:
> divider = BXT_CDCLK_CD2X_DIV_SEL_1_5;
> - ratio = BXT_DE_PLL_RATIO(60);
> - break;
> - case 576000:
> - divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> - ratio = BXT_DE_PLL_RATIO(60);
> break;
> - case 624000:
> + case 2:
> divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> - ratio = BXT_DE_PLL_RATIO(65);
> - break;
> - case 19200:
> - /*
> - * Bypass frequency with DE PLL disabled. Init ratio, divider
> - * to suppress GCC warning.
> - */
> - ratio = 0;
> - divider = 0;
> break;
> default:
> - DRM_ERROR("unsupported CDCLK freq %d", cdclk);
> + WARN_ON(cdclk != dev_priv->cdclk_pll.ref);
> + WARN_ON(vco != 0);
>
> - return;
> + divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> + break;
> }
>
> - mutex_lock(&dev_priv->rps.hw_lock);
> /* Inform power controller of upcoming frequency change */
> + mutex_lock(&dev_priv->rps.hw_lock);
> ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> 0x80000000);
> mutex_unlock(&dev_priv->rps.hw_lock);
> @@ -5442,40 +5455,26 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> return;
> }
>
> - current_cdclk = I915_READ(CDCLK_CTL) & CDCLK_FREQ_DECIMAL_MASK;
> - /* convert from .1 fixpoint MHz with -1MHz offset to kHz */
> - current_cdclk = current_cdclk * 500 + 1000;
> -
> - /*
> - * DE PLL has to be disabled when
> - * - setting to 19.2MHz (bypass, PLL isn't used)
> - * - before setting to 624MHz (PLL needs toggling)
> - * - before setting to any frequency from 624MHz (PLL needs toggling)
> - */
> - if (cdclk == 19200 || cdclk == 624000 ||
> - current_cdclk == 624000) {
> + if (dev_priv->cdclk_pll.vco != 0 &&
> + dev_priv->cdclk_pll.vco != vco)
> bxt_de_pll_disable(dev_priv);
> - }
> -
> - if (cdclk != 19200) {
> - uint32_t val;
>
> - bxt_de_pll_enable(dev_priv, ratio);
> + if (dev_priv->cdclk_pll.vco != vco)
> + bxt_de_pll_enable(dev_priv, vco);
>
> - val = divider | skl_cdclk_decimal(cdclk);
> - /*
> - * FIXME if only the cd2x divider needs changing, it could be done
> - * without shutting off the pipe (if only one pipe is active).
> - */
> - val |= BXT_CDCLK_CD2X_PIPE_NONE;
> - /*
> - * Disable SSA Precharge when CD clock frequency < 500 MHz,
> - * enable otherwise.
> - */
> - if (cdclk >= 500000)
> - val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> - I915_WRITE(CDCLK_CTL, val);
> - }
> + val = divider | skl_cdclk_decimal(cdclk);
> + /*
> + * FIXME if only the cd2x divider needs changing, it could be done
> + * without shutting off the pipe (if only one pipe is active).
> + */
> + val |= BXT_CDCLK_CD2X_PIPE_NONE;
> + /*
> + * Disable SSA Precharge when CD clock frequency < 500 MHz,
> + * enable otherwise.
> + */
> + if (cdclk >= 500000)
> + val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> + I915_WRITE(CDCLK_CTL, val);
>
> mutex_lock(&dev_priv->rps.hw_lock);
> ret = sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ,
> @@ -5525,8 +5524,7 @@ void broxton_init_cdclk(struct drm_i915_private *dev_priv)
>
> void broxton_uninit_cdclk(struct drm_i915_private *dev_priv)
> {
> - /* Set minimum (bypass) frequency, in effect turning off the DE PLL */
> - broxton_set_cdclk(dev_priv, 19200);
> + broxton_set_cdclk(dev_priv, dev_priv->cdclk_pll.ref);
> }
>
> static int skl_calc_cdclk(int max_pixclk, int vco)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
