[PATCH 86/94] drm/i915: Enabling rc6 and rps have different requirements, so separate them

Chris Wilson chris at chris-wilson.co.uk
Fri Sep 28 10:38:24 UTC 2018


On Ironlake, we are required to not enable rc6 until the GPU is loaded
with a valid context; after that point it can start to use a powersaving
context for rc6. This seems a reasonable requirement to impose on all
generations as we are already priming the system by loading a context on
resume. We can simply then delay enabling rc6 until we know the GPU is
awake.

v2: Reorder intel_gt_pm_fini in i915_gem_fini to match setup ordering,
and remove the superfluous intel_gt_pm_sanitize() on mmio cleanup.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c    |  1 -
 drivers/gpu/drm/i915/i915_gem.c    | 69 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_gt_pm.c |  2 +
 3 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index edb32a28a7cf..f1dc7298030b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1052,7 +1052,6 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_cleanup_mmio(struct drm_i915_private *dev_priv)
 {
-	intel_gt_pm_sanitize(dev_priv);
 	intel_uncore_fini(dev_priv);
 	i915_mmio_cleanup(dev_priv);
 	pci_dev_put(dev_priv->bridge_dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89a98d24c8a7..4dec06dc9237 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -210,10 +210,6 @@ void i915_gem_unpark(struct drm_i915_private *i915)
 	if (unlikely(++i915->gt.epoch == 0)) /* keep 0 as invalid */
 		i915->gt.epoch = 1;
 
-	intel_gt_pm_enable_rps(i915);
-	intel_gt_pm_enable_rc6(i915);
-	intel_gt_pm_enable_llc(i915);
-
 	i915_update_gfx_val(i915);
 	if (INTEL_GEN(i915) >= 6)
 		gen6_rps_busy(i915);
@@ -4798,6 +4794,46 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
 	i915_gem_sanitize(i915);
 }
 
+static int load_power_context(struct drm_i915_private *i915)
+{
+	int err;
+
+	lockdep_assert_held(&i915->drm.struct_mutex);
+
+	intel_gt_pm_sanitize(i915);
+	intel_gt_pm_enable_rps(i915);
+
+	err = i915_gem_switch_to_kernel_context(i915);
+	if (err)
+		goto err;
+
+	if (i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED, HZ / 5)) {
+		i915_gem_set_wedged(i915);
+		err = -EIO; /* Caller will declare us wedged */
+		goto err;
+	}
+
+	/*
+	 * Immediately park the GPU as we enable powersaving and treat it as
+	 * idle. The next time we issue a request, we will unpark and start
+	 * using the engine->pinned_default_state, otherwise
+	 * it is in limbo and an early reset may fail.
+	 */
+	assert_kernel_context_is_current(i915);
+	GEM_BUG_ON(i915->gt.active_requests);
+	GEM_BUG_ON(!i915->gt.awake);
+	__i915_gem_park(i915);
+
+	intel_gt_pm_enable_rc6(i915);
+	intel_gt_pm_enable_llc(i915);
+
+	return 0;
+
+err:
+	intel_gt_pm_sanitize(i915);
+	return err;
+}
+
 void i915_gem_resume(struct drm_i915_private *i915)
 {
 	GEM_TRACE("\n");
@@ -4823,7 +4859,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
 	intel_uc_resume(i915);
 
 	/* Always reload a context for powersaving. */
-	if (i915_gem_switch_to_kernel_context(i915))
+	if (load_power_context(i915))
 		goto err_wedged;
 
 out_unlock:
@@ -4833,7 +4869,8 @@ void i915_gem_resume(struct drm_i915_private *i915)
 
 err_wedged:
 	if (!i915_terminally_wedged(&i915->gpu_error)) {
-		DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n");
+		dev_err(i915->drm.dev,
+			"Failed to re-initialize GPU, declaring wedged!\n");
 		i915_gem_set_wedged(i915);
 	}
 	goto out_unlock;
@@ -5016,26 +5053,11 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
 			goto err_active;
 	}
 
-	err = i915_gem_switch_to_kernel_context(i915);
+	/* Flush the default context image to memory, and enable powersaving. */
+	err = load_power_context(i915);
 	if (err)
 		goto err_active;
 
-	if (i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED, HZ / 5)) {
-		i915_gem_set_wedged(i915);
-		err = -EIO; /* Caller will declare us wedged */
-		goto err_active;
-	}
-
-	assert_kernel_context_is_current(i915);
-
-	/*
-	 * Immediately park the GPU so that we enable powersaving and
-	 * treat it as idle. The next time we issue a request, we will
-	 * unpark and start using the engine->pinned_default_state, otherwise
-	 * it is in limbo and an early reset may fail.
-	 */
-	__i915_gem_park(i915);
-
 	for_each_engine(engine, i915, id) {
 		struct i915_vma *state;
 		void *vaddr;
@@ -5343,6 +5365,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_cleanup_engines(dev_priv);
 	i915_gem_contexts_fini(dev_priv);
 	i915_gem_fini_scratch(dev_priv);
+	intel_gt_pm_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
 	intel_gt_pm_fini(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_gt_pm.c b/drivers/gpu/drm/i915/intel_gt_pm.c
index 1140f77c8f02..9b439eff61dc 100644
--- a/drivers/gpu/drm/i915/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/intel_gt_pm.c
@@ -2797,6 +2797,8 @@ void intel_gt_pm_disable_llc(struct drm_i915_private *i915)
 
 void intel_gt_pm_fini(struct drm_i915_private *i915)
 {
+	intel_gt_pm_sanitize(i915);
+
 	if (IS_VALLEYVIEW(i915))
 		valleyview_cleanup_gt_powersave(i915);
 
-- 
2.19.0



More information about the Intel-gfx-trybot mailing list