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