On Fri, 2016-05-13 at 23:41 +0300, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > 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ä <[email protected]> > --- > 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 <[email protected]> > + > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
