Mika's rps/rc6 fixes

Mika's rps/rc6 fixes

[Chris Wilson]

Fallout from the init tweaks from a couple of months ago...
-Chris

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

[PATCH 01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping

[Chris Wilson]

Never go to sleep waiting on the GPU without first ensuring that we will
get woken up.

We have a choice of queuing the hangcheck before every schedule() or the
first time we wakeup. In order to simply accommodate both the signaler
and the ordinary waiter, move the queuing to the common point of
enabling the irq. We lose the paranoid safety of ensuring that the
hangcheck is active before the sleep, but avoid code duplication (and
redundant hangcheck queuing).

Testcase: igt/prime_busy
Fixes: c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c          | 9 ---------
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8f50919ba9b4..7fd44980798f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1501,15 +1501,6 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 			break;
 		}
 
-		/* Ensure that even if the GPU hangs, we get woken up.
-		 *
-		 * However, note that if no one is waiting, we never notice
-		 * a gpu hang. Eventually, we will have to wait for a resource
-		 * held by the GPU and so trigger a hangcheck. In the most
-		 * pathological case, this will be upon memory starvation!
-		 */
-		i915_queue_hangcheck(req->i915);
-
 		timeout_remain = io_schedule_timeout(timeout_remain);
 		if (timeout_remain == 0) {
 			ret = -ETIME;
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index d89b2c963618..b074f3d6d127 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -93,6 +93,15 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
 	if (!b->irq_enabled ||
 	    test_bit(engine->id, &i915->gpu_error.missed_irq_rings))
 		mod_timer(&b->fake_irq, jiffies + 1);
+
+	/* Ensure that even if the GPU hangs, we get woken up.
+	 *
+	 * However, note that if no one is waiting, we never notice
+	 * a gpu hang. Eventually, we will have to wait for a resource
+	 * held by the GPU and so trigger a hangcheck. In the most
+	 * pathological case, this will be upon memory starvation!
+	 */
+	i915_queue_hangcheck(i915);
 }
 
 static void __intel_breadcrumbs_disable_irq(struct intel_breadcrumbs *b)
-- 
2.8.1

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

[PATCH 02/10] drm/i915: Kick hangcheck from retire worker

[Chris Wilson]

Let's ensure that we cannot run indefinitely without the hangcheck
worker being queued. We removed it from being kicked on every request
because we were kicking it a few millions times in every hangcheck
interval and only once is necessary! However, that leaves us with the
issue of what if userspace never waits for a request, or runs out of
resources, what if userspace just issues a request then spins on
BUSY_IOCTL?

Testcase: igt/gem_busy
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7fd44980798f..adeca0ec4cfb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3281,10 +3281,12 @@ i915_gem_retire_work_handler(struct work_struct *work)
 	 * We do not need to do this test under locking as in the worst-case
 	 * we queue the retire worker once too often.
 	 */
-	if (READ_ONCE(dev_priv->gt.awake))
+	if (READ_ONCE(dev_priv->gt.awake)) {
+		i915_queue_hangcheck(dev_priv);
 		queue_delayed_work(dev_priv->wq,
 				   &dev_priv->gt.retire_work,
 				   round_jiffies_up_relative(HZ));
+	}
 }
 
 static void
-- 
2.8.1

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

[PATCH 03/10] drm/i915: Preserve current RPS frequency across init

[Chris Wilson]

Select idle frequency during initialisation, then reset the last known
frequency when re-enabling. This allows us to preserve the user selected
frequency across resets.

v2: Stop CHV from overriding the user's choice in cherryview_enable_rps()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 46 ++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5a8ee0c76593..df72f8e7b4b3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5149,6 +5149,7 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
 	}
 
 	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
+	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
 
 	/* Preserve min/max settings in case of re-init */
 	if (dev_priv->rps.max_freq_softlimit == 0)
@@ -5165,6 +5166,18 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void reset_rps(struct drm_i915_private *dev_priv,
+		      void (*set)(struct drm_i915_private *, u8))
+{
+	u8 freq = dev_priv->rps.cur_freq;
+
+	/* force a reset */
+	dev_priv->rps.power = -1;
+	dev_priv->rps.cur_freq = -1;
+
+	set(dev_priv, freq);
+}
+
 /* See the Gen9_GT_PM_Programming_Guide doc for the below */
 static void gen9_enable_rps(struct drm_i915_private *dev_priv)
 {
@@ -5201,8 +5214,7 @@ 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 */
-	dev_priv->rps.power = HIGH_POWER; /* force a reset */
-	gen6_set_rps(dev_priv, dev_priv->rps.idle_freq);
+	reset_rps(dev_priv, gen6_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
@@ -5348,8 +5360,7 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 
 	/* 6: Ring frequency + overclocking (our driver does this later */
 
-	dev_priv->rps.power = HIGH_POWER; /* force a reset */
-	gen6_set_rps(dev_priv, dev_priv->rps.idle_freq);
+	reset_rps(dev_priv, gen6_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
@@ -5442,8 +5453,7 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 		dev_priv->rps.max_freq = pcu_mbox & 0xff;
 	}
 
-	dev_priv->rps.power = HIGH_POWER; /* force a reset */
-	gen6_set_rps(dev_priv, dev_priv->rps.idle_freq);
+	reset_rps(dev_priv, gen6_set_rps);
 
 	rc6vids = 0;
 	ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
@@ -5807,6 +5817,7 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 			 dev_priv->rps.min_freq);
 
 	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
+	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
 
 	/* Preserve min/max settings in case of re-init */
 	if (dev_priv->rps.max_freq_softlimit == 0)
@@ -5871,6 +5882,7 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 		  "Odd GPU freq values\n");
 
 	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
+	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
 
 	/* Preserve min/max settings in case of re-init */
 	if (dev_priv->rps.max_freq_softlimit == 0)
@@ -5970,16 +5982,7 @@ 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);
 
-	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
-	DRM_DEBUG_DRIVER("current GPU freq: %d MHz (%u)\n",
-			 intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
-			 dev_priv->rps.cur_freq);
-
-	DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n",
-			 intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq),
-			 dev_priv->rps.idle_freq);
-
-	valleyview_set_rps(dev_priv, dev_priv->rps.idle_freq);
+	reset_rps(dev_priv, valleyview_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
@@ -6059,16 +6062,7 @@ 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);
 
-	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
-	DRM_DEBUG_DRIVER("current GPU freq: %d MHz (%u)\n",
-			 intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
-			 dev_priv->rps.cur_freq);
-
-	DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n",
-			 intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq),
-			 dev_priv->rps.idle_freq);
-
-	valleyview_set_rps(dev_priv, dev_priv->rps.idle_freq);
+	reset_rps(dev_priv, valleyview_set_rps);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
-- 
2.8.1

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

[PATCH 04/10] drm/i915: Perform static RPS frequency setup before userspace

[Chris Wilson]

As these RPS frequency values are part of our userspace interface, they
must be established before that userspace interface is registered.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 98 +++++++++++++----------------------------
 1 file changed, 31 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index df72f8e7b4b3..54f739fbd133 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5102,35 +5102,31 @@ int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
 
 static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
 {
-	uint32_t rp_state_cap;
-	u32 ddcc_status = 0;
-	int ret;
-
 	/* All of these values are in units of 50MHz */
-	dev_priv->rps.cur_freq		= 0;
+
 	/* static values from HW: RP0 > RP1 > RPn (min_freq) */
 	if (IS_BROXTON(dev_priv)) {
-		rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
+		u32 rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
 		dev_priv->rps.rp0_freq = (rp_state_cap >> 16) & 0xff;
 		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
 		dev_priv->rps.min_freq = (rp_state_cap >>  0) & 0xff;
 	} else {
-		rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+		u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
 		dev_priv->rps.rp0_freq = (rp_state_cap >>  0) & 0xff;
 		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
 		dev_priv->rps.min_freq = (rp_state_cap >> 16) & 0xff;
 	}
-
 	/* hw_max = RP0 until we check for overclocking */
-	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
+	dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
 
 	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) ||
 	    IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
-		ret = sandybridge_pcode_read(dev_priv,
-					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
-					&ddcc_status);
-		if (0 == ret)
+		u32 ddcc_status = 0;
+
+		if (sandybridge_pcode_read(dev_priv,
+					   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
+					   &ddcc_status) == 0)
 			dev_priv->rps.efficient_freq =
 				clamp_t(u8,
 					((ddcc_status >> 8) & 0xff),
@@ -5140,30 +5136,14 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
 
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
 		/* Store the frequency values in 16.66 MHZ units, which is
-		   the natural hardware unit for SKL */
+		 * the natural hardware unit for SKL
+		 */
 		dev_priv->rps.rp0_freq *= GEN9_FREQ_SCALER;
 		dev_priv->rps.rp1_freq *= GEN9_FREQ_SCALER;
 		dev_priv->rps.min_freq *= GEN9_FREQ_SCALER;
 		dev_priv->rps.max_freq *= GEN9_FREQ_SCALER;
 		dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
 	}
-
-	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
-	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
-
-	/* Preserve min/max settings in case of re-init */
-	if (dev_priv->rps.max_freq_softlimit == 0)
-		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
-
-	if (dev_priv->rps.min_freq_softlimit == 0) {
-		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			dev_priv->rps.min_freq_softlimit =
-				max_t(int, dev_priv->rps.efficient_freq,
-				      intel_freq_opcode(dev_priv, 450));
-		else
-			dev_priv->rps.min_freq_softlimit =
-				dev_priv->rps.min_freq;
-	}
 }
 
 static void reset_rps(struct drm_i915_private *dev_priv,
@@ -5183,8 +5163,6 @@ static void gen9_enable_rps(struct drm_i915_private *dev_priv)
 {
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
-	gen6_init_rps_frequencies(dev_priv);
-
 	/* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
 	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
 		/*
@@ -5301,9 +5279,6 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 	/* 2a: Disable RC states. */
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 
-	/* Initialize rps frequencies */
-	gen6_init_rps_frequencies(dev_priv);
-
 	/* 2b: Program RC6 thresholds.*/
 	I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
 	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
@@ -5392,9 +5367,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
-	/* Initialize rps frequencies */
-	gen6_init_rps_frequencies(dev_priv);
-
 	/* disable the counters and set deterministic thresholds */
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 
@@ -5778,8 +5750,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	vlv_init_gpll_ref_freq(dev_priv);
 
-	mutex_lock(&dev_priv->rps.hw_lock);
-
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 	switch ((val >> 6) & 3) {
 	case 0:
@@ -5815,18 +5785,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
 			 dev_priv->rps.min_freq);
-
-	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
-	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
-
-	/* Preserve min/max settings in case of re-init */
-	if (dev_priv->rps.max_freq_softlimit == 0)
-		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
-
-	if (dev_priv->rps.min_freq_softlimit == 0)
-		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
-
-	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
 static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
@@ -5837,8 +5795,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	vlv_init_gpll_ref_freq(dev_priv);
 
-	mutex_lock(&dev_priv->rps.hw_lock);
-
 	mutex_lock(&dev_priv->sb_lock);
 	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
 	mutex_unlock(&dev_priv->sb_lock);
@@ -5880,18 +5836,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 		   dev_priv->rps.rp1_freq |
 		   dev_priv->rps.min_freq) & 1,
 		  "Odd GPU freq values\n");
-
-	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
-	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
-
-	/* Preserve min/max settings in case of re-init */
-	if (dev_priv->rps.max_freq_softlimit == 0)
-		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
-
-	if (dev_priv->rps.min_freq_softlimit == 0)
-		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
-
-	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
 static void valleyview_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
@@ -6559,10 +6503,30 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 		intel_runtime_pm_get(dev_priv);
 	}
 
+	mutex_lock(&dev_priv->rps.hw_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
+		gen6_init_rps_frequencies(dev_priv);
+
+	/* Derive initial user preferences/limits from the hardware limits */
+	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
+	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
+
+	dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
+	dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
+
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		dev_priv->rps.min_freq_softlimit =
+			max_t(int,
+			      dev_priv->rps.efficient_freq,
+			      intel_freq_opcode(dev_priv, 450));
+
+	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
 void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
-- 
2.8.1

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

[PATCH 05/10] drm/i915: Move overclocking detection to alongside RPS frequency detection

[Chris Wilson]

Move the overclocking max frequency detection alongside the regular
frequency detection, before we expose the undefined value to userspace.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 54f739fbd133..24b23a51c56b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5343,7 +5343,7 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
 static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
-	u32 rc6vids, pcu_mbox = 0, rc6_mask = 0;
+	u32 rc6vids, rc6_mask = 0;
 	u32 gtfifodbg;
 	int rc6_mode;
 	int ret;
@@ -5417,14 +5417,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	if (ret)
 		DRM_DEBUG_DRIVER("Failed to set the min frequency\n");
 
-	ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
-	if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */
-		DRM_DEBUG_DRIVER("Overclocking supported. Max: %dMHz, Overclock max: %dMHz\n",
-				 (dev_priv->rps.max_freq_softlimit & 0xff) * 50,
-				 (pcu_mbox & 0xff) * 50);
-		dev_priv->rps.max_freq = pcu_mbox & 0xff;
-	}
-
 	reset_rps(dev_priv, gen6_set_rps);
 
 	rc6vids = 0;
@@ -6526,6 +6518,20 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 			      dev_priv->rps.efficient_freq,
 			      intel_freq_opcode(dev_priv, 450));
 
+	/* After setting max-softlimit, find the overclock max freq */
+	if (IS_GEN6(dev_priv) ||
+	    IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv)) {
+		u32 params = 0;
+
+		sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &params);
+		if (params & BIT(31)) { /* OC supported */
+			DRM_DEBUG_DRIVER("Overclocking supported, max: %dMHz, overclock: %dMHz\n",
+					 (dev_priv->rps.max_freq & 0xff) * 50,
+					 (params & 0xff) * 50);
+			dev_priv->rps.max_freq = params & 0xff;
+		}
+	}
+
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
-- 
2.8.1

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

[PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Chris Wilson]

To allow the user finer control over waitboosting, allow them to set the
frequency we request for the boost. This also them allows to effectively
disable the boosting by setting the boost request to a low frequency.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 ++
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_irq.c     |  9 +++++----
 drivers/gpu/drm/i915/i915_sysfs.c   | 38 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c     |  5 ++++-
 5 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 844fea795bae..d1ff4cb9b90e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1381,6 +1381,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 			   intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
 		seq_printf(m, "Min freq: %d MHz\n",
 			   intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
+		seq_printf(m, "Boost freq: %d MHz\n",
+			   intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
 		seq_printf(m, "Max freq: %d MHz\n",
 			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
 		seq_printf(m,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4478cc852489..bb59bc637f7b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1170,6 +1170,7 @@ struct intel_gen6_power_mgmt {
 	u8 max_freq_softlimit;	/* Max frequency permitted by the driver */
 	u8 max_freq;		/* Maximum frequency, RP0 if not overclocking */
 	u8 min_freq;		/* AKA RPn. Minimum frequency */
+	u8 boost_freq;		/* Frequency to request when wait boosting */
 	u8 idle_freq;		/* Frequency to request when we are idle */
 	u8 efficient_freq;	/* AKA RPe. Pre-determined balanced frequency */
 	u8 rp1_freq;		/* "less than" RP0 power/freqency */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1c2aec392412..c8ed36765301 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1105,9 +1105,10 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	new_delay = dev_priv->rps.cur_freq;
 	min = dev_priv->rps.min_freq_softlimit;
 	max = dev_priv->rps.max_freq_softlimit;
-
-	if (client_boost) {
-		new_delay = dev_priv->rps.max_freq_softlimit;
+	if (client_boost || any_waiters(dev_priv))
+		max = dev_priv->rps.max_freq;
+	if (client_boost && new_delay < dev_priv->rps.boost_freq) {
+		new_delay = dev_priv->rps.boost_freq;
 		adj = 0;
 	} else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
 		if (adj > 0)
@@ -1122,7 +1123,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			new_delay = dev_priv->rps.efficient_freq;
 			adj = 0;
 		}
-	} else if (any_waiters(dev_priv)) {
+	} else if (client_boost || any_waiters(dev_priv)) {
 		adj = 0;
 	} else if (pm_iir & GEN6_PM_RP_DOWN_TIMEOUT) {
 		if (dev_priv->rps.cur_freq > dev_priv->rps.efficient_freq)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index d61829e54f93..8c045ff47f0e 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -318,6 +318,41 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
 }
 
+static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct drm_minor *minor = dev_to_drm_minor(kdev);
+	struct drm_i915_private *dev_priv = to_i915(minor->dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
+}
+
+static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct drm_minor *minor = dev_to_drm_minor(kdev);
+	struct drm_device *dev = minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	/* Validate against (static) hardware limits */
+	val = intel_freq_opcode(dev_priv, val);
+	if (val < dev_priv->rps.min_freq || val > dev_priv->rps.max_freq)
+		return -EINVAL;
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	dev_priv->rps.boost_freq = val;
+	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	return count;
+}
+
 static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
 				     struct device_attribute *attr, char *buf)
 {
@@ -465,6 +500,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 
 static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show, NULL);
 static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
+static DEVICE_ATTR(gt_boost_freq_mhz, S_IRUGO, gt_boost_freq_mhz_show, gt_boost_freq_mhz_store);
 static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
 static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
 
@@ -498,6 +534,7 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
 static const struct attribute *gen6_attrs[] = {
 	&dev_attr_gt_act_freq_mhz.attr,
 	&dev_attr_gt_cur_freq_mhz.attr,
+	&dev_attr_gt_boost_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
 	&dev_attr_gt_RP0_freq_mhz.attr,
@@ -509,6 +546,7 @@ static const struct attribute *gen6_attrs[] = {
 static const struct attribute *vlv_attrs[] = {
 	&dev_attr_gt_act_freq_mhz.attr,
 	&dev_attr_gt_cur_freq_mhz.attr,
+	&dev_attr_gt_boost_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
 	&dev_attr_gt_RP0_freq_mhz.attr,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 24b23a51c56b..aab1e0b5d2eb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4911,7 +4911,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
 	 */
 	if (!(dev_priv->gt.awake &&
 	      dev_priv->rps.enabled &&
-	      dev_priv->rps.cur_freq < dev_priv->rps.max_freq_softlimit))
+	      dev_priv->rps.cur_freq < dev_priv->rps.boost_freq))
 		return;
 
 	/* Force a RPS boost (and don't count it against the client) if
@@ -6532,6 +6532,9 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 		}
 	}
 
+	/* Finally allow us to boost to max by default */
+	dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
+
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
-- 
2.8.1

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

[PATCH 07/10] drm/i915: Remove superfluous powersave work flushing

[Chris Wilson]

Instead of flushing the outstanding enabling, remember the requested
frequency to apply when the powersave work runs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 30 ++-------------------
 drivers/gpu/drm/i915/i915_sysfs.c   | 52 ++++++++++---------------------------
 2 files changed, 16 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d1ff4cb9b90e..90aef4540193 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1205,8 +1205,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 
 	intel_runtime_pm_get(dev_priv);
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	if (IS_GEN5(dev)) {
 		u16 rgvswctl = I915_READ16(MEMSWCTL);
 		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
@@ -1898,8 +1896,6 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
 
 	intel_runtime_pm_get(dev_priv);
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
 		goto out;
@@ -4952,20 +4948,11 @@ i915_max_freq_get(void *data, u64 *val)
 {
 	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
 
 	if (INTEL_INFO(dev)->gen < 6)
 		return -ENODEV;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
-	if (ret)
-		return ret;
-
 	*val = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
-	mutex_unlock(&dev_priv->rps.hw_lock);
-
 	return 0;
 }
 
@@ -4980,8 +4967,6 @@ i915_max_freq_set(void *data, u64 val)
 	if (INTEL_INFO(dev)->gen < 6)
 		return -ENODEV;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
 
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
@@ -5019,20 +5004,11 @@ i915_min_freq_get(void *data, u64 *val)
 {
 	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
 
-	if (INTEL_INFO(dev)->gen < 6)
+	if (INTEL_GEN(dev_priv) < 6)
 		return -ENODEV;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
-	if (ret)
-		return ret;
-
 	*val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
-	mutex_unlock(&dev_priv->rps.hw_lock);
-
 	return 0;
 }
 
@@ -5044,11 +5020,9 @@ i915_min_freq_set(void *data, u64 val)
 	u32 hw_max, hw_min;
 	int ret;
 
-	if (INTEL_INFO(dev)->gen < 6)
+	if (INTEL_GEN(dev_priv) < 6)
 		return -ENODEV;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
 
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 8c045ff47f0e..d47281b4b1c1 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -271,8 +271,6 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int ret;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
@@ -303,19 +301,10 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	struct drm_minor *minor = dev_to_drm_minor(kdev);
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
-
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	intel_runtime_pm_get(dev_priv);
-
-	mutex_lock(&dev_priv->rps.hw_lock);
-	ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
-	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	intel_runtime_pm_put(dev_priv);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv,
+				       dev_priv->rps.cur_freq));
 }
 
 static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
@@ -324,7 +313,8 @@ static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribu
 	struct drm_i915_private *dev_priv = to_i915(minor->dev);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n",
-			intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
+			intel_gpu_freq(dev_priv,
+				       dev_priv->rps.boost_freq));
 }
 
 static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
@@ -360,9 +350,9 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	return snprintf(buf, PAGE_SIZE,
-			"%d\n",
-			intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv,
+				       dev_priv->rps.efficient_freq));
 }
 
 static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
@@ -370,15 +360,10 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	struct drm_minor *minor = dev_to_drm_minor(kdev);
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
-
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
-	mutex_lock(&dev_priv->rps.hw_lock);
-	ret = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
-	mutex_unlock(&dev_priv->rps.hw_lock);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv,
+				       dev_priv->rps.max_freq_softlimit));
 }
 
 static ssize_t gt_max_freq_mhz_store(struct device *kdev,
@@ -395,8 +380,6 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 	if (ret)
 		return ret;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
@@ -438,15 +421,10 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
 	struct drm_minor *minor = dev_to_drm_minor(kdev);
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	mutex_lock(&dev_priv->rps.hw_lock);
-	ret = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
-	mutex_unlock(&dev_priv->rps.hw_lock);
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv,
+				       dev_priv->rps.min_freq_softlimit));
 }
 
 static ssize_t gt_min_freq_mhz_store(struct device *kdev,
@@ -463,8 +441,6 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	if (ret)
 		return ret;
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-- 
2.8.1

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

[PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Chris Wilson]

Some hardware requires a valid render context before it can initiate
rc6 power gating of the GPU; the default state of the GPU is not
sufficient and may lead to undefined behaviour. The first execution of
any batch will load the "golden render state", at which point it is safe
to enable rc6. As we do not forcibly load the kernel context at resume,
we have to hook into the batch submission to be sure that the render
state is setup before enabling rc6.

However, since we don't enable powersaving until that first batch, we
queued a delayed task in order to guarantee that the batch is indeed
submitted.

v2: Rearrange intel_disable_gt_powersave() to match.
v3: Apply user specified cur_freq (or idle_freq if not set).
v4: Give in, and supply a delayed work to autoenable rc6
v5: Mika suggested a couple of better names for delayed_resume_work

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   7 +-
 drivers/gpu/drm/i915/i915_drv.h      |   2 +-
 drivers/gpu/drm/i915/i915_gem.c      |   1 +
 drivers/gpu/drm/i915/intel_display.c |   2 +-
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_pm.c      | 122 +++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
 7 files changed, 89 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b9a811750ca8..ff3342b78a74 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1343,7 +1343,7 @@ void i915_driver_unload(struct drm_device *dev)
 	i915_destroy_error_state(dev);
 
 	/* Flush any outstanding unpin_work. */
-	flush_workqueue(dev_priv->wq);
+	drain_workqueue(dev_priv->wq);
 
 	intel_guc_fini(dev);
 	i915_gem_fini(dev);
@@ -1652,6 +1652,7 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_opregion_notify_adapter(dev_priv, PCI_D0);
 
+	intel_autoenable_gt_powersave(dev_priv);
 	drm_kms_helper_poll_enable(dev);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -1835,8 +1836,7 @@ int i915_reset(struct drm_i915_private *dev_priv)
 	 * previous concerns that it doesn't respond well to some forms
 	 * of re-init after reset.
 	 */
-	if (INTEL_INFO(dev)->gen > 5)
-		intel_enable_gt_powersave(dev_priv);
+	intel_autoenable_gt_powersave(dev_priv);
 
 	return 0;
 
@@ -2459,7 +2459,6 @@ static int intel_runtime_resume(struct device *device)
 	 * we can do is to hope that things will still work (and disable RPM).
 	 */
 	i915_gem_init_swizzling(dev);
-	gen6_update_ring_freq(dev_priv);
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bb59bc637f7b..8d74db096189 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1188,7 +1188,7 @@ struct intel_gen6_power_mgmt {
 	bool client_boost;
 
 	bool enabled;
-	struct delayed_work delayed_resume_work;
+	struct delayed_work autoenable_work;
 	unsigned boosts;
 
 	struct intel_rps_client semaphores, mmioflips;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index adeca0ec4cfb..6f2227b24711 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2842,6 +2842,7 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
+	intel_enable_gt_powersave(dev_priv);
 	i915_update_gfx_val(dev_priv);
 	if (INTEL_GEN(dev_priv) >= 6)
 		gen6_rps_busy(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be3b2cab2640..92cd0a70f8c8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15456,7 +15456,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
 	dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
 
 	intel_init_clock_gating(dev);
-	intel_enable_gt_powersave(dev_priv);
 }
 
 /*
@@ -16252,6 +16251,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
+	intel_suspend_gt_powersave(dev_priv);
 	intel_disable_gt_powersave(dev_priv);
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 55aeaf041749..c68dd6166c52 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1697,7 +1697,9 @@ 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_autoenable_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_reset_gt_powersave(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 aab1e0b5d2eb..c6ba923425ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6549,13 +6549,6 @@ void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
 		intel_runtime_pm_put(dev_priv);
 }
 
-static void gen6_suspend_rps(struct drm_i915_private *dev_priv)
-{
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
-	gen6_disable_rps_interrupts(dev_priv);
-}
-
 /**
  * intel_suspend_gt_powersave - suspend PM work and helper threads
  * @dev_priv: i915 device
@@ -6569,50 +6562,65 @@ void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) < 6)
 		return;
 
-	gen6_suspend_rps(dev_priv);
+	cancel_delayed_work_sync(&dev_priv->rps.autoenable_work);
+
+	gen6_disable_rps_interrupts(dev_priv);
 
 	/* Force GPU to min freq during suspend */
 	gen6_rps_idle(dev_priv);
 }
 
+void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
+{
+	dev_priv->rps.enabled = true; /* force disabling */
+	intel_disable_gt_powersave(dev_priv);
+
+	gen6_reset_rps_interrupts(dev_priv);
+}
+
 void intel_disable_gt_powersave(struct drm_i915_private *dev_priv)
 {
-	if (IS_IRONLAKE_M(dev_priv)) {
-		ironlake_disable_drps(dev_priv);
-	} else if (INTEL_INFO(dev_priv)->gen >= 6) {
-		intel_suspend_gt_powersave(dev_priv);
+	if (!READ_ONCE(dev_priv->rps.enabled))
+		return;
 
-		mutex_lock(&dev_priv->rps.hw_lock);
-		if (INTEL_INFO(dev_priv)->gen >= 9) {
-			gen9_disable_rc6(dev_priv);
-			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
-			gen6_disable_rps(dev_priv);
+	mutex_lock(&dev_priv->rps.hw_lock);
 
-		dev_priv->rps.enabled = false;
-		mutex_unlock(&dev_priv->rps.hw_lock);
+	if (INTEL_GEN(dev_priv) >= 9) {
+		gen9_disable_rc6(dev_priv);
+		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);
 	}
+
+	dev_priv->rps.enabled = false;
+	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
-static void intel_gen6_powersave_work(struct work_struct *work)
+void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv =
-		container_of(work, struct drm_i915_private,
-			     rps.delayed_resume_work.work);
+	/* We shouldn't be disabling as we submit, so this should be less
+	 * racy than it appears!
+	 */
+	if (READ_ONCE(dev_priv->rps.enabled))
+		return;
 
-	mutex_lock(&dev_priv->rps.hw_lock);
+	/* Powersaving is controlled by the host when inside a VM */
+	if (intel_vgpu_active(dev_priv))
+		return;
 
-	gen6_reset_rps_interrupts(dev_priv);
+	mutex_lock(&dev_priv->rps.hw_lock);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		cherryview_enable_rps(dev_priv);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		valleyview_enable_rps(dev_priv);
-	} else if (INTEL_INFO(dev_priv)->gen >= 9) {
+	} else if (INTEL_GEN(dev_priv) >= 9) {
 		gen9_enable_rc6(dev_priv);
 		gen9_enable_rps(dev_priv);
 		if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
@@ -6620,9 +6628,12 @@ static void intel_gen6_powersave_work(struct work_struct *work)
 	} else if (IS_BROADWELL(dev_priv)) {
 		gen8_enable_rps(dev_priv);
 		__gen6_update_ring_freq(dev_priv);
-	} else {
+	} else if (INTEL_GEN(dev_priv) >= 6) {
 		gen6_enable_rps(dev_priv);
 		__gen6_update_ring_freq(dev_priv);
+	} else if (IS_IRONLAKE_M(dev_priv)) {
+		ironlake_enable_drps(dev_priv);
+		intel_init_emon(dev_priv);
 	}
 
 	WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);
@@ -6632,18 +6643,45 @@ static void intel_gen6_powersave_work(struct work_struct *work)
 	WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);
 
 	dev_priv->rps.enabled = true;
+	mutex_unlock(&dev_priv->rps.hw_lock);
+}
 
-	gen6_enable_rps_interrupts(dev_priv);
+static void __intel_autoenable_gt_powersave(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv), rps.autoenable_work.work);
+	struct intel_engine_cs *rcs;
+	struct drm_i915_gem_request *req;
 
-	mutex_unlock(&dev_priv->rps.hw_lock);
+	if (READ_ONCE(dev_priv->rps.enabled))
+		return;
+
+	rcs = &dev_priv->engine[RCS];
+	if (rcs->last_context)
+		return;
+
+	if (!rcs->init_context)
+		return;
+
+	intel_runtime_pm_get(dev_priv);
+	mutex_lock(&dev_priv->drm.struct_mutex);
 
+	req = i915_gem_request_alloc(rcs, dev_priv->kernel_context);
+	if (IS_ERR(req))
+		goto unlock;
+
+	if (!i915.enable_execlists && i915_switch_context(req) == 0)
+		rcs->init_context(req);
+	i915_add_request_no_flush(req);
+
+unlock:
+	mutex_unlock(&dev_priv->drm.struct_mutex);
 	intel_runtime_pm_put(dev_priv);
 }
 
-void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
+void intel_autoenable_gt_powersave(struct drm_i915_private *dev_priv)
 {
-	/* Powersaving is controlled by the host when inside a VM */
-	if (intel_vgpu_active(dev_priv))
+	if (READ_ONCE(dev_priv->rps.enabled))
 		return;
 
 	if (IS_IRONLAKE_M(dev_priv)) {
@@ -6664,8 +6702,8 @@ void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 		 * paths, so the _noresume version is enough (and in case of
 		 * runtime resume it's necessary).
 		 */
-		if (schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
-					   round_jiffies_up_relative(HZ)))
+		if (schedule_delayed_work(&dev_priv->rps.autoenable_work,
+					  round_jiffies_up_relative(HZ)))
 			intel_runtime_pm_get_noresume(dev_priv);
 	}
 }
@@ -6675,7 +6713,7 @@ void intel_reset_gt_powersave(struct drm_i915_private *dev_priv)
 	if (INTEL_INFO(dev_priv)->gen < 6)
 		return;
 
-	gen6_suspend_rps(dev_priv);
+	gen6_disable_rps_interrupts(dev_priv);
 	dev_priv->rps.enabled = false;
 }
 
@@ -7785,8 +7823,8 @@ void intel_pm_setup(struct drm_device *dev)
 	mutex_init(&dev_priv->rps.hw_lock);
 	spin_lock_init(&dev_priv->rps.client_lock);
 
-	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
-			  intel_gen6_powersave_work);
+	INIT_DELAYED_WORK(&dev_priv->rps.autoenable_work,
+			  __intel_autoenable_gt_powersave);
 	INIT_LIST_HEAD(&dev_priv->rps.clients);
 	INIT_LIST_HEAD(&dev_priv->rps.semaphores.link);
 	INIT_LIST_HEAD(&dev_priv->rps.mmioflips.link);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index ff80a81b1a84..eeb4cbce19ff 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -435,7 +435,7 @@ void intel_uncore_sanitize(struct drm_i915_private *dev_priv)
 	i915.enable_rc6 = sanitize_rc6_option(dev_priv, i915.enable_rc6);
 
 	/* BIOS often leaves RC6 enabled, but disable it for hw init */
-	intel_disable_gt_powersave(dev_priv);
+	intel_sanitize_gt_powersave(dev_priv);
 }
 
 static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
-- 
2.8.1

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

[PATCH 09/10] drm/i915: Hide gen6_update_ring_freq()

[Chris Wilson]

This function is no longer used outside of intel_pm.c so we can stop
exposing it and rename the __gen6_update_ring_freq() to take its place.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 drivers/gpu/drm/i915/intel_pm.c  | 18 ++++--------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c68dd6166c52..9c78fda491f0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1703,7 +1703,6 @@ void intel_autoenable_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_reset_gt_powersave(struct drm_i915_private *dev_priv);
-void gen6_update_ring_freq(struct drm_i915_private *dev_priv);
 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 c6ba923425ce..89cd9676b5ab 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5436,7 +5436,7 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 }
 
-static void __gen6_update_ring_freq(struct drm_i915_private *dev_priv)
+static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 {
 	int min_freq = 15;
 	unsigned int gpu_freq;
@@ -5520,16 +5520,6 @@ static void __gen6_update_ring_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
-void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
-{
-	if (!HAS_CORE_RING_FREQ(dev_priv))
-		return;
-
-	mutex_lock(&dev_priv->rps.hw_lock);
-	__gen6_update_ring_freq(dev_priv);
-	mutex_unlock(&dev_priv->rps.hw_lock);
-}
-
 static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp0;
@@ -6624,13 +6614,13 @@ void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
 		gen9_enable_rc6(dev_priv);
 		gen9_enable_rps(dev_priv);
 		if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
-			__gen6_update_ring_freq(dev_priv);
+			gen6_update_ring_freq(dev_priv);
 	} else if (IS_BROADWELL(dev_priv)) {
 		gen8_enable_rps(dev_priv);
-		__gen6_update_ring_freq(dev_priv);
+		gen6_update_ring_freq(dev_priv);
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		gen6_enable_rps(dev_priv);
-		__gen6_update_ring_freq(dev_priv);
+		gen6_update_ring_freq(dev_priv);
 	} else if (IS_IRONLAKE_M(dev_priv)) {
 		ironlake_enable_drps(dev_priv);
 		intel_init_emon(dev_priv);
-- 
2.8.1

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

[PATCH 10/10] drm/i915: Remove temporary RPM wakeref assert disables

[Chris Wilson]

Now that the last couple of hacks have been removed from the runtime
powermanagement users, we can fully enable the asserts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_drv.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9c78fda491f0..26bfcccad57e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1665,13 +1665,6 @@ enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
 	atomic_dec(&dev_priv->pm.wakeref_count);
 }
 
-/* TODO: convert users of these to rely instead on proper RPM refcounting */
-#define DISABLE_RPM_WAKEREF_ASSERTS(dev_priv)	\
-	disable_rpm_wakeref_asserts(dev_priv)
-
-#define ENABLE_RPM_WAKEREF_ASSERTS(dev_priv)	\
-	enable_rpm_wakeref_asserts(dev_priv)
-
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
 bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping

[Patchwork]

== Series Details ==

Series: series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping
URL   : https://patchwork.freedesktop.org/series/9688/
State : failure

== Summary ==

Series 9688v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/9688/revisions/1/mbox

Test core_auth:
        Subgroup basic-auth:
                pass       -> DMESG-WARN (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> FAIL       (ro-bdw-i7-5557U)
                pass       -> FAIL       (ro-hsw-i3-4010u)
                pass       -> FAIL       (fi-skl-i5-6260u)
                pass       -> FAIL       (ro-bdw-i7-5600u)
                pass       -> FAIL       (ro-bdw-i5-5250u)
                pass       -> FAIL       (fi-skl-i7-6700k)
                pass       -> FAIL       (ro-hsw-i7-4770r)
                pass       -> FAIL       (fi-snb-i7-2600)
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)
        Subgroup basic-rte:
                pass       -> FAIL       (ro-bdw-i7-5557U)
                pass       -> FAIL       (ro-hsw-i3-4010u)
                pass       -> FAIL       (fi-skl-i5-6260u)
                pass       -> FAIL       (ro-bdw-i7-5600u)
                pass       -> FAIL       (ro-bdw-i5-5250u)
                pass       -> FAIL       (ro-snb-i7-2620M)
                pass       -> FAIL       (ro-hsw-i7-4770r)
                pass       -> FAIL       (fi-snb-i7-2600)
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (fi-skl-i7-6700k)
                pass       -> FAIL       (ro-skl3-i5-6260u)
Test vgem_basic:
        Subgroup debugfs:
                pass       -> FAIL       (ro-bdw-i7-5557U)
                pass       -> FAIL       (ro-ilk1-i5-650)
                pass       -> FAIL       (ro-hsw-i3-4010u)
                pass       -> FAIL       (ro-bdw-i7-5600u)
                pass       -> FAIL       (ro-snb-i7-2620M)
                pass       -> FAIL       (ro-bdw-i5-5250u)
                pass       -> FAIL       (ro-hsw-i7-4770r)
                pass       -> FAIL       (ro-ivb-i7-3770)
                pass       -> FAIL       (ro-byt-n2820)
                pass       -> FAIL       (ro-skl3-i5-6260u)

fi-kbl-qkkr      total:237  pass:168  dwarn:26  dfail:4   fail:5   skip:34 
fi-skl-i5-6260u  total:237  pass:209  dwarn:0   dfail:1   fail:7   skip:20 
fi-skl-i7-6700k  total:237  pass:195  dwarn:0   dfail:1   fail:7   skip:34 
fi-snb-i7-2600   total:237  pass:181  dwarn:0   dfail:1   fail:7   skip:48 
ro-bdw-i5-5250u  total:237  pass:204  dwarn:1   dfail:1   fail:7   skip:24 
ro-bdw-i7-5557U  total:237  pass:204  dwarn:1   dfail:1   fail:7   skip:24 
ro-bdw-i7-5600u  total:237  pass:190  dwarn:0   dfail:1   fail:7   skip:39 
ro-bsw-n3050     total:217  pass:168  dwarn:0   dfail:0   fail:6   skip:42 
ro-byt-n2820     total:237  pass:179  dwarn:1   dfail:1   fail:10  skip:46 
ro-hsw-i3-4010u  total:237  pass:197  dwarn:0   dfail:1   fail:7   skip:32 
ro-hsw-i7-4770r  total:237  pass:197  dwarn:0   dfail:1   fail:7   skip:32 
ro-ilk-i7-620lm  total:237  pass:159  dwarn:0   dfail:1   fail:6   skip:71 
ro-ilk1-i5-650   total:232  pass:159  dwarn:0   dfail:1   fail:6   skip:66 
ro-ivb-i7-3770   total:237  pass:190  dwarn:0   dfail:1   fail:5   skip:41 
ro-skl3-i5-6260u total:237  pass:208  dwarn:1   dfail:1   fail:7   skip:20 
ro-snb-i7-2620M  total:237  pass:180  dwarn:0   dfail:1   fail:7   skip:49 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1456/

689b286 drm-intel-nightly: 2016y-07m-08d-12h-37m-09s UTC integration manifest
c2c0cf9 drm/i915: Remove temporary RPM wakeref assert disables
6125ea4 drm/i915: Hide gen6_update_ring_freq()
5de58b7 drm/i915: Defer enabling rc6 til after we submit the first batch/context
f435900 drm/i915: Remove superfluous powersave work flushing
a61330c drm/i915: Define a separate variable and control for RPS waitboost frequency
210fd07 drm/i915: Move overclocking detection to alongside RPS frequency detection
9ee1451 drm/i915: Perform static RPS frequency setup before userspace
f826a5c drm/i915: Preserve current RPS frequency across init
4e4d82d drm/i915: Kick hangcheck from retire worker
c841283 drm/i915/breadcrumbs: Queue hangcheck before sleeping

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

Re: ✗ Ro.CI.BAT: failure for series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping

[Chris Wilson]

On Sat, Jul 09, 2016 at 09:58:00AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping
> URL   : https://patchwork.freedesktop.org/series/9688/
> State : failure
> 
> == Summary ==
> 
> Series 9688v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9688/revisions/1/mbox
> 
> Test core_auth:
>         Subgroup basic-auth:
>                 pass       -> DMESG-WARN (ro-byt-n2820)
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> FAIL       (ro-bdw-i7-5557U)
>                 pass       -> FAIL       (ro-hsw-i3-4010u)
>                 pass       -> FAIL       (fi-skl-i5-6260u)
>                 pass       -> FAIL       (ro-bdw-i7-5600u)
>                 pass       -> FAIL       (ro-bdw-i5-5250u)
>                 pass       -> FAIL       (fi-skl-i7-6700k)
>                 pass       -> FAIL       (ro-hsw-i7-4770r)
>                 pass       -> FAIL       (fi-snb-i7-2600)
>                 pass       -> FAIL       (ro-byt-n2820)
>                 pass       -> FAIL       (ro-skl3-i5-6260u)
>         Subgroup basic-rte:
>                 pass       -> FAIL       (ro-bdw-i7-5557U)
>                 pass       -> FAIL       (ro-hsw-i3-4010u)
>                 pass       -> FAIL       (fi-skl-i5-6260u)
>                 pass       -> FAIL       (ro-bdw-i7-5600u)
>                 pass       -> FAIL       (ro-bdw-i5-5250u)
>                 pass       -> FAIL       (ro-snb-i7-2620M)
>                 pass       -> FAIL       (ro-hsw-i7-4770r)
>                 pass       -> FAIL       (fi-snb-i7-2600)
>                 pass       -> FAIL       (ro-byt-n2820)
>                 pass       -> FAIL       (fi-skl-i7-6700k)
>                 pass       -> FAIL       (ro-skl3-i5-6260u)

Note this appears to a .config fail and a broken assert instead of
require.

> Test vgem_basic:
>         Subgroup debugfs:
>                 pass       -> FAIL       (ro-bdw-i7-5557U)
>                 pass       -> FAIL       (ro-ilk1-i5-650)
>                 pass       -> FAIL       (ro-hsw-i3-4010u)
>                 pass       -> FAIL       (ro-bdw-i7-5600u)
>                 pass       -> FAIL       (ro-snb-i7-2620M)
>                 pass       -> FAIL       (ro-bdw-i5-5250u)
>                 pass       -> FAIL       (ro-hsw-i7-4770r)
>                 pass       -> FAIL       (ro-ivb-i7-3770)
>                 pass       -> FAIL       (ro-byt-n2820)
>                 pass       -> FAIL       (ro-skl3-i5-6260u)

Fixed in igt (bad path).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> To allow the user finer control over waitboosting, allow them to set the
> frequency we request for the boost. This also them allows to effectively
> disable the boosting by setting the boost request to a low frequency.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_irq.c     |  9 +++++----
>  drivers/gpu/drm/i915/i915_sysfs.c   | 38 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.c     |  5 ++++-
>  5 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 844fea795bae..d1ff4cb9b90e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1381,6 +1381,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>  			   intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
>  		seq_printf(m, "Min freq: %d MHz\n",
>  			   intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
> +		seq_printf(m, "Boost freq: %d MHz\n",
> +			   intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
>  		seq_printf(m, "Max freq: %d MHz\n",
>  			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
>  		seq_printf(m,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4478cc852489..bb59bc637f7b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1170,6 +1170,7 @@ struct intel_gen6_power_mgmt {
>  	u8 max_freq_softlimit;	/* Max frequency permitted by the driver */
>  	u8 max_freq;		/* Maximum frequency, RP0 if not overclocking */
>  	u8 min_freq;		/* AKA RPn. Minimum frequency */
> +	u8 boost_freq;		/* Frequency to request when wait boosting */
>  	u8 idle_freq;		/* Frequency to request when we are idle */
>  	u8 efficient_freq;	/* AKA RPe. Pre-determined balanced frequency */
>  	u8 rp1_freq;		/* "less than" RP0 power/freqency */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c2aec392412..c8ed36765301 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1105,9 +1105,10 @@ static void gen6_pm_rps_work(struct work_struct *work)
>  	new_delay = dev_priv->rps.cur_freq;
>  	min = dev_priv->rps.min_freq_softlimit;
>  	max = dev_priv->rps.max_freq_softlimit;
> -
> -	if (client_boost) {
> -		new_delay = dev_priv->rps.max_freq_softlimit;
> +	if (client_boost || any_waiters(dev_priv))
> +		max = dev_priv->rps.max_freq;
> +	if (client_boost && new_delay < dev_priv->rps.boost_freq) {
> +		new_delay = dev_priv->rps.boost_freq;

What should be the rules for boost_freq? With this change it can
be over max_freq_softlimit and thus will break pm_rps.

-Mika

>  		adj = 0;
>  	} else if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
>  		if (adj > 0)
> @@ -1122,7 +1123,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
>  			new_delay = dev_priv->rps.efficient_freq;
>  			adj = 0;
>  		}
> -	} else if (any_waiters(dev_priv)) {
> +	} else if (client_boost || any_waiters(dev_priv)) {
>  		adj = 0;
>  	} else if (pm_iir & GEN6_PM_RP_DOWN_TIMEOUT) {
>  		if (dev_priv->rps.cur_freq > dev_priv->rps.efficient_freq)
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index d61829e54f93..8c045ff47f0e 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -318,6 +318,41 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>  	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
>  }
>  
> +static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> +	struct drm_i915_private *dev_priv = to_i915(minor->dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
> +}
> +
> +static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t count)
> +{
> +	struct drm_minor *minor = dev_to_drm_minor(kdev);
> +	struct drm_device *dev = minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 val;
> +	ssize_t ret;
> +
> +	ret = kstrtou32(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/* Validate against (static) hardware limits */
> +	val = intel_freq_opcode(dev_priv, val);
> +	if (val < dev_priv->rps.min_freq || val > dev_priv->rps.max_freq)
> +		return -EINVAL;
> +
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +	dev_priv->rps.boost_freq = val;
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	return count;
> +}
> +
>  static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
>  				     struct device_attribute *attr, char *buf)
>  {
> @@ -465,6 +500,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  
>  static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show, NULL);
>  static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, NULL);
> +static DEVICE_ATTR(gt_boost_freq_mhz, S_IRUGO, gt_boost_freq_mhz_show, gt_boost_freq_mhz_store);
>  static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
>  static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
>  
> @@ -498,6 +534,7 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
>  static const struct attribute *gen6_attrs[] = {
>  	&dev_attr_gt_act_freq_mhz.attr,
>  	&dev_attr_gt_cur_freq_mhz.attr,
> +	&dev_attr_gt_boost_freq_mhz.attr,
>  	&dev_attr_gt_max_freq_mhz.attr,
>  	&dev_attr_gt_min_freq_mhz.attr,
>  	&dev_attr_gt_RP0_freq_mhz.attr,
> @@ -509,6 +546,7 @@ static const struct attribute *gen6_attrs[] = {
>  static const struct attribute *vlv_attrs[] = {
>  	&dev_attr_gt_act_freq_mhz.attr,
>  	&dev_attr_gt_cur_freq_mhz.attr,
> +	&dev_attr_gt_boost_freq_mhz.attr,
>  	&dev_attr_gt_max_freq_mhz.attr,
>  	&dev_attr_gt_min_freq_mhz.attr,
>  	&dev_attr_gt_RP0_freq_mhz.attr,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 24b23a51c56b..aab1e0b5d2eb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4911,7 +4911,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
>  	 */
>  	if (!(dev_priv->gt.awake &&
>  	      dev_priv->rps.enabled &&
> -	      dev_priv->rps.cur_freq < dev_priv->rps.max_freq_softlimit))
> +	      dev_priv->rps.cur_freq < dev_priv->rps.boost_freq))
>  		return;
>  
>  	/* Force a RPS boost (and don't count it against the client) if
> @@ -6532,6 +6532,9 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		}
>  	}
>  
> +	/* Finally allow us to boost to max by default */
> +	dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
> +
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Never go to sleep waiting on the GPU without first ensuring that we will
> get woken up.
>
> We have a choice of queuing the hangcheck before every schedule() or the
> first time we wakeup. In order to simply accommodate both the signaler
> and the ordinary waiter, move the queuing to the common point of
> enabling the irq. We lose the paranoid safety of ensuring that the
> hangcheck is active before the sleep, but avoid code duplication (and
> redundant hangcheck queuing).
>
> Testcase: igt/prime_busy
> Fixes: c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c          | 9 ---------
>  drivers/gpu/drm/i915/intel_breadcrumbs.c | 9 +++++++++
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8f50919ba9b4..7fd44980798f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1501,15 +1501,6 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  			break;
>  		}
>  
> -		/* Ensure that even if the GPU hangs, we get woken up.
> -		 *
> -		 * However, note that if no one is waiting, we never notice
> -		 * a gpu hang. Eventually, we will have to wait for a resource
> -		 * held by the GPU and so trigger a hangcheck. In the most
> -		 * pathological case, this will be upon memory starvation!
> -		 */
> -		i915_queue_hangcheck(req->i915);
> -
>  		timeout_remain = io_schedule_timeout(timeout_remain);
>  		if (timeout_remain == 0) {
>  			ret = -ETIME;
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index d89b2c963618..b074f3d6d127 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -93,6 +93,15 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
>  	if (!b->irq_enabled ||
>  	    test_bit(engine->id, &i915->gpu_error.missed_irq_rings))
>  		mod_timer(&b->fake_irq, jiffies + 1);
> +
> +	/* Ensure that even if the GPU hangs, we get woken up.
> +	 *
> +	 * However, note that if no one is waiting, we never notice
> +	 * a gpu hang. Eventually, we will have to wait for a resource
> +	 * held by the GPU and so trigger a hangcheck. In the most
> +	 * pathological case, this will be upon memory starvation!
> +	 */
> +	i915_queue_hangcheck(i915);
>  }
>  
>  static void __intel_breadcrumbs_disable_irq(struct intel_breadcrumbs *b)
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Chris Wilson]

On Mon, Jul 11, 2016 at 02:39:19PM +0300, Mika Kuoppala wrote:
> What should be the rules for boost_freq? With this change it can
> be over max_freq_softlimit and thus will break pm_rps.

The point is to be an independent variable that the user can set in
addition to max freq, just like the cpu turbo.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Kick hangcheck from retire worker

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Let's ensure that we cannot run indefinitely without the hangcheck
> worker being queued. We removed it from being kicked on every request
> because we were kicking it a few millions times in every hangcheck
> interval and only once is necessary! However, that leaves us with the
> issue of what if userspace never waits for a request, or runs out of
> resources, what if userspace just issues a request then spins on
> BUSY_IOCTL?
>
> Testcase: igt/gem_busy
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7fd44980798f..adeca0ec4cfb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3281,10 +3281,12 @@ i915_gem_retire_work_handler(struct work_struct *work)
>  	 * We do not need to do this test under locking as in the worst-case
>  	 * we queue the retire worker once too often.
>  	 */
> -	if (READ_ONCE(dev_priv->gt.awake))
> +	if (READ_ONCE(dev_priv->gt.awake)) {
> +		i915_queue_hangcheck(dev_priv);
>  		queue_delayed_work(dev_priv->wq,
>  				   &dev_priv->gt.retire_work,
>  				   round_jiffies_up_relative(HZ));
> +	}
>  }
>  
>  static void
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Kick hangcheck from retire worker

[Chris Wilson]

On Mon, Jul 11, 2016 at 03:21:37PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Let's ensure that we cannot run indefinitely without the hangcheck
> > worker being queued. We removed it from being kicked on every request
> > because we were kicking it a few millions times in every hangcheck
> > interval and only once is necessary! However, that leaves us with the
> > issue of what if userspace never waits for a request, or runs out of
> > resources, what if userspace just issues a request then spins on
> > BUSY_IOCTL?
> >
> > Testcase: igt/gem_busy
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Ta, I'm going to push this pair as they fix a couple of (new) igt.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Mon, Jul 11, 2016 at 02:39:19PM +0300, Mika Kuoppala wrote:
>> What should be the rules for boost_freq? With this change it can
>> be over max_freq_softlimit and thus will break pm_rps.
>
> The point is to be an independent variable that the user can set in
> addition to max freq, just like the cpu turbo.

But can it be over max_freq? If so, pm_rps needs tweaking.

Also, noticed that this is not set up in gen6_init_rps_frequencies()

-Mika

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency

[Chris Wilson]

On Mon, Jul 11, 2016 at 03:55:35PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Mon, Jul 11, 2016 at 02:39:19PM +0300, Mika Kuoppala wrote:
> >> What should be the rules for boost_freq? With this change it can
> >> be over max_freq_softlimit and thus will break pm_rps.
> >
> > The point is to be an independent variable that the user can set in
> > addition to max freq, just like the cpu turbo.
> 
> But can it be over max_freq? If so, pm_rps needs tweaking.

max_freq being the hw limit?
 
> Also, noticed that this is not set up in gen6_init_rps_frequencies()

It's set after we determine the hw frequencies, in the common block
after the backends do their bit.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/10] drm/i915: Perform static RPS frequency setup before userspace

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> As these RPS frequency values are part of our userspace interface, they
> must be established before that userspace interface is registered.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 98 +++++++++++++----------------------------
>  1 file changed, 31 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index df72f8e7b4b3..54f739fbd133 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5102,35 +5102,31 @@ int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
>  
>  static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t rp_state_cap;
> -	u32 ddcc_status = 0;
> -	int ret;
> -
>  	/* All of these values are in units of 50MHz */
> -	dev_priv->rps.cur_freq		= 0;
> +
>  	/* static values from HW: RP0 > RP1 > RPn (min_freq) */
>  	if (IS_BROXTON(dev_priv)) {
> -		rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
> +		u32 rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
>  		dev_priv->rps.rp0_freq = (rp_state_cap >> 16) & 0xff;
>  		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
>  		dev_priv->rps.min_freq = (rp_state_cap >>  0) & 0xff;
>  	} else {
> -		rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +		u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
>  		dev_priv->rps.rp0_freq = (rp_state_cap >>  0) & 0xff;
>  		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
>  		dev_priv->rps.min_freq = (rp_state_cap >> 16) & 0xff;
>  	}
> -
>  	/* hw_max = RP0 until we check for overclocking */
> -	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
> +	dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
>  
>  	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) ||
>  	    IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> -		ret = sandybridge_pcode_read(dev_priv,
> -					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> -					&ddcc_status);
> -		if (0 == ret)
> +		u32 ddcc_status = 0;
> +
> +		if (sandybridge_pcode_read(dev_priv,
> +					   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> +					   &ddcc_status) == 0)


We do the read now without forcewake, but this should not matter.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  			dev_priv->rps.efficient_freq =
>  				clamp_t(u8,
>  					((ddcc_status >> 8) & 0xff),
> @@ -5140,30 +5136,14 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
>  
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		/* Store the frequency values in 16.66 MHZ units, which is
> -		   the natural hardware unit for SKL */
> +		 * the natural hardware unit for SKL
> +		 */
>  		dev_priv->rps.rp0_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.rp1_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.min_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.max_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
>  	}
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0) {
> -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -			dev_priv->rps.min_freq_softlimit =
> -				max_t(int, dev_priv->rps.efficient_freq,
> -				      intel_freq_opcode(dev_priv, 450));
> -		else
> -			dev_priv->rps.min_freq_softlimit =
> -				dev_priv->rps.min_freq;
> -	}
>  }
>  
>  static void reset_rps(struct drm_i915_private *dev_priv,
> @@ -5183,8 +5163,6 @@ static void gen9_enable_rps(struct drm_i915_private *dev_priv)
>  {
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
>  	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
>  		/*
> @@ -5301,9 +5279,6 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
>  	/* 2a: Disable RC states. */
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  
> -	/* Initialize rps frequencies */
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* 2b: Program RC6 thresholds.*/
>  	I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
>  	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
> @@ -5392,9 +5367,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
>  
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
> -	/* Initialize rps frequencies */
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* disable the counters and set deterministic thresholds */
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  
> @@ -5778,8 +5750,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	vlv_init_gpll_ref_freq(dev_priv);
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  	switch ((val >> 6) & 3) {
>  	case 0:
> @@ -5815,18 +5785,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0)
> -		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> -
> -	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -5837,8 +5795,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	vlv_init_gpll_ref_freq(dev_priv);
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -
>  	mutex_lock(&dev_priv->sb_lock);
>  	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
>  	mutex_unlock(&dev_priv->sb_lock);
> @@ -5880,18 +5836,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		   dev_priv->rps.rp1_freq |
>  		   dev_priv->rps.min_freq) & 1,
>  		  "Odd GPU freq values\n");
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0)
> -		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> -
> -	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  static void valleyview_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -6559,10 +6503,30 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		intel_runtime_pm_get(dev_priv);
>  	}
>  
> +	mutex_lock(&dev_priv->rps.hw_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
> +		gen6_init_rps_frequencies(dev_priv);
> +
> +	/* Derive initial user preferences/limits from the hardware limits */
> +	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> +	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> +
> +	dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> +	dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> +
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +		dev_priv->rps.min_freq_softlimit =
> +			max_t(int,
> +			      dev_priv->rps.efficient_freq,
> +			      intel_freq_opcode(dev_priv, 450));
> +
> +	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Move overclocking detection to alongside RPS frequency detection

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Move the overclocking max frequency detection alongside the regular
> frequency detection, before we expose the undefined value to userspace.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 54f739fbd133..24b23a51c56b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5343,7 +5343,7 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
>  static void gen6_enable_rps(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_engine_cs *engine;
> -	u32 rc6vids, pcu_mbox = 0, rc6_mask = 0;
> +	u32 rc6vids, rc6_mask = 0;
>  	u32 gtfifodbg;
>  	int rc6_mode;
>  	int ret;
> @@ -5417,14 +5417,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
>  	if (ret)
>  		DRM_DEBUG_DRIVER("Failed to set the min frequency\n");
>  
> -	ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
> -	if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */
> -		DRM_DEBUG_DRIVER("Overclocking supported. Max: %dMHz, Overclock max: %dMHz\n",
> -				 (dev_priv->rps.max_freq_softlimit & 0xff) * 50,
> -				 (pcu_mbox & 0xff) * 50);
> -		dev_priv->rps.max_freq = pcu_mbox & 0xff;
> -	}
> -
>  	reset_rps(dev_priv, gen6_set_rps);
>  
>  	rc6vids = 0;
> @@ -6526,6 +6518,20 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  			      dev_priv->rps.efficient_freq,
>  			      intel_freq_opcode(dev_priv, 450));
>  
> +	/* After setting max-softlimit, find the overclock max freq */
> +	if (IS_GEN6(dev_priv) ||
> +	    IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv)) {
> +		u32 params = 0;
> +
> +		sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &params);
> +		if (params & BIT(31)) { /* OC supported */
> +			DRM_DEBUG_DRIVER("Overclocking supported, max: %dMHz, overclock: %dMHz\n",
> +					 (dev_priv->rps.max_freq & 0xff) * 50,
> +					 (params & 0xff) * 50);

Yeah max_freq makes more sense.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> +			dev_priv->rps.max_freq = params & 0xff;
> +		}
> +	}
> +
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/10] drm/i915: Remove superfluous powersave work flushing

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Instead of flushing the outstanding enabling, remember the requested
> frequency to apply when the powersave work runs.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 30 ++-------------------
>  drivers/gpu/drm/i915/i915_sysfs.c   | 52 ++++++++++---------------------------
>  2 files changed, 16 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d1ff4cb9b90e..90aef4540193 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1205,8 +1205,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	if (IS_GEN5(dev)) {
>  		u16 rgvswctl = I915_READ16(MEMSWCTL);
>  		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
> @@ -1898,8 +1896,6 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>  	if (ret)
>  		goto out;
> @@ -4952,20 +4948,11 @@ i915_max_freq_get(void *data, u64 *val)
>  {
>  	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret;
>  
>  	if (INTEL_INFO(dev)->gen < 6)
>  		return -ENODEV;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> -	if (ret)
> -		return ret;
> -
>  	*val = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
> -	mutex_unlock(&dev_priv->rps.hw_lock);
> -
>  	return 0;
>  }
>  
> @@ -4980,8 +4967,6 @@ i915_max_freq_set(void *data, u64 val)
>  	if (INTEL_INFO(dev)->gen < 6)
>  		return -ENODEV;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> @@ -5019,20 +5004,11 @@ i915_min_freq_get(void *data, u64 *val)
>  {
>  	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret;
>  
> -	if (INTEL_INFO(dev)->gen < 6)
> +	if (INTEL_GEN(dev_priv) < 6)
>  		return -ENODEV;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> -	if (ret)
> -		return ret;
> -
>  	*val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
> -	mutex_unlock(&dev_priv->rps.hw_lock);
> -
>  	return 0;
>  }
>  
> @@ -5044,11 +5020,9 @@ i915_min_freq_set(void *data, u64 val)
>  	u32 hw_max, hw_min;
>  	int ret;
>  
> -	if (INTEL_INFO(dev)->gen < 6)
> +	if (INTEL_GEN(dev_priv) < 6)
>  		return -ENODEV;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 8c045ff47f0e..d47281b4b1c1 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -271,8 +271,6 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	int ret;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	intel_runtime_pm_get(dev_priv);
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> @@ -303,19 +301,10 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>  	struct drm_minor *minor = dev_to_drm_minor(kdev);
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret;
> -
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	intel_runtime_pm_get(dev_priv);
> -
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -	ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
> -	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	intel_runtime_pm_put(dev_priv);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			intel_gpu_freq(dev_priv,
> +				       dev_priv->rps.cur_freq));
>  }
>  
>  static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> @@ -324,7 +313,8 @@ static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribu
>  	struct drm_i915_private *dev_priv = to_i915(minor->dev);
>  
>  	return snprintf(buf, PAGE_SIZE, "%d\n",
> -			intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
> +			intel_gpu_freq(dev_priv,
> +				       dev_priv->rps.boost_freq));
>  }
>  
>  static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> @@ -360,9 +350,9 @@ static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  
> -	return snprintf(buf, PAGE_SIZE,
> -			"%d\n",
> -			intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			intel_gpu_freq(dev_priv,
> +				       dev_priv->rps.efficient_freq));
>  }
>  
>  static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> @@ -370,15 +360,10 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
>  	struct drm_minor *minor = dev_to_drm_minor(kdev);
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret;
> -
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -	ret = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit);
> -	mutex_unlock(&dev_priv->rps.hw_lock);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			intel_gpu_freq(dev_priv,
> +				       dev_priv->rps.max_freq_softlimit));
>  }
>  
>  static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> @@ -395,8 +380,6 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  	if (ret)
>  		return ret;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	intel_runtime_pm_get(dev_priv);
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> @@ -438,15 +421,10 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
>  	struct drm_minor *minor = dev_to_drm_minor(kdev);
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -	ret = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit);
> -	mutex_unlock(&dev_priv->rps.hw_lock);
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> +	return snprintf(buf, PAGE_SIZE, "%d\n",
> +			intel_gpu_freq(dev_priv,
> +				       dev_priv->rps.min_freq_softlimit));
>  }
>  
>  static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> @@ -463,8 +441,6 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	if (ret)
>  		return ret;
>  
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
>  	intel_runtime_pm_get(dev_priv);
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Some hardware requires a valid render context before it can initiate
> rc6 power gating of the GPU; the default state of the GPU is not
> sufficient and may lead to undefined behaviour. The first execution of
> any batch will load the "golden render state", at which point it is safe
> to enable rc6. As we do not forcibly load the kernel context at resume,
> we have to hook into the batch submission to be sure that the render
> state is setup before enabling rc6.
>
> However, since we don't enable powersaving until that first batch, we
> queued a delayed task in order to guarantee that the batch is indeed
> submitted.
>
> v2: Rearrange intel_disable_gt_powersave() to match.
> v3: Apply user specified cur_freq (or idle_freq if not set).
> v4: Give in, and supply a delayed work to autoenable rc6
> v5: Mika suggested a couple of better names for delayed_resume_work
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |   7 +-
>  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
>  drivers/gpu/drm/i915/i915_gem.c      |   1 +
>  drivers/gpu/drm/i915/intel_display.c |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  drivers/gpu/drm/i915/intel_pm.c      | 122 +++++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
>  7 files changed, 89 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b9a811750ca8..ff3342b78a74 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1343,7 +1343,7 @@ void i915_driver_unload(struct drm_device *dev)
>  	i915_destroy_error_state(dev);
>  
>  	/* Flush any outstanding unpin_work. */
> -	flush_workqueue(dev_priv->wq);
> +	drain_workqueue(dev_priv->wq);
>

For this to make sense, should we also use the dev priv workqueue
for the gt autoenabling?

-Mika


>  	intel_guc_fini(dev);
>  	i915_gem_fini(dev);
> @@ -1652,6 +1652,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_opregion_notify_adapter(dev_priv, PCI_D0);
>  
> +	intel_autoenable_gt_powersave(dev_priv);
>  	drm_kms_helper_poll_enable(dev);
>  
>  	enable_rpm_wakeref_asserts(dev_priv);
> @@ -1835,8 +1836,7 @@ int i915_reset(struct drm_i915_private *dev_priv)
>  	 * previous concerns that it doesn't respond well to some forms
>  	 * of re-init after reset.
>  	 */
> -	if (INTEL_INFO(dev)->gen > 5)
> -		intel_enable_gt_powersave(dev_priv);
> +	intel_autoenable_gt_powersave(dev_priv);
>  
>  	return 0;
>  
> @@ -2459,7 +2459,6 @@ static int intel_runtime_resume(struct device *device)
>  	 * we can do is to hope that things will still work (and disable RPM).
>  	 */
>  	i915_gem_init_swizzling(dev);
> -	gen6_update_ring_freq(dev_priv);
>  
>  	intel_runtime_pm_enable_interrupts(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bb59bc637f7b..8d74db096189 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1188,7 +1188,7 @@ struct intel_gen6_power_mgmt {
>  	bool client_boost;
>  
>  	bool enabled;
> -	struct delayed_work delayed_resume_work;
> +	struct delayed_work autoenable_work;
>  	unsigned boosts;
>  
>  	struct intel_rps_client semaphores, mmioflips;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index adeca0ec4cfb..6f2227b24711 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2842,6 +2842,7 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
>  	intel_runtime_pm_get_noresume(dev_priv);
>  	dev_priv->gt.awake = true;
>  
> +	intel_enable_gt_powersave(dev_priv);
>  	i915_update_gfx_val(dev_priv);
>  	if (INTEL_GEN(dev_priv) >= 6)
>  		gen6_rps_busy(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index be3b2cab2640..92cd0a70f8c8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15456,7 +15456,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  	dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
>  
>  	intel_init_clock_gating(dev);
> -	intel_enable_gt_powersave(dev_priv);
>  }
>  
>  /*
> @@ -16252,6 +16251,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  
> +	intel_suspend_gt_powersave(dev_priv);
>  	intel_disable_gt_powersave(dev_priv);
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 55aeaf041749..c68dd6166c52 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1697,7 +1697,9 @@ 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_autoenable_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_reset_gt_powersave(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 aab1e0b5d2eb..c6ba923425ce 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6549,13 +6549,6 @@ void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
>  		intel_runtime_pm_put(dev_priv);
>  }
>  
> -static void gen6_suspend_rps(struct drm_i915_private *dev_priv)
> -{
> -	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> -	gen6_disable_rps_interrupts(dev_priv);
> -}
> -
>  /**
>   * intel_suspend_gt_powersave - suspend PM work and helper threads
>   * @dev_priv: i915 device
> @@ -6569,50 +6562,65 @@ void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
>  	if (INTEL_GEN(dev_priv) < 6)
>  		return;
>  
> -	gen6_suspend_rps(dev_priv);
> +	cancel_delayed_work_sync(&dev_priv->rps.autoenable_work);
> +
> +	gen6_disable_rps_interrupts(dev_priv);
>  
>  	/* Force GPU to min freq during suspend */
>  	gen6_rps_idle(dev_priv);
>  }
>  
> +void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv)
> +{
> +	dev_priv->rps.enabled = true; /* force disabling */
> +	intel_disable_gt_powersave(dev_priv);
> +
> +	gen6_reset_rps_interrupts(dev_priv);
> +}
> +
>  void intel_disable_gt_powersave(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_IRONLAKE_M(dev_priv)) {
> -		ironlake_disable_drps(dev_priv);
> -	} else if (INTEL_INFO(dev_priv)->gen >= 6) {
> -		intel_suspend_gt_powersave(dev_priv);
> +	if (!READ_ONCE(dev_priv->rps.enabled))
> +		return;
>  
> -		mutex_lock(&dev_priv->rps.hw_lock);
> -		if (INTEL_INFO(dev_priv)->gen >= 9) {
> -			gen9_disable_rc6(dev_priv);
> -			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
> -			gen6_disable_rps(dev_priv);
> +	mutex_lock(&dev_priv->rps.hw_lock);
>  
> -		dev_priv->rps.enabled = false;
> -		mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		gen9_disable_rc6(dev_priv);
> +		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);
>  	}
> +
> +	dev_priv->rps.enabled = false;
> +	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> -static void intel_gen6_powersave_work(struct work_struct *work)
> +void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv =
> -		container_of(work, struct drm_i915_private,
> -			     rps.delayed_resume_work.work);
> +	/* We shouldn't be disabling as we submit, so this should be less
> +	 * racy than it appears!
> +	 */
> +	if (READ_ONCE(dev_priv->rps.enabled))
> +		return;
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> +	/* Powersaving is controlled by the host when inside a VM */
> +	if (intel_vgpu_active(dev_priv))
> +		return;
>  
> -	gen6_reset_rps_interrupts(dev_priv);
> +	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		cherryview_enable_rps(dev_priv);
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		valleyview_enable_rps(dev_priv);
> -	} else if (INTEL_INFO(dev_priv)->gen >= 9) {
> +	} else if (INTEL_GEN(dev_priv) >= 9) {
>  		gen9_enable_rc6(dev_priv);
>  		gen9_enable_rps(dev_priv);
>  		if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> @@ -6620,9 +6628,12 @@ static void intel_gen6_powersave_work(struct work_struct *work)
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		gen8_enable_rps(dev_priv);
>  		__gen6_update_ring_freq(dev_priv);
> -	} else {
> +	} else if (INTEL_GEN(dev_priv) >= 6) {
>  		gen6_enable_rps(dev_priv);
>  		__gen6_update_ring_freq(dev_priv);
> +	} else if (IS_IRONLAKE_M(dev_priv)) {
> +		ironlake_enable_drps(dev_priv);
> +		intel_init_emon(dev_priv);
>  	}
>  
>  	WARN_ON(dev_priv->rps.max_freq < dev_priv->rps.min_freq);
> @@ -6632,18 +6643,45 @@ static void intel_gen6_powersave_work(struct work_struct *work)
>  	WARN_ON(dev_priv->rps.efficient_freq > dev_priv->rps.max_freq);
>  
>  	dev_priv->rps.enabled = true;
> +	mutex_unlock(&dev_priv->rps.hw_lock);
> +}
>  
> -	gen6_enable_rps_interrupts(dev_priv);
> +static void __intel_autoenable_gt_powersave(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, typeof(*dev_priv), rps.autoenable_work.work);
> +	struct intel_engine_cs *rcs;
> +	struct drm_i915_gem_request *req;
>  
> -	mutex_unlock(&dev_priv->rps.hw_lock);
> +	if (READ_ONCE(dev_priv->rps.enabled))
> +		return;
> +
> +	rcs = &dev_priv->engine[RCS];
> +	if (rcs->last_context)
> +		return;
> +
> +	if (!rcs->init_context)
> +		return;
> +
> +	intel_runtime_pm_get(dev_priv);
> +	mutex_lock(&dev_priv->drm.struct_mutex);
>  
> +	req = i915_gem_request_alloc(rcs, dev_priv->kernel_context);
> +	if (IS_ERR(req))
> +		goto unlock;
> +
> +	if (!i915.enable_execlists && i915_switch_context(req) == 0)
> +		rcs->init_context(req);
> +	i915_add_request_no_flush(req);
> +
> +unlock:
> +	mutex_unlock(&dev_priv->drm.struct_mutex);
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> -void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
> +void intel_autoenable_gt_powersave(struct drm_i915_private *dev_priv)
>  {
> -	/* Powersaving is controlled by the host when inside a VM */
> -	if (intel_vgpu_active(dev_priv))
> +	if (READ_ONCE(dev_priv->rps.enabled))
>  		return;
>  
>  	if (IS_IRONLAKE_M(dev_priv)) {
> @@ -6664,8 +6702,8 @@ void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
>  		 * paths, so the _noresume version is enough (and in case of
>  		 * runtime resume it's necessary).
>  		 */
> -		if (schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
> -					   round_jiffies_up_relative(HZ)))
> +		if (schedule_delayed_work(&dev_priv->rps.autoenable_work,
> +					  round_jiffies_up_relative(HZ)))
>  			intel_runtime_pm_get_noresume(dev_priv);
>  	}
>  }
> @@ -6675,7 +6713,7 @@ void intel_reset_gt_powersave(struct drm_i915_private *dev_priv)
>  	if (INTEL_INFO(dev_priv)->gen < 6)
>  		return;
>  
> -	gen6_suspend_rps(dev_priv);
> +	gen6_disable_rps_interrupts(dev_priv);
>  	dev_priv->rps.enabled = false;
>  }
>  
> @@ -7785,8 +7823,8 @@ void intel_pm_setup(struct drm_device *dev)
>  	mutex_init(&dev_priv->rps.hw_lock);
>  	spin_lock_init(&dev_priv->rps.client_lock);
>  
> -	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
> -			  intel_gen6_powersave_work);
> +	INIT_DELAYED_WORK(&dev_priv->rps.autoenable_work,
> +			  __intel_autoenable_gt_powersave);
>  	INIT_LIST_HEAD(&dev_priv->rps.clients);
>  	INIT_LIST_HEAD(&dev_priv->rps.semaphores.link);
>  	INIT_LIST_HEAD(&dev_priv->rps.mmioflips.link);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index ff80a81b1a84..eeb4cbce19ff 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -435,7 +435,7 @@ void intel_uncore_sanitize(struct drm_i915_private *dev_priv)
>  	i915.enable_rc6 = sanitize_rc6_option(dev_priv, i915.enable_rc6);
>  
>  	/* BIOS often leaves RC6 enabled, but disable it for hw init */
> -	intel_disable_gt_powersave(dev_priv);
> +	intel_sanitize_gt_powersave(dev_priv);
>  }
>  
>  static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context

[Chris Wilson]

On Mon, Jul 11, 2016 at 05:14:22PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Some hardware requires a valid render context before it can initiate
> > rc6 power gating of the GPU; the default state of the GPU is not
> > sufficient and may lead to undefined behaviour. The first execution of
> > any batch will load the "golden render state", at which point it is safe
> > to enable rc6. As we do not forcibly load the kernel context at resume,
> > we have to hook into the batch submission to be sure that the render
> > state is setup before enabling rc6.
> >
> > However, since we don't enable powersaving until that first batch, we
> > queued a delayed task in order to guarantee that the batch is indeed
> > submitted.
> >
> > v2: Rearrange intel_disable_gt_powersave() to match.
> > v3: Apply user specified cur_freq (or idle_freq if not set).
> > v4: Give in, and supply a delayed work to autoenable rc6
> > v5: Mika suggested a couple of better names for delayed_resume_work
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c      |   7 +-
> >  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
> >  drivers/gpu/drm/i915/i915_gem.c      |   1 +
> >  drivers/gpu/drm/i915/intel_display.c |   2 +-
> >  drivers/gpu/drm/i915/intel_drv.h     |   2 +
> >  drivers/gpu/drm/i915/intel_pm.c      | 122 +++++++++++++++++++++++------------
> >  drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
> >  7 files changed, 89 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index b9a811750ca8..ff3342b78a74 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1343,7 +1343,7 @@ void i915_driver_unload(struct drm_device *dev)
> >  	i915_destroy_error_state(dev);
> >  
> >  	/* Flush any outstanding unpin_work. */
> > -	flush_workqueue(dev_priv->wq);
> > +	drain_workqueue(dev_priv->wq);
> >
> 
> For this to make sense, should we also use the dev priv workqueue
> for the gt autoenabling?

Sure. (That change was a bit of paranoia that stuck.) There's nothing
stopping the autoenable work from using the dev_priv->wq and that would
indeed make both it semantically cleaner.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Remove temporary RPM wakeref assert disables

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Now that the last couple of hacks have been removed from the runtime
> powermanagement users, we can fully enable the asserts.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c78fda491f0..26bfcccad57e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1665,13 +1665,6 @@ enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
>  	atomic_dec(&dev_priv->pm.wakeref_count);
>  }
>  
> -/* TODO: convert users of these to rely instead on proper RPM refcounting */
> -#define DISABLE_RPM_WAKEREF_ASSERTS(dev_priv)	\
> -	disable_rpm_wakeref_asserts(dev_priv)
> -
> -#define ENABLE_RPM_WAKEREF_ASSERTS(dev_priv)	\
> -	enable_rpm_wakeref_asserts(dev_priv)
> -
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
>  bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx