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

Imre Deak
2019-11-06T16:38:32Z

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