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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx