[PATCH v2 00/14] fix VLV S4 suspend/resume, unify S3/S4 handlers

[PATCH v2 00/14] fix VLV S4 suspend/resume, unify S3/S4 handlers

[Imre Deak]

This is v2 of [1]. I addressed Chris' and Ville's comments in it and
rebased it against the latest -nightly kernel.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2014-September/052199.html

Imre Deak (14):
  drm/i915: factor out i915_drm_suspend_late
  drm/i915: unify legacy S3 suspend and S4 freeze handlers
  drm/i915: propagate error from legacy resume handler
  drm/i915: vlv: fix switcheroo/legacy suspend/resume
  drm/i915: fix S4 suspend while switcheroo state is off
  drm/i915: remove unused restore_gtt_mappings optimization during
    suspend
  drm/i915: check for GT faults in all resume handlers and driver load
    time
  drm/i915: enable output polling during S4 thaw
  drm/i915: disable/re-enable PCI device around S4 freeze/thaw
  drm/i915: unify S3 and S4 suspend/resume handlers
  drm/i915: sanitize suspend/resume helper function names
  drm/i915: add poweroff_late handler
  drm/i915: unify switcheroo and legacy suspend/resume handlers
  drm/i915: add comments on what stage a given PM handler is called

 drivers/gpu/drm/i915/i915_dma.c     |   4 +-
 drivers/gpu/drm/i915/i915_drv.c     | 208 ++++++++++++++----------------------
 drivers/gpu/drm/i915/i915_drv.h     |   4 +-
 drivers/gpu/drm/i915/intel_uncore.c |  13 ++-
 4 files changed, 98 insertions(+), 131 deletions(-)

-- 
1.8.4

[PATCH v2 01/14] drm/i915: factor out i915_drm_suspend_late

[Imre Deak]

This is needed by an upcoming patch fixing the switcheroo/legacy suspend
paths.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ffd672e..753d44a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -630,6 +630,25 @@ static int i915_drm_freeze(struct drm_device *dev)
 	return 0;
 }
 
+static int i915_drm_suspend_late(struct drm_device *drm_dev)
+{
+	struct drm_i915_private *dev_priv = drm_dev->dev_private;
+	int ret;
+
+	ret = intel_suspend_complete(dev_priv);
+
+	if (ret) {
+		DRM_ERROR("Suspend complete failed: %d\n", ret);
+
+		return ret;
+	}
+
+	pci_disable_device(drm_dev->pdev);
+	pci_set_power_state(drm_dev->pdev, PCI_D3hot);
+
+	return 0;
+}
+
 int i915_suspend(struct drm_device *dev, pm_message_t state)
 {
 	int error;
@@ -955,8 +974,6 @@ 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;
-	int ret;
 
 	/*
 	 * We have a suspedn ordering issue with the snd-hda driver also
@@ -970,16 +987,7 @@ static int i915_pm_suspend_late(struct device *dev)
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	ret = intel_suspend_complete(dev_priv);
-
-	if (ret)
-		DRM_ERROR("Suspend complete failed: %d\n", ret);
-	else {
-		pci_disable_device(pdev);
-		pci_set_power_state(pdev, PCI_D3hot);
-	}
-
-	return ret;
+	return i915_drm_suspend_late(drm_dev);
 }
 
 static int i915_pm_resume_early(struct device *dev)
-- 
1.8.4

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

[PATCH v2 02/14] drm/i915: unify legacy S3 suspend and S4 freeze handlers

[Imre Deak]

i915_suspend() is called from the DRM legacy S3 suspend/S4 freeze paths
and the switcheroo suspend path. For switcheroo we only ever need to
perform a full suspend (PM_EVENT_SUSPEND) and for the DRM legacy path
we can handle the S4 freeze (PM_EVENT_FREEZE) the same way as S3
suspend. The only difference atm between suspend and freeze is that
during freeze we don't disable the PCI device, but there is no reason
why we can't do so. So unify the two cases to reduce complexity.

Note that for the DRM legacy case the thaw event is not handled, so
we disable the display before creating the hibernation image and it
won't get re-enabled until reboot. We could fix this leaving the
display enabled for the image creation/writing (if we care enough
about UMS), but this can be done as a follow-up.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 753d44a..56c7570 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -670,11 +670,9 @@ int i915_suspend(struct drm_device *dev, pm_message_t state)
 	if (error)
 		return error;
 
-	if (state.event == PM_EVENT_SUSPEND) {
-		/* Shut down the device */
-		pci_disable_device(dev->pdev);
-		pci_set_power_state(dev->pdev, PCI_D3hot);
-	}
+	/* Shut down the device */
+	pci_disable_device(dev->pdev);
+	pci_set_power_state(dev->pdev, PCI_D3hot);
 
 	return 0;
 }
-- 
1.8.4

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

[PATCH v2 03/14] drm/i915: propagate error from legacy resume handler

[Imre Deak]

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 56c7570..524c3df 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -810,10 +810,13 @@ int i915_resume(struct drm_device *dev)
 
 static int i915_resume_legacy(struct drm_device *dev)
 {
-	i915_resume_early(dev);
-	i915_resume(dev);
+	int ret;
 
-	return 0;
+	ret = i915_resume_early(dev);
+	if (ret)
+		return ret;
+
+	return i915_resume(dev);
 }
 
 /**
-- 
1.8.4

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

[PATCH v2 04/14] drm/i915: vlv: fix switcheroo/legacy suspend/resume

[Imre Deak]

During switcheroo/legacy suspend we don't call the suspend_late handler
but when resuming afterwards we call resume_early. This happened to work
so far, since suspend_late only disabled the PCI device. This changed in

commit 016970beb05da6285c2f3ed2bee1c676cb75972e
Author: Sagar Kamble <sagar.a.kamble@intel.com>
Date:   Wed Aug 13 23:07:06 2014 +0530

    drm/i915: Sharing platform specific sequence between runtime and system susp

after which we also saved/restored the VLV Gunit HW state in
suspend_late/resume_early. So now since we don't save the state during
suspend a following resume will restore a corrupted state.

Fix this by calling the suspend_late handler during both switcheroo and
legacy suspend.

CC: Sagar Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 524c3df..203bd82 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -670,11 +670,7 @@ int i915_suspend(struct drm_device *dev, pm_message_t state)
 	if (error)
 		return error;
 
-	/* Shut down the device */
-	pci_disable_device(dev->pdev);
-	pci_set_power_state(dev->pdev, PCI_D3hot);
-
-	return 0;
+	return i915_drm_suspend_late(dev);
 }
 
 static int i915_drm_thaw_early(struct drm_device *dev)
@@ -790,7 +786,7 @@ static int i915_resume_early(struct drm_device *dev)
 	return i915_drm_thaw_early(dev);
 }
 
-int i915_resume(struct drm_device *dev)
+static int i915_drm_resume(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
@@ -816,7 +812,12 @@ static int i915_resume_legacy(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	return i915_resume(dev);
+	return i915_drm_resume(dev);
+}
+
+int i915_resume(struct drm_device *dev)
+{
+	return i915_resume_legacy(dev);
 }
 
 /**
@@ -1004,7 +1005,7 @@ static int i915_pm_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
-	return i915_resume(drm_dev);
+	return i915_drm_resume(drm_dev);
 }
 
 static int i915_pm_freeze(struct device *dev)
-- 
1.8.4

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

[PATCH v2 05/14] drm/i915: fix S4 suspend while switcheroo state is off

[Imre Deak]

If the device is suspended already through the switcheroo interface we
shouldn't suspend it again or resume it after suspend. We have the
corresponding check for S3 suspend already, add it for all the other
S3 and S4 handlers. Also move the check from i915_resume_early() to
i915_resume_legacy(), so that it's done in the high level handler for
all PM events.

v2:
- fix the resume path too, we don't need to special case there
  DRM_SWITCH_POWER_OFF with the device being enabled (in which case we'd
  have to disable the device), since that never happens (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 203bd82..6993632 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -766,9 +766,6 @@ static int i915_drm_thaw(struct drm_device *dev)
 
 static int i915_resume_early(struct drm_device *dev)
 {
-	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
 	/*
 	 * We have a resume ordering issue with the snd-hda driver also
 	 * requiring our device to be power up. Due to the lack of a
@@ -808,6 +805,9 @@ static int i915_resume_legacy(struct drm_device *dev)
 {
 	int ret;
 
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	ret = i915_resume_early(dev);
 	if (ret)
 		return ret;
@@ -997,6 +997,9 @@ static int i915_pm_resume_early(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_resume_early(drm_dev);
 }
 
@@ -1005,6 +1008,9 @@ static int i915_pm_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_drm_resume(drm_dev);
 }
 
@@ -1018,6 +1024,9 @@ static int i915_pm_freeze(struct device *dev)
 		return -ENODEV;
 	}
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_drm_freeze(drm_dev);
 }
 
@@ -1027,6 +1036,9 @@ static int i915_pm_freeze_late(struct device *dev)
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 	struct drm_i915_private *dev_priv = drm_dev->dev_private;
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return intel_suspend_complete(dev_priv);
 }
 
@@ -1035,6 +1047,9 @@ static int i915_pm_thaw_early(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_drm_thaw_early(drm_dev);
 }
 
@@ -1043,6 +1058,9 @@ static int i915_pm_thaw(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_drm_thaw(drm_dev);
 }
 
@@ -1051,6 +1069,9 @@ static int i915_pm_poweroff(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 
+	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return 0;
+
 	return i915_drm_freeze(drm_dev);
 }
 
-- 
1.8.4

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

[PATCH v2 06/14] drm/i915: remove unused restore_gtt_mappings optimization during suspend

[Imre Deak]

The logic to skip restoring GTT mappings was added to speed up
suspend/resume, but not on old GENs where not restoring them caused
problems. The check for old GENs is based on the existence of OpRegion,
but this doesn't work since opregion is initialized only after
the check. So we end up always restoring the mappings.

On my BYT - which has OpRegion - skipping restoring the mappings during
suspend doesn't work, I get a GPU hang after resume. Also the logic of
when to allow the optimization during S4 is reversed: we should allow it
during S4 thaw but not during S4 restore, but atm we have it the other
way around in the code.

Since correctness wins over optimal code and since the optimization
wasn't used anyway I decided not to try to fix it at this point, but
just remove it. This allows us to unify the S3 and S4 handlers in the
following patches.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6993632..9c934be 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -689,12 +689,11 @@ static int i915_drm_thaw_early(struct drm_device *dev)
 	return ret;
 }
 
-static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
+static int __i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (drm_core_check_feature(dev, DRIVER_MODESET) &&
-	    restore_gtt_mappings) {
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
 		i915_gem_restore_gtt_mappings(dev);
 		mutex_unlock(&dev->struct_mutex);
@@ -761,7 +760,7 @@ static int i915_drm_thaw(struct drm_device *dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_check_and_clear_faults(dev);
 
-	return __i915_drm_thaw(dev, true);
+	return __i915_drm_thaw(dev);
 }
 
 static int i915_resume_early(struct drm_device *dev)
@@ -785,15 +784,9 @@ static int i915_resume_early(struct drm_device *dev)
 
 static int i915_drm_resume(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	/*
-	 * Platforms with opregion should have sane BIOS, older ones (gen3 and
-	 * earlier) need to restore the GTT mappings since the BIOS might clear
-	 * all our scratch PTEs.
-	 */
-	ret = __i915_drm_thaw(dev, !dev_priv->opregion.header);
+	ret = __i915_drm_thaw(dev);
 	if (ret)
 		return ret;
 
-- 
1.8.4

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

[PATCH v2 07/14] drm/i915: check for GT faults in all resume handlers and driver load time

[Imre Deak]

Checking for GT faults is not specific in any way to S4 thaw, so do it
also during S3 resume, S4 restore and driver load time. This allows us to
unify the Sx handlers in an upcoming patch.

v2:
- move the check to intel_uncore_early_sanitize(), so we check at driver
  load time too (Chris)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c     |  3 ---
 drivers/gpu/drm/i915/intel_uncore.c | 13 +++++++++++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c934be..d0f721f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -757,9 +757,6 @@ static int __i915_drm_thaw(struct drm_device *dev)
 
 static int i915_drm_thaw(struct drm_device *dev)
 {
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		i915_check_and_clear_faults(dev);
-
 	return __i915_drm_thaw(dev);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 0b0f4f8..3b5d384 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -360,7 +360,8 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
+static void __intel_uncore_early_sanitize(struct drm_device *dev,
+					  bool restore_forcewake)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -386,6 +387,12 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
 	intel_uncore_forcewake_reset(dev, restore_forcewake);
 }
 
+void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
+{
+	__intel_uncore_early_sanitize(dev, restore_forcewake);
+	i915_check_and_clear_faults(dev);
+}
+
 void intel_uncore_sanitize(struct drm_device *dev)
 {
 	/* BIOS often leaves RC6 enabled, but disable it for hw init */
@@ -830,7 +837,7 @@ void intel_uncore_init(struct drm_device *dev)
 	setup_timer(&dev_priv->uncore.force_wake_timer,
 		    gen6_force_wake_timer, (unsigned long)dev_priv);
 
-	intel_uncore_early_sanitize(dev, false);
+	__intel_uncore_early_sanitize(dev, false);
 
 	if (IS_VALLEYVIEW(dev)) {
 		dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
@@ -948,6 +955,8 @@ void intel_uncore_init(struct drm_device *dev)
 		dev_priv->uncore.funcs.mmio_readq  = gen4_read64;
 		break;
 	}
+
+	i915_check_and_clear_faults(dev);
 }
 
 void intel_uncore_fini(struct drm_device *dev)
-- 
1.8.4

[PATCH v2 08/14] drm/i915: enable output polling during S4 thaw

[Imre Deak]

To avoid processing hotplug events we disable connector polling for the
duration of S3 suspend. We also disable it for S4 freeze, and keep it
disabled after S4 thaw. This won't prevent though hotplug processing,
since we re-enable interrupts anyway. There is also no need to prevent
it at that time, since we reinitialize everything during thaw, so the
device is in a consistent state. So to simplify things enable polling
during thaw, which will allow us to handle S4 thaw the same way as S3
resume in an upcoming patch.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d0f721f..4524c61 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -752,6 +752,8 @@ static int __i915_drm_thaw(struct drm_device *dev)
 
 	intel_opregion_notify_adapter(dev, PCI_D0);
 
+	drm_kms_helper_poll_enable(dev);
+
 	return 0;
 }
 
@@ -781,14 +783,7 @@ static int i915_resume_early(struct drm_device *dev)
 
 static int i915_drm_resume(struct drm_device *dev)
 {
-	int ret;
-
-	ret = __i915_drm_thaw(dev);
-	if (ret)
-		return ret;
-
-	drm_kms_helper_poll_enable(dev);
-	return 0;
+	return __i915_drm_thaw(dev);
 }
 
 static int i915_resume_legacy(struct drm_device *dev)
-- 
1.8.4

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

[PATCH v2 09/14] drm/i915: disable/re-enable PCI device around S4 freeze/thaw

[Imre Deak]

We already disable everything during S4 freeze, except the PCI device
itself. There is no reason why we couldn't disable that too and doing
so allows us to unify these handlers in the next patch with the
corresponding S3 suspend/resume handlers.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4524c61..be4cb1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1019,12 +1019,11 @@ static int i915_pm_freeze_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;
 
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	return intel_suspend_complete(dev_priv);
+	return i915_drm_suspend_late(drm_dev);
 }
 
 static int i915_pm_thaw_early(struct device *dev)
@@ -1035,7 +1034,7 @@ static int i915_pm_thaw_early(struct device *dev)
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	return i915_drm_thaw_early(drm_dev);
+	return i915_resume_early(drm_dev);
 }
 
 static int i915_pm_thaw(struct device *dev)
-- 
1.8.4

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

[PATCH v2 10/14] drm/i915: unify S3 and S4 suspend/resume handlers

[Imre Deak]

The S3 and S4 events are now handled the same way internally, there is no
need to keep separate wrapper functions around them. Simply reuse the
suspend/resume versions everywhere.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 104 +++++++---------------------------------
 1 file changed, 17 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index be4cb1d..2190d0f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -673,22 +673,6 @@ int i915_suspend(struct drm_device *dev, pm_message_t state)
 	return i915_drm_suspend_late(dev);
 }
 
-static int i915_drm_thaw_early(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret;
-
-	ret = intel_resume_prepare(dev_priv, false);
-	if (ret)
-		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
-
-	intel_uncore_early_sanitize(dev, true);
-	intel_uncore_sanitize(dev);
-	intel_power_domains_init_hw(dev_priv);
-
-	return ret;
-}
-
 static int __i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -757,13 +741,11 @@ static int __i915_drm_thaw(struct drm_device *dev)
 	return 0;
 }
 
-static int i915_drm_thaw(struct drm_device *dev)
-{
-	return __i915_drm_thaw(dev);
-}
-
 static int i915_resume_early(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
 	/*
 	 * We have a resume ordering issue with the snd-hda driver also
 	 * requiring our device to be power up. Due to the lack of a
@@ -778,7 +760,15 @@ static int i915_resume_early(struct drm_device *dev)
 
 	pci_set_master(dev->pdev);
 
-	return i915_drm_thaw_early(dev);
+	ret = intel_resume_prepare(dev_priv, false);
+	if (ret)
+		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
+
+	intel_uncore_early_sanitize(dev, true);
+	intel_uncore_sanitize(dev);
+	intel_power_domains_init_hw(dev_priv);
+
+	return ret;
 }
 
 static int i915_drm_resume(struct drm_device *dev)
@@ -999,66 +989,6 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
-static int i915_pm_freeze(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-
-	if (!drm_dev || !drm_dev->dev_private) {
-		dev_err(dev, "DRM not initialized, aborting suspend.\n");
-		return -ENODEV;
-	}
-
-	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
-	return i915_drm_freeze(drm_dev);
-}
-
-static int i915_pm_freeze_late(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-
-	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
-	return i915_drm_suspend_late(drm_dev);
-}
-
-static int i915_pm_thaw_early(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-
-	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
-	return i915_resume_early(drm_dev);
-}
-
-static int i915_pm_thaw(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-
-	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
-	return i915_drm_thaw(drm_dev);
-}
-
-static int i915_pm_poweroff(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-
-	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
-		return 0;
-
-	return i915_drm_freeze(drm_dev);
-}
-
 static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
 {
 	hsw_enable_pc8(dev_priv);
@@ -1618,11 +1548,11 @@ static const struct dev_pm_ops i915_pm_ops = {
 	.suspend_late = i915_pm_suspend_late,
 	.resume_early = i915_pm_resume_early,
 	.resume = i915_pm_resume,
-	.freeze = i915_pm_freeze,
-	.freeze_late = i915_pm_freeze_late,
-	.thaw_early = i915_pm_thaw_early,
-	.thaw = i915_pm_thaw,
-	.poweroff = i915_pm_poweroff,
+	.freeze = i915_pm_suspend,
+	.freeze_late = i915_pm_suspend_late,
+	.thaw_early = i915_pm_resume_early,
+	.thaw = i915_pm_resume,
+	.poweroff = i915_pm_suspend,
 	.restore_early = i915_pm_resume_early,
 	.restore = i915_pm_resume,
 	.runtime_suspend = intel_runtime_suspend,
-- 
1.8.4

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

[PATCH v2 11/14] drm/i915: sanitize suspend/resume helper function names

[Imre Deak]

By now the S4 freeze/thaw and S3 suspend/resume events are handled the
same way, so we can rename the freeze/thaw internal helpers to
suspend/resume accordingly to make clearer what the helpers do. Also
rename i915_resume_early to i915_drm_resume_early aligning it with the
rest of the helper names.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2190d0f..8b2b34c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -554,7 +554,7 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv);
 static int intel_resume_prepare(struct drm_i915_private *dev_priv,
 				bool rpm_resume);
 
-static int i915_drm_freeze(struct drm_device *dev)
+static int i915_drm_suspend(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
@@ -666,14 +666,14 @@ int i915_suspend(struct drm_device *dev, pm_message_t state)
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	error = i915_drm_freeze(dev);
+	error = i915_drm_suspend(dev);
 	if (error)
 		return error;
 
 	return i915_drm_suspend_late(dev);
 }
 
-static int __i915_drm_thaw(struct drm_device *dev)
+static int i915_drm_resume(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -741,7 +741,7 @@ static int __i915_drm_thaw(struct drm_device *dev)
 	return 0;
 }
 
-static int i915_resume_early(struct drm_device *dev)
+static int i915_drm_resume_early(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
@@ -771,11 +771,6 @@ static int i915_resume_early(struct drm_device *dev)
 	return ret;
 }
 
-static int i915_drm_resume(struct drm_device *dev)
-{
-	return __i915_drm_thaw(dev);
-}
-
 static int i915_resume_legacy(struct drm_device *dev)
 {
 	int ret;
@@ -783,7 +778,7 @@ static int i915_resume_legacy(struct drm_device *dev)
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	ret = i915_resume_early(dev);
+	ret = i915_drm_resume_early(dev);
 	if (ret)
 		return ret;
 
@@ -944,7 +939,7 @@ static int i915_pm_suspend(struct device *dev)
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	return i915_drm_freeze(drm_dev);
+	return i915_drm_suspend(drm_dev);
 }
 
 static int i915_pm_suspend_late(struct device *dev)
@@ -975,7 +970,7 @@ static int i915_pm_resume_early(struct device *dev)
 	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	return i915_resume_early(drm_dev);
+	return i915_drm_resume_early(drm_dev);
 }
 
 static int i915_pm_resume(struct device *dev)
-- 
1.8.4

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

[PATCH v2 12/14] drm/i915: add poweroff_late handler

[Imre Deak]

The suspend_late handler saves some registers and powers off the device,
so it doesn't have a big overhead. Calling it at S4 poweroff_late time
makes the power off handling identical to the S3 suspend and S4 freeze
handling, so do this for consistency.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8b2b34c..808f659 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1548,6 +1548,7 @@ static const struct dev_pm_ops i915_pm_ops = {
 	.thaw_early = i915_pm_resume_early,
 	.thaw = i915_pm_resume,
 	.poweroff = i915_pm_suspend,
+	.poweroff_late = i915_pm_suspend_late,
 	.restore_early = i915_pm_resume_early,
 	.restore = i915_pm_resume,
 	.runtime_suspend = intel_runtime_suspend,
-- 
1.8.4

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

[PATCH v2 13/14] drm/i915: unify switcheroo and legacy suspend/resume handlers

[Imre Deak]

By now we handle switcheroo and legacy suspend/resume the same way, so
no need to keep separate functions for them.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |  4 ++--
 drivers/gpu/drm/i915/i915_drv.c | 11 +++--------
 drivers/gpu/drm/i915/i915_drv.h |  4 ++--
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1b39807..9a73533 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1275,12 +1275,12 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 		/* i915 resume handler doesn't set to D0 */
 		pci_set_power_state(dev->pdev, PCI_D0);
-		i915_resume(dev);
+		i915_resume_legacy(dev);
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	} else {
 		pr_err("switched off\n");
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
-		i915_suspend(dev, pmm);
+		i915_suspend_legacy(dev, pmm);
 		dev->switch_power_state = DRM_SWITCH_POWER_OFF;
 	}
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 808f659..bce58d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -649,7 +649,7 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev)
 	return 0;
 }
 
-int i915_suspend(struct drm_device *dev, pm_message_t state)
+int i915_suspend_legacy(struct drm_device *dev, pm_message_t state)
 {
 	int error;
 
@@ -771,7 +771,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 	return ret;
 }
 
-static int i915_resume_legacy(struct drm_device *dev)
+int i915_resume_legacy(struct drm_device *dev)
 {
 	int ret;
 
@@ -785,11 +785,6 @@ static int i915_resume_legacy(struct drm_device *dev)
 	return i915_drm_resume(dev);
 }
 
-int i915_resume(struct drm_device *dev)
-{
-	return i915_resume_legacy(dev);
-}
-
 /**
  * i915_reset - reset chip after a hang
  * @dev: drm device to reset
@@ -1592,7 +1587,7 @@ static struct drm_driver driver = {
 	.set_busid = drm_pci_set_busid,
 
 	/* Used in place of i915_pm_ops for non-DRIVER_MODESET */
-	.suspend = i915_suspend,
+	.suspend = i915_suspend_legacy,
 	.resume = i915_resume_legacy,
 
 	.device_is_agp = i915_driver_device_is_agp,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3212d62..059330c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2221,8 +2221,8 @@ struct drm_i915_cmd_table {
 extern const struct drm_ioctl_desc i915_ioctls[];
 extern int i915_max_ioctl;
 
-extern int i915_suspend(struct drm_device *dev, pm_message_t state);
-extern int i915_resume(struct drm_device *dev);
+extern int i915_suspend_legacy(struct drm_device *dev, pm_message_t state);
+extern int i915_resume_legacy(struct drm_device *dev);
 extern int i915_master_create(struct drm_device *dev, struct drm_master *master);
 extern void i915_master_destroy(struct drm_device *dev, struct drm_master *master);
 
-- 
1.8.4

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

[PATCH v2 14/14] drm/i915: add comments on what stage a given PM handler is called

[Imre Deak]

This will hopefully make it easier to navigate the code without the need
to consult the full PM documentation.

v2:
- add a comment that the freeze handler is also called after rebooting
- add a comment that the thaw handler is also called to recover from
  errors (Ville)
- add the PM event names (PMSG_THAW etc.) for reference (Ville)
- add comments that s0ix can be handled both via system and runtime
  suspend (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index bce58d3..035ec94 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1534,10 +1534,30 @@ static int intel_resume_prepare(struct drm_i915_private *dev_priv,
 }
 
 static const struct dev_pm_ops i915_pm_ops = {
+	/*
+	 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
+	 * PMSG_RESUME]
+	 */
 	.suspend = i915_pm_suspend,
 	.suspend_late = i915_pm_suspend_late,
 	.resume_early = i915_pm_resume_early,
 	.resume = i915_pm_resume,
+
+	/*
+	 * S4 event handlers
+	 * @freeze, @freeze_late    : called (1) before creating the
+	 *                            hibernation image [PMSG_FREEZE] and
+	 *                            (2) after rebooting, before restoring
+	 *                            the image [PMSG_QUIESCE]
+	 * @thaw, @thaw_early       : called (1) after creating the hibernation
+	 *                            image, before writing it [PMSG_THAW]
+	 *                            and (2) after failing to create or
+	 *                            restore the image [PMSG_RECOVER]
+	 * @poweroff, @poweroff_late: called after writing the hibernation
+	 *                            image, before rebooting [PMSG_HIBERNATE]
+	 * @restore, @restore_early : called after rebooting and restoring the
+	 *                            hibernation image [PMSG_RESTORE]
+	 */
 	.freeze = i915_pm_suspend,
 	.freeze_late = i915_pm_suspend_late,
 	.thaw_early = i915_pm_resume_early,
@@ -1546,6 +1566,8 @@ static const struct dev_pm_ops i915_pm_ops = {
 	.poweroff_late = i915_pm_suspend_late,
 	.restore_early = i915_pm_resume_early,
 	.restore = i915_pm_resume,
+
+	/* S0ix (via runtime suspend) event handlers */
 	.runtime_suspend = intel_runtime_suspend,
 	.runtime_resume = intel_runtime_resume,
 };
-- 
1.8.4

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

Re: [PATCH v2 09/14] drm/i915: disable/re-enable PCI device around S4 freeze/thaw

[Daniel Vetter]

On Thu, Oct 23, 2014 at 07:23:23PM +0300, Imre Deak wrote:
> We already disable everything during S4 freeze, except the PCI device
> itself. There is no reason why we couldn't disable that too and doing
> so allows us to unify these handlers in the next patch with the
> corresponding S3 suspend/resume handlers.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

It feels like I've asked you this already, but could this perhaps fix the
ominous S4 issues we're seeing on hsw+? There should be a few QA reports
in bugzilla about it.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4524c61..be4cb1d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1019,12 +1019,11 @@ static int i915_pm_freeze_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;
>  
>  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> -	return intel_suspend_complete(dev_priv);
> +	return i915_drm_suspend_late(drm_dev);
>  }
>  
>  static int i915_pm_thaw_early(struct device *dev)
> @@ -1035,7 +1034,7 @@ static int i915_pm_thaw_early(struct device *dev)
>  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> -	return i915_drm_thaw_early(drm_dev);
> +	return i915_resume_early(drm_dev);
>  }
>  
>  static int i915_pm_thaw(struct device *dev)
> -- 
> 1.8.4
> 
> _______________________________________________
> 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

Re: [PATCH v2 00/14] fix VLV S4 suspend/resume, unify S3/S4 handlers

[Daniel Vetter]

On Thu, Oct 23, 2014 at 07:23:14PM +0300, Imre Deak wrote:
> This is v2 of [1]. I addressed Chris' and Ville's comments in it and
> rebased it against the latest -nightly kernel.
> 
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2014-September/052199.html
> 
> Imre Deak (14):
>   drm/i915: factor out i915_drm_suspend_late
>   drm/i915: unify legacy S3 suspend and S4 freeze handlers
>   drm/i915: propagate error from legacy resume handler
>   drm/i915: vlv: fix switcheroo/legacy suspend/resume
>   drm/i915: fix S4 suspend while switcheroo state is off
>   drm/i915: remove unused restore_gtt_mappings optimization during
>     suspend
>   drm/i915: check for GT faults in all resume handlers and driver load
>     time
>   drm/i915: enable output polling during S4 thaw
>   drm/i915: disable/re-enable PCI device around S4 freeze/thaw
>   drm/i915: unify S3 and S4 suspend/resume handlers
>   drm/i915: sanitize suspend/resume helper function names
>   drm/i915: add poweroff_late handler
>   drm/i915: unify switcheroo and legacy suspend/resume handlers
>   drm/i915: add comments on what stage a given PM handler is called

Entire series merged, thanks for resending.
-Daniel
> 
>  drivers/gpu/drm/i915/i915_dma.c     |   4 +-
>  drivers/gpu/drm/i915/i915_drv.c     | 208 ++++++++++++++----------------------
>  drivers/gpu/drm/i915/i915_drv.h     |   4 +-
>  drivers/gpu/drm/i915/intel_uncore.c |  13 ++-
>  4 files changed, 98 insertions(+), 131 deletions(-)
> 
> -- 
> 1.8.4
> 
> _______________________________________________
> 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

Re: [PATCH v2 09/14] drm/i915: disable/re-enable PCI device around S4 freeze/thaw

[Imre Deak]

On Fri, 2014-10-24 at 10:51 +0200, Daniel Vetter wrote:
> On Thu, Oct 23, 2014 at 07:23:23PM +0300, Imre Deak wrote:
> > We already disable everything during S4 freeze, except the PCI device
> > itself. There is no reason why we couldn't disable that too and doing
> > so allows us to unify these handlers in the next patch with the
> > corresponding S3 suspend/resume handlers.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It feels like I've asked you this already, but could this perhaps fix the
> ominous S4 issues we're seeing on hsw+? There should be a few QA reports
> in bugzilla about it.

Yes, it's possible. I still think that if it fixes things then it's just
a workaround, since by the time we disable the PCI device everything
should be quiescent anyway. Or the reinitialization during thaw depends
on an explicit HW reset (which we'll get now with the PCI
disable/enable), but imo this shouldn't be needed either.

Anyway there is:
https://bugzilla.kernel.org/show_bug.cgi?id=59321
I can ask the reporter to test again -nightly and also test with the
patchset reverted. I'll check also fdo for similar bugs and ask for
retesting.

--Imre

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 4524c61..be4cb1d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1019,12 +1019,11 @@ static int i915_pm_freeze_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;
> >  
> >  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> >  		return 0;
> >  
> > -	return intel_suspend_complete(dev_priv);
> > +	return i915_drm_suspend_late(drm_dev);
> >  }
> >  
> >  static int i915_pm_thaw_early(struct device *dev)
> > @@ -1035,7 +1034,7 @@ static int i915_pm_thaw_early(struct device *dev)
> >  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> >  		return 0;
> >  
> > -	return i915_drm_thaw_early(drm_dev);
> > +	return i915_resume_early(drm_dev);
> >  }
> >  
> >  static int i915_pm_thaw(struct device *dev)
> > -- 
> > 1.8.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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