Re: [Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support
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"
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
---
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(>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,
+ );
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(>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(>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 0x1000ULL /* 4
KiB */
#define LOW_HEAP_MAX_ADDRESS 0xbfffULL
@@ -121,12 +124,9 @@ struct
Re: [Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support
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"
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
---
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(>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,
+ );
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(>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(>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 0x1000ULL /* 4
KiB */
#define LOW_HEAP_MAX_ADDRESS 0xbfffULL
@@ -121,12 +124,9 @@ struct gen_l3_config;
#define INSTRUCTION_STATE_POOL_MIN_ADDRESS 0x00018000ULL /* 6
GiB */
#define INSTRUCTION_STATE_POOL_MAX_ADDRESS
Re: [Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support
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"
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
---
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(>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,
+);
+ 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(>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(>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 0x1000ULL /* 4 KiB */
#define LOW_HEAP_MAX_ADDRESS 0xbfffULL
@@ -121,12 +124,9 @@ struct gen_l3_config;
#define INSTRUCTION_STATE_POOL_MIN_ADDRESS 0x00018000ULL /* 6 GiB */
#define INSTRUCTION_STATE_POOL_MAX_ADDRESS 0x0001bfffULL
#define HIGH_HEAP_MIN_ADDRESS 0x0001c000ULL /* 7 GiB */
-#define HIGH_HEAP_MAX_ADDRESS 0xfffeULL
#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 \
[Mesa-dev] [PATCH 1/1] anv/ehl: 36bits ppgtt support
From: "Xiong, James"
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
---
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(>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,
+);
+ 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(>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(>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 0x1000ULL /* 4 KiB */
#define LOW_HEAP_MAX_ADDRESS 0xbfffULL
@@ -121,12 +124,9 @@ struct gen_l3_config;
#define INSTRUCTION_STATE_POOL_MIN_ADDRESS 0x00018000ULL /* 6 GiB */
#define INSTRUCTION_STATE_POOL_MAX_ADDRESS 0x0001bfffULL
#define HIGH_HEAP_MIN_ADDRESS 0x0001c000ULL /* 7 GiB */
-#define HIGH_HEAP_MAX_ADDRESS 0xfffeULL
#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_heapvma_hi;
uint64_tvma_lo_available;
uint64_t
