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

Lionel Landwerlin lionel.g.landwerlin at intel.com
Thu Apr 11 16:02:48 UTC 2019


I started this MR : 
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/637

On 11/04/2019 13:06, Lionel Landwerlin wrote:
> 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
>
>
> _______________________________________________
> 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