[Intel-gfx] [PATCH] drm/i915: Bug fixes to ring 'head' updating

[Chris Wilson]

On Mon, Nov 03, 2014 at 01:29:04PM +0000, Dave Gordon wrote:
> Fixes to both the LRC and the legacy ringbuffer code to correctly
> calculate and update the available space in a ring.
> 
> The logical ring code was updating the software ring 'head' value
> by reading the hardware 'HEAD' register. In LRC mode, this is not
> valid as the hardware is not necessarily executing the same context
> that is being processed by the software. Thus reading the h/w HEAD
> could put an unrelated (undefined, effectively random) value into
> the s/w 'head' -- A Bad Thing for the free space calculations.
> 
> In addition, the old code could update a ringbuffer's 'head' value
> from the 'last_retired_head' even when the latter hadn't been recently
> updated and therefore had a value of -1; this would also confuse the
> freespace calculations. Now, we consume 'last_retired_head' in just
> one place, ensuring that this confusion does not arise.

What? Not unless someone had broken it...

This is the code I expect to see here based on requests:

static int ring_wait(struct intel_ringbuffer *ring, int n)
{
        int ret;

        trace_intel_ringbuffer_wait(ring, n);

        do {
                struct i915_gem_request *rq;

                i915_gem_retire_requests__engine(ring->engine);
                if (ring->retired_head != -1) {
                        ring->head = ring->retired_head;
                        ring->retired_head = -1;

                        ring->space = intel_ring_space(ring);
                        if (ring->space >= n)
                                return 0;
                }

                list_for_each_entry(rq, &ring->breadcrumbs, breadcrumb_link)
                        if (__intel_ring_space(rq->tail, ring->tail,
                                               ring->size, I915_RING_RSVD) >= n)
                                break;

                if (WARN_ON(&rq->breadcrumb_link == &ring->breadcrumbs))
                        return -EDEADLK;

                ret = i915_request_wait(rq);
        } while (ret == 0);

        return ret;
}

and that works for both execlists and regular...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre