On 11/01/16 22:49, Chris Wilson wrote:
On Mon, Jan 11, 2016 at 05:32:27PM +0000, Tvrtko Ursulin wrote:
+struct i915_gem_active {
+ struct drm_i915_gem_request *request;
+};
+
+static inline void
+i915_gem_request_mark_active(struct drm_i915_gem_request *request,
+ struct i915_gem_active *active)
+{
+ i915_gem_request_assign(&active->request, request);
+}
This function name bothers me since I think the name is misleading
and unintuitive. It is not marking a request as active but
associating it with the second data structure.
Maybe i915_gem_request_move_to_active to keep the mental association
with the well established vma_move_to_active ?
That's backwards imo, since it is the i915_gem_active that gets added to
the request. (Or at least will be.)
Maybe struct i915_gem_active could also be better called
i915_gem_active_list ?
It's not a list but a node. I started with drm_i915_gem_request_node,
but that's too unwieldy and I felt even more confusing.
It is not ideal because of the future little reversal of who is in
who's list, so maybe there is something even better. But I think an
intuitive name is really important for code clarity and
maintainability.
In userspace, I have the request (which is actually a userspace fence
itself) containing a list of fences (that are identical to i915_gem_active,
they track which request contains the reference and a callback for
signalling) and those fences have a direct correspondence to,
ARB_sync_objects, for example. But we already have plenty of conflict
regarding the term fence, so that's no go.
i915_gem_active, for me, made the association with the active-reference
tracking that is ingrained into the objects and beyond. I quite like the
connection with GPU activity
i915_gem_retire_notifier? Hmm, I still like how
i915_gem_active.request != NULL is quite self-descriptive.
Yes the last bit is neat.
Perhaps then leave the structure name as is and just rename the function
to i915_gem_request_assign_active? I think that describes better what is
actually happening.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
