[Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Thu Apr 11 12:06:06 UTC 2019
Sorry, upon rereading the code of the various drivers, it seems
i965/iris handle this properly already.
I have some comments below.
On 11/04/2019 11:36, Lionel Landwerlin wrote:
> 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);
I don't think you need to requery the gtt size, this is already done
when initializing the physical device.
I think we can do something better by storing the bounds in the physical
device and just reusing that at logical device creation.
>> + 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;
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list