Re: [PATCH 3/6] drm/i915: Disable render idle on user forcewake

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 18, 2014 at 06:54:15PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Thu, Sep 18, 2014 at 05:58:32PM +0300, Mika Kuoppala wrote:
> >> Render context on gen8 is not guaranteed to be loaded (active)
> >> at the instant when forcewake ack shows up. So we need extra
> >> steps to get it in before we access specific registers.
> >> 
> >> We could do register white listing to do the extra dance only on
> >> specific render context access. However the main concern is
> >> is ring initialization after reset/resume, so only take the extra
> >> steps on user forcewake as it is already guarding ring init. And not incur
> >> extra perf penalty to regular register accesses. This allows us to be
> >> sure that we don't read all zeros on RMW cycles. And we to show a
> >> consistent state to igt when user fw has been acquired.
> >
> > Might I just ask why? This talks about the need to serialise with the
> > LRI use to load the context registers, so be explicit. When reading
> > those registers, we should know what is going on with the GPU or just
> > don't care. Writing to those registers once the ring is running is
> > verboten.
> >
> > This is just very thin papering over an unexplained problem.
> 
> It is not clear why but on fw ack showing up, we have a short
> delay before some registers on render contexts start to return
> other than all zeroes. Usually you can get 2-3 reads before
> the values show up. This manifests by sometimes the dfs/
> i915_ppgtt_info will return 'render ring:PDP0' as zeroes or
> top 32bits zeroes, ss it is the first read(s).
> 
> Also as our wa verification tests reads through mmio, after
> getting user forcewake, we see sometimes read returning zeros.
> 
> If we do workaround verification by STORE_REGISTER_MEMs on
> igt side and accept that debugfs/i915_forcewake_user doesn't
> guarantee anything with regards to access to render context,
> then I think that we can drop this patch.
> 
> And be very extra careful with workarounds that need RWM
> on this area.

While the spec is quite unclear, I've come up with this theory
on the HW operation:

1. forcewak req=1
2. load power context (includes CCID)
3. forcewake ack=1
4. load render context based on CCID

So if you manage to read the context saved registers via mmio
before step 4 you get the power on defaults instead of the
values from the context.

And on GPU reset we lose CCID so we also lose the workarounds
because following the reset we switch to the default context
with restore_inbhibit=1. Once/if we switch to an already initialized
proper context, the workarounds should magically reappear and should
subsequently be saved to the default context(s) even. So afterwards
going at least once into rc6 should bring the workarounds back even
for a default context. And similarly I suppose we lose the power
context+CCID on suspend/resume so we also lose the workarounds there.

Would be somewhat interesting to see if these theories hold up to
reality.

-- 
Ville Syrjälä
Intel OTC