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 <John.C.Harrison@Intel.com>
Reviewed-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Tomas Elf <tomas.elf@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6504689467b15ffda1d0e4ef2d93ba483c20e813..1c3834fc5608e27bb1501bd62f59037f38e3fb8e 100644 (file)
--- 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 99fb2f06710ac4ec771a2c94db7a634e0a8edd1a..a26bdf89e270a7fea993cb85a0a30b068202bd31 100644 (file)
--- 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;
 }