Re: [PATCH 02/11] drm/i915: Refactor activity tracking for requests

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

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.

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

And I don't like drm_i915_gem_request_active, wouldn't 
drm_i915_gem_active_request be better?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx