[2/2] radv: fix VK_EXT_memory_budget if one heap isn't available

Submitted by Samuel Pitoiset on June 11, 2019, 2:46 p.m.

Details

Message ID 20190611144633.22564-3-samuel.pitoiset@gmail.com
State New
Headers show
Series "radv: two fixes for Intel NUC&VegaM" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset June 11, 2019, 2:46 p.m.
On VegaM, the visible VRAM size is equal to the VRAM size, which
means only two heaps are exposed.

This fixes dEQP-VK.api.info.device.memory_budget.

Cc: 19.0 19.1 <mesa-stable@lists.freedesktop.org>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_device.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 358fc7cb30a..31b9b4e9875 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -1489,9 +1489,11 @@  radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
 	RADV_FROM_HANDLE(radv_physical_device, device, physicalDevice);
 	VkPhysicalDeviceMemoryProperties *memory_properties = &device->memory_properties;
 	uint64_t visible_vram_size = radv_get_visible_vram_size(device);
+	int vram_index = -1, visible_vram_index = -1, gtt_index = -1;
 	uint64_t vram_size = radv_get_vram_size(device);
 	uint64_t gtt_size = device->rad_info.gart_size;
 	uint64_t heap_budget, heap_usage;
+	uint32_t heap_count = 0;
 
 	/* For all memory heaps, the computation of budget is as follow:
 	 *	heap_budget = heap_size - global_heap_usage + app_heap_usage
@@ -1503,6 +1505,7 @@  radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
 	 * in presence of shared buffers).
 	 */
 	if (vram_size) {
+		vram_index = heap_count++;
 		heap_usage = device->ws->query_value(device->ws,
 						     RADEON_ALLOCATED_VRAM);
 
@@ -1510,11 +1513,12 @@  radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
 			device->ws->query_value(device->ws, RADEON_VRAM_USAGE) +
 			heap_usage;
 
-		memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM] = heap_budget;
-		memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM] = heap_usage;
+		memoryBudget->heapBudget[vram_index] = heap_budget;
+		memoryBudget->heapUsage[vram_index] = heap_usage;
 	}
 
 	if (visible_vram_size) {
+		visible_vram_index = heap_count++;
 		heap_usage = device->ws->query_value(device->ws,
 						     RADEON_ALLOCATED_VRAM_VIS);
 
@@ -1522,11 +1526,12 @@  radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
 			device->ws->query_value(device->ws, RADEON_VRAM_VIS_USAGE) +
 			heap_usage;
 
-		memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_budget;
-		memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_usage;
+		memoryBudget->heapBudget[visible_vram_index] = heap_budget;
+		memoryBudget->heapUsage[visible_vram_index] = heap_usage;
 	}
 
 	if (gtt_size) {
+		gtt_index = heap_count++;
 		heap_usage = device->ws->query_value(device->ws,
 						     RADEON_ALLOCATED_GTT);
 
@@ -1534,8 +1539,8 @@  radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
 			device->ws->query_value(device->ws, RADEON_GTT_USAGE) +
 			heap_usage;
 
-		memoryBudget->heapBudget[RADV_MEM_HEAP_GTT] = heap_budget;
-		memoryBudget->heapUsage[RADV_MEM_HEAP_GTT] = heap_usage;
+		memoryBudget->heapBudget[gtt_index] = heap_budget;
+		memoryBudget->heapUsage[gtt_index++] = heap_usage;
 	}
 
 	/* The heapBudget and heapUsage values must be zero for array elements

Comments

On Tue, Jun 11, 2019 at 10:43 AM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
> On VegaM, the visible VRAM size is equal to the VRAM size, which
> means only two heaps are exposed.

FWIW, this is not VegaM specific.  The vram size could be equal to the
visible vram size on any asic depending on whether the platform
supports large or resizeable BARs or not.

Alex

>
> This fixes dEQP-VK.api.info.device.memory_budget.
>
> Cc: 19.0 19.1 <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_device.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 358fc7cb30a..31b9b4e9875 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -1489,9 +1489,11 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>         RADV_FROM_HANDLE(radv_physical_device, device, physicalDevice);
>         VkPhysicalDeviceMemoryProperties *memory_properties = &device->memory_properties;
>         uint64_t visible_vram_size = radv_get_visible_vram_size(device);
> +       int vram_index = -1, visible_vram_index = -1, gtt_index = -1;
>         uint64_t vram_size = radv_get_vram_size(device);
>         uint64_t gtt_size = device->rad_info.gart_size;
>         uint64_t heap_budget, heap_usage;
> +       uint32_t heap_count = 0;
>
>         /* For all memory heaps, the computation of budget is as follow:
>          *      heap_budget = heap_size - global_heap_usage + app_heap_usage
> @@ -1503,6 +1505,7 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>          * in presence of shared buffers).
>          */
>         if (vram_size) {
> +               vram_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_VRAM);
>
> @@ -1510,11 +1513,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_VRAM_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM] = heap_usage;
> +               memoryBudget->heapBudget[vram_index] = heap_budget;
> +               memoryBudget->heapUsage[vram_index] = heap_usage;
>         }
>
>         if (visible_vram_size) {
> +               visible_vram_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_VRAM_VIS);
>
> @@ -1522,11 +1526,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_VRAM_VIS_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_usage;
> +               memoryBudget->heapBudget[visible_vram_index] = heap_budget;
> +               memoryBudget->heapUsage[visible_vram_index] = heap_usage;
>         }
>
>         if (gtt_size) {
> +               gtt_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_GTT);
>
> @@ -1534,8 +1539,8 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_GTT_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_GTT] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_GTT] = heap_usage;
> +               memoryBudget->heapBudget[gtt_index] = heap_budget;
> +               memoryBudget->heapUsage[gtt_index++] = heap_usage;
>         }
>
>         /* The heapBudget and heapUsage values must be zero for array elements
> --
> 2.22.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 6/11/19 4:56 PM, Alex Deucher wrote:
> On Tue, Jun 11, 2019 at 10:43 AM Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> On VegaM, the visible VRAM size is equal to the VRAM size, which
>> means only two heaps are exposed.
> FWIW, this is not VegaM specific.  The vram size could be equal to the
> visible vram size on any asic depending on whether the platform
> supports large or resizeable BARs or not.
Yeah, not saying it's specific to VegaM. It's just the first time I hit 
this, so I included that patch in the VegaM series. :-)
>
> Alex
>
>> This fixes dEQP-VK.api.info.device.memory_budget.
>>
>> Cc: 19.0 19.1 <mesa-stable@lists.freedesktop.org>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/radv_device.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>> index 358fc7cb30a..31b9b4e9875 100644
>> --- a/src/amd/vulkan/radv_device.c
>> +++ b/src/amd/vulkan/radv_device.c
>> @@ -1489,9 +1489,11 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>>          RADV_FROM_HANDLE(radv_physical_device, device, physicalDevice);
>>          VkPhysicalDeviceMemoryProperties *memory_properties = &device->memory_properties;
>>          uint64_t visible_vram_size = radv_get_visible_vram_size(device);
>> +       int vram_index = -1, visible_vram_index = -1, gtt_index = -1;
>>          uint64_t vram_size = radv_get_vram_size(device);
>>          uint64_t gtt_size = device->rad_info.gart_size;
>>          uint64_t heap_budget, heap_usage;
>> +       uint32_t heap_count = 0;
>>
>>          /* For all memory heaps, the computation of budget is as follow:
>>           *      heap_budget = heap_size - global_heap_usage + app_heap_usage
>> @@ -1503,6 +1505,7 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>>           * in presence of shared buffers).
>>           */
>>          if (vram_size) {
>> +               vram_index = heap_count++;
>>                  heap_usage = device->ws->query_value(device->ws,
>>                                                       RADEON_ALLOCATED_VRAM);
>>
>> @@ -1510,11 +1513,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>>                          device->ws->query_value(device->ws, RADEON_VRAM_USAGE) +
>>                          heap_usage;
>>
>> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM] = heap_budget;
>> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM] = heap_usage;
>> +               memoryBudget->heapBudget[vram_index] = heap_budget;
>> +               memoryBudget->heapUsage[vram_index] = heap_usage;
>>          }
>>
>>          if (visible_vram_size) {
>> +               visible_vram_index = heap_count++;
>>                  heap_usage = device->ws->query_value(device->ws,
>>                                                       RADEON_ALLOCATED_VRAM_VIS);
>>
>> @@ -1522,11 +1526,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>>                          device->ws->query_value(device->ws, RADEON_VRAM_VIS_USAGE) +
>>                          heap_usage;
>>
>> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_budget;
>> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_usage;
>> +               memoryBudget->heapBudget[visible_vram_index] = heap_budget;
>> +               memoryBudget->heapUsage[visible_vram_index] = heap_usage;
>>          }
>>
>>          if (gtt_size) {
>> +               gtt_index = heap_count++;
>>                  heap_usage = device->ws->query_value(device->ws,
>>                                                       RADEON_ALLOCATED_GTT);
>>
>> @@ -1534,8 +1539,8 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>>                          device->ws->query_value(device->ws, RADEON_GTT_USAGE) +
>>                          heap_usage;
>>
>> -               memoryBudget->heapBudget[RADV_MEM_HEAP_GTT] = heap_budget;
>> -               memoryBudget->heapUsage[RADV_MEM_HEAP_GTT] = heap_usage;
>> +               memoryBudget->heapBudget[gtt_index] = heap_budget;
>> +               memoryBudget->heapUsage[gtt_index++] = heap_usage;
>>          }
>>
>>          /* The heapBudget and heapUsage values must be zero for array elements
>> --
>> 2.22.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
So can we rephrase this entire thing based on
physical_devices->mem_type_indices, instead of opencoding ordering &
detection?

On Tue, Jun 11, 2019 at 4:43 PM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
> On VegaM, the visible VRAM size is equal to the VRAM size, which
> means only two heaps are exposed.
>
> This fixes dEQP-VK.api.info.device.memory_budget.
>
> Cc: 19.0 19.1 <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_device.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 358fc7cb30a..31b9b4e9875 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -1489,9 +1489,11 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>         RADV_FROM_HANDLE(radv_physical_device, device, physicalDevice);
>         VkPhysicalDeviceMemoryProperties *memory_properties = &device->memory_properties;
>         uint64_t visible_vram_size = radv_get_visible_vram_size(device);
> +       int vram_index = -1, visible_vram_index = -1, gtt_index = -1;
>         uint64_t vram_size = radv_get_vram_size(device);
>         uint64_t gtt_size = device->rad_info.gart_size;
>         uint64_t heap_budget, heap_usage;
> +       uint32_t heap_count = 0;
>
>         /* For all memory heaps, the computation of budget is as follow:
>          *      heap_budget = heap_size - global_heap_usage + app_heap_usage
> @@ -1503,6 +1505,7 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>          * in presence of shared buffers).
>          */
>         if (vram_size) {
> +               vram_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_VRAM);
>
> @@ -1510,11 +1513,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_VRAM_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM] = heap_usage;
> +               memoryBudget->heapBudget[vram_index] = heap_budget;
> +               memoryBudget->heapUsage[vram_index] = heap_usage;
>         }
>
>         if (visible_vram_size) {
> +               visible_vram_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_VRAM_VIS);
>
> @@ -1522,11 +1526,12 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_VRAM_VIS_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_VRAM_CPU_ACCESS] = heap_usage;
> +               memoryBudget->heapBudget[visible_vram_index] = heap_budget;
> +               memoryBudget->heapUsage[visible_vram_index] = heap_usage;
>         }
>
>         if (gtt_size) {
> +               gtt_index = heap_count++;
>                 heap_usage = device->ws->query_value(device->ws,
>                                                      RADEON_ALLOCATED_GTT);
>
> @@ -1534,8 +1539,8 @@ radv_get_memory_budget_properties(VkPhysicalDevice physicalDevice,
>                         device->ws->query_value(device->ws, RADEON_GTT_USAGE) +
>                         heap_usage;
>
> -               memoryBudget->heapBudget[RADV_MEM_HEAP_GTT] = heap_budget;
> -               memoryBudget->heapUsage[RADV_MEM_HEAP_GTT] = heap_usage;
> +               memoryBudget->heapBudget[gtt_index] = heap_budget;
> +               memoryBudget->heapUsage[gtt_index++] = heap_usage;
>         }
>
>         /* The heapBudget and heapUsage values must be zero for array elements
> --
> 2.22.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 2019-06-11 5:03 p.m., Samuel Pitoiset wrote:
> On 6/11/19 4:56 PM, Alex Deucher wrote:
>> On Tue, Jun 11, 2019 at 10:43 AM Samuel Pitoiset
>> <samuel.pitoiset@gmail.com> wrote:
>>> On VegaM, the visible VRAM size is equal to the VRAM size, which
>>> means only two heaps are exposed.
>> FWIW, this is not VegaM specific.  The vram size could be equal to the
>> visible vram size on any asic depending on whether the platform
>> supports large or resizeable BARs or not.
> Yeah, not saying it's specific to VegaM. It's just the first time I hit
> this, so I included that patch in the VegaM series. :-)

I think Alex's point (which I agree with) is that the commit log is
inaccurate and misleading.
On 6/12/19 9:58 AM, Michel Dänzer wrote:
> On 2019-06-11 5:03 p.m., Samuel Pitoiset wrote:
>> On 6/11/19 4:56 PM, Alex Deucher wrote:
>>> On Tue, Jun 11, 2019 at 10:43 AM Samuel Pitoiset
>>> <samuel.pitoiset@gmail.com> wrote:
>>>> On VegaM, the visible VRAM size is equal to the VRAM size, which
>>>> means only two heaps are exposed.
>>> FWIW, this is not VegaM specific.  The vram size could be equal to the
>>> visible vram size on any asic depending on whether the platform
>>> supports large or resizeable BARs or not.
>> Yeah, not saying it's specific to VegaM. It's just the first time I hit
>> this, so I included that patch in the VegaM series. :-)
> I think Alex's point (which I agree with) is that the commit log is
> inaccurate and misleading.
I will update the commit log before pushing. Forgot to do that for v2.
>
>
r-b with the vega m ref changed in the commit message.

On Wed, Jun 12, 2019 at 11:18 AM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
>
> On 6/12/19 9:58 AM, Michel Dänzer wrote:
> > On 2019-06-11 5:03 p.m., Samuel Pitoiset wrote:
> >> On 6/11/19 4:56 PM, Alex Deucher wrote:
> >>> On Tue, Jun 11, 2019 at 10:43 AM Samuel Pitoiset
> >>> <samuel.pitoiset@gmail.com> wrote:
> >>>> On VegaM, the visible VRAM size is equal to the VRAM size, which
> >>>> means only two heaps are exposed.
> >>> FWIW, this is not VegaM specific.  The vram size could be equal to the
> >>> visible vram size on any asic depending on whether the platform
> >>> supports large or resizeable BARs or not.
> >> Yeah, not saying it's specific to VegaM. It's just the first time I hit
> >> this, so I included that patch in the VegaM series. :-)
> > I think Alex's point (which I agree with) is that the commit log is
> > inaccurate and misleading.
> I will update the commit log before pushing. Forgot to do that for v2.
> >
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev