[PATCH 1/5] drm/i915: disable power wells on suspend

[PATCH 1/5] drm/i915: disable power wells on suspend

[Jesse Barnes]

From: Kristen Carlson Accardi <kristen@linux.intel.com>

We want to make sure everything is disabled and at its lowest power when
freezing.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e2bfdda..66c6ffb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -551,6 +551,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 	dev_priv->suspend_count++;
 
+	intel_display_set_init_power(dev_priv, false);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH 2/5] drm/i915: leave rc6 enabled at suspend time v4

[Jesse Barnes]

This allows the system to enter the lowest power mode during system freeze.

v2: delete force wake timer at suspend (Imre)
v3: add GT work suspend function (Imre)
v4: use uncore forcewake reset (Daniel)

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c     |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_pm.c     | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.c |  2 +-
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66c6ffb..7148eac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 		drm_irq_uninstall(dev);
 		dev_priv->enable_hotplug_processing = false;
 
-		intel_disable_gt_powersave(dev);
+		intel_suspend_gt_powersave(dev);
 
 		/*
 		 * Disable CRTCs directly since we want to preserve sw state
@@ -542,8 +542,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 	i915_save_state(dev);
 
+	intel_uncore_forcewake_reset(dev, false);
 	intel_opregion_fini(dev);
-	intel_uncore_fini(dev);
 
 	console_lock();
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bea9ab40..89d6b47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2084,6 +2084,7 @@ extern void intel_uncore_early_sanitize(struct drm_device *dev);
 extern void intel_uncore_init(struct drm_device *dev);
 extern void intel_uncore_check_errors(struct drm_device *dev);
 extern void intel_uncore_fini(struct drm_device *dev);
+extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore);
 
 void
 i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c597b0d..74fbe4d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -957,6 +957,7 @@ void intel_init_gt_powersave(struct drm_device *dev);
 void intel_cleanup_gt_powersave(struct drm_device *dev);
 void intel_enable_gt_powersave(struct drm_device *dev);
 void intel_disable_gt_powersave(struct drm_device *dev);
+void intel_suspend_gt_powersave(struct drm_device *dev);
 void intel_reset_gt_powersave(struct drm_device *dev);
 void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1840d15..139eebe 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4864,6 +4864,26 @@ void intel_cleanup_gt_powersave(struct drm_device *dev)
 		valleyview_cleanup_gt_powersave(dev);
 }
 
+/**
+ * intel_suspend_gt_powersave - suspend PM work and helper threads
+ * @dev: drm device
+ *
+ * We don't want to disable RC6 or other features here, we just want
+ * to make sure any work we've queued has finished and won't bother
+ * us while we're suspended.
+ */
+void intel_suspend_gt_powersave(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* Interrupts should be disabled already to avoid re-arming. */
+	WARN_ON(dev->irq_enabled);
+
+	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
+
+	cancel_work_sync(&dev_priv->rps.work);
+}
+
 void intel_disable_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 871c284..741a4e3 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -316,7 +316,7 @@ static void gen6_force_wake_timer(unsigned long arg)
 	intel_runtime_pm_put(dev_priv);
 }
 
-static void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
+void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long irqflags;
-- 
1.9.1

[PATCH] drm/i915: Unifiy GT powersave suspend logic

[Daniel Vetter]

Jesse's patch to only quiescent our rps work and Imre's fix to address
a race with runtime pm and the forcewake reference held by the used
diverging means to address the same bug: Jesse's patch uses
flush_delayed_work while (since we want to make sure rps is set up)
while Imre's used a cancel+manuel refcount adjustment.

Unify them again by simply reusing intel_suspend_gt_powersave in
intel_disable_gt_powersave.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 685b4910eb93..49122204a001 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4706,10 +4706,8 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 		ironlake_disable_drps(dev);
 		ironlake_disable_rc6(dev);
 	} else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {
-		if (cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work))
-			intel_runtime_pm_put(dev_priv);
+		intel_suspend_gt_powersave(dev);
 
-		cancel_work_sync(&dev_priv->rps.work);
 		mutex_lock(&dev_priv->rps.hw_lock);
 		if (IS_VALLEYVIEW(dev))
 			valleyview_disable_rps(dev);
-- 
2.0.0

Re: [PATCH] drm/i915: Unifiy GT powersave suspend logic

[Jesse Barnes]

On Thu, 12 Jun 2014 17:53:45 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Jesse's patch to only quiescent our rps work and Imre's fix to address
> a race with runtime pm and the forcewake reference held by the used
> diverging means to address the same bug: Jesse's patch uses
> flush_delayed_work while (since we want to make sure rps is set up)
> while Imre's used a cancel+manuel refcount adjustment.
> 
> Unify them again by simply reusing intel_suspend_gt_powersave in
> intel_disable_gt_powersave.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 685b4910eb93..49122204a001 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4706,10 +4706,8 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  		ironlake_disable_drps(dev);
>  		ironlake_disable_rc6(dev);
>  	} else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {
> -		if (cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work))
> -			intel_runtime_pm_put(dev_priv);
> +		intel_suspend_gt_powersave(dev);
>  
> -		cancel_work_sync(&dev_priv->rps.work);
>  		mutex_lock(&dev_priv->rps.hw_lock);
>  		if (IS_VALLEYVIEW(dev))
>  			valleyview_disable_rps(dev);

Yeah looks good, though we may end up toggling RC6 on rather than
canceling an oustanding enable, but that doesn't matter much on the
disable path.

We can do some more unification of the various freeze/suspend/resume
paths too, though we'll want lots of testing...

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH 3/5] ACPI: export target system state for use by drivers

[Jesse Barnes]

From: Kristen Carlson Accardi <kristen@linux.intel.com>

Needed in ->freeze routines to figure out the target system state and
take appropriate action.

v2: split out ACPI patch

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/acpi/sleep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index c40fb2e..807f333 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -89,6 +89,7 @@ u32 acpi_target_system_state(void)
 {
 	return acpi_target_sleep_state;
 }
+EXPORT_SYMBOL(acpi_target_system_state);
 
 static bool pwr_btn_event_pending;
 
-- 
1.9.1

[PATCH 4/5] drm/i915: send proper opregion notifications on suspend/resume

[Jesse Barnes]

This indicates to the firmware that it can power down various other
components or bring them back up, depending on the target system state.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7148eac..e54dc6c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -28,6 +28,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/acpi.h>
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
@@ -491,6 +492,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
+	pci_power_t opregion_target_state;
 
 	intel_runtime_pm_get(dev_priv);
 
@@ -542,6 +544,12 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 	i915_save_state(dev);
 
+	if (acpi_target_system_state() >= ACPI_STATE_S3)
+		opregion_target_state = PCI_D3cold;
+	else
+		opregion_target_state = PCI_D1;
+	intel_opregion_notify_adapter(dev, opregion_target_state);
+
 	intel_uncore_forcewake_reset(dev, false);
 	intel_opregion_fini(dev);
 
@@ -674,6 +682,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 	dev_priv->modeset_restore = MODESET_DONE;
 	mutex_unlock(&dev_priv->modeset_restore_lock);
 
+	intel_opregion_notify_adapter(dev, PCI_D0);
+
 	intel_runtime_pm_put(dev_priv);
 	return 0;
 }
-- 
1.9.1

[PATCH 5/5] drm/i915: make sure PC8 is enabled on suspend and disabled on resume v4

[Jesse Barnes]

From: Kristen Carlson Accardi <kristen@linux.intel.com>

This matches the runtime suspend paths and allows the system to enter
the lowest power mode at freeze time.

v2: move disable_pc8 call to thaw_early (Imre)
    move enable_pc8 to freeze_late (Imre/Jesse)
v3: drop spurious hunk from _freeze now that we have freeze_late (Jesse)
v4: move back to suspend_late (Imre was right)

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e54dc6c..4823435 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -610,6 +610,9 @@ static int i915_drm_thaw_early(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+		hsw_disable_pc8(dev_priv);
+
 	intel_uncore_early_sanitize(dev);
 	intel_uncore_sanitize(dev);
 	intel_power_domains_init_hw(dev_priv);
@@ -893,6 +896,7 @@ static int i915_pm_suspend_late(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	struct drm_i915_private *dev_priv = drm_dev->dev_private;
 
 	/*
 	 * We have a suspedn ordering issue with the snd-hda driver also
@@ -906,6 +910,9 @@ static int i915_pm_suspend_late(struct device *dev)
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
+	if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
+		hsw_enable_pc8(dev_priv);
+
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
 
-- 
1.9.1

Re: [PATCH 5/5] drm/i915: make sure PC8 is enabled on suspend and disabled on resume v4

[Daniel Vetter]

On Thu, Jun 12, 2014 at 08:35:48AM -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi <kristen@linux.intel.com>
> 
> This matches the runtime suspend paths and allows the system to enter
> the lowest power mode at freeze time.
> 
> v2: move disable_pc8 call to thaw_early (Imre)
>     move enable_pc8 to freeze_late (Imre/Jesse)
> v3: drop spurious hunk from _freeze now that we have freeze_late (Jesse)
> v4: move back to suspend_late (Imre was right)
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e54dc6c..4823435 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -610,6 +610,9 @@ static int i915_drm_thaw_early(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> +		hsw_disable_pc8(dev_priv);

Not terribly happy really about this one here, since it leaks platform
logic into the generic codepaths. Sooner or later we'll add the same for
byt/chv and then it'll slowly diverge ...

Queued for now (since runtime pm is suffering from too much platform
specifics a bit in general), but grumpily.
-Daniel

> +
>  	intel_uncore_early_sanitize(dev);
>  	intel_uncore_sanitize(dev);
>  	intel_power_domains_init_hw(dev_priv);
> @@ -893,6 +896,7 @@ static int i915_pm_suspend_late(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> +	struct drm_i915_private *dev_priv = drm_dev->dev_private;
>  
>  	/*
>  	 * We have a suspedn ordering issue with the snd-hda driver also
> @@ -906,6 +910,9 @@ static int i915_pm_suspend_late(struct device *dev)
>  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> +	if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
> +		hsw_enable_pc8(dev_priv);
> +
>  	pci_disable_device(pdev);
>  	pci_set_power_state(pdev, PCI_D3hot);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch