[Intel-gfx] [PATCH] drm/i915/cnl: Avoid ioremap_wc on Cannonlake as well.

[Vivi, Rodrigo]

On Wed, 2017-08-30 at 14:38 +0300, Joonas Lahtinen wrote:
> On Wed, 2017-08-30 at 12:26 +0100, Chris Wilson wrote:
> > Quoting Joonas Lahtinen (2017-08-30 12:13:29)
> > > On Tue, 2017-08-29 at 16:09 -0700, Rodrigo Vivi wrote:
> > > > Driver’s CPU access to GTT is via the GTTMMADR BAR.
> > > > 
> > > > The current HW implementation of that BAR is to only
> > > > support <= DW (and maybe QW) writes—not 16/32/64B writes
> > > > that could occur with WC and/or SSE/AVX moves.
> > > > 
> > > > GTTMMADR must be marked uncacheable (UC).
> > > > Accesses to GTTMMADR(GTT), must be 64 bits or less (ie. 1 GTT entry).
> > > > 
> > > > v2: Get clarification on the reasons and spec is getting
> > > >     updated to reflect it now.
> > > > 
> > > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > > Suggested-by: Ben Widawsky <benjamin.widawsky at intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > 
> > > Rodrigo, can you double-check how this interacts with the patch from
> > > Zhi that adds the WB flag to PPAT_CACHE_INDEX on CNL.
> > 
> > Different issue (or should be). The ioremap concerns access through the
> > PCI BAR, affecting how fast we insert entries into the GGTT (so
> > establishing new mmaps following frequent runtime pm, loading of new
> > contexts + rings, as well as the stressful GGTT thrashing). PPAT affects
> > how the device accesses the physical pages, not the PTE themselves.
> 
> Yes, I know it should be :) But Rodrigo also described pretty random
> hangs, IIRC not much was pinpointing to either of the issues. With
> these two bugs present, device could be operating without write-back on
> certain pages, or could be operating on wrong pages altogether.
> 
> I'd just like one round of testing to try to avoid this change if we
> can.

I had tried already put PAT to non-cached, but I will double check Zhi's
work just in case.

I wish we could avoid this patch here, but it seems by definition this
BAR should be uncached. By BAR's non-Prefetchable attribute.

So probably the ioremap_wc should check that attribute and fail to
allocate that with wc so we would try wc and fallback to uncached.

But since we know this is uncached only for this case and this handle
don't exist yet the best is to move along with this patch.

> 
> Regards, Joonas