From c1f2445772281898d48f5533432b69e8b7ecdd13 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Wed, 13 Jul 2022 22:09:09 +0530 Subject: [PATCH] Set state to `CpuDirty` directly in `SynchronizeGuest` `SynchronizeGuest` could only set the dirty state to `Clean` which was redundant since calls to it from inside the write trap handler would set it to `CpuDirty` directly after, this fixes that by doing it inside the function when necessary. --- app/src/main/cpp/skyline/gpu/buffer.cpp | 14 +++++++------ app/src/main/cpp/skyline/gpu/buffer.h | 3 ++- .../main/cpp/skyline/gpu/texture/texture.cpp | 20 ++++++++++--------- .../main/cpp/skyline/gpu/texture/texture.h | 3 ++- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp index c478a7d4..45e5af1d 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.cpp +++ b/app/src/main/cpp/skyline/gpu/buffer.cpp @@ -31,8 +31,7 @@ namespace skyline::gpu { std::unique_lock lock{*this, std::try_to_lock}; if (!lock) return false; - SynchronizeGuest(true); - dirtyState = DirtyState::CpuDirty; // We need to assume the buffer is dirty since we don't know what the guest is writing + SynchronizeGuest(true, false, true); // We need to assume the buffer is dirty since we don't know what the guest is writing return true; }); } @@ -161,15 +160,15 @@ namespace skyline::gpu { gpu.state.nce->RetrapRegions(*trapHandle, !rwTrap); // Trap any future CPU reads (optionally) + writes to this buffer } - void Buffer::SynchronizeGuest(bool skipTrap, bool nonBlocking) { + void Buffer::SynchronizeGuest(bool skipTrap, bool nonBlocking, bool setDirty) { if (!guest) return; auto currentState{dirtyState.load(std::memory_order_relaxed)}; do { - if (currentState != DirtyState::GpuDirty) - return; // If the buffer has not been used on the GPU, there is no need to synchronize it - } while (!dirtyState.compare_exchange_strong(currentState, DirtyState::Clean, std::memory_order_relaxed)); + if (currentState == DirtyState::CpuDirty || (currentState == DirtyState::Clean && setDirty)) + return; // If the buffer is synchronized (Clean/CpuDirty), there is no need to synchronize it + } while (!dirtyState.compare_exchange_strong(currentState, setDirty ? DirtyState::CpuDirty : DirtyState::Clean, std::memory_order_relaxed)); if (nonBlocking && !PollFence()) return; @@ -179,6 +178,9 @@ namespace skyline::gpu { if (!skipTrap) gpu.state.nce->RetrapRegions(*trapHandle, true); + if (setDirty && currentState == DirtyState::Clean) + return; // If the texture was simply transitioned from Clean to CpuDirty, there is no need to synchronize it + if (!nonBlocking) WaitOnFence(); diff --git a/app/src/main/cpp/skyline/gpu/buffer.h b/app/src/main/cpp/skyline/gpu/buffer.h index 822ef816..f7333833 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.h +++ b/app/src/main/cpp/skyline/gpu/buffer.h @@ -233,9 +233,10 @@ namespace skyline::gpu { * @brief Synchronizes the guest buffer with the host buffer * @param skipTrap If true, setting up a CPU trap will be skipped and the dirty state will be Clean/CpuDirty * @param nonBlocking If true, the call will return immediately if the fence is not signalled, skipping the sync + * @param setDirty If true, the buffer will be marked as CpuDirty rather than Clean * @note The buffer **must** be locked prior to calling this */ - void SynchronizeGuest(bool skipTrap = false, bool nonBlocking = false); + void SynchronizeGuest(bool skipTrap = false, bool nonBlocking = false, bool setDirty = false); /** * @brief Synchronizes the guest buffer with the host buffer immediately, flushing GPU work if necessary diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp index 4480f95f..ce0ed782 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp +++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp @@ -160,8 +160,7 @@ namespace skyline::gpu { std::unique_lock lock{*this, std::try_to_lock}; if (!lock) return false; - SynchronizeGuest(true); - dirtyState = DirtyState::CpuDirty; // We need to assume the texture is dirty since we don't know what the guest is writing + SynchronizeGuest(true, true); // We need to assume the texture is dirty since we don't know what the guest is writing WaitOnFence(); return true; }); @@ -555,9 +554,9 @@ namespace skyline::gpu { } Texture::~Texture() { + SynchronizeGuest(true); if (trapHandle) gpu.state.nce->DeleteTrap(*trapHandle); - SynchronizeGuest(true); if (alignedMirror.valid()) munmap(alignedMirror.data(), alignedMirror.size()); WaitOnFence(); @@ -705,20 +704,23 @@ namespace skyline::gpu { gpu.state.nce->RetrapRegions(*trapHandle, !rwTrap); // Trap any future CPU reads (optionally) + writes to this texture } - void Texture::SynchronizeGuest(bool skipTrap) { + void Texture::SynchronizeGuest(bool skipTrap, bool setDirty) { if (!guest) return; auto currentState{dirtyState.load(std::memory_order_relaxed)}; do { - if (currentState != DirtyState::GpuDirty) - return; // If the buffer has not been used on the GPU, there is no need to synchronize it - } while (!dirtyState.compare_exchange_strong(currentState, DirtyState::Clean, std::memory_order_relaxed)); + if (currentState == DirtyState::CpuDirty || (currentState == DirtyState::Clean && setDirty)) + return; // If the texture is synchronized (Clean/CpuDirty), there is no need to synchronize it + } while (!dirtyState.compare_exchange_strong(currentState, setDirty ? DirtyState::CpuDirty : DirtyState::Clean, std::memory_order_relaxed)); TRACE_EVENT("gpu", "Texture::SynchronizeGuest"); if (!skipTrap) - gpu.state.nce->RetrapRegions(*trapHandle, true); + gpu.state.nce->RetrapRegions(*trapHandle, !setDirty); + + if (setDirty && currentState == DirtyState::Clean) + return; // If the texture was simply transitioned from Clean to CpuDirty, there is no need to synchronize it if (layout == vk::ImageLayout::eUndefined || format != guest->format) // If the state of the host texture is undefined then so can the guest @@ -752,7 +754,7 @@ namespace skyline::gpu { auto currentState{dirtyState.load(std::memory_order_relaxed)}; do { if (currentState != DirtyState::GpuDirty) - return; // If the buffer has not been used on the GPU, there is no need to synchronize it + return; // If the texture has not been used on the GPU, there is no need to synchronize it } while (!dirtyState.compare_exchange_strong(currentState, DirtyState::Clean, std::memory_order_relaxed)); TRACE_EVENT("gpu", "Texture::SynchronizeGuestWithBuffer"); diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.h b/app/src/main/cpp/skyline/gpu/texture/texture.h index ebb838e8..d08472dd 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.h +++ b/app/src/main/cpp/skyline/gpu/texture/texture.h @@ -559,10 +559,11 @@ namespace skyline::gpu { /** * @brief Synchronizes the guest texture with the host texture after it has been modified * @param skipTrap If true, setting up a CPU trap will be skipped and the dirty state will be Clean/CpuDirty + * @param setDirty If true, the texture will be marked as CpuDirty rather than Clean * @note The texture **must** be locked prior to calling this * @note The guest texture should not be null prior to calling this */ - void SynchronizeGuest(bool skipTrap = false); + void SynchronizeGuest(bool skipTrap = false, bool setDirty = false); /** * @brief Synchronizes the guest texture with the host texture after it has been modified