[PATCH 31/88] drm/i915: Flush the RPS bottom-half when the GPU idles
Chris Wilson
chris at chris-wilson.co.uk
Mon Apr 18 22:45:10 UTC 2016
Make sure that the RPS bottom-half is flushed before we set the idle
frequency when we decide the GPU is idle. This should prevent any races
with the bottom-half and setting the idle frequency, and ensures that
the bottom-half is bounded by the GPU's rpm reference taken for when it
is active (i.e. between gen6_rps_busy() and gen6_rps_idle()).
v2: Avoid recursively using the i915->wq - RPS does not touch the
struct_mutex so has no place being on the ordered i915->wq.
v3: Enable/disable interrupts for RPS busy/idle in order to prevent
further HW access from RPS outside of the wakeref.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Imre Deak <imre.deak at intel.com>
Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
---
drivers/gpu/drm/i915/i915_drv.c | 1 -
drivers/gpu/drm/i915/i915_irq.c | 45 +++++++++++++++---------------------
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 6 ++---
drivers/gpu/drm/i915/intel_pm.c | 23 +++++++++---------
5 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 905ad1d12dbd..d0321c6a7eb3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1494,7 +1494,6 @@ static int intel_runtime_suspend(struct device *device)
intel_guc_suspend(dev);
- intel_suspend_gt_powersave(dev);
intel_runtime_pm_disable_interrupts(dev_priv);
ret = intel_suspend_complete(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 367026389120..17e321a0806b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,9 +336,8 @@ void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
__gen6_disable_pm_irq(dev_priv, mask);
}
-void gen6_reset_rps_interrupts(struct drm_device *dev)
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
i915_reg_t reg = gen6_pm_iir(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
@@ -349,14 +348,14 @@ void gen6_reset_rps_interrupts(struct drm_device *dev)
spin_unlock_irq(&dev_priv->irq_lock);
}
-void gen6_enable_rps_interrupts(struct drm_device *dev)
+void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
+ if (dev_priv->rps.interrupts_enabled)
+ return;
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);
+ WARN_ON_ONCE(dev_priv->rps.pm_iir);
+ WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
dev_priv->rps.interrupts_enabled = true;
I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
dev_priv->pm_rps_events);
@@ -382,17 +381,13 @@ u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask)
return mask;
}
-void gen6_disable_rps_interrupts(struct drm_device *dev)
+void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
+ if (!dev_priv->rps.interrupts_enabled)
+ return;
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);
-
- spin_lock_irq(&dev_priv->irq_lock);
I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0));
@@ -401,8 +396,15 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
~dev_priv->pm_rps_events);
spin_unlock_irq(&dev_priv->irq_lock);
+ synchronize_irq(dev_priv->dev->irq);
- synchronize_irq(dev->irq);
+ /* Now that we will not be generating any more work, flush any
+ * outsanding tasks. As we are called on the RPS idle path,
+ * we will reset the GPU to minimum frequencies, so the current
+ * state of the worker can be discarded.
+ */
+ cancel_work_sync(&dev_priv->rps.work);
+ gen6_reset_rps_interrupts(dev_priv);
}
/**
@@ -1102,13 +1104,6 @@ static void gen6_pm_rps_work(struct work_struct *work)
return;
}
- /*
- * The RPS work is synced during runtime suspend, we don't require a
- * wakeref. TODO: instead of disabling the asserts make sure that we
- * always hold an RPM reference while the work is running.
- */
- DISABLE_RPM_WAKEREF_ASSERTS(dev_priv);
-
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 */
@@ -1121,7 +1116,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
WARN_ON(pm_iir & ~dev_priv->pm_rps_events);
if ((pm_iir & dev_priv->pm_rps_events) == 0 && !client_boost)
- goto out;
+ return;
mutex_lock(&dev_priv->rps.hw_lock);
@@ -1176,8 +1171,6 @@ static void gen6_pm_rps_work(struct work_struct *work)
intel_set_rps(dev_priv->dev, new_delay);
mutex_unlock(&dev_priv->rps.hw_lock);
-out:
- ENABLE_RPM_WAKEREF_ASSERTS(dev_priv);
}
@@ -1621,7 +1614,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
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);
+ schedule_work(&dev_priv->rps.work);
}
spin_unlock(&dev_priv->irq_lock);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5fa1bdf26f25..caff656417c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16038,6 +16038,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_connector *connector;
+ intel_suspend_gt_powersave(dev);
intel_disable_gt_powersave(dev);
intel_backlight_unregister(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e13ce2290de7..484112dc6c7d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1026,9 +1026,9 @@ 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 gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
+void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv);
+void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv);
u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask);
void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
void intel_runtime_pm_enable_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 2a85aea3c177..b964c448bc03 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4518,17 +4518,24 @@ void gen6_rps_busy(struct drm_i915_private *dev_priv)
gen6_rps_reset_ei(dev_priv);
I915_WRITE(GEN6_PMINTRMSK,
gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
+
+ gen6_enable_rps_interrupts(dev_priv);
}
mutex_unlock(&dev_priv->rps.hw_lock);
}
void gen6_rps_idle(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = dev_priv->dev;
+ /* Flush our bottom-half so that it does not race with us
+ * setting the idle frequency and so that it is bounded by
+ * our rpm wakeref. And then disable the interrupts to stop any
+ * futher RPS reclocking whilst we are asleep.
+ */
+ gen6_disable_rps_interrupts(dev_priv);
mutex_lock(&dev_priv->rps.hw_lock);
if (dev_priv->rps.enabled) {
- if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
vlv_set_rps_idle(dev_priv);
else
gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq);
@@ -4566,7 +4573,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->rps.interrupts_enabled) {
dev_priv->rps.client_boost = true;
- queue_work(dev_priv->wq, &dev_priv->rps.work);
+ schedule_work(&dev_priv->rps.work);
}
spin_unlock_irq(&dev_priv->irq_lock);
@@ -6233,8 +6240,6 @@ static void gen6_suspend_rps(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
flush_delayed_work(&dev_priv->rps.delayed_resume_work);
-
- gen6_disable_rps_interrupts(dev);
}
/**
@@ -6265,8 +6270,6 @@ void intel_disable_gt_powersave(struct drm_device *dev)
if (IS_IRONLAKE_M(dev)) {
ironlake_disable_drps(dev);
} else if (INTEL_INFO(dev)->gen >= 6) {
- intel_suspend_gt_powersave(dev);
-
mutex_lock(&dev_priv->rps.hw_lock);
if (INTEL_INFO(dev)->gen >= 9)
gen9_disable_rps(dev);
@@ -6290,8 +6293,7 @@ static void intel_gen6_powersave_work(struct work_struct *work)
struct drm_device *dev = dev_priv->dev;
mutex_lock(&dev_priv->rps.hw_lock);
-
- gen6_reset_rps_interrupts(dev);
+ gen6_reset_rps_interrupts(dev_priv);
if (IS_CHERRYVIEW(dev)) {
cherryview_enable_rps(dev);
@@ -6317,9 +6319,6 @@ 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;
-
- gen6_enable_rps_interrupts(dev);
-
mutex_unlock(&dev_priv->rps.hw_lock);
intel_runtime_pm_put(dev_priv);
--
2.8.0.rc3
More information about the Intel-gfx-trybot
mailing list