From 682d82faf3bfc96603bf9b2c77436b1b23af24e0 Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Sat, 13 Feb 2021 05:11:48 -0300 Subject: [PATCH 1/3] gl_stream_buffer/vk_staging_buffer_pool: Fix size check Fix a tragic off-by-one condition that causes Vulkan's stream buffer to think it's always full, using fallback memory. The OpenGL was also affected by this bug to a lesser extent. --- src/video_core/renderer_opengl/gl_stream_buffer.cpp | 2 +- src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_stream_buffer.cpp b/src/video_core/renderer_opengl/gl_stream_buffer.cpp index bfb992a796..77b3ee0fe6 100644 --- a/src/video_core/renderer_opengl/gl_stream_buffer.cpp +++ b/src/video_core/renderer_opengl/gl_stream_buffer.cpp @@ -40,7 +40,7 @@ std::pair, size_t> StreamBuffer::Request(size_t size) noexcept { glClientWaitSync(fences[region].handle, 0, GL_TIMEOUT_IGNORED); fences[region].Release(); } - if (iterator + size > free_iterator) { + if (iterator + size >= free_iterator) { free_iterator = iterator + size; } if (iterator + size > STREAM_BUFFER_SIZE) { diff --git a/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp b/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp index dfd8c8e5a1..9b5786fcb7 100644 --- a/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp +++ b/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp @@ -153,7 +153,7 @@ StagingBufferRef StagingBufferPool::GetStreamBuffer(size_t size) { used_iterator = iterator; free_iterator = std::max(free_iterator, iterator + size); - if (iterator + size > STREAM_BUFFER_SIZE) { + if (iterator + size >= STREAM_BUFFER_SIZE) { std::fill(sync_ticks.begin() + Region(used_iterator), sync_ticks.begin() + NUM_SYNCS, current_tick); used_iterator = 0; From 6171566296154963933fb8553c15b5316b5dda2f Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Sat, 13 Feb 2021 05:13:29 -0300 Subject: [PATCH 2/3] vk_staging_buffer_pool: Inline tick tests Load the current tick to a local variable, moving it out of an atomic and allowing us to compare the value without going through a pointer each time. This should make the loop more optimizable. --- src/video_core/renderer_vulkan/vk_master_semaphore.h | 5 +++++ src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/video_core/renderer_vulkan/vk_master_semaphore.h b/src/video_core/renderer_vulkan/vk_master_semaphore.h index f336f1862b..33216d24a7 100644 --- a/src/video_core/renderer_vulkan/vk_master_semaphore.h +++ b/src/video_core/renderer_vulkan/vk_master_semaphore.h @@ -24,6 +24,11 @@ public: return current_tick; } + /// Returns the last known GPU tick. + [[nodiscard]] u64 KnownGpuTick() const noexcept { + return gpu_tick; + } + /// Returns the timeline semaphore handle. [[nodiscard]] VkSemaphore Handle() const noexcept { return *semaphore; diff --git a/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp b/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp index 9b5786fcb7..7a12324973 100644 --- a/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp +++ b/src/video_core/renderer_vulkan/vk_staging_buffer_pool.cpp @@ -175,8 +175,9 @@ StagingBufferRef StagingBufferPool::GetStreamBuffer(size_t size) { } bool StagingBufferPool::AreRegionsActive(size_t region_begin, size_t region_end) const { + const u64 gpu_tick = scheduler.GetMasterSemaphore().KnownGpuTick(); return std::any_of(sync_ticks.begin() + region_begin, sync_ticks.begin() + region_end, - [this](u64 sync_tick) { return !scheduler.IsFree(sync_tick); }); + [gpu_tick](u64 sync_tick) { return gpu_tick < sync_tick; }); }; StagingBufferRef StagingBufferPool::GetStagingBuffer(size_t size, MemoryUsage usage) { From dd9caf9aa02fb46dad4799055a85ce132ae20172 Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Sat, 13 Feb 2021 05:57:28 -0300 Subject: [PATCH 3/3] vk_master_semaphore: Mark gpu_tick atomic operations with relaxed order --- src/video_core/renderer_vulkan/vk_master_semaphore.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_master_semaphore.h b/src/video_core/renderer_vulkan/vk_master_semaphore.h index 33216d24a7..2c7ed654db 100644 --- a/src/video_core/renderer_vulkan/vk_master_semaphore.h +++ b/src/video_core/renderer_vulkan/vk_master_semaphore.h @@ -21,12 +21,12 @@ public: /// Returns the current logical tick. [[nodiscard]] u64 CurrentTick() const noexcept { - return current_tick; + return current_tick.load(std::memory_order_relaxed); } /// Returns the last known GPU tick. [[nodiscard]] u64 KnownGpuTick() const noexcept { - return gpu_tick; + return gpu_tick.load(std::memory_order_relaxed); } /// Returns the timeline semaphore handle. @@ -36,7 +36,7 @@ public: /// Returns true when a tick has been hit by the GPU. [[nodiscard]] bool IsFree(u64 tick) { - return gpu_tick >= tick; + return gpu_tick.load(std::memory_order_relaxed) >= tick; } /// Advance to the logical tick. @@ -46,7 +46,7 @@ public: /// Refresh the known GPU tick void Refresh() { - gpu_tick = semaphore.GetCounter(); + gpu_tick.store(semaphore.GetCounter(), std::memory_order_relaxed); } /// Waits for a tick to be hit on the GPU