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

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-11 08:23:37)
> 
> On 08/07/2019 14:39, Chris Wilson wrote:
> > 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).
> 
> Will there be a separate blitter context?

Yes, no. I will try to push as much of the migration into the culprit,
but there will always be a need for kswapd/kcompactd for background
cleanup and a get-out-of-jail. That should be the kernel_context as far
as I am concerned.

> OA can handle the changing config between kernel and other contexts?

Imo, the less spying on things it is not privileged to spy on the better.
I have no idea how stable it is, the code gives a very underwhelming
impression.
-Chris