[Intel-gfx] [RFC PATCH 00/11] Refactor HW workaround code

[Oscar Mateo]

On 10/10/2017 10:37 AM, Oscar Mateo wrote:
>
>
> On 10/09/2017 02:08 PM, Chris Wilson wrote:
>> Quoting Oscar Mateo (2017-10-09 21:58:15)
>>> Currently, deciding how/where to apply new workarounds is 
>>> challenging. Often,
>>> workarounds end up applied incorrectly and get lost under certain 
>>> circumstances
>>> (e.g. a context switch or a GPU reset). This is a proposal to 
>>> attempt to
>>> eliminate some of this pain, by clarifying the current 
>>> classification of
>>> workarounds (context saved/restored, global registers, whitelisting, 
>>> BB),
>>> putting them together on the same file, and improving the existing 
>>> validation
>>> infrastructure (debugfs/i-g-t).
>> One thing I've been dreaming of is if we can have an external file for
>> importing the w/a (reg offset + corrected value) that we could source
>> directly from spec. (Hoping for some xml translation to C or DT.)
>
> Hmmm... I'm afraid this is impossible at the moment, since many WAs in 
> the BSpec are simply a link to the ticket where the workaround was 
> devised (and there you have to parse the conversation to figure out 
> what the WA should do).
>
>> We need something like this so that we can set all the nonpriv registers
>> to the default value in the proto-context. Or at least lots of patience
>> and careful proofreading.
>> -Chris
>
> You mean applying the workarounds directly to the context image 
> instead of the LRI commands we use now? That can be done (as you said, 
> with *a lot* of patience and careful proofreading) but we would need 
> to force the proto-context to be restored first (with 
> CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT bit set), because the workarounds 
> do not always give us the full default value for a register, only the 
> fields that are wrong in the hardware.

Hmmmm... I'm giving out conflicting information. Registers in the engine 
(those that live in the engine part of the context image) are not 
guaranteed to be initialized, so inhibit-restoring the context and then 
saving it is not enough. Yes, we would have to get the default values 
for them from the BSpec, either automatically or by hand, then apply the 
workarounds (either by modifying the default values or with the emission 
of LRIs we have now).

This is an insane amount of work (and sometimes the BSpec does not 
include all the information you need), that's why the recommendation in 
the BSpec is to force the engine to populate these registers for us (the 
infamous null state batchbuffer):

"[...] Engine context starts immediately following the logical ring 
context in memory. This state is very specific to an engine and differs 
from engine to engine. This part of the context consists of the state 
from all the units in the engine that needs to be save/restored across 
context switches. Engine restores the engine context following the 
logical ring context restore. It is tedious for software to populate the 
engine context as per the requirements, it is recommended to implicitly 
use engine to populate this portion of the context. Below method can be 
followed to achieve the same:

* When a context is submitted for the first time for execution, SW can 
inhibit engine from restoring engine context by setting the “Engine 
Context Restore Inhibit” bit in CTXT_SR_CTL register of the logical ring 
context. This will avoid software from populating the Engine Context. 
Software must program all the state required to initialize the engine in 
the ring buffer which would initialize the hardware state. On a 
subsequent context save engine will populate the engine context with 
appropriate values.

* Above method can be used to create a complete logical context with 
engine context populated by the hardware. This Logical context can be 
used as an Golden Context Image or template for subsequently created 
contexts.

Engine saves the engine context following the logical ring context on 
switching out a context."