Re: [PATCH 1/2] drm/i915/bdw: Apply workarounds using the golden render state

["Siluvery, Arun" <arun.siluvery@linux.intel.com>]

On 26/08/2014 13:53, Daniel Vetter wrote:
> On Fri, Aug 22, 2014 at 01:10:26PM +0100, Siluvery, Arun wrote:
>> On 22/08/2014 12:06, Mika Kuoppala wrote:
>>> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
>>>
>>>> On Wed, Aug 20, 2014 at 03:19:17PM +0100, Arun Siluvery wrote:
>>>>> Workarounds for bdw are currently applied in init_clock_gating() but they
>>>>> are lost following a gpu reset. Some of the WA registers are part of register
>>>>> state context and they are restored with every context switch so initializing
>>>>> them in golden render state ensures that they are applied even when we start
>>>>> with an uninitialized context or during hw initlialization followed by a reset.
>>>>>
>>>>> v2: Add comments corresponding to WAs in golden render state (Chris).
>>>>>
>>>>> The generation of render state is not a straighforward process, it would
>>>>> be ideal to augment WA values from during the setup state as opposed to
>>>>> using a tool but that would be a follow up patch.
>>>>
>>>> I'd still prefer just emitting the LRIs from code rather tha mucking
>>>> about with null batch. Less hoops to jump through when adding a new w/a.
>>>
>>> I agree with this. We should aim to keep null state as per
>>> gen. Workaround set is different for gtX inside particular
>>> gen so we would need then multiple null states per gen.
>>>
>>> After brief chat with Ville, I think that the correct
>>> spot to init the context specific workarounds is after MI_SET_CONTEXT
>>> to default and right before null batch is run. If we do these
>>> with emitting LRIs to ring, we should be safe as they are then saved
>>> with default ctx.
>>>
>>> The default ctx is then used as a 'parent' for newly created
>>> contexts. Ofcource if registers get globbered, then we inherit
>>> crap.
>>>
>>> If we have the per gen null state and the ring is initializing
>>> workarounds for the default context, then in future we can
>>> save this state as 'read only golden context'. And use it as the
>>> initial state for all newly created contexts.
>>>
>>> Then the full plan how to init would look like this:
>>>
>>> #1 reset the gpu (on driver load, on resume or on hang recovery)
>>> #2 if we have 'read only golden context', copy it to default ctx
>>> #3 switch to default context
>>> #4 if we had 'read only golden context' we are done with the init.
>>>
>>> ---
>>>
>>> #5 if this is driver load thus there is no 'read only golden context' yet.
>>> #6 init workarounds through ring LRIs
>>> #7 run null/golden state batch
>>> #8 save this state as a 'read only golden context'
>>>
>>> ---
>>>
>>> #9 for each new context, initialize ctx obj with 'read only golden
>>>   context' (either by memcpy or restoring from it when switching to new)
>>>
>> I understand applying WAs using null batch has its issues but as I mentioned
>> in the commit msg I will fix this as a follow up patch.
>> It is going to take some time though to change the patch as per the new
>> sequence.
>> The patch in its current state helps fix WA issues after reset; so it can
>> only be accepted if it is updated as per the new sequence?
>
> We already have a lot of "let's fix it later" experiments running, so I
> don't want to overload the ship. So I highly prefer to merge the revised
> version directly.
> -Daniel
>
I understand, a revised version with LRIs emitting from the driver is 
already submitted and is being reviewed.

regards
Arun