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

[Tvrtko Ursulin]

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?

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

Regards,

Tvrtko