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

[Joonas Lahtinen]

On Wed, 2017-08-30 at 17:58 +0000, Vivi, Rodrigo wrote:
> 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.

Right, you can have the R-b.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation