[Intel-gfx] [PATCH 1/2] drm/i915/oa: Reconfigure contexts on the fly

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-08 13:24:39)
> 
> On 08/07/2019 13:16, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-07-08 13:10:34)
> >>
> >> On 08/07/2019 12:19, Chris Wilson wrote:
> >>> Avoid a global idle barrier by reconfiguring each context by rewriting
> >>> them with MI_STORE_DWORD from the kernel context.
> >>>
> >>> v2: We only need to determine the desired register values once, they are
> >>> the same for all contexts.
> >>>
> >>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +
> >>>    drivers/gpu/drm/i915/gt/intel_lrc.c         |   7 +-
> >>>    drivers/gpu/drm/i915/i915_perf.c            | 248 +++++++++++++++-----
> >>>    3 files changed, 195 insertions(+), 62 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> index e367dce2a696..1f0d10bb88c1 100644
> >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> @@ -624,7 +624,9 @@ i915_gem_context_create_kernel(struct drm_i915_private *i915, int prio)
> >>>        ctx->sched.priority = I915_USER_PRIORITY(prio);
> >>>        ctx->ring_size = PAGE_SIZE;
> >>>    
> >>> +     /* Isolate the kernel context from prying eyes and sticky fingers */
> >>>        GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> >>> +     list_del_init(&ctx->link);
> >>
> >> Why exactly?
> > 
> > Otherwise we recursively try to modify the context.
> 
>  From gen8_configure_all_contexts, twice, or really recursively? If 
> former isn't that solvable by simply skipping kernel contexts in the 
> first loop?
> 
> >> Any repercussions for i915_sysfs/i915_l3_write? debugfs I gather you
> >> won't care about?
> > 
> > No, because what matters for those is user contexts.
> 
> There isn't some time cost associated with l3_remap calls when switching 
> contexts?

No, it's even weirder than that as it is not a context register (at
least on hsw where we support it). I guess, we should just unlazy and emit
a request from the sysfs.

> >> Should adding to i915->contexts.list be done from gem_context_register?
> >> What remains not a kernel context, but on a list?
> > 
> > And I also plan to pull it under its own mutex.
> >   
> >> Won't preempt context be missed in re-configuration?
> > 
> > What preempt-context? :-p And I would skip kernel_context if I could, but
> > for whatever reason oa events are being sent to userspace even while the
> > GPU is idle and igt expects the regular tick.
> 
> My tree still has it? Is it out of date? Solution with keeping it on the 
> list and skipping sounds more future proof if doable.

It's only used by guc for the guc command to preempt-to-idle. It should
not be recorded as an oa event but an inter-context blip (which is
exactly what it is).

Similarly, I also think oa should not prying into the kernel context
(even though it's not used for much at present).
-Chris