From a358b847c1fac43579067c0e14286c78bdad3b02 Mon Sep 17 00:00:00 2001 From: nmzik Date: Thu, 22 Oct 2026 18:26:35 +0200 Subject: [PATCH] graphics: stall waitUntilSafeForRendering in its command processor instead of the GPU thread --- CMakeLists.txt | 2 + src/graphics/guest_gpu/graphicsRun.cpp | 21 ++++-- src/graphics/guest_gpu/graphicsRun.h | 3 + src/graphics/presentation/videoOut.cpp | 41 ++++++----- src/graphics/presentation/videoOut.h | 2 +- tests/ShaderRecompilerComputeTests.cpp | 95 ++++++++++++++++++++++++++ 6 files changed, 142 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0356c124..0d0bebc68 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -676,6 +676,8 @@ if(BUILD_TESTING) COMMAND $ --stream-buffer-only) add_test(NAME gpu_command_lane COMMAND $ --gpu-command-lane-only) + add_test(NAME wait_until_safe_for_rendering + COMMAND $ --wait-until-safe-for-rendering-only) add_test(NAME pm4_context_state COMMAND $ --context-state-only) add_test(NAME gpu_tiler diff --git a/src/graphics/guest_gpu/graphicsRun.cpp b/src/graphics/guest_gpu/graphicsRun.cpp index 80e6cdb63..36522bbff 100644 --- a/src/graphics/guest_gpu/graphicsRun.cpp +++ b/src/graphics/guest_gpu/graphicsRun.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include namespace Libs::Graphics { @@ -91,6 +92,12 @@ void GuestGpu::SendCommand(Common::UniqueFunction&& command) { m_work_available.Signal(); } +void GuestGpu::Wake() { + Common::LockGuard lock(m_queue_mutex); + m_wake_pending = true; + m_work_available.Signal(); +} + void GuestGpu::ProcessCommands() { EXIT_IF(!IsGpuThread()); while (m_pending_commands.load(std::memory_order_acquire) != 0) { @@ -464,7 +471,10 @@ void GuestGpu::ThreadRun(void* data) { } if (selected_queue < 0) { gpu->m_processing = false; - gpu->m_work_available.WaitFor(&gpu->m_queue_mutex, 100); + // A wake that came before this wait must not be lost. + if (!std::exchange(gpu->m_wake_pending, false)) { + gpu->m_work_available.WaitFor(&gpu->m_queue_mutex, 100); + } for (auto& queue: gpu->m_queues) { if (!queue.empty()) { queue.front().blocked = false; @@ -998,10 +1008,11 @@ void CommandProcessor::DrawIndexAuto(DrawAutoArgs args) { } void CommandProcessor::WaitFlipDone(uint32_t video_out_handle, uint32_t display_buffer_index) { - BufferFlush(); - - m_renderer.GetVideoOut().WaitFlipDone(static_cast(video_out_handle), - static_cast(display_buffer_index)); + // The SDK stalls only this command processor, not the host command lane or other queues. + if (!m_renderer.GetVideoOut().IsSafeForRendering(static_cast(video_out_handle), + static_cast(display_buffer_index))) { + SuspendPm4(); + } } template diff --git a/src/graphics/guest_gpu/graphicsRun.h b/src/graphics/guest_gpu/graphicsRun.h index 781bda2d3..399c56356 100644 --- a/src/graphics/guest_gpu/graphicsRun.h +++ b/src/graphics/guest_gpu/graphicsRun.h @@ -31,6 +31,8 @@ public: [[nodiscard]] bool IsStopping(); void SendCommand(Common::UniqueFunction&& command); void SendCommandSync(Common::UniqueFunction&& command); + // Retries blocked submissions now instead of after the 100 ms stall timeout. + void Wake(); // Submitted command memory is borrowed and must remain valid until GPU execution completes. void Submit(std::span draw_commands, @@ -87,6 +89,7 @@ private: bool m_processing = false; bool m_accepting = true; bool m_stopping = false; + bool m_wake_pending = false; bool m_shutdown_complete = false; // Completion callbacks can outlive GuestGpu during renderer shutdown. std::shared_ptr m_suspend_point_ready = diff --git a/src/graphics/presentation/videoOut.cpp b/src/graphics/presentation/videoOut.cpp index 247df7a48..269d8f23b 100644 --- a/src/graphics/presentation/videoOut.cpp +++ b/src/graphics/presentation/videoOut.cpp @@ -221,7 +221,7 @@ public: void WaitForSubmitSlot(VideoOutConfig& cfg); bool Flip(uint32_t micros); void GetFlipStatus(VideoOutConfig& cfg, VideoOutFlipStatus& out); - void Wait(VideoOutConfig& cfg, int index); + bool IsSafeForRendering(VideoOutConfig& cfg, int index); private: enum class RequestState { Reserved, Recording, Ready, Presenting }; @@ -240,6 +240,7 @@ private: Graphics::Presenter::Frame* frame; bool premultiplied_alpha; }; + void SignalDoneLocked(); Graphics::Presenter& m_presenter; Common::Mutex m_mutex; @@ -250,6 +251,7 @@ private: std::list m_cpu_requests; std::list m_cancelled_requests; bool m_processing = false; + bool m_gpu_waiting = false; uint64_t m_next_request_id = 1; }; @@ -953,7 +955,7 @@ bool FlipQueue::Reserve(VideoOutConfig& cfg, int index, int64_t flip_arg, FlipRe } } } - m_done_cond_var.SignalAll(); + SignalDoneLocked(); } if (cfg.flip_status.flipPendingNum >= VIDEO_OUT_FLIP_QUEUE_CAPACITY) { Common::DebugCounters::Add(Common::DebugCounters::Counter::FlipsRejected); @@ -1033,7 +1035,7 @@ void FlipQueue::Cancel(VideoOutConfig& cfg) { it = queue->erase(it); } } - m_done_cond_var.SignalAll(); + SignalDoneLocked(); m_submit_slot_cond_var.SignalAll(); m_submit_cond_var.SignalAll(); m_mutex.Unlock(); @@ -1205,19 +1207,26 @@ void FlipQueue::WaitForSubmitSlot(VideoOutConfig& cfg) { } } -void FlipQueue::Wait(VideoOutConfig& cfg, int index) { - Common::LockGuard lock(m_mutex); - - auto has_request = [this, &cfg, index] { - auto matches = [&cfg, index](const auto& r) { return r.cfg == &cfg && r.index == index; }; - return std::any_of(m_requests.begin(), m_requests.end(), matches) || - std::any_of(m_cpu_requests.begin(), m_cpu_requests.end(), matches); - }; - while (has_request()) { - m_done_cond_var.Wait(&m_mutex); +void FlipQueue::SignalDoneLocked() { + m_done_cond_var.SignalAll(); + // A stalled GPU cannot drain and shut down without re-checking under m_mutex, so it is alive here. + if (m_gpu_waiting) { + m_gpu_waiting = false; + m_presenter.Renderer().GetGpu().Wake(); } } +// A display buffer is safe for rendering once it is neither displayed nor waiting to be. +bool FlipQueue::IsSafeForRendering(VideoOutConfig& cfg, int index) { + Common::LockGuard lock(m_mutex); + auto matches = [&cfg, index](const auto& r) { return r.cfg == &cfg && r.index == index; }; + const bool safe = cfg.flip_status.currentBuffer != index && + std::none_of(m_requests.begin(), m_requests.end(), matches) && + std::none_of(m_cpu_requests.begin(), m_cpu_requests.end(), matches); + m_gpu_waiting = !safe; + return safe; +} + bool FlipQueue::Flip(uint32_t micros) { KYTY_PROFILER_BLOCK("FlipQueue::Flip"); @@ -1311,7 +1320,7 @@ bool FlipQueue::Flip(uint32_t micros) { it = m_requests.erase(it); } m_processing = false; - m_done_cond_var.SignalAll(); + SignalDoneLocked(); m_submit_slot_cond_var.SignalAll(); m_mutex.Unlock(); for (size_t i = count; i != 0; i--) { @@ -1694,12 +1703,12 @@ void VideoOutDriver::WaitForSubmitSlot(int handle) { m_impl->GetFlipQueue().WaitForSubmitSlot(*cfg); } -void VideoOutDriver::WaitFlipDone(int handle, int index) { +bool VideoOutDriver::IsSafeForRendering(int handle, int index) { auto* ctx = m_impl->Get(handle); EXIT_IF(ctx == nullptr); EXIT_NOT_IMPLEMENTED(!IsValidBufferIndex(index)); - m_impl->GetFlipQueue().Wait(*ctx, index); + return m_impl->GetFlipQueue().IsSafeForRendering(*ctx, index); } KYTY_SYSV_ABI int VideoOutGetFlipStatus(int handle, VideoOutFlipStatus* status) { diff --git a/src/graphics/presentation/videoOut.h b/src/graphics/presentation/videoOut.h index dad4aaeb2..72ba64bea 100644 --- a/src/graphics/presentation/videoOut.h +++ b/src/graphics/presentation/videoOut.h @@ -38,7 +38,7 @@ public: void CompleteFlip(uint64_t request_id); void SubmitFlipPreparation(uint64_t request_id); void WaitForSubmitSlot(int handle); - void WaitFlipDone(int handle, int index); + [[nodiscard]] bool IsSafeForRendering(int handle, int index); [[nodiscard]] Impl& State() noexcept; diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index e7a94da06..8f8a11045 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -39,6 +39,7 @@ #include "graphics/host_gpu/renderer/sync.h" #include "graphics/host_gpu/vulkanCommon.h" #include +#include "graphics/presentation/videoOut.h" #include "graphics/presentation/window/windowInternal.h" #include "graphics/shader/recompiler/BufferFormat.h" #include "graphics/shader/recompiler/ShaderRecompiler.h" @@ -49108,6 +49109,94 @@ void CheckPm4IndirectControlFlow(RenderContext &renderer) { std::printf("[host] %-32s ok\n", name); } +// waitUntilSafeForRendering stalls only its own command processor (SDK), and a display buffer +// stays busy while its flip is pending or while it is on screen. +void CheckWaitUntilSafeForRendering() { + constexpr const char *name = "WaitUntilSafeForRendering"; + Require(name, "video", SDL_InitSubSystem(SDL_INIT_VIDEO), "SDL video init failed"); + WindowContext window; + window.graphic_ctx.screen_width = 64; + window.graphic_ctx.screen_height = 64; + window.window = SDL_CreateWindow(name, 64, 64, SDL_WINDOW_VULKAN | SDL_WINDOW_HIDDEN); + Require(name, "window", window.window != nullptr, "hidden Vulkan window failed"); + window.CreateVulkan(); + auto &video_out = VideoOut::VideoOutInit(64, 64, *window.presenter); + auto &renderer = *window.render_context; + renderer.InitializeGpu(&video_out); + auto &gpu = renderer.GetGpu(); + + constexpr int user_system = 255; + constexpr int bus_main = 0; + constexpr int flip_mode_vsync = 1; + // Special buffer indices flip without a registered surface. + constexpr int index_blank = -1; + constexpr int index_black = -2; + const int handle = VideoOut::VideoOutOpen(user_system, bus_main, 0, nullptr); + Require(name, "open", handle > 0, "video-out open failed"); + const auto wait_until = [](const auto &done) { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (!done() && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + return done(); + }; + const auto address = [](const void *value) { return reinterpret_cast(value); }; + + // Hold the graphics queue so the CPU flip of BLACK, prepared behind it, stays pending. + std::atomic_uint32_t gate{0}; + std::array hold{}; + hold[0] = KYTY_PM4(7, Pm4::IT_WAIT_REG_MEM, 0); + hold[1] = 0x10u | 3u; + hold[2] = static_cast(address(&gate)); + hold[3] = static_cast(address(&gate) >> 32u); + hold[4] = 1; + hold[5] = UINT32_MAX; + gpu.Submit(hold, {}); + Require(name, "flip black", + VideoOut::VideoOutSubmitFlip(handle, index_black, flip_mode_vsync, 0) == 0, + "CPU flip failed"); + + std::atomic_uint32_t rendered{0}; + std::array render{}; + render[0] = KYTY_PM4(7, Pm4::IT_NOP, Pm4::R_WAIT_FLIP_DONE); + render[1] = static_cast(handle); + render[2] = static_cast(index_black); + render[7] = KYTY_PM4(5, Pm4::IT_WRITE_DATA, 0); + render[9] = static_cast(address(&rendered)); + render[10] = static_cast(address(&rendered) >> 32u); + render[11] = 1; + gpu.SubmitCompute(0x20, render); + + std::binary_semaphore lane{0}; + std::jthread command_lane([&] { + gpu.SendCommandSync([] {}); + lane.release(); + }); + Require(name, "command lane", lane.try_acquire_for(std::chrono::seconds(2)), + "a busy display buffer blocked the GPU thread"); + Require(name, "pending", rendered.load() == 0, + "rendering started while its display buffer had a pending flip"); + + gate.store(1); + Require(name, "flip shown", wait_until([handle] { return VideoOut::VideoOutIsFlipPending(handle) == 0; }), + "the BLACK flip was not presented"); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + Require(name, "displayed", rendered.load() == 0, + "rendering started while its display buffer was on screen"); + + Require(name, "flip blank", + VideoOut::VideoOutSubmitFlip(handle, index_blank, flip_mode_vsync, 0) == 0, + "CPU flip failed"); + Require(name, "resume", wait_until([&] { return rendered.load() == 1; }), + "rendering did not resume once its display buffer left the screen"); + + gpu.WaitForIdle(); + Require(name, "close", VideoOut::VideoOutClose(handle) == 0, "video-out close failed"); + renderer.ShutdownGpu(); + VideoOut::VideoOutShutdown(); + std::printf("[host] %-32s ok\n", name); +} + void CheckPm4WaitResume(RenderContext &renderer) { GraphicsInitJmpTables(); CommandProcessor processor(renderer, 0); @@ -49987,6 +50076,10 @@ int main(int argc, char **argv) { vulkan.CheckScalarLeafCall(); return 0; } + if (argc == 2 && std::strcmp(argv[1], "--wait-until-safe-for-rendering-only") == 0) { + CheckWaitUntilSafeForRendering(); + return 0; + } if (argc == 2 && std::strcmp(argv[1], "--gpu-command-lane-only") == 0) { VulkanHarness vulkan; vulkan.CheckGpuCommandLane(); @@ -50540,6 +50633,8 @@ int main(int argc, char **argv) { vulkan.CheckGpuSuspendPoint(); vulkan.CheckGpuCommandLane(); vulkan.CheckScalarLeafCall(); + // Last: its window installs global GPU resources. + CheckWaitUntilSafeForRendering(); if (skipped_device_checks) { std::printf( "ShaderRecompilerComputeTests: device rasterization checks skipped, this "