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.


         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

Reply via email to