[Intel-gfx] [PATCH 02/11] drm/i915: Refactor activity tracking for requests

[Chris Wilson]

On Wed, Dec 16, 2015 at 05:16:19PM +0000, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 14/12/15 11:36, Chris Wilson wrote:
> >With the introduction of requests, we amplified the number of atomic
> >refcounted objects we use and update every execbuffer; from none to
> >several references, and a set of references that need to be changed. We
> >also introduced interesting side-effects in the order of retiring
> >requests and objects.
> >
> >Instead of independently tracking the last request for an object, track
> >the active objects for each request. The object will reside in the
> >buffer list of its most recent active request and so we reduce the kref
> >interchange to a list_move. Now retirements are entirely driven by the
> >request, dramatically simplifying activity tracking on the object
> >themselves, and removing the ambiguity between retiring objects and
> >retiring requests.
> >
> >All told, less code, simpler and faster, and more extensible.
> 
> I get the general idea but it seems unfinished. For example there is
> no handling for last_write and last_fence. So as for bisectability
> requirement it does not seem appropriate.

Pardon. It is there, working and complete. If it wasn't for rebasing, it
would have ~10 months of testing behind it. Plus comparable
implementations in userspace.

last_fence is no a no-op (we just need to track the request, we don't
need to anything special when it completes, I provided a stub callback
for the rare case rather than have a conditional function call),
last_write just needs to update the status on the framebuffer object.
 
> I also wanted to suggest splitting out the req->list to link
> renaming but see you've already done it in your branch.
> 
> I915_BO_ACTIVE_SHIFT is undefined up to and including this patch.

The curse of rebasing.

> Variable naming conventions for requests is still a mess but whatever.

I know, I am slowly fixing it up.
 
> And I don't like drm_i915_gem_request_active, wouldn't
> drm_i915_gem_active_request be better?

Yeah, active_request is better. Still lacks something.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre