[PATCH 28/33] simplify-rc6-init

Chris Wilson chris at chris-wilson.co.uk
Wed Dec 20 23:18:36 UTC 2017


---
 drivers/gpu/drm/i915/i915_debugfs.c |   5 +-
 drivers/gpu/drm/i915/i915_drv.c     |   3 -
 drivers/gpu/drm/i915/i915_drv.h     |  12 +-
 drivers/gpu/drm/i915/i915_gem.c     |  12 +-
 drivers/gpu/drm/i915/intel_gt_pm.c  | 232 ++++++++++++------------------------
 5 files changed, 86 insertions(+), 178 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d337c47fe7da..cc07b87269b4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2154,7 +2154,6 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
 	struct drm_file *file;
 
-	seq_printf(m, "RPS enabled? %d\n", rps->enabled);
 	seq_printf(m, "GPU busy? %s [%d requests]\n",
 		   yesno(dev_priv->gt.awake), dev_priv->gt.active_requests);
 	seq_printf(m, "CPU waiting? %d\n", count_irq_waiters(dev_priv));
@@ -2189,9 +2188,7 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 		   atomic_read(&rps->boosts));
 	mutex_unlock(&dev->filelist_mutex);
 
-	if (INTEL_GEN(dev_priv) >= 6 &&
-	    rps->enabled &&
-	    dev_priv->gt.active_requests) {
+	if (INTEL_GEN(dev_priv) >= 6 && dev_priv->gt.awake) {
 		u32 rpup, rpupei;
 		u32 rpdown, rpdownei;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d72248afe28f..d6952c892cf0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2558,9 +2558,6 @@ static int intel_runtime_suspend(struct device *kdev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int ret;
 
-	if (WARN_ON_ONCE(!(dev_priv->gt_pm.rc6.enabled && HAS_RC6(dev_priv))))
-		return -ENODEV;
-
 	if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
 		return -ENODEV;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb6679316187..b3413054e332 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1378,7 +1378,6 @@ struct intel_rps {
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
-	bool enabled;
 	atomic_t num_waiters;
 	atomic_t boosts;
 
@@ -1386,13 +1385,8 @@ struct intel_rps {
 	struct intel_rps_ei ei;
 };
 
-struct intel_rc6 {
-	bool enabled;
-};
-
-struct intel_gen6_power_mgmt {
+struct intel_gt_pm {
 	struct intel_rps rps;
-	struct intel_rc6 rc6;
 };
 
 /* defined intel_pm.c */
@@ -2478,9 +2472,7 @@ struct drm_i915_private {
 	 * talking to hw - so only take it when talking to hw!
 	 */
 	struct mutex pcu_lock;
-
-	/* gen6+ GT PM state */
-	struct intel_gen6_power_mgmt gt_pm;
+	struct intel_gt_pm gt_pm;
 
 	/* ilk-only ips/rps state. Everything in here is protected by the global
 	 * mchdev_lock in intel_pm.c */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 976beac52071..bcbbb4847b61 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3150,11 +3150,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv)
 
 	i915_gem_restore_fences(dev_priv);
 
-	if (dev_priv->gt_pm.rc6.enabled) {
-		dev_priv->gt_pm.rc6.enabled = false;
-		intel_gt_enable_rc6(dev_priv);
-	}
-
+	intel_gt_enable_rc6(dev_priv);
 	if (dev_priv->gt.awake) {
 		if (INTEL_GEN(dev_priv) >= 6)
 			gen6_rps_busy(dev_priv);
@@ -3436,13 +3432,13 @@ i915_gem_idle_work_handler(struct work_struct *work)
 
 	i915_pmu_gt_parked(dev_priv);
 
+	if (INTEL_GEN(dev_priv) >= 6)
+		gen6_rps_idle(dev_priv);
+
 	GEM_BUG_ON(!dev_priv->gt.awake);
 	dev_priv->gt.awake = false;
 	rearm_hangcheck = false;
 
-	if (INTEL_GEN(dev_priv) >= 6)
-		gen6_rps_idle(dev_priv);
-
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GT_IRQ);
 
 	intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_gt_pm.c b/drivers/gpu/drm/i915/intel_gt_pm.c
index 2fbc1e5ad657..29d5f7713ae4 100644
--- a/drivers/gpu/drm/i915/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/intel_gt_pm.c
@@ -327,36 +327,29 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
  * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
 static int gen6_set_rps(struct drm_i915_private *dev_priv, u8 val)
 {
-	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	if (INTEL_GEN(dev_priv) >= 9)
+		I915_WRITE(GEN6_RPNSWREQ, GEN9_FREQUENCY(val));
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		I915_WRITE(GEN6_RPNSWREQ, HSW_FREQUENCY(val));
+	else
+		I915_WRITE(GEN6_RPNSWREQ,
+			   GEN6_FREQUENCY(val) |
+			   GEN6_OFFSET(0) |
+			   GEN6_AGGRESSIVE_TURBO);
 
-	/* min/max delay may still have been modified so be sure to
+	/*
+	 * min/max delay may still have been modified so be sure to
 	 * write the limits value.
 	 */
-	if (val != rps->cur_freq) {
-		gen6_set_rps_thresholds(dev_priv, val);
-
-		if (INTEL_GEN(dev_priv) >= 9)
-			I915_WRITE(GEN6_RPNSWREQ,
-				   GEN9_FREQUENCY(val));
-		else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			I915_WRITE(GEN6_RPNSWREQ,
-				   HSW_FREQUENCY(val));
-		else
-			I915_WRITE(GEN6_RPNSWREQ,
-				   GEN6_FREQUENCY(val) |
-				   GEN6_OFFSET(0) |
-				   GEN6_AGGRESSIVE_TURBO);
-	}
+	gen6_set_rps_thresholds(dev_priv, val);
 
-	/* Make sure we continue to get interrupts
+	/*
+	 * Make sure we continue to get interrupts
 	 * until we hit the minimum or maximum frequencies.
 	 */
 	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, intel_rps_limits(dev_priv, val));
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
-	rps->cur_freq = val;
-	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
-
 	return 0;
 }
 
@@ -368,89 +361,54 @@ static int valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val)
 		      "Odd GPU freq value\n"))
 		val &= ~1;
 
-	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
-
-	if (val != dev_priv->gt_pm.rps.cur_freq) {
-		err = vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-		if (err)
-			return err;
-
-		gen6_set_rps_thresholds(dev_priv, val);
-	}
+	err = vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+	if (err)
+		return err;
 
-	dev_priv->gt_pm.rps.cur_freq = val;
-	trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
+	gen6_set_rps_thresholds(dev_priv, val);
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	return 0;
 }
 
-/* vlv_set_rps_idle: Set the frequency to idle, if Gfx clocks are down
- *
- * * If Gfx is Idle, then
- * 1. Forcewake Media well.
- * 2. Request idle freq.
- * 3. Release Forcewake of Media well.
-*/
-static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+static int __intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
 {
-	struct intel_rps *rps = &dev_priv->gt_pm.rps;
-	u32 val = rps->idle_freq;
-	int err;
-
-	if (rps->cur_freq <= val)
-		return;
-
-	/* The punit delays the write of the frequency and voltage until it
-	 * determines the GPU is awake. During normal usage we don't want to
-	 * waste power changing the frequency if the GPU is sleeping (rc6).
-	 * However, the GPU and driver is now idle and we do not want to delay
-	 * switching to minimum voltage (reducing power whilst idle) as we do
-	 * not expect to be woken in the near future and so must flush the
-	 * change by waking the device.
-	 *
-	 * We choose to take the media powerwell (either would do to trick the
-	 * punit into committing the voltage change) as that takes a lot less
-	 * power than the render powerwell.
-	 */
-	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
-	err = valleyview_set_rps(dev_priv, val);
-	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA);
-
-	if (err)
-		DRM_ERROR("Failed to set RPS for idle\n");
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		return valleyview_set_rps(dev_priv, val);
+	else if (INTEL_GEN(dev_priv) >= 6)
+		return gen6_set_rps(dev_priv, val);
+	else
+		return 0;
 }
 
 void gen6_rps_busy(struct drm_i915_private *dev_priv)
 {
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	u8 freq;
 
 	if (!HAS_RPS(dev_priv))
 		return;
 
 	mutex_lock(&dev_priv->pcu_lock);
-	if (rps->enabled) {
-		u8 freq;
 
-		if (dev_priv->pm_rps_events & GEN6_PM_RP_UP_EI_EXPIRED)
-			gen6_rps_reset_ei(dev_priv);
-		I915_WRITE(GEN6_PMINTRMSK,
-			   gen6_rps_pm_mask(dev_priv, rps->cur_freq));
+	if (dev_priv->pm_rps_events & GEN6_PM_RP_UP_EI_EXPIRED)
+		gen6_rps_reset_ei(dev_priv);
 
-		gen6_enable_rps_interrupts(dev_priv);
-
-		/* Use the user's desired frequency as a guide, but for better
-		 * performance, jump directly to RPe as our starting frequency.
-		 */
-		freq = max(rps->cur_freq,
-			   rps->efficient_freq);
+	/*
+	 * Use the user's desired frequency as a guide, but for better
+	 * performance, jump directly to RPe as our starting frequency.
+	 */
+	freq = max(rps->cur_freq, rps->efficient_freq);
+	if (intel_set_rps(dev_priv,
+			  clamp(freq,
+				rps->min_freq_softlimit,
+				rps->max_freq_softlimit)))
+		DRM_DEBUG_DRIVER("Failed to set busy frequency\n");
 
-		if (intel_set_rps(dev_priv,
-				  clamp(freq,
-					rps->min_freq_softlimit,
-					rps->max_freq_softlimit)))
-			DRM_DEBUG_DRIVER("Failed to set idle frequency\n");
-	}
+	rps->last_adj = 0;
 	mutex_unlock(&dev_priv->pcu_lock);
+
+	gen6_enable_rps_interrupts(dev_priv);
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
@@ -460,7 +418,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 	if (!HAS_RPS(dev_priv))
 		return;
 
-	/* Flush our bottom-half so that it does not race with us
+	/*
+	 * Flush our bottom-half so that it does not race with us
 	 * setting the idle frequency and so that it is bounded by
 	 * our rpm wakeref. And then disable the interrupts to stop any
 	 * futher RPS reclocking whilst we are asleep.
@@ -468,15 +427,31 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 	gen6_disable_rps_interrupts(dev_priv);
 
 	mutex_lock(&dev_priv->pcu_lock);
-	if (rps->enabled) {
-		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-			vlv_set_rps_idle(dev_priv);
-		else
-			gen6_set_rps(dev_priv, rps->idle_freq);
-		rps->last_adj = 0;
-		I915_WRITE(GEN6_PMINTRMSK,
-			   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
+
+	if (rps->cur_freq > rps->idle_freq) {
+		/*
+		 * The punit delays the write of the frequency and voltage
+		 * until it determines the GPU is awake. During normal usage we
+		 * don't want to waste power changing the frequency if the GPU
+		 * is sleeping (rc6).  However, the GPU and driver is now idle
+		 * and we do not want to delay switching to minimum voltage
+		 * (reducing power whilst idle) as we do not expect to be woken
+		 * in the near future and so must flush the change by waking
+		 * the device.
+		 *
+		 * We choose to take the media powerwell (either would do to
+		 * trick the punit into committing the voltage change) as that
+		 * takes a lot less power than the render powerwell.
+		 */
+		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+		if (intel_set_rps(dev_priv, rps->idle_freq))
+			DRM_DEBUG_DRIVER("Failed to set idle frequency\n");
+		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA);
 	}
+
+	I915_WRITE(GEN6_PMINTRMSK,
+		   gen6_sanitize_rps_pm_mask(dev_priv, ~0));
+
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
@@ -490,12 +465,6 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
 	if (!HAS_RPS(rq->i915))
 		return;
 
-	/* This is intentionally racy! We peek at the state here, then
-	 * validate inside the RPS worker.
-	 */
-	if (!rps->enabled)
-		return;
-
 	boost = false;
 	spin_lock_irqsave(&rq->lock, flags);
 	if (!rq->waitboost && !i915_gem_request_completed(rq)) {
@@ -516,23 +485,23 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
 int intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
 {
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
-	int err;
 
 	lockdep_assert_held(&dev_priv->pcu_lock);
 	GEM_BUG_ON(val > rps->max_freq);
 	GEM_BUG_ON(val < rps->min_freq);
 
-	if (!rps->enabled) {
-		rps->cur_freq = val;
-		return 0;
+	if (dev_priv->gt.awake) {
+		int err = __intel_set_rps(dev_priv, val);
+		if (err)
+			return err;
 	}
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		err = valleyview_set_rps(dev_priv, val);
-	else
-		err = gen6_set_rps(dev_priv, val);
+	if (val != rps->cur_freq) {
+		trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
+		rps->cur_freq = val;
+	}
 
-	return err;
+	return 0;
 }
 
 static void gen9_disable_rc6(struct drm_i915_private *dev_priv)
@@ -719,20 +688,6 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void reset_rps(struct drm_i915_private *dev_priv,
-		      int (*set)(struct drm_i915_private *, u8))
-{
-	struct intel_rps *rps = &dev_priv->gt_pm.rps;
-	u8 freq = rps->cur_freq;
-
-	/* force a reset */
-	rps->power = -1;
-	rps->cur_freq = -1;
-
-	if (set(dev_priv, freq))
-		DRM_ERROR("Failed to reset RPS to initial values\n");
-}
-
 /* See the Gen9_GT_PM_Programming_Guide doc for the below */
 static void gen9_enable_rps(struct drm_i915_private *dev_priv)
 {
@@ -752,7 +707,6 @@ static void gen9_enable_rps(struct drm_i915_private *dev_priv)
 	/* Leaning on the below call to gen6_set_rps to program/setup the
 	 * Up/Down EI & threshold registers, as well as the RP_CONTROL,
 	 * RP_INTERRUPT_LIMITS & RPNSWREQ registers */
-	reset_rps(dev_priv, gen6_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
@@ -897,8 +851,6 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
-	reset_rps(dev_priv, gen6_set_rps);
-
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -984,8 +936,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 50000);
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
-	reset_rps(dev_priv, gen6_set_rps);
-
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -1478,8 +1428,6 @@ static void cherryview_enable_rps(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", yesno(val & GPLLENABLE));
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
-	reset_rps(dev_priv, valleyview_set_rps);
-
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -1562,8 +1510,6 @@ static void valleyview_enable_rps(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", yesno(val & GPLLENABLE));
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
-	reset_rps(dev_priv, valleyview_set_rps);
-
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -2048,10 +1994,7 @@ static void intel_init_emon(struct drm_i915_private *dev_priv)
 
 void intel_gt_pm_sanitize(struct drm_i915_private *dev_priv)
 {
-	dev_priv->gt_pm.rps.enabled = true; /* force RPS disabling */
 	intel_gt_disable_rps(dev_priv);
-
-	dev_priv->gt_pm.rc6.enabled = true; /* force RC6 disabling */
 	intel_gt_disable_rc6(dev_priv);
 
 	gen6_reset_rps_interrupts(dev_priv);
@@ -2120,9 +2063,6 @@ static void __enable_rc6(struct drm_i915_private *dev_priv)
 {
 	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))
@@ -2133,8 +2073,6 @@ static void __enable_rc6(struct drm_i915_private *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;
 }
 
 static void __enable_rps(struct drm_i915_private *dev_priv)
@@ -2143,9 +2081,6 @@ static void __enable_rps(struct drm_i915_private *dev_priv)
 
 	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	if (rps->enabled)
-		return;
-
 	if (IS_CHERRYVIEW(dev_priv)) {
 		cherryview_enable_rps(dev_priv);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
@@ -2167,7 +2102,8 @@ static void __enable_rps(struct drm_i915_private *dev_priv)
 	WARN_ON(rps->efficient_freq < rps->min_freq);
 	WARN_ON(rps->efficient_freq > rps->max_freq);
 
-	rps->enabled = true;
+	rps->cur_freq = rps->idle_freq;
+	rps->power = -1;
 }
 
 void intel_gt_enable_rc6(struct drm_i915_private *dev_priv)
@@ -2194,9 +2130,6 @@ static void __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))
@@ -2205,8 +2138,6 @@ static void __disable_rc6(struct drm_i915_private *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;
 }
 
 void intel_gt_disable_rc6(struct drm_i915_private *dev_priv)
@@ -2220,9 +2151,6 @@ static void __disable_rps(struct drm_i915_private *dev_priv)
 {
 	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	if (!dev_priv->gt_pm.rps.enabled)
-		return;
-
 	if (INTEL_GEN(dev_priv) >= 9)
 		gen9_disable_rps(dev_priv);
 	else if (IS_CHERRYVIEW(dev_priv))
@@ -2233,8 +2161,6 @@ static void __disable_rps(struct drm_i915_private *dev_priv)
 		gen6_disable_rps(dev_priv);
 	else if (INTEL_GEN(dev_priv) >= 5)
 		ironlake_disable_drps(dev_priv);
-
-	dev_priv->gt_pm.rps.enabled = false;
 }
 
 void intel_gt_disable_rps(struct drm_i915_private *dev_priv)
-- 
2.15.1



More information about the Intel-gfx-trybot mailing list