[Intel-gfx] [PATCH] drm/i915: Keep ring->active_list and ring->requests_list consistent

[Daniel Vetter]

On Wed, Mar 18, 2015 at 06:19:22PM +0000, Chris Wilson wrote:
> If we retire requests last, we may use a later seqno and so clear
> the requests lists without clearing the active list, leading to
> confusion. Hence we should retire requests first for consistency with
> the early return. The order used to be important as the lifecycle for
> the object on the active list was determined by request->seqno. However,
> the requests themselves are now reference counted removing the
> constraint from the order of retirement.
> 
> Fixes regression from
> 
> commit 1b5a433a4dd967b125131da42b89b5cc0d5b1f57
> Author: John Harrison <John.C.Harrison at Intel.com>
> Date:   Mon Nov 24 18:49:42 2014 +0000
> 
>     drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed
> '
> 
> and a
> 
> 	WARNING: CPU: 0 PID: 1383 at drivers/gpu/drm/i915/i915_gem_evict.c:279 i915_gem_evict_vm+0x10c/0x140()
> 	WARN_ON(!list_empty(&vm->active_list))

How does this come about - we call gpu_idle before this seems to blow up,
so all requests should be completed? And I don't think we can blame this
on racy seqno signalling, since gpu_idle does all the waiting already ...

> Identified by updating WATCH_LISTS:
> 
> 	[drm:i915_verify_lists] *ERROR* blitter ring: active list not empty, but no requests
> 	WARNING: CPU: 0 PID: 681 at drivers/gpu/drm/i915/i915_gem.c:2751 i915_gem_retire_requests_ring+0x149/0x230()
> 	WARN_ON(i915_verify_lists(ring->dev))
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>

Since we've just discussed this on irc: Doesn't this now enshrine that
every bo needs to hold a full request?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch