[Intel-gfx] [PATCH v2] drm/i915/execlists: Inhibit context save/restore for the fake preempt context

[Michel Thierry]

On 1/24/2018 12:46 PM, Chris Wilson wrote:
> Quoting Chris Wilson (2018-01-24 09:44:45)
>> Quoting Michel Thierry (2018-01-24 01:24:25)
>>> On 1/23/2018 1:04 PM, Chris Wilson wrote:
>>>> We only use the preempt context to inject an idle point into execlists.
>>>> We never need to reference its logical state, so tell the GPU never to
>>>> load it or save it.
>>>>
>>>> v2: BIT(2) for save-inhibit.
>>>>
>>>> N.B. Daniele mentioned this bit mbz for ICL, and has been moved into the
>>>> submission process rather than the context image.
>>>>
>>>> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>>>> Cc: Michel Thierry <michel.thierry at intel.com>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
>>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_lrc.c | 4 ++++
>>>>    drivers/gpu/drm/i915/intel_lrc.h | 1 +
>>>>    2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>>> index 22d471a4228d..c28f267a8417 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>> @@ -2285,6 +2285,10 @@ populate_lr_context(struct i915_gem_context *ctx,
>>>>        if (!engine->default_state)
>>>>                regs[CTX_CONTEXT_CONTROL + 1] |=
>>>>                        _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
>>>> +     if (ctx->hw_id == PREEMPT_ID)
>>>> +             regs[CTX_CONTEXT_CONTROL + 1] |=
>>>> +                     _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
>>>> +                                        CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
>>>>
>>>
>>> This shouldn't break anything and ICL is not merged yet (plus things may
>>> still change) so if you want to merge this,
>>
>> I think it's the right thing conceptually to do.
> 
> Uhoh, seeing GPU hangs on Broxton when thrashing preemption.
> Is the bit still valid?

It should be valid, but nobody used cxt save inhibit until now (it 
doesn't surprise bxt is doing something different).