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

* [Intel-gfx] [PATCH 1/2] drm/i915/runtime_pm: Consolidate runtime_pm functions
@ 2021-08-24 15:44 Rodrigo Vivi
  2021-08-24 15:44 ` [Intel-gfx] [PATCH 2/2] drm/i915/runtime_pm: Let's avoid the undocumented D1 opregion notification Rodrigo Vivi
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2021-08-24 15:44 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi, Imre Deak, Tilak Tangudu

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

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         | 141 ++----------------------
 drivers/gpu/drm/i915/intel_runtime_pm.c | 139 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.h |   2 +
 3 files changed, 151 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 59fb4c710c8c..14770a404bbd 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,144 +1516,24 @@ static int i915_pm_restore(struct device *kdev)
 	return i915_pm_resume(kdev);
 }
 
-static int intel_runtime_suspend(struct device *kdev)
+static int i915_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;
+	struct drm_i915_private *i915 = kdev_to_i915(kdev);
 
-	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
+	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
 		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;
+	return intel_runtime_pm_suspend(&i915->runtime_pm);
 }
 
-static int intel_runtime_resume(struct device *kdev)
+static int i915_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;
+	struct drm_i915_private *i915 = kdev_to_i915(kdev);
 
-	if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv)))
+	if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915)))
 		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;
+	return intel_runtime_pm_resume(&i915->runtime_pm);
 }
 
 const struct dev_pm_ops i915_pm_ops = {
@@ -1693,8 +1572,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 = i915_runtime_suspend,
+	.runtime_resume = i915_runtime_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..43cdc2f3ff9e 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,137 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm *rpm)
 
 	init_intel_runtime_pm_wakeref(rpm);
 }
+
+int intel_runtime_pm_suspend(struct intel_runtime_pm *rpm)
+{
+	struct drm_i915_private *i915 =
+		container_of(rpm, struct drm_i915_private, runtime_pm);
+	int ret;
+
+	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 intel_runtime_pm *rpm)
+{
+	struct drm_i915_private *i915 =
+		container_of(rpm, struct drm_i915_private, runtime_pm);
+	int ret;
+
+	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..bde26f497914 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 intel_runtime_pm *rpm);
+int intel_runtime_pm_resume(struct intel_runtime_pm *rpm);
 
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread