From dbe4646d6e3467adbe2ec8dbe6b7ceea57dee2ec Mon Sep 17 00:00:00 2001 From: John Harrison Date: Thu, 19 Mar 2015 12:30:09 +0000 Subject: [PATCH] drm/i915: Fix for ringbuf space wait in LRC mode The legacy and LRC code paths have an almost identical procedure for waiting for space in the ring buffer. They both search for a request in the free list that will advance the tail to a point where sufficient space is available. They then wait for that request, retire it and recalculate the free space value. Unfortunately, a bug in the LRC side meant that the resulting free space might not be as large as expected and indeed, might not be sufficient. This is because it was testing against the value of request->tail not request->postfix. Whereas, when a request is retired, ringbuf->tail is updated to req->postfix not req->tail. Another significant difference between the two is that the LRC one did not trust the wait for request to work! It redid the is there enough space available test and would fail the call if insufficient. Whereas, the legacy version just said 'return 0' - it assumed the preceeding code works. This difference meant that the LRC version still worked even with the bug - it just fell back to the polling wait path. For: VIZ-5115 Signed-off-by: John Harrison Reviewed-by: Thomas Daniel Reviewed-by: Tomas Elf Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++---- drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 6504689467b1..1c3834fc5608 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -634,7 +634,7 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, { struct intel_engine_cs *ring = ringbuf->ring; struct drm_i915_gem_request *request; - int ret; + int ret, new_space; if (intel_ring_space(ringbuf) >= bytes) return 0; @@ -650,10 +650,10 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, continue; /* Would completion of this request free enough space? */ - if (__intel_ring_space(request->tail, ringbuf->tail, - ringbuf->size) >= bytes) { + new_space = __intel_ring_space(request->postfix, ringbuf->tail, + ringbuf->size); + if (new_space >= bytes) break; - } } if (&request->list == &ring->request_list) @@ -665,6 +665,8 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, i915_gem_retire_requests_ring(ring); + WARN_ON(intel_ring_space(ringbuf) < new_space); + return intel_ring_space(ringbuf) >= bytes ? 0 : -ENOSPC; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 99fb2f06710a..a26bdf89e270 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2059,16 +2059,16 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) { struct intel_ringbuffer *ringbuf = ring->buffer; struct drm_i915_gem_request *request; - int ret; + int ret, new_space; if (intel_ring_space(ringbuf) >= n) return 0; list_for_each_entry(request, &ring->request_list, list) { - if (__intel_ring_space(request->postfix, ringbuf->tail, - ringbuf->size) >= n) { + new_space = __intel_ring_space(request->postfix, ringbuf->tail, + ringbuf->size); + if (new_space >= n) break; - } } if (&request->list == &ring->request_list) @@ -2080,6 +2080,8 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) i915_gem_retire_requests_ring(ring); + WARN_ON(intel_ring_space(ringbuf) < new_space); + return 0; } -- 2.20.1