[Mesa-dev] [PATCH v2 1/5] anv: implementation of VK_EXT_debug_report extension
Jason Ekstrand
jason at jlekstrand.net
Thu Sep 7 16:51:17 UTC 2017
On Thu, Sep 7, 2017 at 9:46 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Mon, Aug 28, 2017 at 10:44 PM, Tapani Pälli <tapani.palli at 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.palli at 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170907/0fecb6d2/attachment-0001.html>
More information about the mesa-dev
mailing list