[Intel-gfx] [PATCH 033/190] drm/i915: Only start retire worker when idle
Chris Wilson
chris at chris-wilson.co.uk
Mon Jan 11 01:16:44 PST 2016
The retire worker is a low frequency task that makes sure we retire
outstanding requests if userspace is being lax. We only need to start it
once as it remains active until the GPU is idle, so do a cheap test
before the more expensive queue_work(). A consequence of this is that we
need correct locking in the worker to make the hot path of request
submission cheap. To keep the symmetry and keep hangcheck strictly bound
by the GPU's wakelock, we move the cancel_sync(hangcheck) to the idle
worker before dropping the wakelock.
v2: Guard against RCU fouling the breadcrumbs bottom-half whilst we kick
the waiter.
v3: Remove the wakeref assertion squelching (now we hold a wakeref for
the hangcheck, any rpm error there is genuine).
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=88437
---
drivers/gpu/drm/i915/i915_drv.c | 2 -
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 83 ++++++++++++++++++++----------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++
drivers/gpu/drm/i915/i915_irq.c | 16 +-----
drivers/gpu/drm/i915/intel_display.c | 29 -----------
6 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5160f1414de4..4c090f1cf69c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1490,8 +1490,6 @@ static int intel_runtime_suspend(struct device *device)
i915_gem_release_all_mmaps(dev_priv);
mutex_unlock(&dev->struct_mutex);
- cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
-
intel_guc_suspend(dev);
intel_suspend_gt_powersave(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7f021505e32f..9ec6f3e9e74d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2987,7 +2987,7 @@ int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno);
struct drm_i915_gem_request *
i915_gem_find_active_request(struct intel_engine_cs *ring);
-bool i915_gem_retire_requests(struct drm_device *dev);
+void i915_gem_retire_requests(struct drm_device *dev);
void i915_gem_retire_requests_ring(struct intel_engine_cs *ring);
static inline u32 i915_reset_counter(struct i915_gpu_error *error)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5ddb2ed0f785..3788fce136f3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2479,6 +2479,37 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
return 0;
}
+static void i915_gem_mark_busy(struct drm_i915_private *dev_priv)
+{
+ if (dev_priv->mm.busy)
+ return;
+
+ intel_runtime_pm_get_noresume(dev_priv);
+
+ i915_update_gfx_val(dev_priv);
+ if (INTEL_INFO(dev_priv)->gen >= 6)
+ gen6_rps_busy(dev_priv);
+
+ queue_delayed_work(dev_priv->wq,
+ &dev_priv->mm.retire_work,
+ round_jiffies_up_relative(HZ));
+
+ dev_priv->mm.busy = true;
+}
+
+static void i915_gem_mark_idle(struct drm_i915_private *dev_priv)
+{
+ dev_priv->mm.busy = false;
+
+ if (cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work))
+ intel_kick_waiters(dev_priv);
+
+ if (INTEL_INFO(dev_priv)->gen >= 6)
+ gen6_rps_idle(dev_priv);
+
+ intel_runtime_pm_put(dev_priv);
+}
+
/*
* NB: This function is not allowed to fail. Doing so would mean the the
* request is not being tracked for completion but the work itself is
@@ -2559,10 +2590,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
trace_i915_gem_request_add(request);
- queue_delayed_work(dev_priv->wq,
- &dev_priv->mm.retire_work,
- round_jiffies_up_relative(HZ));
- intel_mark_busy(dev_priv->dev);
+ i915_gem_mark_busy(dev_priv);
/* Sanity check that the reserved size was large enough. */
intel_ring_reserved_space_end(ringbuf);
@@ -2892,7 +2920,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
WARN_ON(i915_verify_lists(ring->dev));
}
-bool
+void
i915_gem_retire_requests(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2900,6 +2928,9 @@ i915_gem_retire_requests(struct drm_device *dev)
bool idle = true;
int i;
+ if (!dev_priv->mm.busy)
+ return;
+
for_each_ring(ring, dev_priv, i) {
i915_gem_retire_requests_ring(ring);
idle &= list_empty(&ring->request_list);
@@ -2918,8 +2949,6 @@ i915_gem_retire_requests(struct drm_device *dev)
mod_delayed_work(dev_priv->wq,
&dev_priv->mm.idle_work,
msecs_to_jiffies(100));
-
- return idle;
}
static void
@@ -2928,17 +2957,21 @@ i915_gem_retire_work_handler(struct work_struct *work)
struct drm_i915_private *dev_priv =
container_of(work, typeof(*dev_priv), mm.retire_work.work);
struct drm_device *dev = dev_priv->dev;
- bool idle;
/* Come back later if the device is busy... */
- idle = false;
if (mutex_trylock(&dev->struct_mutex)) {
- idle = i915_gem_retire_requests(dev);
+ i915_gem_retire_requests(dev);
mutex_unlock(&dev->struct_mutex);
}
- if (!idle) {
+
+ /* Keep the retire handler running until we are finally idle.
+ * We do not need to do this test under locking as in the worst-case
+ * we queue the retire worker once too often.
+ */
+ if (READ_ONCE(dev_priv->mm.busy)) {
i915_queue_hangcheck(dev_priv);
- queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work,
+ queue_delayed_work(dev_priv->wq,
+ &dev_priv->mm.retire_work,
round_jiffies_up_relative(HZ));
}
}
@@ -2952,25 +2985,23 @@ i915_gem_idle_work_handler(struct work_struct *work)
struct intel_engine_cs *ring;
int i;
- for_each_ring(ring, dev_priv, i)
- if (!list_empty(&ring->request_list))
- return;
+ if (!mutex_trylock(&dev->struct_mutex))
+ return;
- /* we probably should sync with hangcheck here, using cancel_work_sync.
- * Also locking seems to be fubar here, ring->request_list is protected
- * by dev->struct_mutex. */
+ if (!dev_priv->mm.busy)
+ goto out;
- intel_mark_idle(dev);
+ for_each_ring(ring, dev_priv, i) {
+ if (!list_empty(&ring->request_list))
+ goto out;
- if (mutex_trylock(&dev->struct_mutex)) {
- struct intel_engine_cs *ring;
- int i;
+ i915_gem_batch_pool_fini(&ring->batch_pool);
+ }
- for_each_ring(ring, dev_priv, i)
- i915_gem_batch_pool_fini(&ring->batch_pool);
+ i915_gem_mark_idle(dev_priv);
- mutex_unlock(&dev->struct_mutex);
- }
+out:
+ mutex_unlock(&dev->struct_mutex);
}
/**
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index b8186bd061c1..da1c6fe5b40e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1475,6 +1475,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
dispatch_flags |= I915_DISPATCH_RS;
}
+ /* Take a local wakeref for preparing to dispatch the execbuf as
+ * we expect to access the hardware fairly frequently in the
+ * process. Upon first dispatch, we acquire another prolonged
+ * wakeref that we hold until the GPU has been idle for at least
+ * 100ms.
+ */
intel_runtime_pm_get(dev_priv);
ret = i915_mutex_lock_interruptible(dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 502663f13cd8..8866e981bcba 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3047,13 +3047,6 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
if (!i915.enable_hangcheck)
return;
- /*
- * The hangcheck work is synced during runtime suspend, we don't
- * require a wakeref. TODO: instead of disabling the asserts make
- * sure that we hold a reference when this work is running.
- */
- DISABLE_RPM_WAKEREF_ASSERTS(dev_priv);
-
/* As enabling the GPU requires fairly extensive mmio access,
* periodically arm the mmio checker to see if we are triggering
* any invalid access.
@@ -3157,17 +3150,12 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
}
}
- if (rings_hung) {
- i915_handle_error(dev, true, "Ring hung");
- goto out;
- }
+ if (rings_hung)
+ return i915_handle_error(dev, true, "Ring hung");
/* Reset timer in case GPU hangs without another request being added */
if (busy_count)
i915_queue_hangcheck(dev_priv);
-
-out:
- ENABLE_RPM_WAKEREF_ASSERTS(dev_priv);
}
static void ibx_irq_reset(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de4d4a0d923a..8e646780c971 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10874,35 +10874,6 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
return mode;
}
-void intel_mark_busy(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (dev_priv->mm.busy)
- return;
-
- intel_runtime_pm_get(dev_priv);
- i915_update_gfx_val(dev_priv);
- if (INTEL_INFO(dev)->gen >= 6)
- gen6_rps_busy(dev_priv);
- dev_priv->mm.busy = true;
-}
-
-void intel_mark_idle(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!dev_priv->mm.busy)
- return;
-
- dev_priv->mm.busy = false;
-
- if (INTEL_INFO(dev)->gen >= 6)
- gen6_rps_idle(dev->dev_private);
-
- intel_runtime_pm_put(dev_priv);
-}
-
static void intel_crtc_destroy(struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
--
2.7.0.rc3
More information about the Intel-gfx
mailing list