All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/i915: Flush GT idle status upon reset
@ 2016-07-13  8:10 Chris Wilson
  2016-07-13  8:10 ` [PATCH 2/8] drm/i915: Preserve current RPS frequency across init Chris Wilson
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

Upon resetting the GPU, we force the engines to be idle by clearing
their request lists. However, I neglected to clear the GT active status
and so the next request following the reset was not marking the device
as busy again. (We had to wait until any outstanding retire worker
finally ran and cleared the active status.)

Fixes: 67d97da34917 ("drm/i915: Only start retire worker when idle")
Testcase: igt/pm_rps/reset
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index adeca0ec4cfb..9d8c26f42dee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3169,6 +3169,8 @@ static void i915_gem_reset_engine_cleanup(struct intel_engine_cs *engine)
 	}
 
 	intel_ring_init_seqno(engine, engine->last_submitted_seqno);
+
+	engine->i915->gt.active_engines &= ~intel_engine_flag(engine);
 }
 
 void i915_gem_reset(struct drm_device *dev)
@@ -3186,6 +3188,7 @@ void i915_gem_reset(struct drm_device *dev)
 
 	for_each_engine(engine, dev_priv)
 		i915_gem_reset_engine_cleanup(engine);
+	mod_delayed_work(dev_priv->wq, &dev_priv->gt.idle_work, 0);
 
 	i915_gem_context_reset(dev);
 
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/8] drm/i915: Preserve current RPS frequency across init
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-13  8:10 ` [PATCH 3/8] drm/i915: Perform static RPS frequency setup before userspace Chris Wilson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/8] drm/i915: Perform static RPS frequency setup before userspace
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
  2016-07-13  8:10 ` [PATCH 2/8] drm/i915: Preserve current RPS frequency across init Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-13  8:10 ` [PATCH 4/8] drm/i915: Move overclocking detection to alongside RPS frequency detection Chris Wilson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/8] drm/i915: Move overclocking detection to alongside RPS frequency detection
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
  2016-07-13  8:10 ` [PATCH 2/8] drm/i915: Preserve current RPS frequency across init Chris Wilson
  2016-07-13  8:10 ` [PATCH 3/8] drm/i915: Perform static RPS frequency setup before userspace Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-13  8:10 ` [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (2 preceding siblings ...)
  2016-07-13  8:10 ` [PATCH 4/8] drm/i915: Move overclocking detection to alongside RPS frequency detection Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-14 10:26   ` Mika Kuoppala
  2016-07-13  8:10 ` [PATCH 6/8] drm/i915: Remove superfluous powersave work flushing Chris Wilson
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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 e76cfe2e2471..fe85235367be 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/8] drm/i915: Remove superfluous powersave work flushing
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (3 preceding siblings ...)
  2016-07-13  8:10 ` [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-13  8:10 ` [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (4 preceding siblings ...)
  2016-07-13  8:10 ` [PATCH 6/8] drm/i915: Remove superfluous powersave work flushing Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-14 13:56   ` Mika Kuoppala
  2016-07-13  8:10 ` [PATCH 8/8] drm/i915: Hide gen6_update_ring_freq() Chris Wilson
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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
v6: Rebalance rpm_put around the autoenable task

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      |  11 +--
 drivers/gpu/drm/i915/i915_drv.h      |   2 +-
 drivers/gpu/drm/i915/i915_gem.c      |   3 +
 drivers/gpu/drm/i915/intel_display.c |   1 -
 drivers/gpu/drm/i915/intel_drv.h     |   3 +-
 drivers/gpu/drm/i915/intel_pm.c      | 136 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
 7 files changed, 94 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b9a811750ca8..c6cc01faaa36 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);
@@ -1458,8 +1458,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 
 	intel_guc_suspend(dev);
 
-	intel_suspend_gt_powersave(dev_priv);
-
 	intel_display_suspend(dev);
 
 	intel_dp_mst_suspend(dev);
@@ -1652,6 +1650,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);
@@ -1778,8 +1777,6 @@ int i915_reset(struct drm_i915_private *dev_priv)
 	unsigned reset_counter;
 	int ret;
 
-	intel_reset_gt_powersave(dev_priv);
-
 	mutex_lock(&dev->struct_mutex);
 
 	/* Clear any previous failed attempts at recovery. Time to try again. */
@@ -1835,8 +1832,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 +2455,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 fe85235367be..1fdca3bb7f33 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 9d8c26f42dee..67c3bffb700d 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);
@@ -4980,6 +4981,8 @@ i915_gem_suspend(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int ret = 0;
 
+	intel_suspend_gt_powersave(dev_priv);
+
 	mutex_lock(&dev->struct_mutex);
 	ret = i915_gem_wait_for_idle(dev_priv);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be3b2cab2640..88589b5bde25 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);
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6c8485c3a44f..c036dfdffe0d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1690,10 +1690,11 @@ 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);
 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);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aab1e0b5d2eb..c77ec106a93c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6536,6 +6536,8 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
 	dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
 
 	mutex_unlock(&dev_priv->rps.hw_lock);
+
+	intel_autoenable_gt_powersave(dev_priv);
 }
 
 void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
@@ -6549,13 +6551,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 +6564,63 @@ void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) < 6)
 		return;
 
-	gen6_suspend_rps(dev_priv);
+	if (cancel_delayed_work_sync(&dev_priv->rps.autoenable_work))
+		intel_runtime_pm_put(dev_priv);
 
-	/* Force GPU to min freq during suspend */
-	gen6_rps_idle(dev_priv);
+	/* gen6_rps_idle() will be called later to disable interrupts */
+}
+
+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,47 @@ 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))
+		goto out;
+
+	rcs = &dev_priv->engine[RCS];
+	if (rcs->last_context)
+		goto out;
+
+	if (!rcs->init_context)
+		goto out;
 
+	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);
+
+	/* Mark the device busy, calling intel_enable_gt_powersave() */
+	i915_add_request_no_flush(req);
+
+unlock:
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+out:
 	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,21 +6704,13 @@ 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 (queue_delayed_work(dev_priv->wq,
+				       &dev_priv->rps.autoenable_work,
+				       round_jiffies_up_relative(HZ)))
 			intel_runtime_pm_get_noresume(dev_priv);
 	}
 }
 
-void intel_reset_gt_powersave(struct drm_i915_private *dev_priv)
-{
-	if (INTEL_INFO(dev_priv)->gen < 6)
-		return;
-
-	gen6_suspend_rps(dev_priv);
-	dev_priv->rps.enabled = false;
-}
-
 static void ibx_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -7785,8 +7817,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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/8] drm/i915: Hide gen6_update_ring_freq()
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (5 preceding siblings ...)
  2016-07-13  8:10 ` [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
@ 2016-07-13  8:10 ` Chris Wilson
  2016-07-13  8:48 ` ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset Patchwork
  2016-07-14  7:53 ` [PATCH 1/8] " Joonas Lahtinen
  8 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13  8:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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.

Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: 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 c036dfdffe0d..57738bae1a28 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1695,7 +1695,6 @@ 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 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 c77ec106a93c..fa6b341c2792 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (6 preceding siblings ...)
  2016-07-13  8:10 ` [PATCH 8/8] drm/i915: Hide gen6_update_ring_freq() Chris Wilson
@ 2016-07-13  8:48 ` Patchwork
  2016-07-13 10:25   ` Mika Kuoppala
  2016-07-14  7:53 ` [PATCH 1/8] " Joonas Lahtinen
  8 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2016-07-13  8:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/8] drm/i915: Flush GT idle status upon reset
URL   : https://patchwork.freedesktop.org/series/9800/
State : failure

== Summary ==

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

Test gem_sync:
        Subgroup basic-store-each:
                pass       -> FAIL       (ro-bdw-i7-5600u)

ro-bdw-i5-5250u  total:237  pass:213  dwarn:1   dfail:0   fail:7   skip:16 
ro-bdw-i7-5557U  total:237  pass:213  dwarn:2   dfail:0   fail:7   skip:15 
ro-bdw-i7-5600u  total:237  pass:198  dwarn:0   dfail:0   fail:8   skip:31 
ro-bsw-n3050     total:217  pass:172  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:237  pass:191  dwarn:0   dfail:0   fail:8   skip:38 
ro-hsw-i3-4010u  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
ro-hsw-i7-4770r  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
ro-skl3-i5-6260u total:237  pass:217  dwarn:1   dfail:0   fail:7   skip:12 
ro-snb-i7-2620M  total:237  pass:188  dwarn:0   dfail:0   fail:8   skip:41 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1480/

9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest
8419e2847 drm/i915: Hide gen6_update_ring_freq()
53bd541 drm/i915: Defer enabling rc6 til after we submit the first batch/context
0d19866 drm/i915: Remove superfluous powersave work flushing
81ce63d drm/i915: Define a separate variable and control for RPS waitboost frequency
d347718 drm/i915: Move overclocking detection to alongside RPS frequency detection
560aadd drm/i915: Perform static RPS frequency setup before userspace
57e5573 drm/i915: Preserve current RPS frequency across init
7628e53 drm/i915: Flush GT idle status upon reset

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset
  2016-07-13  8:48 ` ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset Patchwork
@ 2016-07-13 10:25   ` Mika Kuoppala
  2016-07-13 10:30     ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2016-07-13 10:25 UTC (permalink / raw)
  To: Patchwork, Chris Wilson; +Cc: intel-gfx

Patchwork <patchwork@emeril.freedesktop.org> writes:

> == Series Details ==
>
> Series: series starting with [1/8] drm/i915: Flush GT idle status upon reset
> URL   : https://patchwork.freedesktop.org/series/9800/
> State : failure
>
> == Summary ==
>
> Series 9800v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9800/revisions/1/mbox
>
> Test gem_sync:
>         Subgroup basic-store-each:
>                 pass       -> FAIL       (ro-bdw-i7-5600u)
>

Seems to have happened first with nightly 649

-Mika

> ro-bdw-i5-5250u  total:237  pass:213  dwarn:1   dfail:0   fail:7   skip:16 
> ro-bdw-i7-5557U  total:237  pass:213  dwarn:2   dfail:0   fail:7   skip:15 
> ro-bdw-i7-5600u  total:237  pass:198  dwarn:0   dfail:0   fail:8   skip:31 
> ro-bsw-n3050     total:217  pass:172  dwarn:0   dfail:0   fail:2   skip:42 
> ro-byt-n2820     total:237  pass:191  dwarn:0   dfail:0   fail:8   skip:38 
> ro-hsw-i3-4010u  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
> ro-hsw-i7-4770r  total:237  pass:206  dwarn:0   dfail:0   fail:7   skip:24 
> ro-ilk-i7-620lm  total:237  pass:166  dwarn:0   dfail:0   fail:8   skip:63 
> ro-ilk1-i5-650   total:232  pass:166  dwarn:0   dfail:0   fail:8   skip:58 
> ro-ivb-i7-3770   total:237  pass:197  dwarn:0   dfail:0   fail:7   skip:33 
> ro-skl3-i5-6260u total:237  pass:217  dwarn:1   dfail:0   fail:7   skip:12 
> ro-snb-i7-2620M  total:237  pass:188  dwarn:0   dfail:0   fail:8   skip:41 
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1480/
>
> 9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest
> 8419e2847 drm/i915: Hide gen6_update_ring_freq()
> 53bd541 drm/i915: Defer enabling rc6 til after we submit the first batch/context
> 0d19866 drm/i915: Remove superfluous powersave work flushing
> 81ce63d drm/i915: Define a separate variable and control for RPS waitboost frequency
> d347718 drm/i915: Move overclocking detection to alongside RPS frequency detection
> 560aadd drm/i915: Perform static RPS frequency setup before userspace
> 57e5573 drm/i915: Preserve current RPS frequency across init
> 7628e53 drm/i915: Flush GT idle status upon reset
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset
  2016-07-13 10:25   ` Mika Kuoppala
@ 2016-07-13 10:30     ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-13 10:30 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Wed, Jul 13, 2016 at 01:25:26PM +0300, Mika Kuoppala wrote:
> Patchwork <patchwork@emeril.freedesktop.org> writes:
> 
> > == Series Details ==
> >
> > Series: series starting with [1/8] drm/i915: Flush GT idle status upon reset
> > URL   : https://patchwork.freedesktop.org/series/9800/
> > State : failure
> >
> > == Summary ==
> >
> > Series 9800v1 Series without cover letter
> > http://patchwork.freedesktop.org/api/1.0/series/9800/revisions/1/mbox
> >
> > Test gem_sync:
> >         Subgroup basic-store-each:
> >                 pass       -> FAIL       (ro-bdw-i7-5600u)
> >
> 
> Seems to have happened first with nightly 649

It's also being sporadically, FAIL not DMESG-FAIL is interesting as it
implies not a missed interrupt...
-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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] drm/i915: Flush GT idle status upon reset
  2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
                   ` (7 preceding siblings ...)
  2016-07-13  8:48 ` ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset Patchwork
@ 2016-07-14  7:53 ` Joonas Lahtinen
  2016-07-14  7:59   ` Chris Wilson
  8 siblings, 1 reply; 16+ messages in thread
From: Joonas Lahtinen @ 2016-07-14  7:53 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Mika Kuoppala

On ke, 2016-07-13 at 09:10 +0100, Chris Wilson wrote:
> Upon resetting the GPU, we force the engines to be idle by clearing
> their request lists. However, I neglected to clear the GT active status
> and so the next request following the reset was not marking the device
> as busy again. (We had to wait until any outstanding retire worker
> finally ran and cleared the active status.)
> 
> Fixes: 67d97da34917 ("drm/i915: Only start retire worker when idle")
> Testcase: igt/pm_rps/reset
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

No FDO bug associated?

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index adeca0ec4cfb..9d8c26f42dee 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3169,6 +3169,8 @@ static void i915_gem_reset_engine_cleanup(struct intel_engine_cs *engine)
>  	}
>  
>  	intel_ring_init_seqno(engine, engine->last_submitted_seqno);
> +
> +	engine->i915->gt.active_engines &= ~intel_engine_flag(engine);
>  }
>  
>  void i915_gem_reset(struct drm_device *dev)
> @@ -3186,6 +3188,7 @@ void i915_gem_reset(struct drm_device *dev)
>  
>  	for_each_engine(engine, dev_priv)
>  		i915_gem_reset_engine_cleanup(engine);
> +	mod_delayed_work(dev_priv->wq, &dev_priv->gt.idle_work, 0);
>  
>  	i915_gem_context_reset(dev);
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] drm/i915: Flush GT idle status upon reset
  2016-07-14  7:53 ` [PATCH 1/8] " Joonas Lahtinen
@ 2016-07-14  7:59   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-14  7:59 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx, Mika Kuoppala

On Thu, Jul 14, 2016 at 10:53:20AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-13 at 09:10 +0100, Chris Wilson wrote:
> > Upon resetting the GPU, we force the engines to be idle by clearing
> > their request lists. However, I neglected to clear the GT active status
> > and so the next request following the reset was not marking the device
> > as busy again. (We had to wait until any outstanding retire worker
> > finally ran and cleared the active status.)
> > 
> > Fixes: 67d97da34917 ("drm/i915: Only start retire worker when idle")
> > Testcase: igt/pm_rps/reset
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> No FDO bug associated?

Unlikely, I had to rewrite the test case to hit it reliably.
-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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency
  2016-07-13  8:10 ` [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
@ 2016-07-14 10:26   ` Mika Kuoppala
  0 siblings, 0 replies; 16+ messages in thread
From: Mika Kuoppala @ 2016-07-14 10:26 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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>

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

> ---
>  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 e76cfe2e2471..fe85235367be 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context
  2016-07-13  8:10 ` [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
@ 2016-07-14 13:56   ` Mika Kuoppala
  2016-07-14 14:06     ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Mika Kuoppala @ 2016-07-14 13:56 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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
> v6: Rebalance rpm_put around the autoenable task
>
> 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>

Ta for splitting stuff out from this patch(set) to smaller
bits. I think it's all done now.

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  11 +--
>  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
>  drivers/gpu/drm/i915/i915_gem.c      |   3 +
>  drivers/gpu/drm/i915/intel_display.c |   1 -
>  drivers/gpu/drm/i915/intel_drv.h     |   3 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 136 +++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_uncore.c  |   2 +-
>  7 files changed, 94 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b9a811750ca8..c6cc01faaa36 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);
> @@ -1458,8 +1458,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>  	intel_guc_suspend(dev);
>  
> -	intel_suspend_gt_powersave(dev_priv);
> -
>  	intel_display_suspend(dev);
>  
>  	intel_dp_mst_suspend(dev);
> @@ -1652,6 +1650,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);
> @@ -1778,8 +1777,6 @@ int i915_reset(struct drm_i915_private *dev_priv)
>  	unsigned reset_counter;
>  	int ret;
>  
> -	intel_reset_gt_powersave(dev_priv);
> -
>  	mutex_lock(&dev->struct_mutex);
>  
>  	/* Clear any previous failed attempts at recovery. Time to try again. */
> @@ -1835,8 +1832,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 +2455,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 fe85235367be..1fdca3bb7f33 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 9d8c26f42dee..67c3bffb700d 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);
> @@ -4980,6 +4981,8 @@ i915_gem_suspend(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	int ret = 0;
>  
> +	intel_suspend_gt_powersave(dev_priv);
> +
>  	mutex_lock(&dev->struct_mutex);
>  	ret = i915_gem_wait_for_idle(dev_priv);
>  	if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index be3b2cab2640..88589b5bde25 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);
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6c8485c3a44f..c036dfdffe0d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1690,10 +1690,11 @@ 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);
>  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);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index aab1e0b5d2eb..c77ec106a93c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6536,6 +6536,8 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  	dev_priv->rps.boost_freq = dev_priv->rps.max_freq;
>  
>  	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	intel_autoenable_gt_powersave(dev_priv);
>  }
>  
>  void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -6549,13 +6551,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 +6564,63 @@ void intel_suspend_gt_powersave(struct drm_i915_private *dev_priv)
>  	if (INTEL_GEN(dev_priv) < 6)
>  		return;
>  
> -	gen6_suspend_rps(dev_priv);
> +	if (cancel_delayed_work_sync(&dev_priv->rps.autoenable_work))
> +		intel_runtime_pm_put(dev_priv);
>  
> -	/* Force GPU to min freq during suspend */
> -	gen6_rps_idle(dev_priv);
> +	/* gen6_rps_idle() will be called later to disable interrupts */
> +}
> +
> +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,47 @@ 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))
> +		goto out;
> +
> +	rcs = &dev_priv->engine[RCS];
> +	if (rcs->last_context)
> +		goto out;
> +
> +	if (!rcs->init_context)
> +		goto out;
>  
> +	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);
> +
> +	/* Mark the device busy, calling intel_enable_gt_powersave() */
> +	i915_add_request_no_flush(req);
> +
> +unlock:
> +	mutex_unlock(&dev_priv->drm.struct_mutex);
> +out:
>  	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,21 +6704,13 @@ 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 (queue_delayed_work(dev_priv->wq,
> +				       &dev_priv->rps.autoenable_work,
> +				       round_jiffies_up_relative(HZ)))
>  			intel_runtime_pm_get_noresume(dev_priv);
>  	}
>  }
>  
> -void intel_reset_gt_powersave(struct drm_i915_private *dev_priv)
> -{
> -	if (INTEL_INFO(dev_priv)->gen < 6)
> -		return;
> -
> -	gen6_suspend_rps(dev_priv);
> -	dev_priv->rps.enabled = false;
> -}
> -
>  static void ibx_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -7785,8 +7817,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context
  2016-07-14 13:56   ` Mika Kuoppala
@ 2016-07-14 14:06     ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2016-07-14 14:06 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Thu, Jul 14, 2016 at 04:56:50PM +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
> > v6: Rebalance rpm_put around the autoenable task
> >
> > 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>
> 
> Ta for splitting stuff out from this patch(set) to smaller
> bits.

It was only in the other because I was waiting for you to come back from
holiday! (And trying to progress the other means sending all patches up
to that point so that CI has a hope of sanity checking them.)
-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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-07-14 14:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-13  8:10 [PATCH 1/8] drm/i915: Flush GT idle status upon reset Chris Wilson
2016-07-13  8:10 ` [PATCH 2/8] drm/i915: Preserve current RPS frequency across init Chris Wilson
2016-07-13  8:10 ` [PATCH 3/8] drm/i915: Perform static RPS frequency setup before userspace Chris Wilson
2016-07-13  8:10 ` [PATCH 4/8] drm/i915: Move overclocking detection to alongside RPS frequency detection Chris Wilson
2016-07-13  8:10 ` [PATCH 5/8] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
2016-07-14 10:26   ` Mika Kuoppala
2016-07-13  8:10 ` [PATCH 6/8] drm/i915: Remove superfluous powersave work flushing Chris Wilson
2016-07-13  8:10 ` [PATCH 7/8] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
2016-07-14 13:56   ` Mika Kuoppala
2016-07-14 14:06     ` Chris Wilson
2016-07-13  8:10 ` [PATCH 8/8] drm/i915: Hide gen6_update_ring_freq() Chris Wilson
2016-07-13  8:48 ` ✗ Ro.CI.BAT: failure for series starting with [1/8] drm/i915: Flush GT idle status upon reset Patchwork
2016-07-13 10:25   ` Mika Kuoppala
2016-07-13 10:30     ` Chris Wilson
2016-07-14  7:53 ` [PATCH 1/8] " Joonas Lahtinen
2016-07-14  7:59   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.