Commit dbe4646d authored by John Harrison's avatar John Harrison Committed by Daniel Vetter

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: default avatarJohn Harrison <John.C.Harrison@Intel.com>
Reviewed-by: default avatarThomas Daniel <thomas.daniel@intel.com>
Reviewed-by: default avatarTomas Elf <tomas.elf@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 6689cb2b
......@@ -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,11 +650,11 @@ 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)
return -ENOSPC;
......@@ -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;
}
......
......@@ -2059,17 +2059,17 @@ 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)
return -ENOSPC;
......@@ -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;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment