[PATCH] drm/i915: We don't need display's suspend/resume operations when !HAS_DISPLAY

* [PATCH] drm/i915: We don't need display's suspend/resume operations when !HAS_DISPLAY
@ 2019-05-23 23:17 Rodrigo Vivi
  2019-05-24  0:06 ` ✓ Fi.CI.BAT: success for drm/i915: We don't need display's suspend/resume operations when !HAS_DISPLAY (rev3) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Rodrigo Vivi @ 2019-05-23 23:17 UTC (permalink / raw)
  To: intel-gfx

Suspend resume is broken if we try to enable/disable dc9 on
cases with disabled displays.

v2: Make checkpatch happy:
    - braces {} are not necessary for single statement blocks

v3: Also move hsw/bdw PC8 sequences since they are related to
    display PM anyways. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com> (v1)
---
 drivers/gpu/drm/i915/i915_drv.c | 117 +++++++++++++++++++++-----------
 1 file changed, 76 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 83d2eb9e74cb..bd73ce57741a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2118,6 +2118,17 @@ get_suspend_mode(struct drm_i915_private *dev_priv, bool hibernate)
 	return I915_DRM_SUSPEND_MEM;
 }
 
+static void intel_display_suspend_late(struct drm_i915_private *dev_priv)
+{
+	if (!HAS_DISPLAY(dev_priv))
+		return;
+
+	if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv))
+		bxt_enable_dc9(dev_priv);
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		hsw_enable_pc8(dev_priv);
+}
+
 static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2133,12 +2144,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 	intel_power_domains_suspend(dev_priv,
 				    get_suspend_mode(dev_priv, hibernation));
 
+	intel_display_suspend_late(dev_priv);
+
 	ret = 0;
-	if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv))
-		bxt_enable_dc9(dev_priv);
-	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		hsw_enable_pc8(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		ret = vlv_suspend_complete(dev_priv);
 
 	if (ret) {
@@ -2266,6 +2275,19 @@ static int i915_drm_resume(struct drm_device *dev)
 	return 0;
 }
 
+static void intel_display_resume_early(struct drm_i915_private *dev_priv)
+{
+	if (!HAS_DISPLAY(dev_priv))
+		return;
+
+	if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
+		gen9_sanitize_dc_state(dev_priv);
+		bxt_disable_dc9(dev_priv);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		hsw_disable_pc8(dev_priv);
+	}
+}
+
 static int i915_drm_resume_early(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2328,12 +2350,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 
 	i915_check_and_clear_faults(dev_priv);
 
-	if (INTEL_GEN(dev_priv) >= 11 || IS_GEN9_LP(dev_priv)) {
-		gen9_sanitize_dc_state(dev_priv);
-		bxt_disable_dc9(dev_priv);
-	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		hsw_disable_pc8(dev_priv);
-	}
+	intel_display_resume_early(dev_priv);
 
 	intel_uncore_sanitize(dev_priv);
 
@@ -2869,6 +2886,22 @@ static int vlv_resume_prepare(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
+static void intel_runtime_display_suspend(struct drm_i915_private *dev_priv)
+{
+	if (!HAS_DISPLAY(dev_priv))
+		return;
+
+	if (INTEL_GEN(dev_priv) >= 11) {
+		icl_display_core_uninit(dev_priv);
+		bxt_enable_dc9(dev_priv);
+	} else if (IS_GEN9_LP(dev_priv)) {
+		bxt_display_core_uninit(dev_priv);
+		bxt_enable_dc9(dev_priv);
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		hsw_enable_pc8(dev_priv);
+	}
+}
+
 static int intel_runtime_suspend(struct device *kdev)
 {
 	struct pci_dev *pdev = to_pci_dev(kdev);
@@ -2898,18 +2931,11 @@ static int intel_runtime_suspend(struct device *kdev)
 
 	intel_uncore_suspend(&dev_priv->uncore);
 
+	intel_runtime_display_suspend(dev_priv);
+
 	ret = 0;
-	if (INTEL_GEN(dev_priv) >= 11) {
-		icl_display_core_uninit(dev_priv);
-		bxt_enable_dc9(dev_priv);
-	} else if (IS_GEN9_LP(dev_priv)) {
-		bxt_display_core_uninit(dev_priv);
-		bxt_enable_dc9(dev_priv);
-	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		hsw_enable_pc8(dev_priv);
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		ret = vlv_suspend_complete(dev_priv);
-	}
 
 	if (ret) {
 		DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
@@ -2967,25 +2993,10 @@ static int intel_runtime_suspend(struct device *kdev)
 	return 0;
 }
 
-static int intel_runtime_resume(struct device *kdev)
+static void intel_runtime_display_resume(struct drm_i915_private *dev_priv)
 {
-	struct pci_dev *pdev = to_pci_dev(kdev);
-	struct drm_device *dev = pci_get_drvdata(pdev);
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret = 0;
-
-	if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
-		return -ENODEV;
-
-	DRM_DEBUG_KMS("Resuming device\n");
-
-	WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count));
-	disable_rpm_wakeref_asserts(dev_priv);
-
-	intel_opregion_notify_adapter(dev_priv, PCI_D0);
-	dev_priv->runtime_pm.suspended = false;
-	if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
-		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
+	if (!HAS_DISPLAY(dev_priv))
+		return;
 
 	if (INTEL_GEN(dev_priv) >= 11) {
 		bxt_disable_dc9(dev_priv);
@@ -3006,9 +3017,33 @@ static int intel_runtime_resume(struct device *kdev)
 			gen9_enable_dc5(dev_priv);
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		hsw_disable_pc8(dev_priv);
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		ret = vlv_resume_prepare(dev_priv, true);
 	}
+}
+
+static int intel_runtime_resume(struct device *kdev)
+{
+	struct pci_dev *pdev = to_pci_dev(kdev);
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	int ret = 0;
+
+	if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
+		return -ENODEV;
+
+	DRM_DEBUG_KMS("Resuming device\n");
+
+	WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count));
+	disable_rpm_wakeref_asserts(dev_priv);
+
+	intel_opregion_notify_adapter(dev_priv, PCI_D0);
+	dev_priv->runtime_pm.suspended = false;
+	if (intel_uncore_unclaimed_mmio(&dev_priv->uncore))
+		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
+
+	intel_runtime_display_resume(dev_priv);
+
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		ret = vlv_resume_prepare(dev_priv, true);
 
 	intel_uncore_runtime_resume(&dev_priv->uncore);
 
-- 
2.20.1

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

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