On 03/20/2015 02:36 PM, Chris Wilson wrote:
This provides a nice boost to mesa in swap bound scenarios (as mesa
throttles itself to the previous frame and given the scenario that will
complete shortly). It will also provide a good boost to systems running
with semaphores disabled and so frequently waiting on the GPU as it
switches rings. In the most favourable of microbenchmarks, this can
increase performance by around 15% - though in practice improvements
will be marginal and rarely noticeable.
v2: Account for user timeouts
v3: Limit the spinning to a single jiffie (~1us) at most. On an
otherwise idle system, there is no scheduler contention and so without a
limit we would spin until the GPU is ready.
v4: Drop forcewake - the lazy coherent access doesn't require it, and we
have no reason to believe that the forcewake itself improves seqno
coherency - it only adds delay.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Cc: Eero Tamminen <eero.t.tamminen@xxxxxxxxx>
Cc: "Rantala, Valtteri" <valtteri.rantala@xxxxxxxxx>
Still against a toggle switch like a simple module parameter?
---
drivers/gpu/drm/i915/i915_gem.c | 44 +++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e17a254aac1..9988e65c1440 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1181,6 +1181,29 @@ static bool missed_irq(struct drm_i915_private *dev_priv,
return test_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings);
}
+static int __i915_spin_request(struct drm_i915_gem_request *rq)
+{
+ unsigned long timeout;
+
+ if (i915_gem_request_get_ring(rq)->irq_refcount)
+ return -EBUSY;
So if someone else is already waiting on this ring skip the spin and do
the sleep-wait.
That would mean earlier waiter didn't manage to spin to completion so
for subsequent ones does it make sense to try to spin? If we assume
waiters are arriving here in submission order then no, they should
proceed to sleep-wait. But if waiters are arriving here in random order,
and that is purely up to userspace I think, then I am not sure?
On the other hand if we allowed this "out-of-order waiters" that would
potentially be too much spinning so maybe it is better like it is.
+ timeout = jiffies + 1;
+ while (!need_resched()) {
+ if (i915_gem_request_completed(rq, true))
+ return 0;
+
+ if (time_after_eq(jiffies, timeout))
+ break;
+
+ cpu_relax_lowlatency();
+ }
+ if (i915_gem_request_completed(rq, false))
+ return 0;
> +
+ return -EAGAIN;
+}
+
/**
* __i915_wait_request - wait until execution of request has finished
* @req: duh!
@@ -1225,12 +1248,20 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
if (ring->id == RCS && INTEL_INFO(dev)->gen >= 6)
gen6_rps_boost(dev_priv, file_priv);
- if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring)))
- return -ENODEV;
-
/* Record current time in case interrupted by signal, or wedged */
trace_i915_gem_request_wait_begin(req);
before = ktime_get_raw_ns();
+
+ /* Optimistic spin for the next jiffie before touching IRQs */
+ ret = __i915_spin_request(req);
+ if (ret == 0)
+ goto out;
+
+ if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
+ ret = -ENODEV;
+ goto out;
+ }
+
for (;;) {
struct timer_list timer;
@@ -1279,14 +1310,15 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
destroy_timer_on_stack(&timer);
}
}
- now = ktime_get_raw_ns();
- trace_i915_gem_request_wait_end(req);
-
if (!irq_test_in_progress)
ring->irq_put(ring);
finish_wait(&ring->irq_queue, &wait);
+out:
+ now = ktime_get_raw_ns();
+ trace_i915_gem_request_wait_end(req);
+
if (timeout) {
s64 tres = *timeout - (now - before);
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
