<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 28, 2017 at 10:44 PM, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Patch adds required functionality for extension to manage a list of<br>
application provided callbacks and handle debug reporting from driver<br>
and application side.<br>
<br>
v2: remove useless helper anv_debug_report_call<br>
    add locking around callbacks list<br>
    use vk_alloc2, vk_free2 helpers<br>
    refactor CreateDebugReportCallbackEXT<br>
    fix bugs found with crucible testing<br>
<br>
Signed-off-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>><br>
---<br>
 src/intel/Makefile.sources          |   1 +<br>
 src/intel/vulkan/anv_debug_<wbr>report.c | 124 ++++++++++++++++++++++++++++++<wbr>++++++<br>
 src/intel/vulkan/anv_device.c       |  46 +++++++++++++<br>
 src/intel/vulkan/anv_<wbr>extensions.py  |   1 +<br>
 src/intel/vulkan/anv_private.h      |  24 +++++++<br>
 5 files changed, 196 insertions(+)<br>
 create mode 100644 src/intel/vulkan/anv_debug_<wbr>report.c<br>
<br>
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources<br>
index 4074ba9ee5..200713b06e 100644<br>
--- a/src/intel/Makefile.sources<br>
+++ b/src/intel/Makefile.sources<br>
@@ -205,6 +205,7 @@ VULKAN_FILES := \<br>
        vulkan/anv_batch_chain.c \<br>
        vulkan/anv_blorp.c \<br>
        vulkan/anv_cmd_buffer.c \<br>
+       vulkan/anv_debug_report.c \<br>
        vulkan/anv_descriptor_set.c \<br>
        vulkan/anv_device.c \<br>
        vulkan/anv_dump.c \<br>
diff --git a/src/intel/vulkan/anv_debug_<wbr>report.c b/src/intel/vulkan/anv_debug_<wbr>report.c<br>
new file mode 100644<br>
index 0000000000..d3239f0cc0<br>
--- /dev/null<br>
+++ b/src/intel/vulkan/anv_debug_<wbr>report.c<br>
@@ -0,0 +1,124 @@<br>
+/*<br>
+ * Copyright © 2017 Intel Corporation<br>
+ *<br>
+ * Permission is hereby granted, free of charge, to any person obtaining a<br>
+ * copy of this software and associated documentation files (the "Software"),<br>
+ * to deal in the Software without restriction, including without limitation<br>
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,<br>
+ * and/or sell copies of the Software, and to permit persons to whom the<br>
+ * Software is furnished to do so, subject to the following conditions:<br>
+ *<br>
+ * The above copyright notice and this permission notice (including the next<br>
+ * paragraph) shall be included in all copies or substantial portions of the<br>
+ * Software.<br>
+ *<br>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR<br>
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,<br>
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL<br>
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER<br>
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING<br>
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS<br>
+ * IN THE SOFTWARE.<br>
+ */<br>
+<br>
+#include "anv_private.h"<br>
+#include "vk_util.h"<br>
+<br>
+/* This file contains implementation for VK_EXT_debug_report. */<br>
+<br>
+VkResult<br>
+anv_<wbr>CreateDebugReportCallbackEXT(<wbr>VkInstance _instance,<br>
+                                 const VkDebugReportCallbackCreateInf<wbr>oEXT* pCreateInfo,<br>
+                                 const VkAllocationCallbacks* pAllocator,<br>
+                                 VkDebugReportCallbackEXT* pCallback)<br>
+{<br>
+   ANV_FROM_HANDLE(anv_instance, instance, _instance);<br>
+<br>
+   struct anv_debug_callback *cb =<br>
+      vk_alloc2(&instance->alloc, pAllocator, sizeof(struct anv_debug_callback),<br>
+                8, VK_SYSTEM_ALLOCATION_SCOPE_<wbr>INSTANCE);<br></blockquote><div><br></div><div>You should use SCOPE_OBJECT here.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+   if (!cb)<br>
+      return vk_error(VK_ERROR_OUT_OF_HOST_<wbr>MEMORY);<br>
+<br>
+   cb->flags = pCreateInfo->flags;<br>
+   cb->callback = pCreateInfo->pfnCallback;<br>
+   cb->data = pCreateInfo->pUserData;<br>
+<br>
+   pthread_mutex_lock(&instance-><wbr>callbacks_mutex);<br>
+   list_addtail(&cb->link, &instance->callbacks);<br>
+   pthread_mutex_unlock(&<wbr>instance->callbacks_mutex);<br>
+<br>
+   *pCallback = (VkDebugReportCallbackEXT) cb;<br></blockquote><div><br></div><div>Please add an ANV_DEFINE_HANDLE_CASTS for anv_debug_report_callback and use anv_debug_report_callback_to_handle here.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+   return VK_SUCCESS;<br>
+}<br>
+<br>
+void<br>
+anv_<wbr>DestroyDebugReportCallbackEXT(<wbr>VkInstance _instance,<br>
+                                  VkDebugReportCallbackEXT callback,<br>
+                                  const VkAllocationCallbacks* pAllocator)<br>
+{<br>
+   ANV_FROM_HANDLE(anv_instance, instance, _instance);<br>
+<br>
+   pthread_mutex_lock(&instance-><wbr>callbacks_mutex);<br>
+<br>
+   list_for_each_entry_safe(<wbr>struct anv_debug_callback, debug_cb,<br>
+                            &instance->callbacks, link) {<br>
+      /* Found a match, remove from list and destroy given callback. */<br>
+      if ((VkDebugReportCallbackEXT)<wbr>debug_cb == callback) {<br>
+         list_del(&debug_cb->link);<br>
+         vk_free2(&instance->alloc, pAllocator, debug_cb);<br>
+      }<br>
+   }<br></blockquote><div><br></div><div>You don't need to walk the list here.  You have the callback just do</div><div><br></div><div>ANV_FROM_HANDLE(anv_debug_report_callback, callback, _callback);</div><div><br></div><div>at the top and then do</div><div><br></div><div>list_del(&callback->link);</div><div><br></div><div>No search needed.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+   pthread_mutex_unlock(&<wbr>instance->callbacks_mutex);<br>
+}<br>
+<br>
+void<br>
+anv_DebugReportMessageEXT(<wbr>VkInstance _instance,<br>
+                          VkDebugReportFlagsEXT flags,<br>
+                          VkDebugReportObjectTypeEXT objectType,<br>
+                          uint64_t object,<br>
+                          size_t location,<br>
+                          int32_t messageCode,<br>
+                          const char* pLayerPrefix,<br>
+                          const char* pMessage)<br>
+{<br>
+   ANV_FROM_HANDLE(anv_instance, instance, _instance);<br>
+   anv_debug_report(instance, flags, objectType, object,<br>
+                    location, messageCode, pLayerPrefix, pMessage);<br>
+}<br>
+<br>
+void<br>
+anv_debug_report(struct anv_instance *instance,<br>
+                 VkDebugReportFlagsEXT flags,<br>
+                 VkDebugReportObjectTypeEXT object_type,<br>
+                 uint64_t handle,<br>
+                 size_t location,<br>
+                 int32_t messageCode,<br>
+                 const char* pLayerPrefix,<br>
+                 const char *pMessage)<br>
+{<br>
+   /* Allow NULL for convinience, return if no callbacks registered. */<br>
+   if (!instance || list_empty(&instance-><wbr>callbacks))<br>
+      return;<br>
+<br>
+   pthread_mutex_lock(&instance-><wbr>callbacks_mutex);<br>
+<br>
+   /* Section 33.2 of the Vulkan 1.0.59 spec says:<br>
+    *<br>
+    *    "callback is an externally synchronized object and must not be<br>
+    *    used on more than one thread at a time. This means that<br>
+    *    vkDestroyDebugReportCallbackEX<wbr>T must not be called when a callback<br>
+    *    is active."<br>
+    */<br>
+   list_for_each_entry_safe(<wbr>struct anv_debug_callback, cb,<br>
+                            &instance->callbacks, link) {<br>
+      if (cb->flags & flags)<br>
+         cb->callback(flags, object_type, handle, location, messageCode,<br>
+                      pLayerPrefix, pMessage, cb->data);<br>
+   }<br>
+<br>
+   pthread_mutex_unlock(&<wbr>instance->callbacks_mutex);<br>
+}<br>
diff --git a/src/intel/vulkan/anv_device.<wbr>c b/src/intel/vulkan/anv_device.<wbr>c<br>
index 2e0fa19b1a..7e091246a8 100644<br>
--- a/src/intel/vulkan/anv_device.<wbr>c<br>
+++ b/src/intel/vulkan/anv_device.<wbr>c<br>
@@ -443,9 +443,28 @@ VkResult anv_CreateInstance(<br>
     VkInstance*                                 pInstance)<br>
 {<br>
    struct anv_instance *instance;<br>
+   VkDebugReportCallbackCreateInf<wbr>oEXT *ctor_cb = NULL;<br>
<br>
    assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_INSTANCE_<wbr>CREATE_INFO);<br>
<br>
+   /* Check if user passed a debug report callback to be used during<br>
+    * Create/Destroy of instance.<br>
+    */<br>
+   vk_foreach_struct(ext, pCreateInfo->pNext) {<br>
+      switch (ext->sType) {<br>
+      case VK_STRUCTURE_TYPE_DEBUG_<wbr>REPORT_CALLBACK_CREATE_INFO_<wbr>EXT: {<br>
+         ctor_cb = (<wbr>VkDebugReportCallbackCreateInf<wbr>oEXT *) ext;<br>
+         break;<br>
+      }<br>
+      default:<br>
+         /* We ignore any other type by purpose here, we are only<br>
+          * interested if debug report callback was found for instance<br>
+          * ctor/dtor.<br>
+          */<br>
+         break;<br>
+      }<br>
+   };<br>
+<br>
    uint32_t client_version;<br>
    if (pCreateInfo->pApplicationInfo &&<br>
        pCreateInfo->pApplicationInfo-<wbr>>apiVersion != 0) {<br>
@@ -456,6 +475,17 @@ VkResult anv_CreateInstance(<br>
<br>
    if (VK_MAKE_VERSION(1, 0, 0) > client_version ||<br>
        client_version > VK_MAKE_VERSION(1, 0, 0xfff)) {<br>
+<br>
+      if (ctor_cb && ctor_cb->flags & VK_DEBUG_REPORT_ERROR_BIT_EXT)<br>
+         ctor_cb->pfnCallback(VK_DEBUG_<wbr>REPORT_ERROR_BIT_EXT,<br>
+                              VK_DEBUG_REPORT_OBJECT_TYPE_<wbr>INSTANCE_EXT,<br>
+                              0, /* No handle available yet. */<br>
+                              __LINE__,<br>
+                              0,<br>
+                              "anv",<br>
+                              "incompatible driver version",<br>
+                              ctor_cb->pUserData);<br>
+<br>
       return vk_errorf(VK_ERROR_<wbr>INCOMPATIBLE_DRIVER,<br>
                        "Client requested version %d.%d.%d",<br>
                        VK_VERSION_MAJOR(client_<wbr>version),<br>
@@ -484,6 +514,20 @@ VkResult anv_CreateInstance(<br>
    instance->apiVersion = client_version;<br>
    instance->physicalDeviceCount = -1;<br>
<br>
+   if (pthread_mutex_init(&instance-<wbr>>callbacks_mutex, NULL) != 0) {<br>
+      vk_free2(&default_alloc, pAllocator, instance);<br>
+      return vk_error(VK_ERROR_<wbr>INITIALIZATION_FAILED);<br>
+   }<br>
+<br>
+   list_inithead(&instance-><wbr>callbacks);<br>
+<br>
+   /* Store report debug callback to be used during DestroyInstance. */<br>
+   if (ctor_cb) {<br>
+      instance->dtor_cb.flags = ctor_cb->flags;<br>
+      instance->dtor_cb.callback = ctor_cb->pfnCallback;<br>
+      instance->dtor_cb.data = ctor_cb->pUserData;<br>
+   }<br>
+<br>
    _mesa_locale_init();<br>
<br>
    VG(VALGRIND_CREATE_MEMPOOL(<wbr>instance, 0, false));<br>
@@ -510,6 +554,8 @@ void anv_DestroyInstance(<br>
<br>
    VG(VALGRIND_DESTROY_MEMPOOL(<wbr>instance));<br>
<br>
+   pthread_mutex_destroy(&<wbr>instance->callbacks_mutex);<br>
+<br>
    _mesa_locale_fini();<br>
<br>
    vk_free(&instance->alloc, instance);<br>
diff --git a/src/intel/vulkan/anv_<wbr>extensions.py b/src/intel/vulkan/anv_<wbr>extensions.py<br>
index 6b3d72e4b4..acec785959 100644<br>
--- a/src/intel/vulkan/anv_<wbr>extensions.py<br>
+++ b/src/intel/vulkan/anv_<wbr>extensions.py<br>
@@ -75,6 +75,7 @@ EXTENSIONS = [<br>
     Extension('VK_KHR_xcb_surface'<wbr>,                       6, 'VK_USE_PLATFORM_XCB_KHR'),<br>
     Extension('VK_KHR_xlib_<wbr>surface',                      6, 'VK_USE_PLATFORM_XLIB_KHR'),<br>
     Extension('VK_KHX_multiview',                         1, True),<br>
+    Extension('VK_EXT_debug_<wbr>report',                      8, True),<br>
 ]<br>
<br>
 class VkVersion:<br>
diff --git a/src/intel/vulkan/anv_<wbr>private.h b/src/intel/vulkan/anv_<wbr>private.h<br>
index b30b71f336..ae66c5439b 100644<br>
--- a/src/intel/vulkan/anv_<wbr>private.h<br>
+++ b/src/intel/vulkan/anv_<wbr>private.h<br>
@@ -61,6 +61,8 @@ typedef uint32_t xcb_window_t;<br>
 struct anv_buffer;<br>
 struct anv_buffer_view;<br>
 struct anv_image_view;<br>
+struct anv_instance;<br>
+struct anv_debug_callback;<br>
<br>
 struct gen_l3_config;<br>
<br>
@@ -237,6 +239,15 @@ void __anv_perf_warn(const char *file, int line, const char *format, ...)<br>
 void anv_loge(const char *format, ...) anv_printflike(1, 2);<br>
 void anv_loge_v(const char *format, va_list va);<br>
<br>
+void anv_debug_report(struct anv_instance *instance,<br>
+                      VkDebugReportFlagsEXT flags,<br>
+                      VkDebugReportObjectTypeEXT object_type,<br>
+                      uint64_t handle,<br>
+                      size_t location,<br>
+                      int32_t messageCode,<br>
+                      const char* pLayerPrefix,<br>
+                      const char *pMessage);<br>
+<br>
 /**<br>
  * Print a FINISHME message, including its source location.<br>
  */<br>
@@ -666,6 +677,14 @@ struct anv_physical_device {<br>
     int                                         local_fd;<br>
 };<br>
<br>
+struct anv_debug_callback {<br>
+   /* Link in the 'callbacks' list in anv_instance struct. */<br>
+   struct list_head                             link;<br>
+   VkDebugReportFlagsEXT                        flags;<br>
+   PFN_vkDebugReportCallbackEXT                 callback;<br>
+   void *                                       data;<br>
+};<br>
+<br>
 struct anv_instance {<br>
     VK_LOADER_DATA                              _loader_data;<br>
<br>
@@ -674,6 +693,11 @@ struct anv_instance {<br>
     uint32_t                                    apiVersion;<br>
     int                                         physicalDeviceCount;<br>
     struct anv_physical_device                  physicalDevice;<br>
+<br>
+    /* VK_EXT_debug_report debug callbacks */<br>
+    pthread_mutex_t                             callbacks_mutex;<br>
+    struct list_head                            callbacks;<br>
+    struct anv_debug_callback                   dtor_cb;<br></blockquote><div><br></div><div>Maybe call this destroy_debug_cb to be a bit more descriptive?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 };<br>
<br>
 VkResult anv_init_wsi(struct anv_physical_device *physical_device);<br>
<span class="gmail-HOEnZb"><font color="#888888">--<br>
2.13.5<br>
<br>
</font></span></blockquote></div><br></div></div>