[PATCH 10/11] drm/i915: Always sanitize GT wakeref before restarting engines

Chris Wilson chris at chris-wilson.co.uk
Sun Nov 5 12:38:42 UTC 2017


The first request submitted to an engine will take the prolonged GT
wakeref. If we discard that wakeref to issue a reset/suspend/etc, then
before restarting the engines, reacquire the GT's wakeref.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c         |   3 +-
 drivers/gpu/drm/i915/i915_gem.c         |  28 +--
 drivers/gpu/drm/i915/i915_gem_request.c |   1 -
 drivers/gpu/drm/i915/intel_display.c    |   4 -
 drivers/gpu/drm/i915/intel_drv.h        |  10 +-
 drivers/gpu/drm/i915/intel_pm.c         | 296 ++++++++++++++++----------------
 6 files changed, 171 insertions(+), 171 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c53989f2a1f2..fe9e539b6c29 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -632,6 +632,8 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_drain_freed_objects(dev_priv);
 
 	WARN_ON(!list_empty(&dev_priv->contexts.list));
+
+	intel_cleanup_gt_powersave(dev_priv);
 }
 
 static int i915_load_modeset_init(struct drm_device *dev)
@@ -1684,7 +1686,6 @@ static int i915_drm_resume(struct drm_device *dev)
 	int ret;
 
 	disable_rpm_wakeref_asserts(dev_priv);
-	intel_sanitize_gt_powersave(dev_priv);
 
 	ret = i915_ggtt_enable_hw(dev_priv);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8941910d3602..74f230652029 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3099,13 +3099,6 @@ void i915_gem_reset(struct drm_i915_private *dev_priv)
 	}
 
 	i915_gem_restore_fences(dev_priv);
-
-	if (dev_priv->gt.awake) {
-		intel_sanitize_gt_powersave(dev_priv);
-		intel_enable_gt_powersave(dev_priv);
-		if (INTEL_GEN(dev_priv) >= 6)
-			gen6_rps_busy(dev_priv);
-	}
 }
 
 void i915_gem_reset_finish_engine(struct intel_engine_cs *engine)
@@ -4727,7 +4720,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	int ret;
 
 	intel_runtime_pm_get(dev_priv);
-	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
 
@@ -4802,11 +4794,11 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 void i915_gem_resume(struct drm_i915_private *i915)
 {
-	WARN_ON(i915->gt.awake);
-
 	mutex_lock(&i915->drm.struct_mutex);
 	intel_uncore_forcewake_get(i915, FORCEWAKE_ALL);
 
+	intel_resume_gt_powersave(i915);
+
 	i915_gem_restore_gtt_mappings(i915);
 	i915_gem_restore_fences(i915);
 
@@ -4889,6 +4881,22 @@ static int __i915_gem_restart_engines(void *data)
 	enum intel_engine_id id;
 	int err;
 
+	/*
+	 * If we are holding a wakeref from a previous cycle, e.g. from
+	 * before a reset, scrub the GT powersaving and reacquire. This is
+	 * just to make sure that the HW matches our SW tracking,
+	 * irrespective of whether or not it was preserved over the
+	 * restart.
+	 */
+	if (i915->gt.awake) {
+		if (INTEL_GEN(i915) >= 6)
+			gen6_rps_busy(i915);
+
+		queue_delayed_work(i915->wq,
+				   &i915->gt.retire_work,
+				   round_jiffies_up_relative(HZ));
+	}
+
 	for_each_engine(engine, i915, id) {
 		err = engine->init_hw(engine);
 		if (err)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index e0d6221022a8..41464c946d3a 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -254,7 +254,6 @@ static void mark_busy(struct drm_i915_private *i915)
 	intel_runtime_pm_get_noresume(i915);
 	i915->gt.awake = true;
 
-	intel_enable_gt_powersave(i915);
 	i915_update_gfx_val(i915);
 	if (INTEL_GEN(i915) >= 6)
 		gen6_rps_busy(i915);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5debb79540a2..de91a17c9a47 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15217,8 +15217,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	flush_work(&dev_priv->atomic_helper.free_work);
 	WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
 
-	intel_disable_gt_powersave(dev_priv);
-
 	/*
 	 * Interrupts and polling as the first thing to avoid creating havoc.
 	 * Too much stuff here (turning of connectors, ...) would
@@ -15246,8 +15244,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_cleanup_overlay(dev_priv);
 
-	intel_cleanup_gt_powersave(dev_priv);
-
 	intel_teardown_gmbus(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 76b12fb1a35f..bf6ad90624e2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1877,12 +1877,10 @@ void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv);
 void intel_pm_setup(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
-void intel_init_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_enable_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_disable_gt_powersave(struct drm_i915_private *dev_priv);
-void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv);
+void intel_sanitize_gt_powersave(struct drm_i915_private *i915);
+void intel_init_gt_powersave(struct drm_i915_private *i915);
+void intel_resume_gt_powersave(struct drm_i915_private *i915);
+void intel_cleanup_gt_powersave(struct drm_i915_private *i915);
 void gen6_rps_busy(struct drm_i915_private *dev_priv);
 void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8f37b4dc9943..3d9b0b7c4a5e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7859,9 +7859,87 @@ static void intel_init_emon(struct drm_i915_private *dev_priv)
 	dev_priv->ips.corr = (lcfuse & LCFUSE_HIV_MASK);
 }
 
-void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
+static inline void intel_enable_llc_pstate(struct drm_i915_private *i915)
 {
-	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	lockdep_assert_held(&i915->pcu_lock);
+
+	if (i915->gt_pm.llc_pstate.enabled)
+		return;
+
+	gen6_update_ring_freq(i915);
+
+	i915->gt_pm.llc_pstate.enabled = true;
+}
+
+static void intel_enable_rc6(struct drm_i915_private *i915)
+{
+	lockdep_assert_held(&i915->pcu_lock);
+
+	if (i915->gt_pm.rc6.enabled)
+		return;
+
+	if (IS_CHERRYVIEW(i915))
+		cherryview_enable_rc6(i915);
+	else if (IS_VALLEYVIEW(i915))
+		valleyview_enable_rc6(i915);
+	else if (INTEL_GEN(i915) >= 9)
+		gen9_enable_rc6(i915);
+	else if (IS_BROADWELL(i915))
+		gen8_enable_rc6(i915);
+	else if (INTEL_GEN(i915) >= 6)
+		gen6_enable_rc6(i915);
+
+	i915->gt_pm.rc6.enabled = true;
+}
+
+static void intel_enable_rps(struct drm_i915_private *i915)
+{
+	struct intel_rps *rps = &i915->gt_pm.rps;
+
+	lockdep_assert_held(&i915->pcu_lock);
+
+	if (rps->enabled)
+		return;
+
+	if (IS_CHERRYVIEW(i915)) {
+		cherryview_enable_rps(i915);
+	} else if (IS_VALLEYVIEW(i915)) {
+		valleyview_enable_rps(i915);
+	} else if (INTEL_GEN(i915) >= 9) {
+		gen9_enable_rps(i915);
+	} else if (IS_BROADWELL(i915)) {
+		gen8_enable_rps(i915);
+	} else if (INTEL_GEN(i915) >= 6) {
+		gen6_enable_rps(i915);
+	} else if (IS_IRONLAKE_M(i915)) {
+		ironlake_enable_drps(i915);
+		intel_init_emon(i915);
+	}
+
+	WARN_ON(rps->max_freq < rps->min_freq);
+	WARN_ON(rps->idle_freq > rps->max_freq);
+
+	WARN_ON(rps->efficient_freq < rps->min_freq);
+	WARN_ON(rps->efficient_freq > rps->max_freq);
+
+	rps->enabled = true;
+}
+
+static void __intel_enable_gt_powersave(struct drm_i915_private *i915)
+{
+	/* Powersaving is controlled by the host when inside a VM */
+	if (intel_vgpu_active(i915))
+		return;
+
+	intel_enable_rc6(i915);
+	intel_enable_rps(i915);
+	if (HAS_LLC(i915))
+		intel_enable_llc_pstate(i915);
+}
+
+void intel_init_gt_powersave(struct drm_i915_private *i915)
+{
+	struct intel_rps *rps = &i915->gt_pm.rps;
 
 	/*
 	 * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
@@ -7869,18 +7947,18 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	 */
 	if (!i915_modparams.enable_rc6) {
 		DRM_INFO("RC6 disabled, disabling runtime PM support\n");
-		intel_runtime_pm_get(dev_priv);
+		intel_runtime_pm_get(i915);
 	}
 
-	mutex_lock(&dev_priv->pcu_lock);
+	mutex_lock(&i915->pcu_lock);
 
 	/* Initialize RPS limits (for userspace) */
-	if (IS_CHERRYVIEW(dev_priv))
-		cherryview_init_gt_powersave(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv))
-		valleyview_init_gt_powersave(dev_priv);
-	else if (INTEL_GEN(dev_priv) >= 6)
-		gen6_init_rps_frequencies(dev_priv);
+	if (IS_CHERRYVIEW(i915))
+		cherryview_init_gt_powersave(i915);
+	else if (IS_VALLEYVIEW(i915))
+		valleyview_init_gt_powersave(i915);
+	else if (INTEL_GEN(i915) >= 6)
+		gen6_init_rps_frequencies(i915);
 
 	/* Derive initial user preferences/limits from the hardware limits */
 	rps->idle_freq = rps->min_freq;
@@ -7889,18 +7967,17 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	rps->max_freq_softlimit = rps->max_freq;
 	rps->min_freq_softlimit = rps->min_freq;
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	if (IS_HASWELL(i915) || IS_BROADWELL(i915))
 		rps->min_freq_softlimit =
 			max_t(int,
 			      rps->efficient_freq,
-			      intel_freq_opcode(dev_priv, 450));
+			      intel_freq_opcode(i915, 450));
 
 	/* After setting max-softlimit, find the overclock max freq */
-	if (IS_GEN6(dev_priv) ||
-	    IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv)) {
+	if (IS_GEN6(i915) || IS_IVYBRIDGE(i915) || IS_HASWELL(i915)) {
 		u32 params = 0;
 
-		sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &params);
+		sandybridge_pcode_read(i915, GEN6_READ_OC_PARAMS, &params);
 		if (params & BIT(31)) { /* OC supported */
 			DRM_DEBUG_DRIVER("Overclocking supported, max: %dMHz, overclock: %dMHz\n",
 					 (rps->max_freq & 0xff) * 50,
@@ -7912,44 +7989,20 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	/* Finally allow us to boost to max by default */
 	rps->boost_freq = rps->max_freq;
 
-	mutex_unlock(&dev_priv->pcu_lock);
-}
-
-void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	struct drm_i915_gem_object *pctx;
+	__intel_enable_gt_powersave(i915);
 
-	pctx = fetch_and_zero(&dev_priv->gt_pm.rc6.pctx);
-	if (pctx)
-		i915_gem_object_put(pctx);
-
-	if (!i915_modparams.enable_rc6)
-		intel_runtime_pm_put(dev_priv);
+	mutex_unlock(&i915->pcu_lock);
 }
 
-/**
- * intel_suspend_gt_powersave - suspend PM work and helper threads
- * @dev_priv: i915 device
- *
- * We don't want to disable RC6 or other features here, we just want
- * to make sure any work we've queued has finished and won't bother
- * us while we're suspended.
- */
-void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
+void intel_resume_gt_powersave(struct drm_i915_private *i915)
 {
-	if (INTEL_GEN(dev_priv) < 6)
-		return;
-
-	/* gen6_rps_idle() will be called later to disable interrupts */
-}
+	mutex_lock(&i915->pcu_lock);
 
-void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	dev_priv->gt_pm.rps.enabled = true; /* force RPS disabling */
-	dev_priv->gt_pm.rc6.enabled = true; /* force RC6 disabling */
-	intel_disable_gt_powersave(dev_priv);
+	i915->gt_pm.rps.enabled = false;
+	i915->gt_pm.rc6.enabled = false;
+	__intel_enable_gt_powersave(i915);
 
-	gen6_reset_rps_interrupts(dev_priv);
+	mutex_unlock(&i915->pcu_lock);
 }
 
 static inline void intel_disable_llc_pstate(struct drm_i915_private *i915)
@@ -7964,138 +8017,83 @@ static inline void intel_disable_llc_pstate(struct drm_i915_private *i915)
 	i915->gt_pm.llc_pstate.enabled = false;
 }
 
-static void intel_disable_rc6(struct drm_i915_private *dev_priv)
-{
-	lockdep_assert_held(&dev_priv->pcu_lock);
-
-	if (!dev_priv->gt_pm.rc6.enabled)
-		return;
-
-	if (INTEL_GEN(dev_priv) >= 9)
-		gen9_disable_rc6(dev_priv);
-	else if (IS_CHERRYVIEW(dev_priv))
-		cherryview_disable_rc6(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv))
-		valleyview_disable_rc6(dev_priv);
-	else if (INTEL_GEN(dev_priv) >= 6)
-		gen6_disable_rc6(dev_priv);
-
-	dev_priv->gt_pm.rc6.enabled = false;
-}
-
-static void intel_disable_rps(struct drm_i915_private *dev_priv)
+static void intel_disable_rc6(struct drm_i915_private *i915)
 {
-	lockdep_assert_held(&dev_priv->pcu_lock);
+	lockdep_assert_held(&i915->pcu_lock);
 
-	if (!dev_priv->gt_pm.rps.enabled)
+	if (!i915->gt_pm.rc6.enabled)
 		return;
 
-	if (INTEL_GEN(dev_priv) >= 9)
-		gen9_disable_rps(dev_priv);
-	else if (IS_CHERRYVIEW(dev_priv))
-		cherryview_disable_rps(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv))
-		valleyview_disable_rps(dev_priv);
-	else if (INTEL_GEN(dev_priv) >= 6)
-		gen6_disable_rps(dev_priv);
-	else if (IS_IRONLAKE_M(dev_priv))
-		ironlake_disable_drps(dev_priv);
+	if (INTEL_GEN(i915) >= 9)
+		gen9_disable_rc6(i915);
+	else if (IS_CHERRYVIEW(i915))
+		cherryview_disable_rc6(i915);
+	else if (IS_VALLEYVIEW(i915))
+		valleyview_disable_rc6(i915);
+	else if (INTEL_GEN(i915) >= 6)
+		gen6_disable_rc6(i915);
 
-	dev_priv->gt_pm.rps.enabled = false;
+	i915->gt_pm.rc6.enabled = false;
 }
 
-void intel_disable_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	mutex_lock(&dev_priv->pcu_lock);
-
-	intel_disable_rc6(dev_priv);
-	intel_disable_rps(dev_priv);
-	if (HAS_LLC(dev_priv))
-		intel_disable_llc_pstate(dev_priv);
-
-	mutex_unlock(&dev_priv->pcu_lock);
-}
-
-static inline void intel_enable_llc_pstate(struct drm_i915_private *i915)
+static void intel_disable_rps(struct drm_i915_private *i915)
 {
 	lockdep_assert_held(&i915->pcu_lock);
 
-	if (i915->gt_pm.llc_pstate.enabled)
+	if (!i915->gt_pm.rps.enabled)
 		return;
 
-	gen6_update_ring_freq(i915);
+	if (INTEL_GEN(i915) >= 9)
+		gen9_disable_rps(i915);
+	else if (IS_CHERRYVIEW(i915))
+		cherryview_disable_rps(i915);
+	else if (IS_VALLEYVIEW(i915))
+		valleyview_disable_rps(i915);
+	else if (INTEL_GEN(i915) >= 6)
+		gen6_disable_rps(i915);
+	else if (IS_IRONLAKE_M(i915))
+		ironlake_disable_drps(i915);
 
-	i915->gt_pm.llc_pstate.enabled = true;
+	i915->gt_pm.rps.enabled = false;
 }
 
-static void intel_enable_rc6(struct drm_i915_private *dev_priv)
+static void __intel_disable_gt_powersave(struct drm_i915_private *i915)
 {
-	lockdep_assert_held(&dev_priv->pcu_lock);
-
-	if (dev_priv->gt_pm.rc6.enabled)
-		return;
-
-	if (IS_CHERRYVIEW(dev_priv))
-		cherryview_enable_rc6(dev_priv);
-	else if (IS_VALLEYVIEW(dev_priv))
-		valleyview_enable_rc6(dev_priv);
-	else if (INTEL_GEN(dev_priv) >= 9)
-		gen9_enable_rc6(dev_priv);
-	else if (IS_BROADWELL(dev_priv))
-		gen8_enable_rc6(dev_priv);
-	else if (INTEL_GEN(dev_priv) >= 6)
-		gen6_enable_rc6(dev_priv);
-
-	dev_priv->gt_pm.rc6.enabled = true;
+	intel_disable_rc6(i915);
+	intel_disable_rps(i915);
+	if (HAS_LLC(i915))
+		intel_disable_llc_pstate(i915);
 }
 
-static void intel_enable_rps(struct drm_i915_private *dev_priv)
+void intel_cleanup_gt_powersave(struct drm_i915_private *i915)
 {
-	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	struct drm_i915_gem_object *pctx;
 
-	lockdep_assert_held(&dev_priv->pcu_lock);
+	mutex_lock(&i915->pcu_lock);
 
-	if (rps->enabled)
-		return;
+	__intel_disable_gt_powersave(i915);
 
-	if (IS_CHERRYVIEW(dev_priv)) {
-		cherryview_enable_rps(dev_priv);
-	} else if (IS_VALLEYVIEW(dev_priv)) {
-		valleyview_enable_rps(dev_priv);
-	} else if (INTEL_GEN(dev_priv) >= 9) {
-		gen9_enable_rps(dev_priv);
-	} else if (IS_BROADWELL(dev_priv)) {
-		gen8_enable_rps(dev_priv);
-	} else if (INTEL_GEN(dev_priv) >= 6) {
-		gen6_enable_rps(dev_priv);
-	} else if (IS_IRONLAKE_M(dev_priv)) {
-		ironlake_enable_drps(dev_priv);
-		intel_init_emon(dev_priv);
-	}
+	pctx = fetch_and_zero(&i915->gt_pm.rc6.pctx);
+	if (pctx)
+		i915_gem_object_put(pctx);
 
-	WARN_ON(rps->max_freq < rps->min_freq);
-	WARN_ON(rps->idle_freq > rps->max_freq);
+	mutex_unlock(&i915->pcu_lock);
 
-	WARN_ON(rps->efficient_freq < rps->min_freq);
-	WARN_ON(rps->efficient_freq > rps->max_freq);
-
-	rps->enabled = true;
+	if (!i915_modparams.enable_rc6)
+		intel_runtime_pm_put(i915);
 }
 
-void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
+void intel_sanitize_gt_powersave(struct drm_i915_private *i915)
 {
-	/* Powersaving is controlled by the host when inside a VM */
-	if (intel_vgpu_active(dev_priv))
-		return;
+	mutex_lock(&i915->pcu_lock);
 
-	mutex_lock(&dev_priv->pcu_lock);
+	i915->gt_pm.rps.enabled = true; /* force RPS disabling */
+	i915->gt_pm.rc6.enabled = true; /* force RC6 disabling */
+	__intel_disable_gt_powersave(i915);
 
-	intel_enable_rc6(dev_priv);
-	intel_enable_rps(dev_priv);
-	if (HAS_LLC(dev_priv))
-		intel_enable_llc_pstate(dev_priv);
+	mutex_unlock(&i915->pcu_lock);
 
-	mutex_unlock(&dev_priv->pcu_lock);
+	gen6_reset_rps_interrupts(i915);
 }
 
 static void ibx_init_clock_gating(struct drm_i915_private *dev_priv)
-- 
2.15.0



More information about the Intel-gfx-trybot mailing list