On Wed, Nov 04, 2015 at 07:24:14PM +0200, Imre Deak wrote: > We need to initialize the display core part early, before initializing > the rest of the display power state. This is also described in the bspec > termed "Display initialization sequence". Atm we run this sequence > during driver loading after power domain HW state initialization which > is too late and during runtime suspend/resume which is unneeded and can > interere with DMC functionality which handles HW resources toggled > by this init/uninit sequence automatically. The init sequence must be > run as the first step of HW power state initialization and during > system resume. The uninit sequence must be run during system suspend. > > To address the above move the init sequence to the initial HW power > state setup and the uninit sequence to a new power domains suspend > function called during system suspend. > > As part of the init sequence we also have to reprogram the DMC firmware > as it's lost across a system suspend/resume cycle. > > After this change CD clock initialization during driver loading will > happen only later after other dependent HW/SW parts are initialized, > while during system resume it will get initialized as the last step of > the init sequence. This distinction can be removed by some refactoring > of platform independent parts. I left this refactoring out from this > series since I didn't want to change non-SKL parts. This is a TODO for > later. > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> Reviewed-by: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_dma.c | 2 +- > drivers/gpu/drm/i915/i915_drv.c | 9 ++---- > drivers/gpu/drm/i915/intel_display.c | 11 ------- > drivers/gpu/drm/i915/intel_drv.h | 3 +- > drivers/gpu/drm/i915/intel_runtime_pm.c | 55 +++++++++++++++++++++++++++++++-- > 5 files changed, 59 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 71e1323..6e9337f 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -396,7 +396,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > if (ret) > goto cleanup_vga_switcheroo; > > - intel_power_domains_init_hw(dev_priv); > + intel_power_domains_init_hw(dev_priv, false); > > intel_csr_ucode_init(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 858d58c..5a63f9a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -704,6 +704,8 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) > struct drm_i915_private *dev_priv = drm_dev->dev_private; > int ret; > > + intel_power_domains_suspend(dev_priv); > + > ret = intel_suspend_complete(dev_priv); > > if (ret) { > @@ -861,7 +863,7 @@ static int i915_drm_resume_early(struct drm_device *dev) > hsw_disable_pc8(dev_priv); > > intel_uncore_sanitize(dev); > - intel_power_domains_init_hw(dev_priv); > + intel_power_domains_init_hw(dev_priv, true); > > return ret; > } > @@ -1070,8 +1072,6 @@ static int i915_pm_resume(struct device *dev) > > static int skl_suspend_complete(struct drm_i915_private *dev_priv) > { > - skl_uninit_cdclk(dev_priv); > - > if (dev_priv->csr.dmc_payload) > skl_enable_dc6(dev_priv); > > @@ -1122,9 +1122,6 @@ static int skl_resume_prepare(struct drm_i915_private *dev_priv) > if (dev_priv->csr.dmc_payload) > skl_disable_dc6(dev_priv); > > - skl_init_cdclk(dev_priv); > - intel_csr_load_program(dev_priv); > - > return 0; > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ef2ebcd..d9ed128 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5701,23 +5701,12 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv) > if (wait_for(!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_LOCK), 1)) > DRM_ERROR("Couldn't disable DPLL0\n"); > } > - > - /* disable PG1 and Misc I/O */ > - skl_pw1_misc_io_fini(dev_priv); > } > > 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 */ > - skl_pw1_misc_io_init(dev_priv); > - > /* DPLL0 not enabled (happens on early BIOS versions) */ > if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE)) { > /* enable DPLL0 */ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 3d9d1d3..e0476c3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1380,7 +1380,8 @@ void intel_psr_single_frame_update(struct drm_device *dev, > /* intel_runtime_pm.c */ > int intel_power_domains_init(struct drm_i915_private *); > void intel_power_domains_fini(struct drm_i915_private *); > -void intel_power_domains_init_hw(struct drm_i915_private *dev_priv); > +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume); > +void intel_power_domains_suspend(struct drm_i915_private *dev_priv); > void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv); > void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv); > void intel_runtime_pm_enable(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 63ad315..dce76ff 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1922,6 +1922,42 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv) > mutex_unlock(&power_domains->lock); > } > > +static void skl_display_core_init(struct drm_i915_private *dev_priv, > + bool resume) > +{ > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > + uint32_t val; > + > + /* 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 */ > + mutex_lock(&power_domains->lock); > + skl_pw1_misc_io_init(dev_priv); > + mutex_unlock(&power_domains->lock); > + > + if (!resume) > + return; > + > + skl_init_cdclk(dev_priv); > + > + intel_csr_load_program(dev_priv); > +} > + > +static void skl_display_core_uninit(struct drm_i915_private *dev_priv) > +{ > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > + > + skl_uninit_cdclk(dev_priv); > + > + /* The spec doesn't call for removing the reset handshake flag */ > + /* disable PG1 and Misc I/O */ > + mutex_lock(&power_domains->lock); > + skl_pw1_misc_io_fini(dev_priv); > + mutex_unlock(&power_domains->lock); > +} > + > static void chv_phy_control_init(struct drm_i915_private *dev_priv) > { > struct i915_power_well *cmn_bc = > @@ -2044,14 +2080,16 @@ static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv) > * This function initializes the hardware power domain state and enables all > * power domains using intel_display_set_init_power(). > */ > -void intel_power_domains_init_hw(struct drm_i915_private *dev_priv) > +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) > { > struct drm_device *dev = dev_priv->dev; > struct i915_power_domains *power_domains = &dev_priv->power_domains; > > power_domains->initializing = true; > > - if (IS_CHERRYVIEW(dev)) { > + if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) { > + skl_display_core_init(dev_priv, resume); > + } else if (IS_CHERRYVIEW(dev)) { > mutex_lock(&power_domains->lock); > chv_phy_control_init(dev_priv); > mutex_unlock(&power_domains->lock); > @@ -2068,6 +2106,19 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv) > } > > /** > + * intel_power_domains_suspend - suspend power domain state > + * @dev_priv: i915 device instance > + * > + * This function prepares the hardware power domain state before entering > + * system suspend. It must be paired with intel_power_domains_init_hw(). > + */ > +void intel_power_domains_suspend(struct drm_i915_private *dev_priv) > +{ > + if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) > + skl_display_core_uninit(dev_priv); > +} > + > +/** > * intel_aux_display_runtime_get - grab an auxiliary power domain reference > * @dev_priv: i915 device instance > * > -- > 2.1.4 > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx