From d0cb99e96a249895d2c5d1045eacc62835123cb3 Mon Sep 17 00:00:00 2001 From: Chad Versace Date: Fri, 12 Aug 2022 09:19:32 -0700 Subject: [PATCH] venus: Enable VK_EXT_pipeline_creation_feedback Implement natively by always returning invalid feedback. This is a legal (but useless) implementation according to the spec. In the future, I want to return the real feedback values from the host, but that requires changes to the venus protocol. The protocol does not know that the VkPipelineCreationFeedback structs in the VkGraphicsPipelineCreateInfo pNext are output parameters. Before VK_EXT_pipeline_creation_feedback, the pNext chain was input-only. Tested with `dEQP-VK.pipeline.*.creation_feedback.*`. The tests in vulkan-cts-1.3.3.0 are buggy. I submitted a fix to dEQP upstream; see below. Results with the bug: Passed: 0/30 ( 0.0%) Failed: 12/30 (40.0%) Not supported: 18/30 (60.0%) Warnings: 0/30 ( 0.0%) Results with bugfix: Passed: 12/30 (40.0%) Failed: 0/30 ( 0.0%) Not supported: 18/30 (60.0%) Warnings: 0/30 ( 0.0%) See: https://gerrit.khronos.org/c/vk-gl-cts/+/10086 See: https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/909 Reviewed-by: Yiwei Zhang Signed-off-by: Chad Versace Part-of: --- docs/features.txt | 2 +- src/virtio/vulkan/vn_physical_device.c | 13 +++++++++++ src/virtio/vulkan/vn_pipeline.c | 42 +++++++++++++++++++++++++++++----- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/docs/features.txt b/docs/features.txt index b7f6209..b485900 100644 --- a/docs/features.txt +++ b/docs/features.txt @@ -487,7 +487,7 @@ Vulkan 1.3 -- all DONE: anv, radv, lvp VK_EXT_extended_dynamic_state2 DONE (anv, lvp, radv, tu, vn) VK_EXT_inline_uniform_block DONE (anv, lvp, radv, v3dv, vn) VK_EXT_pipeline_creation_cache_control DONE (anv, lvp, radv, tu, v3dv, vn) - VK_EXT_pipeline_creation_feedback DONE (anv, lvp, radv, tu, v3dv) + VK_EXT_pipeline_creation_feedback DONE (anv, lvp, radv, tu, v3dv, vn) VK_EXT_private_data DONE (anv, lvp, pvr, radv, tu, v3dv, vn) VK_EXT_image_robustness DONE (anv, lvp, radv, tu, vn) VK_EXT_shader_demote_to_helper_invocation DONE (anv, lvp, radv, tu, vn) diff --git a/src/virtio/vulkan/vn_physical_device.c b/src/virtio/vulkan/vn_physical_device.c index b5018ab..f9969e6 100644 --- a/src/virtio/vulkan/vn_physical_device.c +++ b/src/virtio/vulkan/vn_physical_device.c @@ -1090,6 +1090,19 @@ vn_physical_device_get_passthrough_extensions( .EXT_image_robustness = true, .EXT_inline_uniform_block = true, .EXT_pipeline_creation_cache_control = true, + /* TODO(VK_EXT_pipeline_creation_feedback): The native implementation + * invalidates all feedback. Teach the venus protocol to receive valid + * feedback from renderer. + * + * Even though we implement this natively, we still require host driver + * support to avoid invalid usage in the renderer, because we (the guest + * driver) do not scrub the extension bits from the + * VkGraphicsPipelineCreateInfo pNext chain. The host driver still writes + * feedback into VkPipelineCreationFeedback, which is harmless, but the + * renderer does not send the returned feedback to us due to protocol + * deficiencies. + */ + .EXT_pipeline_creation_feedback = true, .EXT_shader_demote_to_helper_invocation = true, .EXT_subgroup_size_control = true, .EXT_texel_buffer_alignment = true, diff --git a/src/virtio/vulkan/vn_pipeline.c b/src/virtio/vulkan/vn_pipeline.c index c4dd4f4..c730359 100644 --- a/src/virtio/vulkan/vn_pipeline.c +++ b/src/virtio/vulkan/vn_pipeline.c @@ -646,6 +646,34 @@ vn_fix_graphics_pipeline_create_info( return fixes->create_infos; } +/** + * We invalidate each VkPipelineCreationFeedback. This is a legal but useless + * implementation. + * + * We invalidate because the venus protocol (as of 2022-08-25) does not know + * that the VkPipelineCreationFeedback structs in the + * VkGraphicsPipelineCreateInfo pNext are output parameters. Before + * VK_EXT_pipeline_creation_feedback, the pNext chain was input-only. + */ +static void +vn_invalidate_pipeline_creation_feedback(const VkBaseInStructure *chain) +{ + const VkPipelineCreationFeedbackCreateInfo *feedback_info = + vk_find_struct_const(chain, PIPELINE_CREATION_FEEDBACK_CREATE_INFO); + + if (!feedback_info) + return; + + feedback_info->pPipelineCreationFeedback->flags &= + ~VK_PIPELINE_CREATION_FEEDBACK_VALID_BIT; + + for (uint32_t i = 0; i < feedback_info->pipelineStageCreationFeedbackCount; + i++) { + feedback_info->pPipelineStageCreationFeedbacks[i].flags &= + ~VK_PIPELINE_CREATION_FEEDBACK_VALID_BIT; + } +} + VkResult vn_CreateGraphicsPipelines(VkDevice device, VkPipelineCache pipelineCache, @@ -673,10 +701,11 @@ vn_CreateGraphicsPipelines(VkDevice device, } for (uint32_t i = 0; i < createInfoCount; i++) { - if ((pCreateInfos[i].flags & VN_PIPELINE_CREATE_SYNC_MASK)) { + if ((pCreateInfos[i].flags & VN_PIPELINE_CREATE_SYNC_MASK)) want_sync = true; - break; - } + + vn_invalidate_pipeline_creation_feedback( + (const VkBaseInStructure *)pCreateInfos[i].pNext); } if (want_sync) { @@ -716,10 +745,11 @@ vn_CreateComputePipelines(VkDevice device, return vn_error(dev->instance, VK_ERROR_OUT_OF_HOST_MEMORY); for (uint32_t i = 0; i < createInfoCount; i++) { - if ((pCreateInfos[i].flags & VN_PIPELINE_CREATE_SYNC_MASK)) { + if ((pCreateInfos[i].flags & VN_PIPELINE_CREATE_SYNC_MASK)) want_sync = true; - break; - } + + vn_invalidate_pipeline_creation_feedback( + (const VkBaseInStructure *)pCreateInfos[i].pNext); } if (want_sync) { -- 2.7.4