<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 26, 2017 at 11:05 PM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Saturday, April 22, 2017 3:55:07 PM PDT Jason Ekstrand wrote:<br>
> This fixes rendering corruptions in DOOM.  Hopefully, it will also make<br>
> Jenkins a bit more stable as we've been seeing some random failures and<br>
> GPU hangs ever since turning on 48bit.<br>
><br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=100620" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=100620</a><br>
> Cc: "17.1" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.<wbr>freedesktop.org</a>><br>
> ---<br>
>  src/intel/vulkan/anv_<wbr>allocator.c | 16 ++++++++++++++++<br>
>  1 file changed, 16 insertions(+)<br>
><br>
> diff --git a/src/intel/vulkan/anv_<wbr>allocator.c b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> index 78327df..28bfac4 100644<br>
> --- a/src/intel/vulkan/anv_<wbr>allocator.c<br>
> +++ b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> @@ -994,6 +994,22 @@ anv_scratch_pool_alloc(struct anv_device *device, struct anv_scratch_pool *pool,<br>
><br>
>     anv_bo_init_new(&bo->bo, device, size);<br>
><br>
> +   /* Even though the Scratch base pointers in 3DSTATE_*S are 64 bits, they<br>
> +    * are still relative to the general state base address.  When we emit<br>
> +    * STATE_BASE_ADDRESS, we set general state base address to 0 and the size<br>
> +    * to the maximum (1 page under 4GB).  This allows us to just place the<br>
> +    * scratch buffers anywhere we wish in the bottom 32 bits of address space<br>
> +    * and just set the scratch base pointer in 3DSTATE_*S using a relocation.<br>
> +    * However, in order to do so, we need to ensure that the kernel does not<br>
> +    * place the scratch BO above the 32-bit boundary.<br>
> +    *<br>
> +    * NOTE: Technically, it can't go "anywhere" because the top page is off<br>
> +    * limits.  However, it will never end up getting placed that high because<br>
> +    * the surface state and general state buffers will get placed first and<br>
> +    * the kernel likes to work top-down.<br>
<br>
</div></div>This NOTE sounds unnecessarily scary.<br>
<br>
If you don't set EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS, then the kernel sets<br>
this to PIN_ZONE_4G...which in i915_vma.c does:<br>
<br>
                end = min_t(u64, end, (1ULL << 32) - I915_GTT_PAGE_SIZE);<br>
<br>
If I'm reading the kernel code correctly, that means the kernel is<br>
explicitly refusing to position it in that top page.  So it's not a<br>
side effect of the kernel "liking to work top-down" - I'm pretty sure<br>
it works by design, not by accident.<br></blockquote><div><br></div><div>Thanks!  I've updated the note to make it less scarry and go along with your comment.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
<div class="HOEnZb"><div class="h5"><br>
> +    */<br>
> +   bo->bo.flags &= ~EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> +<br>
>     /* Set the exists last because it may be read by other threads */<br>
>     __sync_synchronize();<br>
>     bo->exists = true;<br>
><br>
</div></div></blockquote></div><br></div></div>