[Mesa-dev] [PATCH] anv: Don't place scratch buffers above the 32-bit boundary

[Jason Ekstrand]

On Wed, Apr 26, 2017 at 11:05 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Saturday, April 22, 2017 3:55:07 PM PDT Jason Ekstrand wrote:
> > This fixes rendering corruptions in DOOM.  Hopefully, it will also make
> > Jenkins a bit more stable as we've been seeing some random failures and
> > GPU hangs ever since turning on 48bit.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100620
> > Cc: "17.1" <mesa-stable at lists.freedesktop.org>
> > ---
> >  src/intel/vulkan/anv_allocator.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_
> allocator.c
> > index 78327df..28bfac4 100644
> > --- a/src/intel/vulkan/anv_allocator.c
> > +++ b/src/intel/vulkan/anv_allocator.c
> > @@ -994,6 +994,22 @@ anv_scratch_pool_alloc(struct anv_device *device,
> struct anv_scratch_pool *pool,
> >
> >     anv_bo_init_new(&bo->bo, device, size);
> >
> > +   /* Even though the Scratch base pointers in 3DSTATE_*S are 64 bits,
> they
> > +    * are still relative to the general state base address.  When we
> emit
> > +    * STATE_BASE_ADDRESS, we set general state base address to 0 and
> the size
> > +    * to the maximum (1 page under 4GB).  This allows us to just place
> the
> > +    * scratch buffers anywhere we wish in the bottom 32 bits of address
> space
> > +    * and just set the scratch base pointer in 3DSTATE_*S using a
> relocation.
> > +    * However, in order to do so, we need to ensure that the kernel
> does not
> > +    * place the scratch BO above the 32-bit boundary.
> > +    *
> > +    * NOTE: Technically, it can't go "anywhere" because the top page is
> off
> > +    * limits.  However, it will never end up getting placed that high
> because
> > +    * the surface state and general state buffers will get placed first
> and
> > +    * the kernel likes to work top-down.
>
> This NOTE sounds unnecessarily scary.
>
> If you don't set EXEC_OBJECT_SUPPORTS_48B_ADDRESS, then the kernel sets
> this to PIN_ZONE_4G...which in i915_vma.c does:
>
>                 end = min_t(u64, end, (1ULL << 32) - I915_GTT_PAGE_SIZE);
>
> If I'm reading the kernel code correctly, that means the kernel is
> explicitly refusing to position it in that top page.  So it's not a
> side effect of the kernel "liking to work top-down" - I'm pretty sure
> it works by design, not by accident.
>

Thanks!  I've updated the note to make it less scarry and go along with
your comment.


> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> > +    */
> > +   bo->bo.flags &= ~EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > +
> >     /* Set the exists last because it may be read by other threads */
> >     __sync_synchronize();
> >     bo->exists = true;
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170427/2dd04484/attachment.html>