From 6da42e9c951b39ff0dd4bd63ba8917fc80b21238 Mon Sep 17 00:00:00 2001 From: Jason Fielder Date: Mon, 14 Oct 2024 17:12:03 +0200 Subject: [PATCH] Fix: Metal: GPU_finish() not syncing when there's no (current) command buffer to sync on Reported by @fclem. On Metal GPU_finish enacts a CPU<-> GPU sync by submitting a command buffer and waiting on the completion event. However if there was no command buffer to submit then the call was returning immediately, regardless of any outstanding GPU work. This fixes that case by keeping track of all outstanding work and blocking on that. Authored by Apple: James McCarthy Co-authored-by: James McCarthy Pull Request: https://projects.blender.org/blender/blender/pulls/128987 --- .../blender/gpu/metal/mtl_command_buffer.mm | 16 +++++++++---- source/blender/gpu/metal/mtl_context.hh | 23 +++++++++++++++++-- source/blender/gpu/metal/mtl_context.mm | 8 ++++--- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/source/blender/gpu/metal/mtl_command_buffer.mm b/source/blender/gpu/metal/mtl_command_buffer.mm index a456ff97ca9..fcb82eca7b1 100644 --- a/source/blender/gpu/metal/mtl_command_buffer.mm +++ b/source/blender/gpu/metal/mtl_command_buffer.mm @@ -20,7 +20,7 @@ using namespace blender::gpu; namespace blender::gpu { /* Counter for active command buffers. */ -int MTLCommandBufferManager::num_active_cmd_bufs = 0; +volatile std::atomic MTLCommandBufferManager::num_active_cmd_bufs_in_system = 0; /* -------------------------------------------------------------------- */ /** \name MTLCommandBuffer initialization and render coordination. @@ -50,7 +50,7 @@ id MTLCommandBufferManager::ensure_begin() * * NOTE: We currently stall until completion of GPU work upon ::submit if we have reached the * in-flight command buffer limit. */ - BLI_assert(MTLCommandBufferManager::num_active_cmd_bufs < + BLI_assert(MTLCommandBufferManager::num_active_cmd_bufs_in_system < GHOST_ContextCGL::max_command_buffer_count); if (G.debug & G_DEBUG_GPU) { @@ -68,7 +68,7 @@ id MTLCommandBufferManager::ensure_begin() } [active_command_buffer_ retain]; - MTLCommandBufferManager::num_active_cmd_bufs++; + context_.main_command_buffer.inc_active_command_buffer_count(); /* Ensure we begin new Scratch Buffer if we are on a new frame. */ MTLScratchBufferManager &mem = context_.memory_manager; @@ -90,6 +90,12 @@ bool MTLCommandBufferManager::submit(bool wait) { /* Skip submission if command buffer is empty. */ if (empty_ || active_command_buffer_ == nil) { + if (wait) { + /* Wait for any previously submitted work on this context to complete. */ + while (context_.main_command_buffer.get_active_command_buffer_count()) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + } return false; } @@ -123,7 +129,7 @@ bool MTLCommandBufferManager::submit(bool wait) [cmd_buffer_ref release]; /* Decrement count. */ - MTLCommandBufferManager::num_active_cmd_bufs--; + context_.main_command_buffer.dec_active_command_buffer_count(); }]; /* Submit command buffer to GPU. */ @@ -131,7 +137,7 @@ bool MTLCommandBufferManager::submit(bool wait) /* If we have too many active command buffers in flight, wait until completed to avoid running * out. We can increase */ - if (MTLCommandBufferManager::num_active_cmd_bufs >= + if (MTLCommandBufferManager::num_active_cmd_bufs_in_system >= (GHOST_ContextCGL::max_command_buffer_count - 1)) { wait = true; diff --git a/source/blender/gpu/metal/mtl_context.hh b/source/blender/gpu/metal/mtl_context.hh index 7a9af163cb2..922a93cd148 100644 --- a/source/blender/gpu/metal/mtl_context.hh +++ b/source/blender/gpu/metal/mtl_context.hh @@ -548,8 +548,8 @@ class MTLCommandBufferManager { friend class MTLContext; public: - /* Counter for active command buffers. */ - static int num_active_cmd_bufs; + /* Counter for all active command buffers. */ + static volatile std::atomic num_active_cmd_bufs_in_system; private: /* Associated Context and properties. */ @@ -559,6 +559,7 @@ class MTLCommandBufferManager { /* CommandBuffer tracking. */ id active_command_buffer_ = nil; id last_submitted_command_buffer_ = nil; + volatile std::atomic num_active_cmd_bufs = 0; /* Active MTLCommandEncoders. */ enum { @@ -653,6 +654,24 @@ class MTLCommandBufferManager { void push_debug_group(const char *name, int index); void pop_debug_group(); + void inc_active_command_buffer_count() + { + num_active_cmd_bufs_in_system++; + num_active_cmd_bufs++; + } + + void dec_active_command_buffer_count() + { + BLI_assert(num_active_cmd_bufs_in_system > 0 && num_active_cmd_bufs > 0); + num_active_cmd_bufs_in_system--; + num_active_cmd_bufs--; + } + + int get_active_command_buffer_count() + { + return num_active_cmd_bufs; + } + private: /* Begin new command buffer. */ id ensure_begin(); diff --git a/source/blender/gpu/metal/mtl_context.mm b/source/blender/gpu/metal/mtl_context.mm index 325ee7a252c..300aa4cd482 100644 --- a/source/blender/gpu/metal/mtl_context.mm +++ b/source/blender/gpu/metal/mtl_context.mm @@ -2706,7 +2706,7 @@ void present(MTLRenderPassDescriptor *blit_descriptor, * possible. This command buffer is separate as it does not utilize the global state * for rendering as the main context does. */ id cmdbuf = [ctx->queue commandBuffer]; - MTLCommandBufferManager::num_active_cmd_bufs++; + ctx->main_command_buffer.inc_active_command_buffer_count(); /* Do Present Call and final Blit to MTLDrawable. */ id enc = [cmdbuf renderCommandEncoderWithDescriptor:blit_descriptor]; @@ -2739,8 +2739,10 @@ void present(MTLRenderPassDescriptor *blit_descriptor, [cmd_buffer_ref release]; /* Decrement count */ - MTLCommandBufferManager::num_active_cmd_bufs--; - MTL_LOG_INFO("Active command buffers: %d", MTLCommandBufferManager::num_active_cmd_bufs); + ctx->main_command_buffer.dec_active_command_buffer_count(); + + MTL_LOG_INFO("Active command buffers: %d", + int(MTLCommandBufferManager::num_active_cmd_bufs_in_system)); /* Drawable count and latency management. */ MTLContext::max_drawables_in_flight--;