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

[Imre Deak]

On ke, 2015-06-10 at 08:33 -0700, Jesse Barnes wrote:
> On 06/10/2015 08:26 AM, 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, pretty sure we want it cached given how often we read from it.  I
> was just curious if the UC mapping would address this just to narrow
> things down even further.

I must admit that I also remembered our A0 power-on event where the
Android guys had a similar issue and came up with a solution to map the
status page uncached. At that time we didn't see this problem with the
upstream kernel. Not sure what was the difference then and why the
problem is back now, maybe we just didn't run the tests long enough. So
I'm pretty sure it's the same issue.

--Imre