[Intel-gfx] [PATCH 1/3] drm/i915: Keep the ctx workarounds tightly packed

[Chris Wilson]

Quoting Ville Syrjälä (2018-06-15 17:37:44)
> On Fri, Jun 15, 2018 at 05:22:40PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-06-15 17:19:14)
> > > On Fri, Jun 15, 2018 at 09:01:37AM -0700, Oscar Mateo Lozano wrote:
> > > > 
> > > > 
> > > > On 6/15/2018 1:59 AM, Chris Wilson wrote:
> > > > > For each platform, we have a few registers that rewritten with multiple
> > > > > values -- they are not part of a sequence, just different parts of a
> > > > > masked register set at different times (e.g. platform and gen
> > > > > workarounds). Consolidate these into a single register write to keep the
> > > > > table compact.
> > > > >
> > > > > While adjusting the construction of the wa table, make it non fatal so
> > > > > that the driver still loads but keeping the warning and extra details
> > > > > for inspection.
> > > > 
> > > > A while ago I sent a patch 
> > > > (https://patchwork.freedesktop.org/patch/205035/) that uses simple MMIO 
> > > > writes to apply ctx workarounds. This is possible since we now have 
> > > > proper golden contexts, and avoids the need for these patches.
> > > > It also has the advantage that an improperly classified WA doesn't get 
> > > > lost (we still need the classification if we want to properly validate 
> > > > the WAs, but that's a different story).
> > > > Are we sure we prefer to do this instead?
> > > 
> > > Wouldn't that require PSMI+FSM dance to make sure execlist has
> > > an active context when you write the regs? Can't see anything like that
> > > in the code currently, nor is there anything in the referenced patch.
> > 
> > We keep forcewake asserted across the bringup, from before we load the
> > default context until after we have saved the context image. These mmio
> > writes should be saved along with the image. That's the theory at least.
> 
> Force wake isn't quite enough from what I understand. Or maybe it was
> that there is at least some delay between forcewake ack asserting and
> the context actually having been loaded (just my recollection of what
> Mika's experiments once showed).
> 
> The spec at least still lists the extra dance for MMIO access into
> context saved registers.

We do at least have the advantage now of checking that all contexts start
with the expected w/a register state. Hmm, if we stuff those registers
with garbage inside gem_workarounds, that will help to confirm that we
do load them correctly. (If having them scrubbed by reset isn't enough!)
-Chris