[Mesa-dev] [PATCH 1/2] vulkan: move anv VK_EXT_debug_report implementation to common code.
Tapani Pälli
tapani.palli at intel.com
Wed Jan 10 12:10:36 UTC 2018
This LGTM. You are taking away the instance constructor/destructor
callback but it wasn't used anyway so that's fine. If someone needs it,
it can be added later.
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
On 01/09/2018 04:04 PM, Bas Nieuwenhuizen wrote:
> For also using it in radv. I moved the remaining stubs back to
> anv_device.c as they were just trivial.
>
> This does not move the vk_errorf/anv_perf_warn or the object
> type macros, as those depend on anv types and logging.
> ---
> src/intel/Makefile.sources | 1 -
> src/intel/vulkan/anv_device.c | 54 +++++++++++---
> src/intel/vulkan/anv_private.h | 28 +------
> src/intel/vulkan/anv_util.c | 32 ++++----
> src/intel/vulkan/meson.build | 1 -
> src/vulkan/Makefile.sources | 2 +
> src/vulkan/util/meson.build | 2 +
> .../util/vk_debug_report.c} | 85 +++++++++++-----------
> src/vulkan/util/vk_debug_report.h | 72 ++++++++++++++++++
> 9 files changed, 182 insertions(+), 95 deletions(-)
> rename src/{intel/vulkan/anv_debug_report.c => vulkan/util/vk_debug_report.c} (56%)
> create mode 100644 src/vulkan/util/vk_debug_report.h
>
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 1c62bad816..7f8379c66a 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -209,7 +209,6 @@ 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_device.c b/src/intel/vulkan/anv_device.c
> index 680f5a752d..1c2590e11c 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -482,6 +482,7 @@ VkResult anv_CreateInstance(
> VkInstance* pInstance)
> {
> struct anv_instance *instance;
> + VkResult result;
>
> assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO);
>
> @@ -541,18 +542,10 @@ VkResult anv_CreateInstance(
> instance->apiVersion = client_version;
> instance->physicalDeviceCount = -1;
>
> - if (pthread_mutex_init(&instance->callbacks_mutex, NULL) != 0) {
> + result = vk_debug_report_instance_init(&instance->debug_report_callbacks);
> + if (result != VK_SUCCESS) {
> 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->destroy_debug_cb.flags = ctor_cb->flags;
> - instance->destroy_debug_cb.callback = ctor_cb->pfnCallback;
> - instance->destroy_debug_cb.data = ctor_cb->pUserData;
> + return vk_error(result);
> }
>
> _mesa_locale_init();
> @@ -581,7 +574,7 @@ void anv_DestroyInstance(
>
> VG(VALGRIND_DESTROY_MEMPOOL(instance));
>
> - pthread_mutex_destroy(&instance->callbacks_mutex);
> + vk_debug_report_instance_destroy(&instance->debug_report_callbacks);
>
> _mesa_locale_fini();
>
> @@ -1066,6 +1059,43 @@ PFN_vkVoidFunction anv_GetDeviceProcAddr(
> return anv_lookup_entrypoint(&device->info, pName);
> }
>
> +VkResult
> +anv_CreateDebugReportCallbackEXT(VkInstance _instance,
> + const VkDebugReportCallbackCreateInfoEXT* pCreateInfo,
> + const VkAllocationCallbacks* pAllocator,
> + VkDebugReportCallbackEXT* pCallback)
> +{
> + ANV_FROM_HANDLE(anv_instance, instance, _instance);
> + return vk_create_debug_report_callback(&instance->debug_report_callbacks,
> + pCreateInfo, pAllocator, &instance->alloc,
> + pCallback);
> +}
> +
> +void
> +anv_DestroyDebugReportCallbackEXT(VkInstance _instance,
> + VkDebugReportCallbackEXT _callback,
> + const VkAllocationCallbacks* pAllocator)
> +{
> + ANV_FROM_HANDLE(anv_instance, instance, _instance);
> + vk_destroy_debug_report_callback(&instance->debug_report_callbacks,
> + _callback, pAllocator, &instance->alloc);
> +}
> +
> +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);
> + vk_debug_report(&instance->debug_report_callbacks, flags, objectType,
> + object, location, messageCode, pLayerPrefix, pMessage);
> +}
> +
> static void
> anv_queue_init(struct anv_device *device, struct anv_queue *queue)
> {
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index b7bde4b8ce..ed711e9434 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -50,6 +50,7 @@
> #include "util/u_atomic.h"
> #include "util/u_vector.h"
> #include "vk_alloc.h"
> +#include "vk_debug_report.h"
>
> /* Pre-declarations needed for WSI entrypoints */
> struct wl_surface;
> @@ -62,7 +63,6 @@ struct anv_buffer;
> struct anv_buffer_view;
> struct anv_image_view;
> struct anv_instance;
> -struct anv_debug_report_callback;
>
> struct gen_l3_config;
>
> @@ -291,7 +291,7 @@ vk_to_isl_color(VkClearColorValue color)
> __builtin_types_compatible_p (__typeof (o), struct wsi_swapchain*), \
> VK_DEBUG_REPORT_OBJECT_TYPE_SWAPCHAIN_KHR_EXT, \
> __builtin_choose_expr ( \
> - __builtin_types_compatible_p (__typeof (o), struct anv_debug_callback*), \
> + __builtin_types_compatible_p (__typeof (o), struct vk_debug_callback*), \
> VK_DEBUG_REPORT_OBJECT_TYPE_DEBUG_REPORT_CALLBACK_EXT_EXT, \
> __builtin_choose_expr ( \
> __builtin_types_compatible_p (__typeof (o), void*), \
> @@ -346,15 +346,6 @@ void __anv_perf_warn(struct anv_instance *instance, const void *object,
> 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.
> */
> @@ -796,14 +787,6 @@ struct anv_physical_device {
> int local_fd;
> };
>
> -struct anv_debug_report_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;
>
> @@ -813,10 +796,7 @@ struct anv_instance {
> 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_report_callback destroy_debug_cb;
> + struct vk_debug_report_instance debug_report_callbacks;
> };
>
> VkResult anv_init_wsi(struct anv_physical_device *physical_device);
> @@ -2922,7 +2902,7 @@ ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_render_pass, VkRenderPass)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_sampler, VkSampler)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_semaphore, VkSemaphore)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_shader_module, VkShaderModule)
> -ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_debug_report_callback, VkDebugReportCallbackEXT)
> +ANV_DEFINE_NONDISP_HANDLE_CASTS(vk_debug_report_callback, VkDebugReportCallbackEXT)
> ANV_DEFINE_NONDISP_HANDLE_CASTS(anv_ycbcr_conversion, VkSamplerYcbcrConversionKHR)
>
> /* Gen-specific function declarations */
> diff --git a/src/intel/vulkan/anv_util.c b/src/intel/vulkan/anv_util.c
> index 59f893492b..6b31224d7f 100644
> --- a/src/intel/vulkan/anv_util.c
> +++ b/src/intel/vulkan/anv_util.c
> @@ -65,14 +65,14 @@ __anv_perf_warn(struct anv_instance *instance, const void *object,
>
> snprintf(report, sizeof(report), "%s: %s", file, buffer);
>
> - anv_debug_report(instance,
> - VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT,
> - type,
> - (uint64_t) (uintptr_t) object,
> - line,
> - 0,
> - "anv",
> - report);
> + vk_debug_report(&instance->debug_report_callbacks,
> + VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT,
> + type,
> + (uint64_t) (uintptr_t) object,
> + line,
> + 0,
> + "anv",
> + report);
>
> intel_logw("%s:%d: PERF: %s", file, line, buffer);
> }
> @@ -99,14 +99,14 @@ __vk_errorf(struct anv_instance *instance, const void *object,
> snprintf(report, sizeof(report), "%s:%d: %s", file, line, error_str);
> }
>
> - anv_debug_report(instance,
> - VK_DEBUG_REPORT_ERROR_BIT_EXT,
> - type,
> - (uint64_t) (uintptr_t) object,
> - line,
> - 0,
> - "anv",
> - report);
> + vk_debug_report(&instance->debug_report_callbacks,
> + VK_DEBUG_REPORT_ERROR_BIT_EXT,
> + type,
> + (uint64_t) (uintptr_t) object,
> + line,
> + 0,
> + "anv",
> + report);
>
> intel_loge("%s", report);
>
> diff --git a/src/intel/vulkan/meson.build b/src/intel/vulkan/meson.build
> index 606a4898fe..5e97a92be9 100644
> --- a/src/intel/vulkan/meson.build
> +++ b/src/intel/vulkan/meson.build
> @@ -100,7 +100,6 @@ libanv_files = files(
> 'anv_batch_chain.c',
> 'anv_blorp.c',
> 'anv_cmd_buffer.c',
> - 'anv_debug_report.c',
> 'anv_descriptor_set.c',
> 'anv_device.c',
> 'anv_dump.c',
> diff --git a/src/vulkan/Makefile.sources b/src/vulkan/Makefile.sources
> index 6e9a09ce78..a0a24ce7de 100644
> --- a/src/vulkan/Makefile.sources
> +++ b/src/vulkan/Makefile.sources
> @@ -19,6 +19,8 @@ VULKAN_WSI_X11_FILES := \
>
> VULKAN_UTIL_FILES := \
> util/vk_alloc.h \
> + util/vk_debug_report.c \
> + util/vk_debug_report.h \
> util/vk_util.c \
> util/vk_util.h
>
> diff --git a/src/vulkan/util/meson.build b/src/vulkan/util/meson.build
> index 6b0ec1e5ef..4508067ad9 100644
> --- a/src/vulkan/util/meson.build
> +++ b/src/vulkan/util/meson.build
> @@ -20,6 +20,8 @@
>
> files_vulkan_util = files(
> 'vk_alloc.h',
> + 'vk_debug_report.c',
> + 'vk_debug_report.h',
> 'vk_util.c',
> 'vk_util.h',
> )
> diff --git a/src/intel/vulkan/anv_debug_report.c b/src/vulkan/util/vk_debug_report.c
> similarity index 56%
> rename from src/intel/vulkan/anv_debug_report.c
> rename to src/vulkan/util/vk_debug_report.c
> index 55d62057c2..22ae4a7d93 100644
> --- a/src/intel/vulkan/anv_debug_report.c
> +++ b/src/vulkan/util/vk_debug_report.c
> @@ -21,26 +21,42 @@
> * IN THE SOFTWARE.
> */
>
> -#include "anv_private.h"
> +#include "vk_debug_report.h"
> +
> +#include "vk_alloc.h"
> #include "vk_util.h"
>
> -/* This file contains implementation for VK_EXT_debug_report. */
> +VkResult vk_debug_report_instance_init(struct vk_debug_report_instance *instance)
> +{
> + if (pthread_mutex_init(&instance->callbacks_mutex, NULL) != 0) {
> + return VK_ERROR_INITIALIZATION_FAILED;
> + }
> +
> + list_inithead(&instance->callbacks);
> +
> + return VK_SUCCESS;
> +}
> +
> +void vk_debug_report_instance_destroy(struct vk_debug_report_instance *instance)
> +{
> + pthread_mutex_destroy(&instance->callbacks_mutex);
> +}
>
> VkResult
> -anv_CreateDebugReportCallbackEXT(VkInstance _instance,
> - const VkDebugReportCallbackCreateInfoEXT* pCreateInfo,
> - const VkAllocationCallbacks* pAllocator,
> - VkDebugReportCallbackEXT* pCallback)
> +vk_create_debug_report_callback(struct vk_debug_report_instance *instance,
> + const VkDebugReportCallbackCreateInfoEXT* pCreateInfo,
> + const VkAllocationCallbacks* pAllocator,
> + const VkAllocationCallbacks* instance_allocator,
> + VkDebugReportCallbackEXT* pCallback)
> {
> - ANV_FROM_HANDLE(anv_instance, instance, _instance);
>
> - struct anv_debug_report_callback *cb =
> - vk_alloc2(&instance->alloc, pAllocator,
> - sizeof(struct anv_debug_report_callback), 8,
> + struct vk_debug_report_callback *cb =
> + vk_alloc2(instance_allocator, pAllocator,
> + sizeof(struct vk_debug_report_callback), 8,
> VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
>
> if (!cb)
> - return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> + return VK_ERROR_OUT_OF_HOST_MEMORY;
>
> cb->flags = pCreateInfo->flags;
> cb->callback = pCreateInfo->pfnCallback;
> @@ -50,50 +66,37 @@ anv_CreateDebugReportCallbackEXT(VkInstance _instance,
> list_addtail(&cb->link, &instance->callbacks);
> pthread_mutex_unlock(&instance->callbacks_mutex);
>
> - *pCallback = anv_debug_report_callback_to_handle(cb);
> + *pCallback = (VkDebugReportCallbackEXT)(uintptr_t)cb;
>
> return VK_SUCCESS;
> }
>
> void
> -anv_DestroyDebugReportCallbackEXT(VkInstance _instance,
> - VkDebugReportCallbackEXT _callback,
> - const VkAllocationCallbacks* pAllocator)
> +vk_destroy_debug_report_callback(struct vk_debug_report_instance *instance,
> + VkDebugReportCallbackEXT _callback,
> + const VkAllocationCallbacks* pAllocator,
> + const VkAllocationCallbacks* instance_allocator)
> {
> - ANV_FROM_HANDLE(anv_instance, instance, _instance);
> - ANV_FROM_HANDLE(anv_debug_report_callback, callback, _callback);
> + struct vk_debug_report_callback *callback =
> + (struct vk_debug_report_callback *)(uintptr_t)_callback;
>
> /* Remove from list and destroy given callback. */
> pthread_mutex_lock(&instance->callbacks_mutex);
> list_del(&callback->link);
> - vk_free2(&instance->alloc, pAllocator, callback);
> + vk_free2(instance_allocator, pAllocator, callback);
> 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)
> +vk_debug_report(struct vk_debug_report_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))
> @@ -108,7 +111,7 @@ anv_debug_report(struct anv_instance *instance,
> * vkDestroyDebugReportCallbackEXT must not be called when a callback
> * is active."
> */
> - list_for_each_entry(struct anv_debug_report_callback, cb,
> + list_for_each_entry(struct vk_debug_report_callback, cb,
> &instance->callbacks, link) {
> if (cb->flags & flags)
> cb->callback(flags, object_type, handle, location, messageCode,
> diff --git a/src/vulkan/util/vk_debug_report.h b/src/vulkan/util/vk_debug_report.h
> new file mode 100644
> index 0000000000..625ecbb69f
> --- /dev/null
> +++ b/src/vulkan/util/vk_debug_report.h
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright © 2018, Google Inc.
> + *
> + * based on the anv driver which is:
> + * 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.
> + */
> +#ifndef VK_DEBUG_REPORT_H
> +#define VK_DEBUG_REPORT_H
> +
> +#include <pthread.h>
> +
> +#include "util/list.h"
> +#include <vulkan/vulkan.h>
> +
> +struct vk_debug_report_callback {
> + /* Link in the 'callbacks' list in anv_instance struct. */
> + struct list_head link;
> + VkDebugReportFlagsEXT flags;
> + PFN_vkDebugReportCallbackEXT callback;
> + void * data;
> +};
> +
> +struct vk_debug_report_instance {
> + /* VK_EXT_debug_report debug callbacks */
> + pthread_mutex_t callbacks_mutex;
> + struct list_head callbacks;
> +};
> +
> +VkResult vk_debug_report_instance_init(struct vk_debug_report_instance *instance);
> +void vk_debug_report_instance_destroy(struct vk_debug_report_instance *instance);
> +
> +VkResult
> +vk_create_debug_report_callback(struct vk_debug_report_instance *instance,
> + const VkDebugReportCallbackCreateInfoEXT* pCreateInfo,
> + const VkAllocationCallbacks* pAllocator,
> + const VkAllocationCallbacks* instance_allocator,
> + VkDebugReportCallbackEXT* pCallback);
> +void
> +vk_destroy_debug_report_callback(struct vk_debug_report_instance *instance,
> + VkDebugReportCallbackEXT _callback,
> + const VkAllocationCallbacks* pAllocator,
> + const VkAllocationCallbacks* instance_allocator);
> +
> +void
> +vk_debug_report(struct vk_debug_report_instance *instance,
> + VkDebugReportFlagsEXT flags,
> + VkDebugReportObjectTypeEXT object_type,
> + uint64_t handle,
> + size_t location,
> + int32_t messageCode,
> + const char* pLayerPrefix,
> + const char *pMessage);
> +#endif
>
More information about the mesa-dev
mailing list