Quoting Tvrtko Ursulin (2019-04-11 14:05:19)
>
> On 10/04/2019 17:18, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-04-10 16:32:18)
> >> Yep as mentioned somewhere above. I definitely think another helper
> >> would help code base redability. Even if called unimaginatively as
> >> __i915_request_add.
> >
> > It is just these two (based on a quick grep).
> >
> > Maybe intel_context_create_request() ?
> >
> > Hmm, but isn't that what i915_request_create() is. Quiet!
>
> Why not __i915_request_add? i915_request_add would then be just wedged
> check calling __i915_request_add, no? (Going from memory.. might not be
> reliable.)
At the moment, we have:
__i915_request_create(): no timeline locking, no runtime-pm
i915_request_create(): no pinning, does timeline + runtime-pm
intel_context_create_request(): pin + call i915_request_create
igt_request_alloc(): lookup intel_context + call intel_context_create_request
I suppose there is some room in bringing i915_request_alloc() back from
the dead, but it's not the simple allocator one would expect from the
name. (igt_request_alloc is just for ease of recognition.)
Outside of igt_request_alloc(), there are two users for
intel_context_create_request() in my tree. One in
__intel_engine_record_defaults() and the other in
context_barrier_task().
For the in-kernel blitter tasks, we'll either be using a pinned kernel
context, or be shoehorning it into a pinned user context. Both should be
using i915_request_create() (as currently planned). And things like the
heartbeat and idle barriers, use the pinned kernel context.
Just to say intel_context_create_request() is the odd one out, and
deserves the longest name :)
> >>> +struct i915_gem_engines {
> >>> + struct rcu_work rcu;
> >>> + struct drm_i915_private *i915;
> >>> + unsigned long num_engines;
> >>
> >> unsigned int?
> >
> > Pointer before, array of pointers after, left an unsigned long hole.
> >
> > I was just filling the space.
>
> Well long makes me think there's a reason int is not enough. Which in
> this case there isn't.
And I thought you were planning for a busy future, full of little
engines :)
> So I would still go for an int regardless of the
> hole or not. There is nothing to be gained by filling space. Could even
> be worse if some instructions expand to longer opcodes. :)
Hmm, in the near future this becomes
struct i915_gem_engines {
struct rcu_head rcu;
unsigned long num_engines;
struct intel_context *engines[];
};
struct rcu_head is a pair of pointers, so that's still a pointer sized
hole. I give in. We'll only support 4 billion engines.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
