On Fri, Aug 05, 2016 at 10:05:38AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> > By applying the same logic as for wait-ioctl, we can query whether a
> > request has completed without holding struct_mutex. The biggest impact
> > system-wide is removing the flush_active and the contention that causes.
> >
> > Testcase: igt/gem_busy
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Akash Goel <akash.goel@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 110 +++++++++++++++++++++++++++++-----------
> > 1 file changed, 80 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 43069b05bdd2..f2f70f5ff9f4 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3721,49 +3721,99 @@ i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj,
> > i915_vma_unpin(i915_gem_obj_to_ggtt_view(obj, view));
> > }
> >
> > +static __always_inline unsigned
> > +__busy_read_flag(const struct drm_i915_gem_request *request)
> > +{
> > + return 0x10000 << request->engine->exec_id;
> > +}
> > +
> > +static __always_inline unsigned int
> > +__busy_write_flag(const struct drm_i915_gem_request *request)
> > +{
> > + return request->engine->exec_id;
>
> Just realized (to my horror) this is not a flag, it's a bare ID, so
> better not call the function _flag, but rather _id?
Bah.
__busy_write_id
__busy_read_flag
__busy_set_if_active
busy_set_active_write_id() { __busy_set_if_active(ptr, __busy_write_id}; )
busy_set_active_read_flag() { __busy_set_if_active(ptr, __busy_read_flag); }
> > + * but before we add its engine into the busy set, the other
> > + * thread reallocates it and assigns it to a task on another
> > + * engine with a fresh and incomplete seqno.
> > + *
> > + * So after we lookup the engine's id, we double check that
> > + * the active request is the same and only then do we add it
> > + * into the busy set.
> > + */
> > + rcu_read_lock();
> > +
> > + for_each_active(active, idx)
> > + args->busy |= busy_read_flag(&obj->last_read[idx]);
>
> So you mean this is double check against __I915_BO_ACTIVE, right?
Yes. The ABI guarantees forward progress but __I915_BO_ACTIVE itself
does not, so we confirm each of the active requests with the hardware.
Will add.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
