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

[Daniel Vetter]

On Tue, Aug 26, 2014 at 01:57:19PM +0100, Siluvery, Arun wrote:
> 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 at 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.

Ah, still catching up from my unusable network connection from last week.
Please ignore me ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch