[PATCH 1/1] drm/i915: Split i915_gem_suspend into gem quiescing and HW suspend

[Sagar Arun Kamble]

GTT mappings are to be suspended post idling GPU and display suspend.
Currently GPU is reset in i915_gem_suspend and then GTT mappings are
suspended in i915_drm_suspend. All GTT flushes should be done prior to
GPU reset. Also part of i915_gem_suspend was about suspending/sanitizing
HW operations. This separated the GEM/HW suspend across reset.

To achieve the complete GEM/HW suspend prior to GPU reset we need to
separate i915_gem_suspend functionality into gem quiescing and HW
suspend. With this patch new function i915_gem_quiesce is created that
will ensure GEM is idle. i915_gem_hw_suspend will ensure all GPU HW
operations are suspended. These currently are RPS, GTT mappings
suspension. In later patches GuC GGTT invalidate can be sanitized here.

This change also ensures that GTT mappings suspension is locked by
struct_mutex.

v2. Split i915_gem_suspend to accommodate GTT mappings suspend as that is
not to be done till display suspend. (Chris)

v3: s/i915_gem_quiescent/i915_gem_quiesce and s/i915_gem_suspend/
i915_gem_hw_suspend. gem_hw_suspend has to be done prior to gem_fini as
engine state/GuC software state would be needed to suspend HW.
s/dev_priv/i915 in i915_gem_quiesce and i915_gem_hw_suspend.
Doing GuC suspend before suspending GTT mappings as GuC needs to access
the shared data. Limiting struct_mutex lock to only gem_sanitize.

v4: Reverted s/dev_priv/i915. Rebase as the patch order is changed in
series. Updated commit message.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Michał Winiarski <michal.winiarski at intel.com>
Cc: Oscar Mateo <oscar.mateo at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 11 +++++++----
 drivers/gpu/drm/i915/i915_drv.h |  3 ++-
 drivers/gpu/drm/i915/i915_gem.c | 28 +++++++++++++++++++++-------
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3db5851..e21d8c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -694,8 +694,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	return 0;
 
 cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_quiesce(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
+	i915_gem_hw_suspend(dev_priv);
 	i915_gem_fini(dev_priv);
 cleanup_uc:
 	intel_uc_fini_fw(dev_priv);
@@ -1396,9 +1397,11 @@ void i915_driver_unload(struct drm_device *dev)
 
 	i915_driver_unregister(dev_priv);
 
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_quiesce(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 
+	i915_gem_hw_suspend(dev_priv);
+
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
 	drm_atomic_helper_shutdown(dev);
@@ -1541,7 +1544,7 @@ static int i915_drm_suspend(struct drm_device *dev)
 
 	pci_save_state(pdev);
 
-	error = i915_gem_suspend(dev_priv);
+	error = i915_gem_quiesce(dev_priv);
 	if (error) {
 		dev_err(&pdev->dev,
 			"GEM idle failed, resume might fail\n");
@@ -1559,7 +1562,7 @@ static int i915_drm_suspend(struct drm_device *dev)
 
 	intel_suspend_hw(dev_priv);
 
-	i915_gem_suspend_gtt_mappings(dev_priv);
+	i915_gem_hw_suspend(dev_priv);
 
 	i915_save_state(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c7b2ca6..f540ca3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3729,7 +3729,8 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine,
 void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
-int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
+int __must_check i915_gem_quiesce(struct drm_i915_private *dev_priv);
+void i915_gem_hw_suspend(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
 int i915_gem_fault(struct vm_fault *vmf);
 int i915_gem_object_wait(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 20fcac3..a901875 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4636,13 +4636,12 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	}
 }
 
-int i915_gem_suspend(struct drm_i915_private *dev_priv)
+int i915_gem_quiesce(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
 	int ret;
 
 	intel_runtime_pm_get(dev_priv);
-	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
 
@@ -4685,6 +4684,26 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	if (WARN_ON(!intel_engines_are_idle(dev_priv)))
 		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
 
+	intel_runtime_pm_put(dev_priv);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	intel_runtime_pm_put(dev_priv);
+	return ret;
+}
+
+void i915_gem_hw_suspend(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = &dev_priv->drm;
+
+	intel_runtime_pm_get(dev_priv);
+	intel_suspend_gt_powersave(dev_priv);
+
+	mutex_lock(&dev->struct_mutex);
+
+	i915_gem_suspend_gtt_mappings(dev_priv);
+
 	/*
 	 * Neither the BIOS, ourselves or any other kernel
 	 * expects the system to be in execlists mode on startup,
@@ -4706,13 +4725,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 */
 	i915_gem_sanitize(dev_priv);
 
-	intel_runtime_pm_put(dev_priv);
-	return 0;
-
-err_unlock:
 	mutex_unlock(&dev->struct_mutex);
 	intel_runtime_pm_put(dev_priv);
-	return ret;
 }
 
 void i915_gem_resume(struct drm_i915_private *dev_priv)
-- 
1.9.1