Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling
@ 2014-11-19 13:30 Imre Deak
  2014-11-19 13:30 ` [PATCH v3 1/5] drm/i915: WARN if we receive any rps interrupts on gen>9 Imre Deak
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

This is the respin of the second half of [1] with the comments from
Paulo and Chris addressed and rebased on the latest -nightly.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2014-November/054942.html

Imre Deak (5):
  drm/i915: WARN if we receive any rps interrupts on gen>9
  drm/i915: move rps irq disable one level up
  drm/i915: sanitize rps irq enabling
  drm/i915: sanitize rps irq disabling
  drm/i915: disable rps irqs earlier during suspend/unload

 drivers/gpu/drm/i915/i915_drv.c      |  9 +++-----
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++-
 drivers/gpu/drm/i915/i915_irq.c      | 43 +++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_display.c |  6 ++---
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 37 +++++++++++++++++--------------
 6 files changed, 64 insertions(+), 38 deletions(-)

-- 
1.8.4

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

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

* [PATCH v3 1/5] drm/i915: WARN if we receive any rps interrupts on gen>9
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
@ 2014-11-19 13:30 ` Imre Deak
  2014-11-19 13:30 ` [PATCH v3 2/5] drm/i915: move rps irq disable one level up Imre Deak
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

This extends

commit 132f3f1767dbabfb01f3c9bd63098c65d91eeac9
Author: Imre Deak <imre.deak@intel.com>
Date:   Mon Nov 10 15:34:33 2014 +0200

    drm/i915: WARN if we receive any gen9 rps interrupts

to GEN>9 platforms as suggested by Paulo.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5e5d3f7..480b87b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1690,9 +1690,9 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe)
  * the work queue. */
 static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 {
-	/* TODO: RPS on GEN9 is not supported yet. */
-	if (WARN_ONCE(INTEL_INFO(dev_priv)->gen == 9,
-		      "GEN9: unexpected RPS IRQ\n"))
+	/* TODO: RPS on GEN9+ is not supported yet. */
+	if (WARN_ONCE(INTEL_INFO(dev_priv)->gen >= 9,
+		      "GEN9+: unexpected RPS IRQ\n"))
 		return;
 
 	if (pm_iir & dev_priv->pm_rps_events) {
-- 
1.8.4

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

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

* [PATCH v3 2/5] drm/i915: move rps irq disable one level up
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
  2014-11-19 13:30 ` [PATCH v3 1/5] drm/i915: WARN if we receive any rps interrupts on gen>9 Imre Deak
@ 2014-11-19 13:30 ` Imre Deak
  2014-11-19 13:30 ` [PATCH v3 3/5] drm/i915: sanitize rps irq enabling Imre Deak
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

We disable the RPS interrupts for all platforms at the same spot, so
move it one level up in the callstack to simplify things.

No functional change.

v2:
- rebase on the GEN9 patches where RPS isn't supported yet, so we don't
  need to disable RPS interrupts on it (Paulo)
v3:
- avoid disabling the interrupts on GEN>9 too (Paulo)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 28a4a0b..e4db2b5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4526,8 +4526,6 @@ static void gen6_disable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
-
-	gen6_disable_rps_interrupts(dev);
 }
 
 static void cherryview_disable_rps(struct drm_device *dev)
@@ -4535,8 +4533,6 @@ static void cherryview_disable_rps(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	I915_WRITE(GEN6_RC_CONTROL, 0);
-
-	gen6_disable_rps_interrupts(dev);
 }
 
 static void valleyview_disable_rps(struct drm_device *dev)
@@ -4550,8 +4546,6 @@ static void valleyview_disable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
-
-	gen6_disable_rps_interrupts(dev);
 }
 
 static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
@@ -6230,6 +6224,14 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 			valleyview_disable_rps(dev);
 		else
 			gen6_disable_rps(dev);
+
+		/*
+		 * TODO: disable RPS interrupts on GEN9+ too once RPS support
+		 * is added for it.
+		 */
+		if (INTEL_INFO(dev)->gen < 9)
+			gen6_disable_rps_interrupts(dev);
+
 		dev_priv->rps.enabled = false;
 		mutex_unlock(&dev_priv->rps.hw_lock);
 	}
-- 
1.8.4

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

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

* [PATCH v3 3/5] drm/i915: sanitize rps irq enabling
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
  2014-11-19 13:30 ` [PATCH v3 1/5] drm/i915: WARN if we receive any rps interrupts on gen>9 Imre Deak
  2014-11-19 13:30 ` [PATCH v3 2/5] drm/i915: move rps irq disable one level up Imre Deak
@ 2014-11-19 13:30 ` Imre Deak
  2014-11-19 13:30 ` [PATCH v3 4/5] drm/i915: sanitize rps irq disabling Imre Deak
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

Atm we first enable the RPS interrupts then we clear any pending ones.
By this we could lose an interrupt arriving after we unmasked it. This
may not be a problem as the caller should handle such a race, but logic
still calls for the opposite order. Also we can delay enabling the
interrupts until after all the RPS initialization is ready with the
following order:

1. disable left-over RPS (earlier via intel_uncore_sanitize)
2. clear any pending RPS interrupts
3. initialize RPS
4. enable RPS interrupts

This also allows us to do the 2. and 4. step the same way for all
platforms, so let's follow this order to simplifying things.

Also make sure any queued interrupts are also cleared.

v2:
- rebase on the GEN9 patches where we don't support RPS yet, so we
  musn't enable RPS interrupts on it (Paulo)
v3:
- avoid enabling RPS interrupts on GEN>9 too (Paulo)
- clarify the RPS init sequence in the log message (Chris)
- add POSTING_READ to gen6_reset_rps_interrupts() (Paulo)
- WARN if any PM_IIR bits are set in gen6_enable_rps_interrupts()
  (Paulo)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c  | 14 +++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 19 +++++++++++--------
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 480b87b..672b711 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -255,14 +255,26 @@ void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
 	snb_update_pm_irq(dev_priv, mask, 0);
 }
 
+void gen6_reset_rps_interrupts(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t reg = gen6_pm_iir(dev_priv);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+	I915_WRITE(reg, dev_priv->pm_rps_events);
+	I915_WRITE(reg, dev_priv->pm_rps_events);
+	POSTING_READ(reg);
+	spin_unlock_irq(&dev_priv->irq_lock);
+}
+
 void gen6_enable_rps_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON(dev_priv->rps.pm_iir);
+	WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
 	gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-	I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1f9b63..f0a46ec 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -791,6 +791,7 @@ void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
+void gen6_reset_rps_interrupts(struct drm_device *dev);
 void gen6_enable_rps_interrupts(struct drm_device *dev);
 void gen6_disable_rps_interrupts(struct drm_device *dev);
 void intel_runtime_pm_disable_interrupts(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 e4db2b5..e46feb6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4747,8 +4747,6 @@ static void gen8_enable_rps(struct drm_device *dev)
 
 	gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) & 0xff00) >> 8);
 
-	gen6_enable_rps_interrupts(dev);
-
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -4845,8 +4843,6 @@ static void gen6_enable_rps(struct drm_device *dev)
 	dev_priv->rps.power = HIGH_POWER; /* force a reset */
 	gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 
-	gen6_enable_rps_interrupts(dev);
-
 	rc6vids = 0;
 	ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
 	if (IS_GEN6(dev) && ret) {
@@ -5348,8 +5344,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
 
-	gen6_enable_rps_interrupts(dev);
-
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -5431,8 +5425,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
 
-	gen6_enable_rps_interrupts(dev);
-
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -6246,6 +6238,13 @@ static void intel_gen6_powersave_work(struct work_struct *work)
 
 	mutex_lock(&dev_priv->rps.hw_lock);
 
+	/*
+	 * TODO: reset/enable RPS interrupts on GEN9+ too, once RPS support is
+	 * added for it.
+	 */
+	if (INTEL_INFO(dev)->gen < 9)
+		gen6_reset_rps_interrupts(dev);
+
 	if (IS_CHERRYVIEW(dev)) {
 		cherryview_enable_rps(dev);
 	} else if (IS_VALLEYVIEW(dev)) {
@@ -6260,6 +6259,10 @@ static void intel_gen6_powersave_work(struct work_struct *work)
 		__gen6_update_ring_freq(dev);
 	}
 	dev_priv->rps.enabled = true;
+
+	if (INTEL_INFO(dev)->gen < 9)
+		gen6_enable_rps_interrupts(dev);
+
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	intel_runtime_pm_put(dev_priv);
-- 
1.8.4

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

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

* [PATCH v3 4/5] drm/i915: sanitize rps irq disabling
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
                   ` (2 preceding siblings ...)
  2014-11-19 13:30 ` [PATCH v3 3/5] drm/i915: sanitize rps irq enabling Imre Deak
@ 2014-11-19 13:30 ` Imre Deak
  2014-11-19 13:30 ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier during suspend/unload Imre Deak
  2014-11-19 14:03 ` [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Daniel Vetter
  5 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

When disabling the RPS interrupts there is a tricky dependency between
the thread disabling the interrupts, the RPS interrupt handler and the
corresponding RPS work. The RPS work can reenable the interrupts, so
there is no straightforward order in the disabling thread to (1) make
sure that any RPS work is flushed and to (2) disable all RPS
interrupts. Currently this is solved by masking the interrupts using two
separate mask registers (first level display IMR and PM IMR) and doing
the disabling when all first level interrupts are disabled.

This works, but the requirement to run with all first level interrupts
disabled is unnecessary making the suspend / unload time ordering of RPS
disabling wrt. other unitialization steps difficult and error prone.
Removing this restriction allows us to disable RPS early during suspend
/ unload and forget about it for the rest of the sequence. By adding a
more explicit method for avoiding the above race, it also becomes easier
to prove its correctness. Finally currently we can hit the WARN in
snb_update_pm_irq(), when a final RPS work runs with the first level
interrupts already disabled. This won't lead to any problem (due to the
separate interrupt masks), but with the change in this and the next
patch we can get rid of the WARN, while leaving it in place for other
scenarios.

To address the above points, add a new RPS interrupts_enabled flag and
use this during RPS disabling to avoid requeuing the RPS work and
reenabling of the RPS interrupts. Since the interrupt disabling happens
now in intel_suspend_gt_powersave(), we will disable RPS interrupts
explicitly during suspend (and not just through the first level mask),
but there is no problem doing so, it's also more consistent and allows
us to unify more of the RPS disabling during suspend and unload time in
the next patch.

v2/v3:
- rebase on patch "drm/i915: move rps irq disable one level up" in the
  patchset

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  6 +++++-
 drivers/gpu/drm/i915/i915_irq.c | 23 ++++++++++++++++-------
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++--------
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c4f2cb6..d8fc160 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -998,8 +998,12 @@ struct intel_rps_ei {
 };
 
 struct intel_gen6_power_mgmt {
-	/* work and pm_iir are protected by dev_priv->irq_lock */
+	/*
+	 * work, interrupts_enabled and pm_iir are protected by
+	 * dev_priv->irq_lock
+	 */
 	struct work_struct work;
+	bool interrupts_enabled;
 	u32 pm_iir;
 
 	/* Frequencies are stored in potentially platform dependent multiples.
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 672b711..fac489e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -274,6 +274,7 @@ void gen6_enable_rps_interrupts(struct drm_device *dev)
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON(dev_priv->rps.pm_iir);
 	WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
+	dev_priv->rps.interrupts_enabled = true;
 	gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -282,14 +283,16 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	spin_lock_irq(&dev_priv->irq_lock);
+	dev_priv->rps.interrupts_enabled = false;
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	cancel_work_sync(&dev_priv->rps.work);
+
 	I915_WRITE(GEN6_PMINTRMSK, INTEL_INFO(dev_priv)->gen >= 8 ?
 		   ~GEN8_PMINTR_REDIRECT_TO_NON_DISP : ~0);
 	I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
 				~dev_priv->pm_rps_events);
-	/* Complete PM interrupt masking here doesn't race with the rps work
-	 * item again unmasking PM interrupts because that is using a different
-	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
-	 * stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	dev_priv->rps.pm_iir = 0;
@@ -1136,6 +1139,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	int new_delay, adj;
 
 	spin_lock_irq(&dev_priv->irq_lock);
+	/* Speed up work cancelation during disabling rps interrupts. */
+	if (!dev_priv->rps.interrupts_enabled) {
+		spin_unlock_irq(&dev_priv->irq_lock);
+		return;
+	}
 	pm_iir = dev_priv->rps.pm_iir;
 	dev_priv->rps.pm_iir = 0;
 	/* Make sure not to corrupt PMIMR state used by ringbuffer on GEN6 */
@@ -1709,11 +1717,12 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 
 	if (pm_iir & dev_priv->pm_rps_events) {
 		spin_lock(&dev_priv->irq_lock);
-		dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
 		gen6_disable_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
+		if (dev_priv->rps.interrupts_enabled) {
+			dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
+			queue_work(dev_priv->wq, &dev_priv->rps.work);
+		}
 		spin_unlock(&dev_priv->irq_lock);
-
-		queue_work(dev_priv->wq, &dev_priv->rps.work);
 	}
 
 	if (INTEL_INFO(dev_priv)->gen >= 8)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e46feb6..9dc29fa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6186,9 +6186,17 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
 	/* Interrupts should be disabled already to avoid re-arming. */
 	WARN_ON(intel_irqs_enabled(dev_priv));
 
+	if (INTEL_INFO(dev)->gen < 6)
+		return;
+
 	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
 
-	cancel_work_sync(&dev_priv->rps.work);
+	/*
+	 * TODO: disable RPS interrupts on GEN9+ too once RPS support
+	 * is added for it.
+	 */
+	if (INTEL_INFO(dev)->gen < 9)
+		gen6_disable_rps_interrupts(dev);
 
 	/* Force GPU to min freq during suspend */
 	gen6_rps_idle(dev_priv);
@@ -6217,13 +6225,6 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 		else
 			gen6_disable_rps(dev);
 
-		/*
-		 * TODO: disable RPS interrupts on GEN9+ too once RPS support
-		 * is added for it.
-		 */
-		if (INTEL_INFO(dev)->gen < 9)
-			gen6_disable_rps_interrupts(dev);
-
 		dev_priv->rps.enabled = false;
 		mutex_unlock(&dev_priv->rps.hw_lock);
 	}
-- 
1.8.4

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

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

* [PATCH v3 5/5] drm/i915: disable rps irqs earlier during suspend/unload
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
                   ` (3 preceding siblings ...)
  2014-11-19 13:30 ` [PATCH v3 4/5] drm/i915: sanitize rps irq disabling Imre Deak
@ 2014-11-19 13:30 ` Imre Deak
  2014-11-20  3:56   ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier shuang.he
  2014-11-19 14:03 ` [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Daniel Vetter
  5 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2014-11-19 13:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

After the previous patch RPS disabling doesn't depend any more on the
first level interrupts being disabled, so we can move it everywhere
earlier. Doing so let's us think about the uninitialization steps
afterwards independently of any asynchronous RPS events that can happen
atm. It also makes the system/runtime suspend time RPS disabling more
uniform. Finally this gets rid of the WARN in
intel_suspend_gt_powersave(), which we can hit if a final RPS work runs
after we disabled the first level interrupts.

Testcase: igt/pm_rpm
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=82939
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 9 +++------
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 drivers/gpu/drm/i915/intel_pm.c      | 3 ---
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 122ea95..19558d6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -584,6 +584,8 @@ static int i915_drm_suspend(struct drm_device *dev)
 			return error;
 		}
 
+		intel_suspend_gt_powersave(dev);
+
 		/*
 		 * Disable CRTCs directly since we want to preserve sw state
 		 * for _thaw. Also, power gate the CRTC power wells.
@@ -595,15 +597,11 @@ static int i915_drm_suspend(struct drm_device *dev)
 
 		intel_dp_mst_suspend(dev);
 
-		flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
 		intel_runtime_pm_disable_interrupts(dev_priv);
 		intel_hpd_cancel_work(dev_priv);
 
 		intel_suspend_encoders(dev_priv);
 
-		intel_suspend_gt_powersave(dev);
-
 		intel_suspend_hw(dev);
 	}
 
@@ -1393,9 +1391,8 @@ static int intel_runtime_suspend(struct device *device)
 	i915_gem_release_all_mmaps(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-	intel_runtime_pm_disable_interrupts(dev_priv);
 	intel_suspend_gt_powersave(dev);
+	intel_runtime_pm_disable_interrupts(dev_priv);
 
 	ret = intel_suspend_complete(dev_priv);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 320bf4c..9de2f69 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13549,11 +13549,13 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector;
 
+	intel_disable_gt_powersave(dev);
+
 	intel_backlight_unregister(dev);
 
 	/*
 	 * Interrupts and polling as the first thing to avoid creating havoc.
-	 * Too much stuff here (turning of rps, connectors, ...) would
+	 * Too much stuff here (turning of connectors, ...) would
 	 * experience fancy races otherwise.
 	 */
 	intel_irq_uninstall(dev_priv);
@@ -13570,8 +13572,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_disable_fbc(dev);
 
-	intel_disable_gt_powersave(dev);
-
 	ironlake_teardown_rc6(dev);
 
 	mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9dc29fa..5cc0ebc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6183,9 +6183,6 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* Interrupts should be disabled already to avoid re-arming. */
-	WARN_ON(intel_irqs_enabled(dev_priv));
-
 	if (INTEL_INFO(dev)->gen < 6)
 		return;
 
-- 
1.8.4

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

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

* Re: [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling
  2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
                   ` (4 preceding siblings ...)
  2014-11-19 13:30 ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier during suspend/unload Imre Deak
@ 2014-11-19 14:03 ` Daniel Vetter
  5 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-11-19 14:03 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, paulo.r.zanoni

On Wed, Nov 19, 2014 at 03:30:00PM +0200, Imre Deak wrote:
> This is the respin of the second half of [1] with the comments from
> Paulo and Chris addressed and rebased on the latest -nightly.
> 
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2014-November/054942.html
> 
> Imre Deak (5):
>   drm/i915: WARN if we receive any rps interrupts on gen>9
>   drm/i915: move rps irq disable one level up
>   drm/i915: sanitize rps irq enabling
>   drm/i915: sanitize rps irq disabling
>   drm/i915: disable rps irqs earlier during suspend/unload
> 
>  drivers/gpu/drm/i915/i915_drv.c      |  9 +++-----
>  drivers/gpu/drm/i915/i915_drv.h      |  6 ++++-
>  drivers/gpu/drm/i915/i915_irq.c      | 43 +++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_display.c |  6 ++---
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 37 +++++++++++++++++--------------
>  6 files changed, 64 insertions(+), 38 deletions(-)

Merged the entire series, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 5/5] drm/i915: disable rps irqs earlier
  2014-11-19 13:30 ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier during suspend/unload Imre Deak
@ 2014-11-20  3:56   ` shuang.he
  0 siblings, 0 replies; 8+ messages in thread
From: shuang.he @ 2014-11-20  3:56 UTC (permalink / raw)
  To: shuang.he, intel-gfx, imre.deak

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              369/369              367/369
ILK              +13-8              384/403              389/403
SNB                 -1              459/459              458/459
IVB                 -5              535/545              530/545
BYT                 -1              290/290              289/290
HSW                 -4              610/610              606/610
BDW                 -1              451/451              450/451
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
PNV  igt_drv_hangman_error-state-capture-render      TIMEOUT(3, M25)PASS(4, M23)      TIMEOUT(1, M23)PASS(3, M23)
PNV  igt_drv_missed_irq_hang      TIMEOUT(6, M23M25)PASS(1, M23)      TIMEOUT(1, M23)PASS(3, M23)
ILK  igt_drv_hangman_error-state-basic      PASS(4, M26)      TIMEOUT(1, M26)PASS(3, M26)
ILK  igt_gem_reset_stats_ban-render      PASS(4, M26)      TIMEOUT(1, M26)PASS(3, M26)
ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      DMESG_WARN(2, M26)PASS(2, M26)
ILK  igt_kms_flip_bcs-wf_vblank-vs-dpms      DMESG_WARN(2, M26)PASS(2, M26)      NSPT(1, M26)PASS(3, M26)
ILK  igt_kms_flip_bcs-wf_vblank-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_blocking-wf_vblank      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_flip-vs-absolute-wf_vblank-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_flip-vs-dpms-off-vs-modeset-interruptible      DMESG_WARN(2, M26)PASS(2, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_flip_flip-vs-expired-vblank-interruptible      DMESG_WARN(2, M26)PASS(2, M26)      PASS(4, M26)
ILK  igt_kms_flip_flip-vs-modeset-vs-hang      DMESG_WARN(2, M26)PASS(2, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_flip_flip-vs-modeset-vs-hang-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_flip-vs-panning-vs-hang-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_flip-vs-wf_vblank      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_nonexisting-fb-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_plain-flip-interruptible      DMESG_WARN(1, M26)PASS(3, M26)      PASS(4, M26)
ILK  igt_kms_flip_rcs-flip-vs-modeset      PASS(4, M26)      DMESG_WARN(2, M26)PASS(2, M26)
ILK  igt_kms_flip_rcs-flip-vs-modeset-interruptible      PASS(4, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_flip_rcs-flip-vs-panning-interruptible      PASS(1, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible      PASS(1, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_flip_wf_vblank-interruptible      DMESG_WARN(1, M26)      PASS(4, M26)
ILK  igt_kms_flip_wf_vblank-vs-dpms      DMESG_WARN(1, M26)      DMESG_WARN(1, M26)PASS(3, M26)
ILK  igt_kms_setmode_invalid-clone-exclusive-crtc      PASS(1, M26)      DMESG_WARN(1, M26)PASS(3, M26)
SNB  igt_kms_pipe_crc_basic_hang-read-crc-pipe-A      PASS(4, M35)      TIMEOUT(1, M35)PASS(3, M35)
IVB  igt_drv_hangman_error-state-basic      TIMEOUT(3, M34)PASS(4, M21)      TIMEOUT(1, M21)PASS(3, M21)
IVB  igt_gem_reset_stats_ban-ctx-render      DMESG_WARN(2, M21)PASS(2, M21)      TIMEOUT(1, M21)PASS(3, M21)
IVB  igt_gem_reset_stats_ban-render      DMESG_WARN(2, M21)PASS(2, M21)      DMESG_WARN(1, M21)PASS(3, M21)
IVB  igt_gem_workarounds_reset      PASS(4, M21)      TIMEOUT(1, M21)PASS(3, M21)
IVB  igt_kms_cursor_crc_cursor-64x64-random      FAIL(1, M21)PASS(3, M21)      FAIL(1, M21)DMESG_WARN(1, M21)PASS(2, M21)
BYT  igt_drv_missed_irq_hang      TIMEOUT(3, M36)PASS(1, M36)      TIMEOUT(1, M36)PASS(3, M36)
HSW  igt_drv_hangman_error-state-basic      PASS(4, M19M40)      TIMEOUT(1, M40)PASS(3, M40)
HSW  igt_gem_reset_stats_ban-bsd      PASS(4, M19M40)      TIMEOUT(1, M40)PASS(3, M40)
HSW  igt_gem_workarounds_reset      PASS(4, M19M40)      TIMEOUT(1, M40)PASS(3, M40)
HSW  igt_pm_rps_min-max-config-idle      PASS(4, M19M40)      FAIL(1, M40)PASS(3, M40)
BDW  igt_drv_hangman_error-state-basic      TIMEOUT(3, M28)PASS(4, M28M30)      TIMEOUT(1, M30)PASS(3, M30)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2014-11-20  3:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-19 13:30 [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Imre Deak
2014-11-19 13:30 ` [PATCH v3 1/5] drm/i915: WARN if we receive any rps interrupts on gen>9 Imre Deak
2014-11-19 13:30 ` [PATCH v3 2/5] drm/i915: move rps irq disable one level up Imre Deak
2014-11-19 13:30 ` [PATCH v3 3/5] drm/i915: sanitize rps irq enabling Imre Deak
2014-11-19 13:30 ` [PATCH v3 4/5] drm/i915: sanitize rps irq disabling Imre Deak
2014-11-19 13:30 ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier during suspend/unload Imre Deak
2014-11-20  3:56   ` [PATCH v3 5/5] drm/i915: disable rps irqs earlier shuang.he
2014-11-19 14:03 ` [PATCH v3 0/5] sanitize RPS interrupt enabling/disabling Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox