[Intel-gfx] [PATCH 1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions

[Intel-gfx] [PATCH 1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions

[Rodrigo Vivi]

No functional changes. Just revamping the functions with
s/dev_priv/i915
and consolidating along with other runtime_pm functions.

v2: avoid the extra redirection (Imre)

Cc: Imre Deak <imre.deak@intel.com>
Cc: Tilak Tangudu <tilak.tangudu@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 145 +-----------------------
 drivers/gpu/drm/i915/intel_runtime_pm.c | 145 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.h |   2 +
 3 files changed, 149 insertions(+), 143 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59fb4c710c8c..a40b5d806321 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -64,7 +64,6 @@
 #include "gem/i915_gem_mman.h"
 #include "gem/i915_gem_pm.h"
 #include "gt/intel_gt.h"
-#include "gt/intel_gt_pm.h"
 #include "gt/intel_rc6.h"
 
 #include "i915_debugfs.h"
@@ -1517,146 +1516,6 @@ static int i915_pm_restore(struct device *kdev)
 	return i915_pm_resume(kdev);
 }
 
-static int intel_runtime_suspend(struct device *kdev)
-{
-	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
-	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
-	int ret;
-
-	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
-		return -ENODEV;
-
-	drm_dbg_kms(&dev_priv->drm, "Suspending device\n");
-
-	disable_rpm_wakeref_asserts(rpm);
-
-	/*
-	 * We are safe here against re-faults, since the fault handler takes
-	 * an RPM reference.
-	 */
-	i915_gem_runtime_suspend(dev_priv);
-
-	intel_gt_runtime_suspend(&dev_priv->gt);
-
-	intel_runtime_pm_disable_interrupts(dev_priv);
-
-	intel_uncore_suspend(&dev_priv->uncore);
-
-	intel_display_power_suspend(dev_priv);
-
-	ret = vlv_suspend_complete(dev_priv);
-	if (ret) {
-		drm_err(&dev_priv->drm,
-			"Runtime suspend failed, disabling it (%d)\n", ret);
-		intel_uncore_runtime_resume(&dev_priv->uncore);
-
-		intel_runtime_pm_enable_interrupts(dev_priv);
-
-		intel_gt_runtime_resume(&dev_priv->gt);
-
-		enable_rpm_wakeref_asserts(rpm);
-
-		return ret;
-	}
-
-	enable_rpm_wakeref_asserts(rpm);
-	intel_runtime_pm_driver_release(rpm);
-
-	if (intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore))
-		drm_err(&dev_priv->drm,
-			"Unclaimed access detected prior to suspending\n");
-
-	rpm->suspended = true;
-
-	/*
-	 * FIXME: We really should find a document that references the arguments
-	 * used below!
-	 */
-	if (IS_BROADWELL(dev_priv)) {
-		/*
-		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
-		 * being detected, and the call we do at intel_runtime_resume()
-		 * won't be able to restore them. Since PCI_D3hot matches the
-		 * actual specification and appears to be working, use it.
-		 */
-		intel_opregion_notify_adapter(dev_priv, PCI_D3hot);
-	} else {
-		/*
-		 * current versions of firmware which depend on this opregion
-		 * notification have repurposed the D1 definition to mean
-		 * "runtime suspended" vs. what you would normally expect (D3)
-		 * to distinguish it from notifications that might be sent via
-		 * the suspend path.
-		 */
-		intel_opregion_notify_adapter(dev_priv, PCI_D1);
-	}
-
-	assert_forcewakes_inactive(&dev_priv->uncore);
-
-	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
-		intel_hpd_poll_enable(dev_priv);
-
-	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
-	return 0;
-}
-
-static int intel_runtime_resume(struct device *kdev)
-{
-	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
-	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
-	int ret;
-
-	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
-		return -ENODEV;
-
-	drm_dbg_kms(&dev_priv->drm, "Resuming device\n");
-
-	drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count));
-	disable_rpm_wakeref_asserts(rpm);
-
-	intel_opregion_notify_adapter(dev_priv, PCI_D0);
-	rpm->suspended = false;
-	if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
-		drm_dbg(&dev_priv->drm,
-			"Unclaimed access during suspend, bios?\n");
-
-	intel_display_power_resume(dev_priv);
-
-	ret = vlv_resume_prepare(dev_priv, true);
-
-	intel_uncore_runtime_resume(&dev_priv->uncore);
-
-	intel_runtime_pm_enable_interrupts(dev_priv);
-
-	/*
-	 * No point of rolling back things in case of an error, as the best
-	 * we can do is to hope that things will still work (and disable RPM).
-	 */
-	intel_gt_runtime_resume(&dev_priv->gt);
-
-	/*
-	 * On VLV/CHV display interrupts are part of the display
-	 * power well, so hpd is reinitialized from there. For
-	 * everyone else do it here.
-	 */
-	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
-		intel_hpd_init(dev_priv);
-		intel_hpd_poll_disable(dev_priv);
-	}
-
-	intel_enable_ipc(dev_priv);
-
-	enable_rpm_wakeref_asserts(rpm);
-
-	if (ret)
-		drm_err(&dev_priv->drm,
-			"Runtime resume failed, disabling it (%d)\n", ret);
-	else
-		drm_dbg_kms(&dev_priv->drm, "Device resumed\n");
-
-	return ret;
-}
-
 const struct dev_pm_ops i915_pm_ops = {
 	/*
 	 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
@@ -1693,8 +1552,8 @@ const struct dev_pm_ops i915_pm_ops = {
 	.restore = i915_pm_restore,
 
 	/* S0ix (via runtime suspend) event handlers */
-	.runtime_suspend = intel_runtime_suspend,
-	.runtime_resume = intel_runtime_resume,
+	.runtime_suspend = intel_runtime_pm_suspend,
+	.runtime_resume = intel_runtime_pm_resume,
 };
 
 static const struct file_operations i915_driver_fops = {
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index eaf7688f517d..f28b5bab61b4 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -32,6 +32,11 @@
 
 #include "i915_drv.h"
 #include "i915_trace.h"
+#include "gt/intel_gt.h"
+#include "gt/intel_gt_pm.h"
+#include "intel_pm.h"
+#include "vlv_suspend.h"
+#include "display/intel_hotplug.h"
 
 /**
  * DOC: runtime pm
@@ -652,3 +657,143 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm)
 
 	init_intel_runtime_pm_wakeref(rpm);
 }
+
+int intel_runtime_pm_suspend(struct device *kdev)
+{
+	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_runtime_pm *rpm = &i915->runtime_pm;
+	int ret;
+
+	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
+		return -ENODEV;
+
+	drm_dbg_kms(&i915->drm, "Suspending device\n");
+
+	disable_rpm_wakeref_asserts(rpm);
+
+	/*
+	 * We are safe here against re-faults, since the fault handler takes
+	 * an RPM reference.
+	 */
+	i915_gem_runtime_suspend(i915);
+
+	intel_gt_runtime_suspend(&i915->gt);
+
+	intel_runtime_pm_disable_interrupts(i915);
+
+	intel_uncore_suspend(&i915->uncore);
+
+	intel_display_power_suspend(i915);
+
+	ret = vlv_suspend_complete(i915);
+	if (ret) {
+		drm_err(&i915->drm,
+			"Runtime suspend failed, disabling it (%d)\n", ret);
+		intel_uncore_runtime_resume(&i915->uncore);
+
+		intel_runtime_pm_enable_interrupts(i915);
+
+		intel_gt_runtime_resume(&i915->gt);
+
+		enable_rpm_wakeref_asserts(rpm);
+
+		return ret;
+	}
+
+	enable_rpm_wakeref_asserts(rpm);
+	intel_runtime_pm_driver_release(rpm);
+
+	if (intel_uncore_arm_unclaimed_mmio_detection(&i915->uncore))
+		drm_err(&i915->drm,
+			"Unclaimed access detected prior to suspending\n");
+
+	rpm->suspended = true;
+
+	/*
+	 * FIXME: We really should find a document that references the arguments
+	 * used below!
+	 */
+	if (IS_BROADWELL(i915)) {
+		/*
+		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
+		 * being detected, and the call we do at intel_runtime_resume()
+		 * won't be able to restore them. Since PCI_D3hot matches the
+		 * actual specification and appears to be working, use it.
+		 */
+		intel_opregion_notify_adapter(i915, PCI_D3hot);
+	} else {
+		/*
+		 * current versions of firmware which depend on this opregion
+		 * notification have repurposed the D1 definition to mean
+		 * "runtime suspended" vs. what you would normally expect (D3)
+		 * to distinguish it from notifications that might be sent via
+		 * the suspend path.
+		 */
+		intel_opregion_notify_adapter(i915, PCI_D1);
+	}
+
+	assert_forcewakes_inactive(&i915->uncore);
+
+	if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915))
+		intel_hpd_poll_enable(i915);
+
+	drm_dbg_kms(&i915->drm, "Device suspended\n");
+	return 0;
+}
+
+int intel_runtime_pm_resume(struct device *kdev)
+{
+	struct drm_i915_private *i915 = kdev_to_i915(kdev);
+	struct intel_runtime_pm *rpm = &i915->runtime_pm;
+	int ret;
+
+	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
+		return -ENODEV;
+
+	drm_dbg_kms(&i915->drm, "Resuming device\n");
+
+	drm_WARN_ON_ONCE(&i915->drm, atomic_read(&rpm->wakeref_count));
+	disable_rpm_wakeref_asserts(rpm);
+
+	intel_opregion_notify_adapter(i915, PCI_D0);
+	rpm->suspended = false;
+	if (intel_uncore_unclaimed_mmio(&i915->uncore))
+		drm_dbg(&i915->drm,
+			"Unclaimed access during suspend, bios?\n");
+
+	intel_display_power_resume(i915);
+
+	ret = vlv_resume_prepare(i915, true);
+
+	intel_uncore_runtime_resume(&i915->uncore);
+
+	intel_runtime_pm_enable_interrupts(i915);
+
+	/*
+	 * No point of rolling back things in case of an error, as the best
+	 * we can do is to hope that things will still work (and disable RPM).
+	 */
+	intel_gt_runtime_resume(&i915->gt);
+
+	/*
+	 * On VLV/CHV display interrupts are part of the display
+	 * power well, so hpd is reinitialized from there. For
+	 * everyone else do it here.
+	 */
+	if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) {
+		intel_hpd_init(i915);
+		intel_hpd_poll_disable(i915);
+	}
+
+	intel_enable_ipc(i915);
+
+	enable_rpm_wakeref_asserts(rpm);
+
+	if (ret)
+		drm_err(&i915->drm,
+			"Runtime resume failed, disabling it (%d)\n", ret);
+	else
+		drm_dbg_kms(&i915->drm, "Device resumed\n");
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
index 47a85fab4130..88ca531165f7 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.h
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
@@ -172,6 +172,8 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm);
 void intel_runtime_pm_enable(struct intel_runtime_pm *rpm);
 void intel_runtime_pm_disable(struct intel_runtime_pm *rpm);
 void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm);
+int intel_runtime_pm_suspend(struct device *kdev);
+int intel_runtime_pm_resume(struct device *kdev);
 
 intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
 intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm *rpm);
-- 
2.31.1

[Intel-gfx] [PATCH 2/4] drm/i915: Disallow D3Cold.

[Rodrigo Vivi]

During runtime or s2idle suspend and resume cases on discrete cards,
if D3Cold is really achieved, we will blow everything up and
freeze the machine because we are not yet handling the pci states
properly.

On Integrated it simply doesn't matter because D3hot is the maximum
that we will get anyway, unless the system is on S3/S4 and our power
is cut.

Let's put this hammer for now everywhere. So we can work to enable
the auto-suspend by default without blowing up the world.

Then, this should be removed when we finally fix the D3Cold flow.

Cc: Tilak Tangudu <tilak.tangudu@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a40b5d806321..086a9a475ce8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -301,6 +301,7 @@ static void sanitize_gpu(struct drm_i915_private *i915)
  */
 static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 {
+	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
 	int ret = 0;
 
 	if (i915_inject_probe_failure(dev_priv))
@@ -331,6 +332,13 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * FIXME: Temporary hammer to avoid freezing the machine on our DGFX
+	 * This should be totally removed when we handle the pci states properly
+	 * on runtime PM and on s2idle cases.
+	 */
+	pci_d3cold_disable(pdev);
+
 	ret = vlv_suspend_init(dev_priv);
 	if (ret < 0)
 		goto err_workqueues;
-- 
2.31.1

[Intel-gfx] [PATCH 3/4] drm/i915: Enable runtime pm autosuspend by default

[Rodrigo Vivi]

Let's enable runtime pm autosuspend by default everywhere.

But at this time let's not touch the autosuspend_delay time,
what caused some regression on our previous attempt.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Tilak Tangudu <tilak.tangudu@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f28b5bab61b4..8f052bd4f58c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -605,6 +605,8 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 		pm_runtime_use_autosuspend(kdev);
 	}
 
+	pm_runtime_allow(kdev);
+
 	/*
 	 * The core calls the driver load handler with an RPM reference held.
 	 * We drop that here and will reacquire it during unloading in
-- 
2.31.1

[Intel-gfx] [PATCH 4/4] drm/i915/runtime_pm: Reduce autosuspend delay to 1s.

[Rodrigo Vivi]

Let's try to be more aggressive on the power savings, but
not as much as 0.1s that caused us some regression in the
past.

Also let's have this in a separated patch so that can be
bisected and increased back (or reverted) as needed.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Tilak Tangudu <tilak.tangudu@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8f052bd4f58c..3244ac85d13c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -585,7 +585,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 	 */
 	dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE);
 
-	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
+	pm_runtime_set_autosuspend_delay(kdev, 1000); /* 1s */
 	pm_runtime_mark_last_busy(kdev);
 
 	/*
-- 
2.31.1

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions
URL   : https://patchwork.freedesktop.org/series/94023/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/i915_gem_context.c:1374:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    expected struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    got struct i915_address_space *
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    expected struct i915_address_space *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    got struct i915_address_space [noderef] __rcu *vm
+drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1392:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/i915_perf.c:1442:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1496:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/intel_wakeref.c:137:19: warning: context imbalance in 'wakeref_auto_timeout' - unexpected unlock
+./include/asm-generic/bitops/find.h:112:45: warning: shift count is negative (-262080)
+./include/asm-generic/bitops/find.h:32:31: warning: shift count is negative (-262080)
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+./include/linux/stddef.h:17:9: this was the original definition
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined
+/usr/lib/gcc/x86_64-linux-gnu/8/include/stddef.h:417:9: warning: preprocessor token offsetof redefined

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6645 bytes --]

== Series Details ==

Series: series starting with [1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions
URL   : https://patchwork.freedesktop.org/series/94023/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10520 -> Patchwork_20893
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_20893 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20893, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_20893:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@objects:
    - fi-bsw-n3050:       [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-bsw-n3050/igt@i915_selftest@live@objects.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bsw-n3050/igt@i915_selftest@live@objects.html
    - fi-bxt-dsi:         [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-bxt-dsi/igt@i915_selftest@live@objects.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bxt-dsi/igt@i915_selftest@live@objects.html
    - fi-kbl-soraka:      [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-kbl-soraka/igt@i915_selftest@live@objects.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-kbl-soraka/igt@i915_selftest@live@objects.html
    - fi-bsw-nick:        [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-bsw-nick/igt@i915_selftest@live@objects.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bsw-nick/igt@i915_selftest@live@objects.html
    - fi-glk-dsi:         [PASS][9] -> [DMESG-FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-glk-dsi/igt@i915_selftest@live@objects.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-glk-dsi/igt@i915_selftest@live@objects.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-rkl-11600:       NOTRUN -> [SKIP][11]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-rkl-11600/igt@kms_flip@basic-flip-vs-modeset.html

  
Known issues
------------

  Here are the changes found in Patchwork_20893 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@runner@aborted:
    - fi-glk-dsi:         NOTRUN -> [FAIL][12] ([i915#3363] / [k.org#202321])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-glk-dsi/igt@runner@aborted.html
    - fi-bsw-nick:        NOTRUN -> [FAIL][13] ([fdo#109271] / [i915#1436])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bsw-nick/igt@runner@aborted.html
    - fi-kbl-soraka:      NOTRUN -> [FAIL][14] ([i915#1436] / [i915#3363])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-kbl-soraka/igt@runner@aborted.html
    - fi-bxt-dsi:         NOTRUN -> [FAIL][15] ([i915#3363])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bxt-dsi/igt@runner@aborted.html
    - fi-bsw-n3050:       NOTRUN -> [FAIL][16] ([fdo#109271] / [i915#1436])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-bsw-n3050/igt@runner@aborted.html

  
#### Warnings ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-tgl-1115g4:      [DMESG-WARN][17] ([i915#1982] / [i915#4002]) -> [DMESG-WARN][18] ([i915#4002])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_pm_rpm@module-reload:
    - fi-tgl-1115g4:      [INCOMPLETE][19] ([i915#4006]) -> [INCOMPLETE][20] ([i915#1385] / [i915#4006])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html

  * igt@kms_psr@primary_page_flip:
    - fi-tgl-1115g4:      [SKIP][21] ([i915#1072]) -> [SKIP][22] ([i915#1072] / [i915#1385])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10520/fi-tgl-1115g4/igt@kms_psr@primary_page_flip.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/fi-tgl-1115g4/igt@kms_psr@primary_page_flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1385]: https://gitlab.freedesktop.org/drm/intel/issues/1385
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2966]: https://gitlab.freedesktop.org/drm/intel/issues/2966
  [i915#3363]: https://gitlab.freedesktop.org/drm/intel/issues/3363
  [i915#4002]: https://gitlab.freedesktop.org/drm/intel/issues/4002
  [i915#4006]: https://gitlab.freedesktop.org/drm/intel/issues/4006
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (40 -> 34)
------------------------------

  Missing    (6): fi-ilk-m540 bat-adls-5 fi-hsw-4200u fi-bsw-cyan fi-bdw-samus bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10520 -> Patchwork_20893

  CI-20190529: 20190529
  CI_DRM_10520: df6d856ea920279c17e875a80fca47a428fd7fcd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6185: 5dca04416f50576f464ebbd9aea96edccd7e4eab @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20893: 431b7d057edd27f841152ed7d1a9f504018ed044 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

431b7d057edd drm/i915/runtime_pm: Reduce autosuspend delay to 1s.
f17647efe72c drm/i915: Enable runtime pm autosuspend by default
33cb8ee3463a drm/i915: Disallow D3Cold.
c548e4ae59d6 drm/i915/runtime_pm: Consolidate runtime_pm functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20893/index.html

[-- Attachment #2: Type: text/html, Size: 7934 bytes --]

Re: [Intel-gfx] [PATCH 4/4] drm/i915/runtime_pm: Reduce autosuspend delay to 1s.

[Gupta, Anshuman]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo
> Vivi
> Sent: Wednesday, August 25, 2021 8:53 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Daniel Vetter
> <daniel.vetter@ffwll.ch>; David Weinehall <david.weinehall@linux.intel.com>;
> Tangudu, Tilak <tilak.tangudu@intel.com>
> Subject: [Intel-gfx] [PATCH 4/4] drm/i915/runtime_pm: Reduce autosuspend
> delay to 1s.
> 
> Let's try to be more aggressive on the power savings, but not as much as 0.1s
> that caused us some regression in the past.
In IGT we are already using 0 second as auto suspend delay.
Looks good to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>

> 
> Also let's have this in a separated patch so that can be bisected and increased
> back (or reverted) as needed.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Tilak Tangudu <tilak.tangudu@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8f052bd4f58c..3244ac85d13c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -585,7 +585,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm
> *rpm)
>  	 */
>  	dev_pm_set_driver_flags(kdev, DPM_FLAG_NO_DIRECT_COMPLETE);
> 
> -	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
> +	pm_runtime_set_autosuspend_delay(kdev, 1000); /* 1s */
>  	pm_runtime_mark_last_busy(kdev);
> 
>  	/*
> --
> 2.31.1

Re: [Intel-gfx] [PATCH 3/4] drm/i915: Enable runtime pm autosuspend by default

[Gupta, Anshuman]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo
> Vivi
> Sent: Wednesday, August 25, 2021 8:53 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Daniel Vetter
> <daniel.vetter@ffwll.ch>; David Weinehall <david.weinehall@linux.intel.com>;
> Tangudu, Tilak <tilak.tangudu@intel.com>; Deak, Imre <imre.deak@intel.com>
> Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Enable runtime pm autosuspend by
> default
> 
> Let's enable runtime pm autosuspend by default everywhere.
> 
> But at this time let's not touch the autosuspend_delay time, what caused some
> regression on our previous attempt.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Tilak Tangudu <tilak.tangudu@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f28b5bab61b4..8f052bd4f58c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -605,6 +605,8 @@ void intel_runtime_pm_enable(struct intel_runtime_pm
> *rpm)
>  		pm_runtime_use_autosuspend(kdev);
>  	}
> 
> +	pm_runtime_allow(kdev);
Looks good to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> +
>  	/*
>  	 * The core calls the driver load handler with an RPM reference held.
>  	 * We drop that here and will reacquire it during unloading in
> --
> 2.31.1

Re: [Intel-gfx] [PATCH 1/4] drm/i915/runtime_pm: Consolidate runtime_pm functions

[Imre Deak]

On Wed, Aug 25, 2021 at 11:22:30AM -0400, Rodrigo Vivi wrote:
> No functional changes. Just revamping the functions with
> s/dev_priv/i915
> and consolidating along with other runtime_pm functions.
> 
> v2: avoid the extra redirection (Imre)
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Tilak Tangudu <tilak.tangudu@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

One nit below.

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 145 +-----------------------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 145 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.h |   2 +
>  3 files changed, 149 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 59fb4c710c8c..a40b5d806321 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -64,7 +64,6 @@
>  #include "gem/i915_gem_mman.h"
>  #include "gem/i915_gem_pm.h"
>  #include "gt/intel_gt.h"
> -#include "gt/intel_gt_pm.h"
>  #include "gt/intel_rc6.h"
>  
>  #include "i915_debugfs.h"
> @@ -1517,146 +1516,6 @@ static int i915_pm_restore(struct device *kdev)
>  	return i915_pm_resume(kdev);
>  }
>  
> -static int intel_runtime_suspend(struct device *kdev)
> -{
> -	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> -	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> -	int ret;
> -
> -	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
> -		return -ENODEV;
> -
> -	drm_dbg_kms(&dev_priv->drm, "Suspending device\n");
> -
> -	disable_rpm_wakeref_asserts(rpm);
> -
> -	/*
> -	 * We are safe here against re-faults, since the fault handler takes
> -	 * an RPM reference.
> -	 */
> -	i915_gem_runtime_suspend(dev_priv);
> -
> -	intel_gt_runtime_suspend(&dev_priv->gt);
> -
> -	intel_runtime_pm_disable_interrupts(dev_priv);
> -
> -	intel_uncore_suspend(&dev_priv->uncore);
> -
> -	intel_display_power_suspend(dev_priv);
> -
> -	ret = vlv_suspend_complete(dev_priv);
> -	if (ret) {
> -		drm_err(&dev_priv->drm,
> -			"Runtime suspend failed, disabling it (%d)\n", ret);
> -		intel_uncore_runtime_resume(&dev_priv->uncore);
> -
> -		intel_runtime_pm_enable_interrupts(dev_priv);
> -
> -		intel_gt_runtime_resume(&dev_priv->gt);
> -
> -		enable_rpm_wakeref_asserts(rpm);
> -
> -		return ret;
> -	}
> -
> -	enable_rpm_wakeref_asserts(rpm);
> -	intel_runtime_pm_driver_release(rpm);
> -
> -	if (intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore))
> -		drm_err(&dev_priv->drm,
> -			"Unclaimed access detected prior to suspending\n");
> -
> -	rpm->suspended = true;
> -
> -	/*
> -	 * FIXME: We really should find a document that references the arguments
> -	 * used below!
> -	 */
> -	if (IS_BROADWELL(dev_priv)) {
> -		/*
> -		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> -		 * being detected, and the call we do at intel_runtime_resume()
> -		 * won't be able to restore them. Since PCI_D3hot matches the
> -		 * actual specification and appears to be working, use it.
> -		 */
> -		intel_opregion_notify_adapter(dev_priv, PCI_D3hot);
> -	} else {
> -		/*
> -		 * current versions of firmware which depend on this opregion
> -		 * notification have repurposed the D1 definition to mean
> -		 * "runtime suspended" vs. what you would normally expect (D3)
> -		 * to distinguish it from notifications that might be sent via
> -		 * the suspend path.
> -		 */
> -		intel_opregion_notify_adapter(dev_priv, PCI_D1);
> -	}
> -
> -	assert_forcewakes_inactive(&dev_priv->uncore);
> -
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_enable(dev_priv);
> -
> -	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
> -	return 0;
> -}
> -
> -static int intel_runtime_resume(struct device *kdev)
> -{
> -	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> -	struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
> -	int ret;
> -
> -	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
> -		return -ENODEV;
> -
> -	drm_dbg_kms(&dev_priv->drm, "Resuming device\n");
> -
> -	drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count));
> -	disable_rpm_wakeref_asserts(rpm);
> -
> -	intel_opregion_notify_adapter(dev_priv, PCI_D0);
> -	rpm->suspended = false;
> -	if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
> -		drm_dbg(&dev_priv->drm,
> -			"Unclaimed access during suspend, bios?\n");
> -
> -	intel_display_power_resume(dev_priv);
> -
> -	ret = vlv_resume_prepare(dev_priv, true);
> -
> -	intel_uncore_runtime_resume(&dev_priv->uncore);
> -
> -	intel_runtime_pm_enable_interrupts(dev_priv);
> -
> -	/*
> -	 * No point of rolling back things in case of an error, as the best
> -	 * we can do is to hope that things will still work (and disable RPM).
> -	 */
> -	intel_gt_runtime_resume(&dev_priv->gt);
> -
> -	/*
> -	 * On VLV/CHV display interrupts are part of the display
> -	 * power well, so hpd is reinitialized from there. For
> -	 * everyone else do it here.
> -	 */
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
> -		intel_hpd_init(dev_priv);
> -		intel_hpd_poll_disable(dev_priv);
> -	}
> -
> -	intel_enable_ipc(dev_priv);
> -
> -	enable_rpm_wakeref_asserts(rpm);
> -
> -	if (ret)
> -		drm_err(&dev_priv->drm,
> -			"Runtime resume failed, disabling it (%d)\n", ret);
> -	else
> -		drm_dbg_kms(&dev_priv->drm, "Device resumed\n");
> -
> -	return ret;
> -}
> -
>  const struct dev_pm_ops i915_pm_ops = {
>  	/*
>  	 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
> @@ -1693,8 +1552,8 @@ const struct dev_pm_ops i915_pm_ops = {
>  	.restore = i915_pm_restore,
>  
>  	/* S0ix (via runtime suspend) event handlers */
> -	.runtime_suspend = intel_runtime_suspend,
> -	.runtime_resume = intel_runtime_resume,
> +	.runtime_suspend = intel_runtime_pm_suspend,
> +	.runtime_resume = intel_runtime_pm_resume,
>  };
>  
>  static const struct file_operations i915_driver_fops = {
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index eaf7688f517d..f28b5bab61b4 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -32,6 +32,11 @@
>  
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> +#include "gt/intel_gt.h"
> +#include "gt/intel_gt_pm.h"
> +#include "intel_pm.h"
> +#include "vlv_suspend.h"
> +#include "display/intel_hotplug.h"

I suppose these should be in the following order:

display/*.h

gt/*.h

i915_*.h
...

(taken from intel_display.c)

Could add docs for the exported funcs at one point.

>  
>  /**
>   * DOC: runtime pm
> @@ -652,3 +657,143 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm)
>  
>  	init_intel_runtime_pm_wakeref(rpm);
>  }
> +
> +int intel_runtime_pm_suspend(struct device *kdev)
> +{
> +	struct drm_i915_private *i915 = kdev_to_i915(kdev);
> +	struct intel_runtime_pm *rpm = &i915->runtime_pm;
> +	int ret;
> +
> +	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
> +		return -ENODEV;
> +
> +	drm_dbg_kms(&i915->drm, "Suspending device\n");
> +
> +	disable_rpm_wakeref_asserts(rpm);
> +
> +	/*
> +	 * We are safe here against re-faults, since the fault handler takes
> +	 * an RPM reference.
> +	 */
> +	i915_gem_runtime_suspend(i915);
> +
> +	intel_gt_runtime_suspend(&i915->gt);
> +
> +	intel_runtime_pm_disable_interrupts(i915);
> +
> +	intel_uncore_suspend(&i915->uncore);
> +
> +	intel_display_power_suspend(i915);
> +
> +	ret = vlv_suspend_complete(i915);
> +	if (ret) {
> +		drm_err(&i915->drm,
> +			"Runtime suspend failed, disabling it (%d)\n", ret);
> +		intel_uncore_runtime_resume(&i915->uncore);
> +
> +		intel_runtime_pm_enable_interrupts(i915);
> +
> +		intel_gt_runtime_resume(&i915->gt);
> +
> +		enable_rpm_wakeref_asserts(rpm);
> +
> +		return ret;
> +	}
> +
> +	enable_rpm_wakeref_asserts(rpm);
> +	intel_runtime_pm_driver_release(rpm);
> +
> +	if (intel_uncore_arm_unclaimed_mmio_detection(&i915->uncore))
> +		drm_err(&i915->drm,
> +			"Unclaimed access detected prior to suspending\n");
> +
> +	rpm->suspended = true;
> +
> +	/*
> +	 * FIXME: We really should find a document that references the arguments
> +	 * used below!
> +	 */
> +	if (IS_BROADWELL(i915)) {
> +		/*
> +		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> +		 * being detected, and the call we do at intel_runtime_resume()
> +		 * won't be able to restore them. Since PCI_D3hot matches the
> +		 * actual specification and appears to be working, use it.
> +		 */
> +		intel_opregion_notify_adapter(i915, PCI_D3hot);
> +	} else {
> +		/*
> +		 * current versions of firmware which depend on this opregion
> +		 * notification have repurposed the D1 definition to mean
> +		 * "runtime suspended" vs. what you would normally expect (D3)
> +		 * to distinguish it from notifications that might be sent via
> +		 * the suspend path.
> +		 */
> +		intel_opregion_notify_adapter(i915, PCI_D1);
> +	}
> +
> +	assert_forcewakes_inactive(&i915->uncore);
> +
> +	if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915))
> +		intel_hpd_poll_enable(i915);
> +
> +	drm_dbg_kms(&i915->drm, "Device suspended\n");
> +	return 0;
> +}
> +
> +int intel_runtime_pm_resume(struct device *kdev)
> +{
> +	struct drm_i915_private *i915 = kdev_to_i915(kdev);
> +	struct intel_runtime_pm *rpm = &i915->runtime_pm;
> +	int ret;
> +
> +	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
> +		return -ENODEV;
> +
> +	drm_dbg_kms(&i915->drm, "Resuming device\n");
> +
> +	drm_WARN_ON_ONCE(&i915->drm, atomic_read(&rpm->wakeref_count));
> +	disable_rpm_wakeref_asserts(rpm);
> +
> +	intel_opregion_notify_adapter(i915, PCI_D0);
> +	rpm->suspended = false;
> +	if (intel_uncore_unclaimed_mmio(&i915->uncore))
> +		drm_dbg(&i915->drm,
> +			"Unclaimed access during suspend, bios?\n");
> +
> +	intel_display_power_resume(i915);
> +
> +	ret = vlv_resume_prepare(i915, true);
> +
> +	intel_uncore_runtime_resume(&i915->uncore);
> +
> +	intel_runtime_pm_enable_interrupts(i915);
> +
> +	/*
> +	 * No point of rolling back things in case of an error, as the best
> +	 * we can do is to hope that things will still work (and disable RPM).
> +	 */
> +	intel_gt_runtime_resume(&i915->gt);
> +
> +	/*
> +	 * On VLV/CHV display interrupts are part of the display
> +	 * power well, so hpd is reinitialized from there. For
> +	 * everyone else do it here.
> +	 */
> +	if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) {
> +		intel_hpd_init(i915);
> +		intel_hpd_poll_disable(i915);
> +	}
> +
> +	intel_enable_ipc(i915);
> +
> +	enable_rpm_wakeref_asserts(rpm);
> +
> +	if (ret)
> +		drm_err(&i915->drm,
> +			"Runtime resume failed, disabling it (%d)\n", ret);
> +	else
> +		drm_dbg_kms(&i915->drm, "Device resumed\n");
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
> index 47a85fab4130..88ca531165f7 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> @@ -172,6 +172,8 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm);
>  void intel_runtime_pm_enable(struct intel_runtime_pm *rpm);
>  void intel_runtime_pm_disable(struct intel_runtime_pm *rpm);
>  void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm);
> +int intel_runtime_pm_suspend(struct device *kdev);
> +int intel_runtime_pm_resume(struct device *kdev);
>  
>  intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm);
>  intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm *rpm);
> -- 
> 2.31.1
> 

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Disallow D3Cold.

[Imre Deak]

On Wed, Aug 25, 2021 at 11:22:31AM -0400, Rodrigo Vivi wrote:
> During runtime or s2idle suspend and resume cases on discrete cards,
> if D3Cold is really achieved, we will blow everything up and
> freeze the machine because we are not yet handling the pci states
> properly.

Is this some ordering problem wrt. pci_save_state()? In theory the RPM
core should take care of this (see pci_pm_runtime_suspend()) and the PCI
rpm docs don't recommend doing this manually from the driver's callback.

> On Integrated it simply doesn't matter because D3hot is the maximum
> that we will get anyway, unless the system is on S3/S4 and our power
> is cut.

Is the support for D3cold indicated in the PCI PM capabailities? I can't
see this being enabled for IGDs, so probably there's no problem with
disabling it on those.

I also checked a DG1 card, but it's also disabled there, do you have any
having it enabled?

> Let's put this hammer for now everywhere. So we can work to enable
> the auto-suspend by default without blowing up the world.
> 
> Then, this should be removed when we finally fix the D3Cold flow.
> 
> Cc: Tilak Tangudu <tilak.tangudu@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Since it helps with the discrete cards:
Acked-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a40b5d806321..086a9a475ce8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -301,6 +301,7 @@ static void sanitize_gpu(struct drm_i915_private *i915)
>   */
>  static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>  	int ret = 0;
>  
>  	if (i915_inject_probe_failure(dev_priv))
> @@ -331,6 +332,13 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	if (ret < 0)
>  		return ret;
>  
> +	/*
> +	 * FIXME: Temporary hammer to avoid freezing the machine on our DGFX
> +	 * This should be totally removed when we handle the pci states properly
> +	 * on runtime PM and on s2idle cases.
> +	 */
> +	pci_d3cold_disable(pdev);
> +
>  	ret = vlv_suspend_init(dev_priv);
>  	if (ret < 0)
>  		goto err_workqueues;
> -- 
> 2.31.1
>