[Intel-gfx] [PATCH 1/2] drm/i915/bxt: work around HW coherency issue when accessing GPU seqno

[Chris Wilson]

On Wed, Jun 10, 2015 at 06:26:58PM +0300, Imre Deak wrote:
> On ke, 2015-06-10 at 08:10 -0700, Jesse Barnes wrote:
> > On 06/10/2015 03:59 AM, Imre Deak wrote:
> > > I think the discussion here is about two separate things:
> > > 1. Possible ordering issue between the seqno store and the completion
> > > interrupt
> > > 2. Coherency issue that leaves the CPU with a stale view of the seqno
> > > indefinitely, which this patch works around
> > > 
> > > I'm confident that in my case the problem is not due to ordering. If it
> > > was "only" ordering then the value would show up eventually. This is not
> > > the case though, __wait_for_request will see the stale value
> > > indefinitely even though it gets woken up periodically afterwards by the
> > > lost IRQ logic (with hangcheck disabled).
> > 
> > Yeah, based on your workaround it sounds like the write from the CS is
> > landing in memory but failing to invalidate the associated CPU
> > cacheline.  I assume mapping the HWSP as uncached also works around this
> > issue?
> 
> I assume it would, but it would of course have a bigger overhead. Based
> on my testing the coherency problem happens only occasionally, so for
> the rest of the times we still would want to benefit from cached reads.
> See especially __i915_spin_request().

Yeah, I am not quite sure about that myself. The reason we don't do
forced coherent reads there is because of the impact that has with fw
and the snb/ivb/hsw workaround.  If we have a viable strategy that gives us
accurate seqno at marginal cost, then it will be beneficial to do so from within
the busy-spin - in order to minimise the amount of cycles we spin.

We always spend a jiffie of CPU time, we might as spend it getting
accurate results (so the coherent read is quick enough and doesn't slow
down the normal case).

I seem to recall that we tried clflushing for gen6+, but we didn't
record any details, so it may be worth retesting ivb with that w/a.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre