Re: [igt-dev] [PATCH v3 1/3] lib/rendercopy: Add AUX page table support

From: Imre Deak <imre.deak@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: igt-dev@lists.freedesktop.org, Brian Welty <brian.welty@intel.com>
Subject: Re: [igt-dev] [PATCH v3 1/3] lib/rendercopy: Add AUX page table support
Date: Wed, 6 Nov 2019 18:36:49 +0200	[thread overview]
Message-ID: <20191106163649.GB2077@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <157305687595.26738.10985391469160168336@skylake-alporthouse-com>

On Wed, Nov 06, 2019 at 04:14:36PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2019-11-06 16:00:16)
> > On Wed, Nov 06, 2019 at 11:11:40AM +0000, Chris Wilson wrote:
> > > What's the coherency model for the pgt->bo? There's a lot of writes here
> > > from the CPU how are you making sure they are visible to the GPU?
> > 
> > My understanding: execbuf will do a clflush for the buf on non-LLC
> > platforms, while on LLC platforms that's not needed; not sure if there
> > will be any non-LLC platforms where the AUX pagetable will be needed.
> 
> No, we don't unless you tell us you modified it between batches. That
> would be the case if you were using drm_intel_bo_map(true) everytime,
> but again that will then stall the GPU between every batch -- which we
> definitely do not want when testing [as it will hide every bug] :)

Do you see a problem with the way I program it here though? I call
drm_intel_bo_map(true) for the purpose of programming a new table. If
you see a problem with that what do you suggest instead?

> > On top of that the AUX GAM has its own caches - found that now in an HSD
> > doc - which will be invalidated whenever GAM's top level table base
> > pointer register is set from the context image, which happens whenver
> > switching to the context.
> 
> But not for a lite-restore?

We still write the top level table regsiter from the batch, so that
could in itself trigger the invalidation.

> So we have a problem when updating existing entries between batches
> inside the same context? We put a big hammer at the front of the
> request for TLB invalidation. If there's a flush we can add, we need
> it there.

Yes, there is, see the CCS_AUX_INV reg in bspec. But I suspect this
would have the same effect as writing the table ptr register, so not
sure when exactly this would be needed. 

> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev