[Mesa-dev] [PATCH mesa] anv: wrap API errors in vk_error()
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Tue Oct 23 15:17:42 UTC 2018
I think there are a few cases below where your change "over reports"
meaning reporting the same error that has been signaled previously
(device lost in particular).
And cases where the application is just giving a wrong value, I might be
wrong but I don't think we should report those ones either.
Cheers,
-
Lionel
On 23/10/2018 15:34, Eric Engestrom wrote:
> Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> ---
> src/intel/vulkan/anv_android.c | 2 +-
> src/intel/vulkan/anv_device.c | 10 +++++-----
> src/intel/vulkan/anv_formats.c | 2 +-
> src/intel/vulkan/anv_image.c | 2 +-
> src/intel/vulkan/anv_queue.c | 6 +++---
> src/intel/vulkan/anv_wsi_display.c | 2 +-
> 6 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
> index 46c41d57861f18f48275..1334cbbc13814893b3b6 100644
> --- a/src/intel/vulkan/anv_android.c
> +++ b/src/intel/vulkan/anv_android.c
> @@ -314,7 +314,7 @@ VkResult anv_GetSwapchainGrallocUsageANDROID(
> }
>
> if (*grallocUsage == 0)
> - return VK_ERROR_FORMAT_NOT_SUPPORTED;
> + return vk_error(VK_ERROR_FORMAT_NOT_SUPPORTED);
Looks like an application error. I don't think we should report it.
>
> return VK_SUCCESS;
> }
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 3ac1bad93ed8713b0da5..211991d6874d52c10ff3 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -2133,7 +2133,7 @@ VkResult anv_DeviceWaitIdle(
> {
> ANV_FROM_HANDLE(anv_device, device, _device);
> if (unlikely(device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
Drop this one, the error should have been reported earlier.
>
> struct anv_batch batch;
>
> @@ -2236,7 +2236,7 @@ VkResult anv_AllocateMemory(
> assert(pAllocateInfo->allocationSize > 0);
>
> if (pAllocateInfo->allocationSize > MAX_MEMORY_ALLOCATION_SIZE)
> - return VK_ERROR_OUT_OF_DEVICE_MEMORY;
> + return vk_error(VK_ERROR_OUT_OF_DEVICE_MEMORY);
Somewhat not sure here. It's the application asking for a value we
report is above the maximum.
Not really a driver error.
>
> /* FINISHME: Fail if allocation request exceeds heap size. */
>
> @@ -2807,7 +2807,7 @@ VkResult anv_QueueBindSparse(
> {
> ANV_FROM_HANDLE(anv_queue, queue, _queue);
> if (unlikely(queue->device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
Drop this one, the error should have been reported earlier.
>
> return vk_error(VK_ERROR_FEATURE_NOT_PRESENT);
> }
> @@ -2865,7 +2865,7 @@ VkResult anv_GetEventStatus(
> ANV_FROM_HANDLE(anv_event, event, _event);
>
> if (unlikely(device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
Drop this one, the error should have been reported earlier.
>
> if (!device->info.has_llc) {
> /* Invalidate read cache before reading event written by GPU. */
> @@ -3095,7 +3095,7 @@ VkResult anv_GetCalibratedTimestampsEXT(
>
> if (ret != 0) {
> device->lost = TRUE;
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
> }
> uint64_t device_period = DIV_ROUND_UP(1000000000, timestamp_frequency);
> max_clock_period = MAX2(max_clock_period, device_period);
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> index 9199567f4454ae6df653..cd1ca9f7b8378a3b24ed 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -907,7 +907,7 @@ anv_get_image_format_properties(
> .maxResourceSize = 0,
> };
>
> - return VK_ERROR_FORMAT_NOT_SUPPORTED;
> + return vk_error(VK_ERROR_FORMAT_NOT_SUPPORTED);
> }
I don't think this is an error, just reporting that the particular
format/tiling/etc... is not supported
>
> VkResult anv_GetPhysicalDeviceImageFormatProperties(
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index e89ce012be7c31f2410a..3e462a3e5739e32b4535 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -355,7 +355,7 @@ make_surface(const struct anv_device *dev,
> .tiling_flags = tiling_flags);
>
> if (!ok)
> - return VK_ERROR_OUT_OF_DEVICE_MEMORY;
> + return vk_error(VK_ERROR_OUT_OF_DEVICE_MEMORY);
>
> image->planes[plane].aux_usage = ISL_AUX_USAGE_NONE;
>
> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> index e0c0a42069f1e52dafeb..db89385b86644417d47a 100644
> --- a/src/intel/vulkan/anv_queue.c
> +++ b/src/intel/vulkan/anv_queue.c
> @@ -399,7 +399,7 @@ VkResult anv_GetFenceStatus(
> ANV_FROM_HANDLE(anv_fence, fence, _fence);
>
> if (unlikely(device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
>
> struct anv_fence_impl *impl =
> fence->temporary.type != ANV_FENCE_TYPE_NONE ?
> @@ -672,7 +672,7 @@ anv_wait_for_bo_fences(struct anv_device *device,
>
> done:
> if (unlikely(device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
>
> return result;
> }
> @@ -762,7 +762,7 @@ VkResult anv_WaitForFences(
> ANV_FROM_HANDLE(anv_device, device, _device);
>
> if (unlikely(device->lost))
> - return VK_ERROR_DEVICE_LOST;
> + return vk_error(VK_ERROR_DEVICE_LOST);
I think the 3 hunks above should be dropped.
>
> if (anv_all_fences_syncobj(fenceCount, pFences)) {
> return anv_wait_for_syncobj_fences(device, fenceCount, pFences,
> diff --git a/src/intel/vulkan/anv_wsi_display.c b/src/intel/vulkan/anv_wsi_display.c
> index 3212c235bab0552970ed..d5fdf81930859711c9d4 100644
> --- a/src/intel/vulkan/anv_wsi_display.c
> +++ b/src/intel/vulkan/anv_wsi_display.c
> @@ -288,7 +288,7 @@ anv_RegisterDisplayEventEXT(VkDevice _device,
> fence = vk_zalloc2(&device->alloc, allocator, sizeof (*fence), 8,
> VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> if (!fence)
> - return VK_ERROR_OUT_OF_HOST_MEMORY;
> + return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
>
> fence->permanent.type = ANV_FENCE_TYPE_WSI;
>
More information about the mesa-dev
mailing list