On Sat, Jan 14, 2017 at 12:20 AM, Andres Rodriguez <andre...@gmail.com> wrote:
>
>
> On 2017-01-13 06:04 PM, Bas Nieuwenhuizen wrote:
>>
>> On Fri, Jan 13, 2017 at 11:06 PM, Andres Rodriguez <andre...@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 <andre...@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.
But we aren't adding extensions as a member to the instance struct, so
I don't think that should matter here?
>
>
>>
>>> 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
>>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
