From 0273c9cc031bf5474b251ae0bb1b356f57514f67 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Mon, 6 Nov 2023 23:58:31 -0800 Subject: [PATCH] venus: always set reply command stream to avoid seek More considerations and details here: - The seek is a bit lighter than set, since it assumes renderer side resource being immutable. It does affect perf when Venus is still making verbose synchronous calls at runtime (e.g. descriptor set, buffer, device memory, etc). - Seek still requires lock protection as the reply shmem must be immutable before the seek and the followed cmd are committed to the ring. - Removing seek without doing set requires renderer change to always bump the encoder end position according to what the original request is instead of being ad-hoc upon what the host driver tells to write. The overhead and extra complexity there isn't negligible. - Further, removing seek requires each ring to track the prior reply pool shmem in the multi-ring scenario. While the additional host side resource lookup isn't costy as the number of resources is must less than the vk object table. - The nice thing is that we can make shmem pool thead safe to be more easily shared across rings. So we just drop it. Signed-off-by: Yiwei Zhang Part-of: --- src/virtio/vulkan/vn_instance.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/virtio/vulkan/vn_instance.c b/src/virtio/vulkan/vn_instance.c index 16ccdcbc952..222726b3144 100644 --- a/src/virtio/vulkan/vn_instance.c +++ b/src/virtio/vulkan/vn_instance.c @@ -496,8 +496,8 @@ vn_instance_get_reply_shmem_locked(struct vn_instance *instance, void **out_ptr) { VN_TRACE_FUNC(); + struct vn_renderer_shmem_pool *pool = &instance->reply_shmem_pool; - const struct vn_renderer_shmem *saved_pool_shmem = pool->shmem; size_t offset; struct vn_renderer_shmem *shmem = @@ -505,31 +505,20 @@ vn_instance_get_reply_shmem_locked(struct vn_instance *instance, if (!shmem) return NULL; - assert(shmem == pool->shmem); - *out_ptr = shmem->mmap_ptr + offset; - - if (shmem != saved_pool_shmem) { - uint32_t set_reply_command_stream_data[16]; - struct vn_cs_encoder local_enc = VN_CS_ENCODER_INITIALIZER_LOCAL( - set_reply_command_stream_data, - sizeof(set_reply_command_stream_data)); - const struct VkCommandStreamDescriptionMESA stream = { - .resourceId = shmem->res_id, - .size = pool->size, - }; - vn_encode_vkSetReplyCommandStreamMESA(&local_enc, 0, &stream); - vn_cs_encoder_commit(&local_enc); - vn_instance_ring_submit_locked(instance, &local_enc, NULL, NULL); - } - - /* TODO avoid this seek command and go lock-free? */ - uint32_t seek_reply_command_stream_data[8]; + uint32_t set_reply_command_stream_data[16]; struct vn_cs_encoder local_enc = VN_CS_ENCODER_INITIALIZER_LOCAL( - seek_reply_command_stream_data, sizeof(seek_reply_command_stream_data)); - vn_encode_vkSeekReplyCommandStreamMESA(&local_enc, 0, offset); + set_reply_command_stream_data, sizeof(set_reply_command_stream_data)); + const struct VkCommandStreamDescriptionMESA stream = { + .resourceId = shmem->res_id, + .offset = offset, + .size = size, + }; + vn_encode_vkSetReplyCommandStreamMESA(&local_enc, 0, &stream); vn_cs_encoder_commit(&local_enc); vn_instance_ring_submit_locked(instance, &local_enc, NULL, NULL); + *out_ptr = shmem->mmap_ptr + offset; + return shmem; }