[Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support

[Lionel Landwerlin]

Hi James,

Thanks a lot for reporting this.

I think this is something we should store in the gen_device_info and 
update with kernel ioctl when supported.
This affects other drivers, not just anv.

-Lionel

On 10/04/2019 23:55, James Xiong wrote:
> From: "Xiong, James" <james.xiong at intel.com>
>
> The vma high heap's capacity and maximum address were pre-defined based
> on 48bits ppgtt support, and the buffers allocated from the vma high heap
> had invalid vma addresses to the ehl platform that only supports 36bits
> ppgtt. As a result, KMD rejected all batchbuffers submitted by vulkan.
>
> This patch:
> 1) initializes the vma high heap by retrieving the gtt capacity from KMD
> and calculating the size and max address on the fly.
> 2) enables softpin when full ppgtt is enabled
>
> V2: change commit messages and comments to refect the changes [Bob, Jason]
>      remove define HIGH_HEAP_SIZE [Bob]
>      make sure there's enough space to enable softspin [Jason]
>
> Signed-off-by: Xiong, James <james.xiong at intel.com>
> ---
>   src/intel/vulkan/anv_device.c  | 30 +++++++++++++++++++++++-------
>   src/intel/vulkan/anv_private.h |  7 ++++---
>   2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 88b34c4..c3eff1c 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -434,7 +434,12 @@ anv_physical_device_init(struct anv_physical_device *device,
>                                 anv_gem_supports_syncobj_wait(fd);
>      device->has_context_priority = anv_gem_has_context_priority(fd);
>   
> +   /*
> +    * make sure there are enough VA space(i.e. 32+bit support) and full ggtt
> +    * is enabled.
> +    */
>      device->use_softpin = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_SOFTPIN)
> +      && (anv_gem_get_param(fd, I915_PARAM_HAS_ALIASING_PPGTT) > 1)
>         && device->supports_48bit_addresses;
>   
>      device->has_context_isolation =
> @@ -1981,14 +1986,25 @@ VkResult anv_CreateDevice(
>         device->vma_lo_available =
>            physical_device->memory.heaps[physical_device->memory.heap_count - 1].size;
>   
> -      /* Leave the last 4GiB out of the high vma range, so that no state base
> -       * address + size can overflow 48 bits. For more information see the
> -       * comment about Wa32bitGeneralStateOffset in anv_allocator.c
> -       */
> -      util_vma_heap_init(&device->vma_hi, HIGH_HEAP_MIN_ADDRESS,
> -                         HIGH_HEAP_SIZE);
>         device->vma_hi_available = physical_device->memory.heap_count == 1 ? 0 :
>            physical_device->memory.heaps[0].size;
> +
> +      /* Retrieve the GTT's capacity and leave the last 4GiB out of the high vma
> +       * range, so that no state base address + size can overflow the vma range. For
> +       * more information see the comment about Wa32bitGeneralStateOffset in
> +       * anv_allocator.c
> +       */
> +      uint64_t size = 0;
> +      anv_gem_get_context_param(device->fd, 0, I915_CONTEXT_PARAM_GTT_SIZE,
> +                                &size);
> +      if(size > HIGH_HEAP_MIN_ADDRESS + (1ull<<32)) {
> +         size -= HIGH_HEAP_MIN_ADDRESS + (1ull<<32);
> +         device->vma_hi_max_addr = HIGH_HEAP_MIN_ADDRESS + size - 1;
> +      } else {
> +         size = device->vma_hi_max_addr = 0;
> +      }
> +
> +      util_vma_heap_init(&device->vma_hi, HIGH_HEAP_MIN_ADDRESS, size);
>      }
>   
>      /* As per spec, the driver implementation may deny requests to acquire
> @@ -2456,7 +2472,7 @@ anv_vma_free(struct anv_device *device, struct anv_bo *bo)
>         device->vma_lo_available += bo->size;
>      } else {
>         assert(addr_48b >= HIGH_HEAP_MIN_ADDRESS &&
> -             addr_48b <= HIGH_HEAP_MAX_ADDRESS);
> +             addr_48b <= device->vma_hi_max_addr);
>         util_vma_heap_free(&device->vma_hi, addr_48b, bo->size);
>         device->vma_hi_available += bo->size;
>      }
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 1664918..ef9b012 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -109,6 +109,9 @@ struct gen_l3_config;
>    * heap. Various hardware units will read past the end of an object for
>    * various reasons. This healthy margin prevents reads from wrapping around
>    * 48-bit addresses.
> + *
> + * (4) the high vma heap size and max address are calculated based on the
> + * gtt capacity retrieved from KMD.
>    */
>   #define LOW_HEAP_MIN_ADDRESS               0x000000001000ULL /* 4 KiB */
>   #define LOW_HEAP_MAX_ADDRESS               0x0000bfffffffULL
> @@ -121,12 +124,9 @@ struct gen_l3_config;
>   #define INSTRUCTION_STATE_POOL_MIN_ADDRESS 0x000180000000ULL /* 6 GiB */
>   #define INSTRUCTION_STATE_POOL_MAX_ADDRESS 0x0001bfffffffULL
>   #define HIGH_HEAP_MIN_ADDRESS              0x0001c0000000ULL /* 7 GiB */
> -#define HIGH_HEAP_MAX_ADDRESS              0xfffeffffffffULL
>   
>   #define LOW_HEAP_SIZE               \
>      (LOW_HEAP_MAX_ADDRESS - LOW_HEAP_MIN_ADDRESS + 1)
> -#define HIGH_HEAP_SIZE              \
> -   (HIGH_HEAP_MAX_ADDRESS - HIGH_HEAP_MIN_ADDRESS + 1)
>   #define DYNAMIC_STATE_POOL_SIZE     \
>      (DYNAMIC_STATE_POOL_MAX_ADDRESS - DYNAMIC_STATE_POOL_MIN_ADDRESS + 1)
>   #define BINDING_TABLE_POOL_SIZE     \
> @@ -1093,6 +1093,7 @@ struct anv_device {
>       struct util_vma_heap                        vma_hi;
>       uint64_t                                    vma_lo_available;
>       uint64_t                                    vma_hi_available;
> +    uint64_t                                    vma_hi_max_addr;
>   
>       struct anv_bo_pool                          batch_bo_pool;
>