Quoting Tvrtko Ursulin (2019-06-05 11:40:27) > > On 04/06/2019 16:24, Chris Wilson wrote: > > The intent was to skip unused HW contexts by checking ce->state. > > However, this only works for execlists where the ppGTT pointers is > > stored inside the HW context. For gen7, the ppGTT is alongside the > > logical state and must be updated on all active engines but, crucially, > > only on active engines. As we need different checks, and to keep > > context_barrier_task() agnostic, pass in the predicate. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110836 > > Fixes: 62c8e423450d ("drm/i915: Skip unused contexts for context_barrier_task()") > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 15 ++++++++++++++- > > .../drm/i915/gem/selftests/i915_gem_context.c | 19 +++++++++++++++---- > > 2 files changed, 29 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > index 08721ef62e4e..6819b598d226 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > @@ -902,6 +902,7 @@ static void cb_retire(struct i915_active *base) > > I915_SELFTEST_DECLARE(static intel_engine_mask_t context_barrier_inject_fault); > > static int context_barrier_task(struct i915_gem_context *ctx, > > intel_engine_mask_t engines, > > + bool (*skip)(struct intel_context *ce, void *data), > > int (*emit)(struct i915_request *rq, void *data), > > void (*task)(void *data), > > void *data) > > @@ -931,7 +932,10 @@ static int context_barrier_task(struct i915_gem_context *ctx, > > break; > > } > > > > - if (!(ce->engine->mask & engines) || !ce->state) > > + if (!(ce->engine->mask & engines)) > > + continue; > > + > > + if (skip && skip(ce, data)) > > continue; > > > > rq = intel_context_create_request(ce); > > @@ -1058,6 +1062,14 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data) > > return 0; > > } > > > > +static bool skip_ppgtt_update(struct intel_context *ce, void *data) > > +{ > > + if (HAS_LOGICAL_RING_CONTEXTS(ce->engine->i915)) > > + return !ce->state; > > + else > > + return !atomic_read(&ce->pin_count); > > Would "return !atomic_read(&ce->pin_count) || !ce->state;" work and > avoid the somewhat irky HAS_LOGICAL_RING_CONTEXTS check? No, because we need the barrier on gen7 !rcs which doesn't have ce->state (but does need to switch mm). -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
