On Thu, May 21, 2015 at 04:37:48PM +0100, Damien Lespiau wrote: > We need to re-init the display hardware when going out of suspend. This > includes: > > - Hooking the PCH to the reset logic > - Restoring CDCDLK > - Enabling the DDB power > > Among those, only the CDCDLK one is a bit tricky. There's some > complexity in that: > > - DPLL0 (which is the source for CDCLK) has two VCOs, each with a set > of supported frequencies. As eDP also uses DPLL0 for its link rate, > once DPLL0 is on, we restrict the possible eDP link rates the chosen > VCO. > - CDCLK also limits the bandwidth available to push pixels. > > So, as a first step, this commit restore what the BIOS set, until I can > do more testing. > > In case that's of interest for the reviewer, I've unit tested the > function that derives the decimal frequency field: > > #include <stdio.h> > #include <stdint.h> > #include <assert.h> > > #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x))) > > static const struct dpll_freq { > unsigned int freq; > unsigned int decimal; > } freqs[] = { > { .freq = 308570, .decimal = 0b01001100111}, > { .freq = 337500, .decimal = 0b01010100001}, > { .freq = 432000, .decimal = 0b01101011110}, > { .freq = 450000, .decimal = 0b01110000010}, > { .freq = 540000, .decimal = 0b10000110110}, > { .freq = 617140, .decimal = 0b10011010000}, > { .freq = 675000, .decimal = 0b10101000100}, > }; > > static void intbits(unsigned int v) > { > int i; > > for(i = 10; i >= 0; i--) > putchar('0' + ((v >> i) & 1)); > } > > static unsigned int freq_decimal(unsigned int freq /* in kHz */) > { > return (freq - 1000) / 500; > } > > static void test_freq(const struct dpll_freq *entry) > { > unsigned int decimal = freq_decimal(entry->freq); > > printf("freq: %d, expected: ", entry->freq); > intbits(entry->decimal); > printf(", got: "); > intbits(decimal); > putchar('\n'); > > assert(decimal == entry->decimal); > } > > int main(int argc, char **argv) > { > int i; > > for (i = 0; i < ARRAY_SIZE(freqs); i++) > test_freq(&freqs[i]); > > return 0; > } > > v2: > - Rebase on top of -nightly > - Use (freq - 1000) / 500 for the decimal frequency (Ville) > - Fix setting the enable bit of HSW_NDE_RSTWRN_OPT (Ville) > - Rename skl_display_{resume,suspend} to skl_{init,uninit}_cdclk to > be consistent with the BXT code (Ville) > - Store boot CDCLK in ddi_pll_init (Ville) > - Merge dev_priv's skl_boot_cdclk into cdclk_freq > - Use LCPLL_PLL_LOCK instead of (1 << 30) (Ville) > - Replace various '0' by SKL_DPLL0 to be a bit more explicit that > we're programming DPLL0 > - Busy poll the PCU before doing the frequency change. It takes about > 3/4 cycles, each separated by 10us, to get the ACK from the CPU > (Ville) > > v3: > - Restore dev_priv->skl_boot_cdclk, leaving unification with > dev_priv->cdclk_freq for a later patch (Daniel, Ville) > > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 3 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 3 + > drivers/gpu/drm/i915/intel_ddi.c | 8 +- > drivers/gpu/drm/i915/intel_display.c | 208 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > 6 files changed, 223 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 93191c1..884b4f9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1047,6 +1047,8 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv) > */ > intel_csr_load_status_set(dev_priv, FW_UNINITIALIZED); > > + skl_uninit_cdclk(dev_priv); > + > return 0; > } > > @@ -1093,6 +1095,7 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = dev_priv->dev; > > + skl_init_cdclk(dev_priv); > intel_csr_load_program(dev); > > return 0; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b4eebd0..44d592f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1705,6 +1705,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 cdclk_freq; > unsigned int hpll_freq; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 5afff3a..4d7282e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6699,6 +6699,9 @@ enum skl_disp_power_wells { > #define GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT 8 > #define GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT 16 > #define GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT 24 > +#define SKL_PCODE_CDCLK_CONTROL 0x7 > +#define SKL_CDCLK_PREPARE_FOR_CHANGE 0x3 > +#define SKL_CDCLK_READY_FOR_CHANGE 0x1 > #define GEN6_PCODE_WRITE_MIN_FREQ_TABLE 0x8 > #define GEN6_PCODE_READ_MIN_FREQ_TABLE 0x9 > #define GEN6_READ_OC_PARAMS 0xc > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index d602db2..cacb07b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2510,6 +2510,7 @@ void intel_ddi_pll_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t val = I915_READ(LCPLL_CTL); > + int cdclk_freq; > > if (IS_SKYLAKE(dev)) > skl_shared_dplls_init(dev_priv); > @@ -2518,12 +2519,15 @@ void intel_ddi_pll_init(struct drm_device *dev) > else > hsw_shared_dplls_init(dev_priv); > > - DRM_DEBUG_KMS("CDCLK running at %dKHz\n", > - dev_priv->display.get_display_clock_speed(dev)); > + cdclk_freq = dev_priv->display.get_display_clock_speed(dev); > + DRM_DEBUG_KMS("CDCLK running at %dKHz\n", cdclk_freq); > > if (IS_SKYLAKE(dev)) { > + dev_priv->skl_boot_cdclk = cdclk_freq; > if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) > DRM_ERROR("LCPLL1 is disabled\n"); > + else > + intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS); > } else if (IS_BROXTON(dev)) { > broxton_init_cdclk(dev); > broxton_ddi_phy_init(dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0485655..268043f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5527,6 +5527,214 @@ void broxton_uninit_cdclk(struct drm_device *dev) > intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > } > > +static const struct skl_cdclk_entry { > + unsigned int freq; > + unsigned int vco; > +} skl_cdclk_frequencies[] = { > + { .freq = 308570, .vco = 8640 }, > + { .freq = 337500, .vco = 8100 }, > + { .freq = 432000, .vco = 8640 }, > + { .freq = 450000, .vco = 8100 }, > + { .freq = 540000, .vco = 8100 }, > + { .freq = 617140, .vco = 8640 }, > + { .freq = 675000, .vco = 8100 }, > +}; > + > +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 i; > + > + for (i = 0; i < ARRAY_SIZE(skl_cdclk_frequencies); i++) { > + const struct skl_cdclk_entry *e = &skl_cdclk_frequencies[i]; > + > + if (e->freq == freq) > + return e->vco; > + } > + > + return 8100; > +} > + > +static void > +skl_dpll0_enable(struct drm_i915_private *dev_priv, unsigned int required_vco) > +{ > + unsigned int min_freq; > + u32 val; > + > + /* select the minimum CDCLK before enabling DPLL 0 */ > + val = I915_READ(CDCLK_CTL); > + val &= ~CDCLK_FREQ_SEL_MASK | ~CDCLK_FREQ_DECIMAL_MASK; > + val |= CDCLK_FREQ_337_308; > + > + if (required_vco == 8640) > + min_freq = 308570; > + else > + min_freq = 337500; > + > + val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_freq); > + > + I915_WRITE(CDCLK_CTL, val); > + POSTING_READ(CDCLK_CTL); > + > + /* > + * We always enable DPLL0 with the lowest link rate possible, but still > + * taking into account the VCO required to operate the eDP panel at the > + * desired frequency. The usual DP link rates operate with a VCO of > + * 8100 while the eDP 1.4 alternate link rates need a VCO of 8640. > + * The modeset code is responsible for the selection of the exact link > + * rate later on, with the constraint of choosing a frequency that > + * works with required_vco. > + */ > + val = I915_READ(DPLL_CTRL1); > + > + val &= ~(DPLL_CTRL1_HDMI_MODE(SKL_DPLL0) | DPLL_CTRL1_SSC(SKL_DPLL0) | > + DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)); > + val |= DPLL_CTRL1_OVERRIDE(SKL_DPLL0); > + if (required_vco == 8640) > + val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, > + SKL_DPLL0); > + else > + val |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, > + SKL_DPLL0); > + > + I915_WRITE(DPLL_CTRL1, val); > + POSTING_READ(DPLL_CTRL1); > + > + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE); > + > + if (wait_for(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK, 5)) > + DRM_ERROR("DPLL0 not locked\n"); > +} > + > +static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv) > +{ > + int ret; > + u32 val; > + > + /* inform PCU we want to change CDCLK */ > + val = SKL_CDCLK_PREPARE_FOR_CHANGE; > + mutex_lock(&dev_priv->rps.hw_lock); > + ret = sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &val); > + mutex_unlock(&dev_priv->rps.hw_lock); > + > + return ret == 0 && (val & SKL_CDCLK_READY_FOR_CHANGE); > +} > + > +static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv) > +{ > + unsigned int i; > + > + for (i = 0; i < 15; i++) { > + if (skl_cdclk_pcu_ready(dev_priv)) > + return true; > + udelay(10); > + } > + > + return false; > +} > + > +static void skl_set_cdclk(struct drm_i915_private *dev_priv, unsigned int freq) > +{ > + u32 freq_select, pcu_ack; > + > + DRM_DEBUG_DRIVER("Changing CDCLK to %dKHz\n", freq); > + > + if (!skl_cdclk_wait_for_pcu_ready(dev_priv)) { > + DRM_ERROR("failed to inform PCU about cdclk change\n"); > + return; > + } > + > + /* set CDCLK_CTL */ > + switch(freq) { > + case 450000: > + case 432000: > + freq_select = CDCLK_FREQ_450_432; > + pcu_ack = 1; > + break; > + case 540000: > + freq_select = CDCLK_FREQ_540; > + pcu_ack = 2; > + break; > + case 308570: > + case 337500: > + default: > + freq_select = CDCLK_FREQ_337_308; > + pcu_ack = 0; > + break; > + case 617140: > + case 675000: > + freq_select = CDCLK_FREQ_675_617; > + pcu_ack = 3; > + break; > + } > + > + I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(freq)); > + POSTING_READ(CDCLK_CTL); > + > + /* inform PCU of the change */ > + mutex_lock(&dev_priv->rps.hw_lock); > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > + mutex_unlock(&dev_priv->rps.hw_lock); > +} > + > +void skl_uninit_cdclk(struct drm_i915_private *dev_priv) > +{ > + /* disable DBUF power */ > + I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST); > + POSTING_READ(DBUF_CTL); > + > + udelay(10); > + > + if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE) > + DRM_ERROR("DBuf power disable timeout\n"); > + > + /* disable DPLL0 */ > + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) & ~LCPLL_PLL_ENABLE); > + if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1)) > + DRM_ERROR("Couldn't disable DPLL0\n"); > + > + intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS); > +} > + > +void skl_init_cdclk(struct drm_i915_private *dev_priv) > +{ > + u32 val; > + unsigned int required_vco; > + > + /* enable PCH reset handshake */ > + val = I915_READ(HSW_NDE_RSTWRN_OPT); > + I915_WRITE(HSW_NDE_RSTWRN_OPT, val | RESET_PCH_HANDSHAKE_ENABLE); > + > + /* enable PG1 and Misc I/O */ > + intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS); > + > + /* DPLL0 already enabed !? */ > + if (I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE) { > + DRM_DEBUG_DRIVER("DPLL0 already running\n"); > + return; > + } > + > + /* enable DPLL0 */ > + required_vco = skl_cdclk_get_vco(dev_priv->skl_boot_cdclk); > + skl_dpll0_enable(dev_priv, required_vco); > + > + /* set CDCLK to the frequency the BIOS chose */ > + skl_set_cdclk(dev_priv, dev_priv->skl_boot_cdclk); > + > + /* enable DBUF power */ > + I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST); > + POSTING_READ(DBUF_CTL); > + > + udelay(10); > + > + if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE)) > + DRM_ERROR("DBuf power enable timeout\n"); > +} > + > /* returns HPLL frequency in kHz */ > static int valleyview_get_vco(struct drm_i915_private *dev_priv) > { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index e94cb6e..02d8317 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1118,6 +1118,8 @@ void broxton_ddi_phy_init(struct drm_device *dev); > void broxton_ddi_phy_uninit(struct drm_device *dev); > void bxt_enable_dc9(struct drm_i915_private *dev_priv); > void bxt_disable_dc9(struct drm_i915_private *dev_priv); > +void skl_init_cdclk(struct drm_i915_private *dev_priv); > +void skl_uninit_cdclk(struct drm_i915_private *dev_priv); > void intel_dp_get_m_n(struct intel_crtc *crtc, > struct intel_crtc_state *pipe_config); > void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n); > -- > 2.1.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx