[Intel-gfx] [PATCH 16/38] drm/i915: Introduce a context barrier callback

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-01 16:12:33)
> 
> On 01/03/2019 14:03, Chris Wilson wrote:
> > +     counter = 0;
> > +     err = context_barrier_task(ctx, 0, mock_barrier_task, &counter);
> > +     if (err) {
> > +             pr_err("Failed at line %d, err=%d\n", __LINE__, err);
> > +             goto out;
> > +     }
> > +     if (counter == 0) {
> > +             pr_err("Did not retire immediately with 0 engines\n");
> > +             err = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     counter = 0;
> > +     err = context_barrier_task(ctx, -1, mock_barrier_task, &counter);
> > +     if (err) {
> > +             pr_err("Failed at line %d, err=%d\n", __LINE__, err);
> > +             goto out;
> > +     }
> > +     if (counter == 0) {
> > +             pr_err("Did not retire immediately for all inactive engines\n");
> 
> Why would this one retire immediately? It will send requests down the 
> pipe, no? So don't you actually need to wait for the tracker to be 
> signalled and that counter == num_engines?

Nothing has used the context at this point, so we don't emit a request
on any engine, and the barrier can be immediately executed.

> > +             err = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     rq = ERR_PTR(-ENODEV);
> > +     with_intel_runtime_pm(i915, wakeref)
> > +             rq = i915_request_alloc(i915->engine[RCS], ctx);
> > +     if (IS_ERR(rq)) {
> > +             pr_err("Request allocation failed!\n");
> > +             goto out;
> > +     }
> > +     i915_request_add(rq);
> 
> Doesn't this need to go under the wakeref as well?

No, we only need the wakeref to start (that's only to avoid blocking
inside request_alloc --- yeah, that's not exactly how it works, we'll be
back later to fix that!). The request then carries the GT wakeref with it.

> > +     GEM_BUG_ON(list_empty(&ctx->active_engines));
> > +
> > +     counter = 0;
> > +     context_barrier_inject_fault = BIT(RCS);
> > +     err = context_barrier_task(ctx, -1, mock_barrier_task, &counter);
> > +     context_barrier_inject_fault = 0;
> > +     if (err == -ENXIO)
> > +             err = 0;
> > +     else
> > +             pr_err("Did not hit fault injection!\n");
> > +     if (counter != 0) {
> > +             pr_err("Invoked callback on error!\n");
> > +             err = -EIO;
> > +     }
> > +     if (err)
> > +             goto out;
> > +
> > +     counter = 0;
> > +     err = context_barrier_task(ctx, -1, mock_barrier_task, &counter);
> > +     if (err) {
> > +             pr_err("Failed at line %d, err=%d\n", __LINE__, err);
> > +             goto out;
> > +     }
> > +     mock_device_flush(i915);
> > +     if (counter == 0) {
> > +             pr_err("Did not retire on each active engines\n");
> > +             err = -EINVAL;
> > +             goto out;
> > +     }
> 
> This one is inline with my understanding, and the context_barrier_task 
> arguments are the same as the one above.. hm.. I am confused.

This time it is active, so we actually have to wait as the barrier waits
for the GPU before firing.
-Chris