[Intel-gfx] [PATCH 2/5] drm/i915: Allow asynchronous waits on the i915_active barriers

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-04-06 13:06:03)
> 
> On 06/04/2020 10:12, Chris Wilson wrote:
> > Allow the caller to also wait upon the barriers stored in i915_active.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_active.c | 60 ++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/i915/i915_active.h |  1 +
> >   2 files changed, 61 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> > index d5e24be759f7..048ab9edd2c2 100644
> > --- a/drivers/gpu/drm/i915/i915_active.c
> > +++ b/drivers/gpu/drm/i915/i915_active.c
> > @@ -542,6 +542,55 @@ static int __await_active(struct i915_active_fence *active,
> >       return 0;
> >   }
> >   
> > +struct wait_barrier {
> > +     struct wait_queue_entry base;
> > +     struct i915_active *ref;
> > +};
> > +
> > +static int
> > +barrier_wake(wait_queue_entry_t *wq, unsigned int mode, int flags, void *key)
> > +{
> > +     struct wait_barrier *wb = container_of(wq, typeof(*wb), base);
> > +
> > +     if (i915_active_is_idle(wb->ref)) { /* shared waitqueue, must check! */
> 
> Who shares it?

__var_waitqueue(ref) => uses a one of a set of global workqueues based
off hash(ref)

Or we add a wait_queue_head_t to active, but we would still need to
recheck as it may be reused as we are signaled.

> > +     if (flags & I915_ACTIVE_AWAIT_BARRIER) {
> > +             err = flush_lazy_signals(ref);
> > +             if (err)
> > +                     return err;
> > +
> > +             err = __await_barrier(ref, arg);
> > +             if (err)
> > +                     return err;
> >
> 
> Could have a single set of active_acquire_if_busy/release over the 
> previous and this new block. Not sure if that would help with any 
> atomicity concerns, or if there are such.

It would not affect correctness, it will just depend on taste.

>   +     }
> > +
> >       return 0;
> >   }
> >   
> > @@ -582,6 +641,7 @@ int i915_request_await_active(struct i915_request *rq,
> >                             struct i915_active *ref,
> >                             unsigned int flags)
> >   {
> > +     GEM_BUG_ON(flags & I915_ACTIVE_AWAIT_BARRIER);
> 
> Why is this an error?

Because I'm being lazy and not hooking up the correct signaling path.

Instead of signaling arg == fence, we would need &request->submit. Just
messy on how to pass down the details.

Maybe 
	return await_active(ref, flags, rq_await_fence, rq, &rq->submit);
and
	return await_active(ref, flags, sw_await_fence, fence, fence);

That seems better than I was expecting.
-Chris