Hi,
On 18/11/15 09:56, Chris Wilson wrote:
Limit busywaiting only to the request currently being processed by the
GPU. If the request is not currently being processed by the GPU, there
is a very low likelihood of it being completed within the 2 microsecond
spin timeout and so we will just be wasting CPU cycles.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 +++++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8afda459a26e..16095b95d2df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2190,7 +2190,7 @@ struct drm_i915_gem_request {
struct intel_engine_cs *ring;
/** GEM sequence number associated with this request. */
- uint32_t seqno;
+ uint32_t seqno, spin_seqno;
Comment needs splitting out.
And spin_seqno is not the best name, I think previous_ring_seqno would
be better. So it would immediately tell you what it is, and then at the
place which uses it it would also be clearer what is the criteria for
spinning.
/** Position in the ringbuffer of the start of the request */
u32 head;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 414150a0b8d5..af9ffa11ef44 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1193,9 +1193,14 @@ static int __i915_spin_request(struct drm_i915_gem_request *req, int state)
* takes to sleep on a request, on the order of a microsecond.
*/
- if (i915_gem_request_get_ring(req)->irq_refcount)
+ if (req->ring->irq_refcount)
return -EBUSY;
+ /* Only spin if we know the GPU is processing this request */
+ if (i915_seqno_passed(req->ring->get_seqno(req->ring, false),
+ req->spin_seqno))
+ return -EAGAIN;
+
timeout = local_clock_us(&cpu) + 2;
while (!need_resched()) {
if (i915_gem_request_completed(req, true))
@@ -2592,6 +2597,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
request->batch_obj = obj;
request->emitted_jiffies = jiffies;
+ request->spin_seqno = ring->last_submitted_seqno;
ring->last_submitted_seqno = request->seqno;
list_add_tail(&request->list, &ring->request_list);
Commit message says it will spin only on the request currently being
processed by the GPU but from here it looks like it will spin for any
request _queued up_ before the last?
For example if we have submitted 1, 2, 3 and 4 and GPU is currently
processing 1.
2 has spin_seqno 1.
3 has spin_seqno 2.
4 has spin_seqno 3.
ring->get_seqno is 0.
Wait on 1: seqno_passed(0, 0) = true -> wait
Wait on 2: seqno_passed(0, 1) = false -> spin
Wait on 3: seqno_passed(0, 2) = false -> spin
Wait on 4: seqno_passed(0, 3) = false -> spin
So it looks the opposite.
Or is it too early for me? :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
