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.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
