[Intel-gfx] [PATCH 10/10] drm/i915: Flush idle barriers when waiting

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-10-14 14:08:12)
> 
> On 11/10/2019 16:11, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-10-11 15:56:35)
> >>
> >> On 10/10/2019 08:14, Chris Wilson wrote:
> >>> If we do find ourselves with an idle barrier inside our active while
> >>> waiting, attempt to flush it by emitting a pulse using the kernel
> >>> context.
> >>
> >> The point of this one completely escapes me at the moment. Idle barriers
> >> are kept in there to be consumed by the engine_pm parking, so if any
> >> random waiter finds some (there will always be some, as long as the
> >> engine executed some user context, right?),
> > 
> > Not any random waiter; the waiter has to be waiting on a context that
> > was active and so setup a barrier.
> > 
> >> why would it want to handle
> >> them? Again just to use the opportunity for some house keeping? But what
> >> if the system is otherwise quite busy and a low-priority client just
> >> happens to want to wait on something silly?
> > 
> > There's no guarantee that it will ever be flushed. So why wouldn't we
> > use a low priority request to give a semblance of forward progress and
> > give a guarantee that the wait will complete.
> > 
> > It's a hypothetical point, there are no waiters that need to wait upon
> > their own barriers at present. We are just completing the picture for
> > idle barrier tracking.
> 
> Hm I was mistakenly remembering things like rpcs reconfiguration would 
> wait on ce->active, but I forgot about your trick with putting kernel 
> context request on an user timeline.
> 
> I guess it is fine there, but since, and as you have said, it is 
> hypothetical, then this patch is dead code and can wait.

Ok, I have a use for this now! In "drm/i915: Allow userspace to specify
ringsize on construction" we need to wait on the context itself to idle,
i.e. i915_active_wait(&ce->active) and so now it is possible for us to
be waiting on an idle_barrier() and so the flush be beneficial.

static int __apply_ringsize(struct intel_context *ce, void *sz)
{
       int err;

       err = i915_active_wait(&ce->active);
       if (err < 0)
               return err;

       if (intel_context_lock_pinned(ce))
               return -EINTR;

       if (intel_context_is_pinned(ce)) {
               err = -EBUSY; /* In active use, come back later! */
               goto unlock;
       }

       if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
               struct intel_ring *ring;

               /* Replace the existing ringbuffer */
               ring = intel_engine_create_ring(ce->engine,
                                               (unsigned long)sz);
               if (IS_ERR(ring)) {
                       err = PTR_ERR(ring);
                       goto unlock;
               }

               intel_ring_put(ce->ring);
               ce->ring = ring;

               /* Context image will be updated on next pin */
       } else {
               ce->ring = sz;
       }

unlock:
       intel_context_unlock_pinned(ce);
       return err;
}

-Chris