[Mesa-dev] [PATCH 3/3] radv: make device extension setup dynamic
Andres Rodriguez
andresx7 at gmail.com
Fri Jan 13 23:20:38 UTC 2017
On 2017-01-13 06:04 PM, Bas Nieuwenhuizen wrote:
> On Fri, Jan 13, 2017 at 11:06 PM, Andres Rodriguez <andresx7 at gmail.com> wrote:
>> Each physical may have different extensions than one another.
>> Furthermore, depending on the software stack, some extensions may not be
>> accessible.
>>
>> If an extension is conditional, it can be registered only when
>> necessary.
>>
>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>> ---
>> src/amd/vulkan/radv_device.c | 196 ++++++++++++++++++++++++++++--------------
>> src/amd/vulkan/radv_private.h | 6 ++
>> 2 files changed, 137 insertions(+), 65 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
>> index 5669fd7..0333688 100644
>> --- a/src/amd/vulkan/radv_device.c
>> +++ b/src/amd/vulkan/radv_device.c
>> @@ -77,6 +77,115 @@ radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
>> return 0;
>> }
>>
>> +static const VkExtensionProperties instance_extensions[] = {
>> + {
>> + .extensionName = VK_KHR_SURFACE_EXTENSION_NAME,
>> + .specVersion = 25,
>> + },
>> +#ifdef VK_USE_PLATFORM_XCB_KHR
>> + {
>> + .extensionName = VK_KHR_XCB_SURFACE_EXTENSION_NAME,
>> + .specVersion = 6,
>> + },
>> +#endif
>> +#ifdef VK_USE_PLATFORM_XLIB_KHR
>> + {
>> + .extensionName = VK_KHR_XLIB_SURFACE_EXTENSION_NAME,
>> + .specVersion = 6,
>> + },
>> +#endif
>> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>> + {
>> + .extensionName = VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME,
>> + .specVersion = 5,
>> + },
>> +#endif
>> +};
>> +
>> +static const VkExtensionProperties common_device_extensions[] = {
>> + {
>> + .extensionName = VK_KHR_SAMPLER_MIRROR_CLAMP_TO_EDGE_EXTENSION_NAME,
>> + .specVersion = 1,
>> + },
>> + {
>> + .extensionName = VK_KHR_SWAPCHAIN_EXTENSION_NAME,
>> + .specVersion = 68,
>> + },
>> + {
>> + .extensionName = VK_AMD_DRAW_INDIRECT_COUNT_EXTENSION_NAME,
>> + .specVersion = 1,
>> + },
>> + {
>> + .extensionName = VK_AMD_NEGATIVE_VIEWPORT_HEIGHT_EXTENSION_NAME,
>> + .specVersion = 1,
>> + },
>> +};
>> +
>> +static VkResult
>> +radv_extensions_register(struct radv_instance *instance,
>> + struct radv_extensions *extensions,
>> + const VkExtensionProperties *new_ext,
>> + uint32_t num_ext)
>> +{
>> + size_t new_size;
>> + VkExtensionProperties *new_ptr;
>> +
>> + assert(new_ext && num_ext > 0);
>> +
>> + if (!new_ext)
>> + return VK_ERROR_INITIALIZATION_FAILED;
>> +
>> + new_size = (extensions->num_ext + num_ext) * sizeof(VkExtensionProperties);
>> + new_ptr = vk_realloc(&instance->alloc, extensions->ext_array,
>> + new_size, 8, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
>> +
>> + /* Old array continues to be valid, update nothing */
>> + if (!new_ptr)
>> + return VK_ERROR_OUT_OF_HOST_MEMORY;
>> +
>> + memcpy(&new_ptr[extensions->num_ext], new_ext,
>> + num_ext * sizeof(VkExtensionProperties));
>> + extensions->ext_array = new_ptr;
>> + extensions->num_ext += num_ext;
>> +
>> + return VK_SUCCESS;
>> +}
>> +
>> +#define radv_extensions_register_single(instance, extensions, name, version) \
>> + radv_extensions_register(instance, extensions, \
>> + &(VkExtensionProperties){ \
>> + .extensionName = name, \
>> + .specVersion = version \
>> + }, 1);
> Please make this a function, I see no reason to keep this a macro. Or
> lose it, as I can't find an user in this patch.
>> +
>> +static void
>> +radv_extensions_finish(struct radv_instance *instance,
>> + struct radv_extensions *extensions)
>> +{
>> + assert(extensions);
>> +
>> + if (!extensions)
>> + radv_loge("Attemted to free invalid extension struct\n");
>> +
>> + if (extensions->ext_array)
>> + vk_free(&instance->alloc, extensions->ext_array);
>> +}
>> +
>> +static bool
>> +is_extension_enabled(const VkExtensionProperties *extensions,
>> + size_t num_ext,
>> + const char *name)
>> +{
>> + assert(extensions && name);
>> +
>> + for (uint32_t i = 0; i < num_ext; i++) {
>> + if (strcmp(name, extensions[i].extensionName) == 0)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> static VkResult
>> radv_physical_device_init(struct radv_physical_device *device,
>> struct radv_instance *instance,
>> @@ -130,6 +239,13 @@ radv_physical_device_init(struct radv_physical_device *device,
>> goto fail;
>> }
>>
>> + result = radv_extensions_register(instance,
>> + &device->extensions,
>> + common_device_extensions,
>> + ARRAY_SIZE(common_device_extensions));
>> + if (result != VK_SUCCESS)
>> + goto fail;
>> +
>> fprintf(stderr, "WARNING: radv is not a conformant vulkan implementation, testing use only.\n");
>> device->name = device->rad_info.name;
>> close(fd);
>> @@ -143,53 +259,11 @@ fail:
>> static void
>> radv_physical_device_finish(struct radv_physical_device *device)
>> {
>> + radv_extensions_finish(device->instance, &device->extensions);
>> radv_finish_wsi(device);
>> device->ws->destroy(device->ws);
>> }
>>
>> -static const VkExtensionProperties instance_extensions[] = {
>> - {
>> - .extensionName = VK_KHR_SURFACE_EXTENSION_NAME,
>> - .specVersion = 25,
>> - },
>> -#ifdef VK_USE_PLATFORM_XCB_KHR
>> - {
>> - .extensionName = VK_KHR_XCB_SURFACE_EXTENSION_NAME,
>> - .specVersion = 6,
>> - },
>> -#endif
>> -#ifdef VK_USE_PLATFORM_XLIB_KHR
>> - {
>> - .extensionName = VK_KHR_XLIB_SURFACE_EXTENSION_NAME,
>> - .specVersion = 6,
>> - },
>> -#endif
>> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>> - {
>> - .extensionName = VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME,
>> - .specVersion = 5,
>> - },
>> -#endif
>> -};
>> -
>> -static const VkExtensionProperties common_device_extensions[] = {
>> - {
>> - .extensionName = VK_KHR_SAMPLER_MIRROR_CLAMP_TO_EDGE_EXTENSION_NAME,
>> - .specVersion = 1,
>> - },
>> - {
>> - .extensionName = VK_KHR_SWAPCHAIN_EXTENSION_NAME,
>> - .specVersion = 68,
>> - },
>> - {
>> - .extensionName = VK_AMD_DRAW_INDIRECT_COUNT_EXTENSION_NAME,
>> - .specVersion = 1,
>> - },
>> - {
>> - .extensionName = VK_AMD_NEGATIVE_VIEWPORT_HEIGHT_EXTENSION_NAME,
>> - .specVersion = 1,
>> - },
>> -};
>>
>> static void *
>> default_alloc_func(void *pUserData, size_t size, size_t align,
>> @@ -257,15 +331,9 @@ VkResult radv_CreateInstance(
>> }
>>
>> for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
>> - bool found = false;
>> - for (uint32_t j = 0; j < ARRAY_SIZE(instance_extensions); j++) {
>> - if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
>> - instance_extensions[j].extensionName) == 0) {
>> - found = true;
>> - break;
>> - }
>> - }
>> - if (!found)
>> + if (!is_extension_enabled(instance_extensions,
>> + ARRAY_SIZE(instance_extensions),
>> + pCreateInfo->ppEnabledExtensionNames[i]))
>> return vk_error(VK_ERROR_EXTENSION_NOT_PRESENT);
>> }
>>
>> @@ -274,6 +342,8 @@ VkResult radv_CreateInstance(
>> if (!instance)
>> return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
>>
>> + memset(instance, 0, sizeof(*instance));
>> +
> Why is this memset necessary for this patch?
>
> Otherwise, with fixed commit message for patch 1 and fixed alignment,
> this series looks good to me.
We need a way to tell whether extensions->ext_array is initialized or not for:
+ new_ptr = vk_realloc(&instance->alloc, extensions->ext_array,
+ new_size, 8, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
If it is unclear, I can create a radv_extensions_initialize() function to clean our the relevant fields. But I was thinking that having the all other instance members zero-initialized may be useful as well.
>
>> instance->_loader_data.loaderMagic = ICD_LOADER_MAGIC;
>>
>> if (pAllocator)
>> @@ -696,15 +766,9 @@ VkResult radv_CreateDevice(
>> struct radv_device *device;
>>
>> for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
>> - bool found = false;
>> - for (uint32_t j = 0; j < ARRAY_SIZE(common_device_extensions); j++) {
>> - if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
>> - common_device_extensions[j].extensionName) == 0) {
>> - found = true;
>> - break;
>> - }
>> - }
>> - if (!found)
>> + if (!is_extension_enabled(physical_device->extensions.ext_array,
>> + physical_device->extensions.num_ext,
>> + pCreateInfo->ppEnabledExtensionNames[i]))
>> return vk_error(VK_ERROR_EXTENSION_NOT_PRESENT);
>> }
>>
>> @@ -850,15 +914,17 @@ VkResult radv_EnumerateDeviceExtensionProperties(
>> uint32_t* pPropertyCount,
>> VkExtensionProperties* pProperties)
>> {
>> + RADV_FROM_HANDLE(radv_physical_device, pdevice, physicalDevice);
>> +
>> if (pProperties == NULL) {
>> - *pPropertyCount = ARRAY_SIZE(common_device_extensions);
>> + *pPropertyCount = pdevice->extensions.num_ext;
>> return VK_SUCCESS;
>> }
>>
>> - *pPropertyCount = MIN2(*pPropertyCount, ARRAY_SIZE(common_device_extensions));
>> - typed_memcpy(pProperties, common_device_extensions, *pPropertyCount);
>> + *pPropertyCount = MIN2(*pPropertyCount, pdevice->extensions.num_ext);
>> + typed_memcpy(pProperties, pdevice->extensions.ext_array, *pPropertyCount);
>>
>> - if (*pPropertyCount < ARRAY_SIZE(common_device_extensions))
>> + if (*pPropertyCount < pdevice->extensions.num_ext)
>> return VK_INCOMPLETE;
>>
>> return VK_SUCCESS;
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index ab4ede6..b095e3f 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -263,6 +263,11 @@ void *radv_lookup_entrypoint(const char *name);
>>
>> extern struct radv_dispatch_table dtable;
>>
>> +struct radv_extensions {
>> + VkExtensionProperties *ext_array;
>> + uint32_t num_ext;
>> +};
>> +
>> struct radv_physical_device {
>> VK_LOADER_DATA _loader_data;
>>
>> @@ -275,6 +280,7 @@ struct radv_physical_device {
>> uint8_t uuid[VK_UUID_SIZE];
>>
>> struct wsi_device wsi_device;
>> + struct radv_extensions extensions;
>> };
>>
>> struct radv_instance {
>> --
>> 2.9.3
>>
More information about the mesa-dev
mailing list