On Thu, May 19, 2016 at 06:48:37PM +0300, Imre Deak wrote: > On pe, 2016-05-13 at 23:41 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > SKL and BXT have the same snippets of code for enabling disabling the > > DBUF. Extract those into helpers and move the calls from > > init/unit_cdclk() to the display core init/init since this stuff isn't > > really about cdclk. Also doing the enable twice shouldn't hurt since > > you're just setting the request bit again when it was already set. > > > > We can also toss in a few WARNs about the register values into > > skl_get_dpll0_vco() now that we know that things should always be > > sane there. > > > > Flatten skl_init_cdclk() while at it. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 58 ++++----------------------------- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++++ > > 2 files changed, 38 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index da903b718c11..e908f360da74 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5480,18 +5480,6 @@ static bool broxton_cdclk_is_enabled(struct drm_i915_private *dev_priv) > > > > /* TODO: Check for a valid CDCLK rate */ > > > > - if (!(I915_READ(DBUF_CTL) & DBUF_POWER_REQUEST)) { > > - DRM_DEBUG_DRIVER("CDCLK enabled, but DBUF power not requested\n"); > > - > > - return false; > > - } > > - > > - if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE)) { > > - DRM_DEBUG_DRIVER("CDCLK enabled, but DBUF power hasn't settled\n"); > > - > > - return false; > > - } > > - > > return true; > > } > > > > @@ -5518,26 +5506,10 @@ void broxton_init_cdclk(struct drm_i915_private *dev_priv) > > * here, it belongs to modeset time > > */ > > broxton_set_cdclk(dev_priv, 624000); > > - > > - 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"); > > } > > > > void broxton_uninit_cdclk(struct drm_i915_private *dev_priv) > > { > > - 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"); > > - > > /* Set minimum (bypass) frequency, in effect turning off the DE PLL */ > > broxton_set_cdclk(dev_priv, 19200); > > } > > @@ -5759,15 +5731,6 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv); > > > > 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"); > > - > > skl_set_cdclk(dev_priv, 24000, 0); > > } > > > > @@ -5785,24 +5748,15 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv) > > if (dev_priv->skl_preferred_vco_freq == 0) > > skl_set_preferred_cdclk_vco(dev_priv, > > dev_priv->skl_vco_freq); > > - } else { > > - /* set CDCLK to the lowest frequency, Modeset follows */ > > - 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); > > + return; > > } > > > > - /* enable DBUF power */ > > - I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST); > > - POSTING_READ(DBUF_CTL); > > - > > - udelay(10); > > + vco = dev_priv->skl_preferred_vco_freq; > > + if (vco == 0) > > + vco = 8100; > > + cdclk = skl_calc_cdclk(0, vco); > > > > - if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE)) > > - DRM_ERROR("DBuf power enable timeout\n"); > > + skl_set_cdclk(dev_priv, cdclk, vco); > > } > > > > static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index fefe22c3c163..6817a3cb5fbc 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -2176,6 +2176,28 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) > > mutex_unlock(&power_domains->lock); > > } > > > > +static void skl_dbuf_enable(struct drm_i915_private *dev_priv) > > I would've used gen9_ for these, but either way: Changed while applying. > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > > > +{ > > + 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"); > > +} > > + > > +static void skl_dbuf_disable(struct drm_i915_private *dev_priv) > > +{ > > + 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"); > > +} > > + > > static void skl_display_core_init(struct drm_i915_private *dev_priv, > > bool resume) > > { > > @@ -2202,6 +2224,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv, > > > > skl_init_cdclk(dev_priv); > > > > + skl_dbuf_enable(dev_priv); > > + > > if (resume && dev_priv->csr.dmc_payload) > > intel_csr_load_program(dev_priv); > > } > > @@ -2213,6 +2237,8 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv) > > > > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > > + skl_dbuf_disable(dev_priv); > > + > > skl_uninit_cdclk(dev_priv); > > > > /* The spec doesn't call for removing the reset handshake flag */ > > @@ -2257,6 +2283,9 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, > > mutex_unlock(&power_domains->lock); > > > > broxton_init_cdclk(dev_priv); > > + > > + skl_dbuf_enable(dev_priv); > > + > > broxton_ddi_phy_init(dev_priv); > > > > broxton_cdclk_verify_state(dev_priv); > > @@ -2274,6 +2303,9 @@ void bxt_display_core_uninit(struct drm_i915_private *dev_priv) > > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > > > > broxton_ddi_phy_uninit(dev_priv); > > + > > + skl_dbuf_disable(dev_priv); > > + > > broxton_uninit_cdclk(dev_priv); > > > > /* The spec doesn't call for removing the reset handshake flag */ -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx