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

[Daniel Vetter]

On Mon, Mar 23, 2015 at 09:15:02AM +0000, Chris Wilson wrote:
> On Mon, Mar 23, 2015 at 09:13:36AM +0000, Chris Wilson wrote:
> > On Mon, Mar 23, 2015 at 09:49:15AM +0100, Daniel Vetter wrote:
> > > In case it's burried too much in the thread:
> > > 
> > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > 
> > > Addadendum for the commit:
> > > 
> > > "Note that this is only a problem in evict_vm where the following happens
> > > after a retire_request has cleaned out all requests, but not all active
> > > bo:
> > > - intel_ring_idle called from i915_gpu_idle notices that no requests are
> > >   outstanding and immediately returns.
> > > - i915_gem_retire_requests_ring called from i915_gem_retire_requests also
> > >   immediately returns when there's no request, still leaving the bo on the
> > >   active list.
> > > - evict_vm hits the WARN_ON(!list_empty(&vm->active_list)) after evicting
> > >   all active objects that there's still stuff left that shouldn't be
> > >   there."
> > > 
> > > Chris, is that an accurate enough description for Jani to add to the
> > > patch?
> > 
> > Not quite. It affects everywhere where we rely on i915_gpu_idle() idling
> > the gpu (e.g. suspend), it is only the canary in i915_gem_evict_vm()
> > that first alerted us to the bug. Maybe we have some recent weird bugs in
> > suspend (or it may be too soon for those reports to start trickling in)?
> 
> I take that back. The gpu is idle (no more requests). It's just the
> bookkeeping that's wrong (and that only affects eviction iirc).
> 
> I stand corrected.

Yeah totally missed that tbh. But since we restore ggtt ptes on resume I
think even if we manage to leave a bunch of objects more behind we should
be fine. Worst-case there's more target area for stray writes from the
bios on S4 ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch