Re: [PATCH 3/6] drm/i915: add SNB runtime PM support

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4822 bytes --]

On Fri, 2014-03-07 at 20:12 -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Just because I have a SNB machine and I can easily test it.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      | 27 +++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>  3 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f75d776..2c62e0c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -832,6 +832,13 @@ static int i915_pm_poweroff(struct device *dev)
>  	return i915_drm_freeze(drm_dev);
>  }
>  
> +static void snb_runtime_suspend(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	intel_runtime_pm_disable_interrupts(dev);
> +}

Afaics RPS is still enabled here, so if we need it enabled at this point
(so that GT stays at RC6 for example) we'd at least need here

cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
cancel_work_sync(&dev_priv->rps.work);

> +
>  static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -840,6 +847,18 @@ static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
>  		hsw_enable_pc8(dev_priv);
>  }
>  
> +static void snb_runtime_resume(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	intel_runtime_pm_restore_interrupts(dev);
> +	intel_init_pch_refclk(dev);
> +	i915_gem_init_swizzling(dev);
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	gen6_update_ring_freq(dev);
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +}

I haven't found any info on what exact state is lost in D3, but in any
case documenting this in the code would be great.

Here there are some bits restored for GTI in i915_gem_init_swizzling(),
but what about the clock gating registers and RPS registers. I'd be
easier if those would be also re-inited here, or a confirmation that
they are saved/restored by HW.

--Imre

> +
>  static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -859,7 +878,9 @@ static int intel_runtime_suspend(struct device *device)
>  
>  	DRM_DEBUG_KMS("Suspending device\n");
>  
> -	if (IS_HASWELL(dev))
> +	if (IS_GEN6(dev))
> +		snb_runtime_suspend(dev_priv);
> +	else if (IS_HASWELL(dev))
>  		hsw_runtime_suspend(dev_priv);
>  
>  	i915_gem_release_all_mmaps(dev_priv);
> @@ -893,7 +914,9 @@ static int intel_runtime_resume(struct device *device)
>  	intel_opregion_notify_adapter(dev, PCI_D0);
>  	dev_priv->pm.suspended = false;
>  
> -	if (IS_HASWELL(dev))
> +	if (IS_GEN6(dev))
> +		snb_runtime_resume(dev_priv);
> +	else if (IS_HASWELL(dev))
>  		hsw_runtime_resume(dev_priv);
>  
>  	DRM_DEBUG_KMS("Device resumed\n");
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 493579d..309852d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1998,7 +1998,7 @@ struct drm_i915_cmd_table {
>  #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev))
>  #define HAS_PC8(dev)		(IS_HASWELL(dev)) /* XXX HSW:ULX */
> -#define HAS_RUNTIME_PM(dev)	(IS_HASWELL(dev))
> +#define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev))
>  
>  #define INTEL_PCH_DEVICE_ID_MASK		0xff00
>  #define INTEL_PCH_IBX_DEVICE_ID_TYPE		0x3b00
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index df69866..35f65c1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6859,6 +6859,11 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void snb_modeset_global_resources(struct drm_device *dev)
> +{
> +	modeset_update_crtc_power_domains(dev);
> +}
> +
>  static void haswell_modeset_global_resources(struct drm_device *dev)
>  {
>  	modeset_update_crtc_power_domains(dev);
> @@ -10758,6 +10763,8 @@ static void intel_init_display(struct drm_device *dev)
>  		} else if (IS_GEN6(dev)) {
>  			dev_priv->display.fdi_link_train = gen6_fdi_link_train;
>  			dev_priv->display.write_eld = ironlake_write_eld;
> +			dev_priv->display.modeset_global_resources =
> +				snb_modeset_global_resources;
>  		} else if (IS_IVYBRIDGE(dev)) {
>  			/* FIXME: detect B0+ stepping and use auto training */
>  			dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx