Re: [PATCH 05/10] drm/i915/skl: init/uninit display core as part of the HW power domain state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux