From: John Harrison <John.C.Harrison@xxxxxxxxx>
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 <John.C.Harrison@xxxxxxxxx>
---
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 6504689..1c3834fc 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 99fb2f0..a26bdf8 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;
}
