From 7bf3580031e847b82e507150b2102c0ba4070d8e Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Sun, 17 Apr 2022 16:01:44 +0100 Subject: [PATCH] Revert "Allow external synchronization for buffers" This reverts commit 372ab8befa5142ca3a102129bfea722e246464a9. --- app/src/main/cpp/skyline/gpu/buffer.cpp | 47 ++++--------------- app/src/main/cpp/skyline/gpu/buffer.h | 20 ++------ .../gpu/interconnect/graphics_context.h | 25 ++++++---- 3 files changed, 29 insertions(+), 63 deletions(-) diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp index 74ae984f..de081a76 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.cpp +++ b/app/src/main/cpp/skyline/gpu/buffer.cpp @@ -45,29 +45,12 @@ namespace skyline::gpu { } void Buffer::MarkGpuDirty() { - if (dirtyState == DirtyState::GpuDirty || externallySynchronized || !guest) { - externallySynchronized = false; // We want to handle synchronize internally after the GPU work is done + if (dirtyState == DirtyState::GpuDirty || !guest) { return; - } gpu.state.nce->RetrapRegions(*trapHandle, false); dirtyState = DirtyState::GpuDirty; } - void Buffer::MarkExternallySynchronized() { - TRACE_EVENT("gpu", "Buffer::MarkExternallySynchronized"); - if (externallySynchronized) - return; - - if (dirtyState == DirtyState::GpuDirty) - std::memcpy(mirror.data(), backing.data(), mirror.size()); - else if (dirtyState == DirtyState::CpuDirty) - std::memcpy(backing.data(), mirror.data(), mirror.size()); - - dirtyState = DirtyState::GpuDirty; // Any synchronization will take place on the GPU which in itself would make the buffer dirty - gpu.state.nce->RetrapRegions(*trapHandle, false); - externallySynchronized = true; - } - void Buffer::WaitOnFence() { TRACE_EVENT("gpu", "Buffer::WaitOnFence"); @@ -84,9 +67,6 @@ namespace skyline::gpu { WaitOnFence(); - if (externallySynchronized) - return; // If the buffer is externally synchronized, we don't need to synchronize it - TRACE_EVENT("gpu", "Buffer::SynchronizeHost"); std::memcpy(backing.data(), mirror.data(), mirror.size()); @@ -101,15 +81,12 @@ namespace skyline::gpu { } void Buffer::SynchronizeHostWithCycle(const std::shared_ptr &pCycle, bool rwTrap) { - if (dirtyState != DirtyState::CpuDirty || !guest || externallySynchronized) + if (dirtyState != DirtyState::CpuDirty || !guest) return; if (!cycle.owner_before(pCycle)) WaitOnFence(); - if (externallySynchronized) - return; - TRACE_EVENT("gpu", "Buffer::SynchronizeHostWithCycle"); std::memcpy(backing.data(), mirror.data(), mirror.size()); @@ -124,15 +101,12 @@ namespace skyline::gpu { } void Buffer::SynchronizeGuest(bool skipTrap, bool skipFence) { - if (dirtyState != DirtyState::GpuDirty || !guest || externallySynchronized) + if (dirtyState != DirtyState::GpuDirty || !guest) return; // If the buffer has not been used on the GPU or there's no guest buffer, there is no need to synchronize it if (!skipFence) WaitOnFence(); - if (externallySynchronized) - return; // If the buffer is externally synchronized, we don't need to synchronize it - TRACE_EVENT("gpu", "Buffer::SynchronizeGuest"); std::memcpy(mirror.data(), backing.data(), mirror.size()); @@ -157,9 +131,6 @@ namespace skyline::gpu { }; void Buffer::SynchronizeGuestWithCycle(const std::shared_ptr &pCycle) { - if (!guest) - return; // If there's no guest buffer, there is no need to synchronize it - if (!cycle.owner_before(pCycle)) WaitOnFence(); @@ -168,16 +139,16 @@ namespace skyline::gpu { } void Buffer::Read(span data, vk::DeviceSize offset) { - if (externallySynchronized || dirtyState == DirtyState::CpuDirty || dirtyState == DirtyState::Clean) + if (dirtyState == DirtyState::CpuDirty || dirtyState == DirtyState::Clean) std::memcpy(data.data(), mirror.data() + offset, data.size()); else if (dirtyState == DirtyState::GpuDirty) std::memcpy(data.data(), backing.data() + offset, data.size()); } - void Buffer::Write(span data, vk::DeviceSize offset) { - if (externallySynchronized || dirtyState == DirtyState::CpuDirty || dirtyState == DirtyState::Clean) + void Buffer::Write(span data, vk::DeviceSize offset, bool skipCleanHostWrite) { + if (dirtyState == DirtyState::CpuDirty || dirtyState == DirtyState::Clean) std::memcpy(mirror.data() + offset, data.data(), data.size()); - if (!externallySynchronized && ((dirtyState == DirtyState::Clean) || dirtyState == DirtyState::GpuDirty)) + if ((!skipCleanHostWrite && dirtyState == DirtyState::Clean) || dirtyState == DirtyState::GpuDirty) std::memcpy(backing.data() + offset, data.data(), data.size()); } @@ -263,7 +234,7 @@ namespace skyline::gpu { bufferDelegate->buffer->Read(data, offset + bufferDelegate->view->offset); } - void BufferView::Write(span data, vk::DeviceSize offset) const { - bufferDelegate->buffer->Write(data, offset + bufferDelegate->view->offset); + void BufferView::Write(span data, vk::DeviceSize offset, bool skipCleanHostWrite) const { + bufferDelegate->buffer->Write(data, offset + bufferDelegate->view->offset, skipCleanHostWrite); } } diff --git a/app/src/main/cpp/skyline/gpu/buffer.h b/app/src/main/cpp/skyline/gpu/buffer.h index 2e347fbf..c1ba6632 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.h +++ b/app/src/main/cpp/skyline/gpu/buffer.h @@ -31,7 +31,6 @@ namespace skyline::gpu { CpuDirty, //!< The CPU mappings have been modified but the GPU buffer is not up to date GpuDirty, //!< The GPU buffer has been modified but the CPU mappings have not been updated } dirtyState{DirtyState::CpuDirty}; //!< The state of the CPU mappings with respect to the GPU buffer - bool externallySynchronized{}; //!< Whether the host buffer is externally synchronized with the guest buffer, disables the buffer synchronization and aims to retain guest/host buffer data across buffer recreation public: /** @@ -124,18 +123,11 @@ namespace skyline::gpu { /** * @brief Marks the buffer as dirty on the GPU, it will be synced on the next call to SynchronizeGuest - * @note This clears the externally synchronized flag automatically * @note This **must** be called after syncing the buffer to the GPU not before * @note The buffer **must** be locked prior to calling this */ void MarkGpuDirty(); - /** - * @brief Marks the buffer as externally synchronized and automatically synchronizes the host buffer and guest buffer, ensuring the buffer is GPU dirty by the end of the current cycle is the responsibility of the API user - * @note The buffer **must** be locked and have the desired fence attached prior to calling this - */ - void MarkExternallySynchronized(); - /** * @brief Waits on a fence cycle if it exists till it's signalled and resets it after * @note The buffer **must** be locked prior to calling this @@ -174,17 +166,14 @@ namespace skyline::gpu { /** * @brief Reads data at the specified offset in the buffer - * @note The buffer **must** be locked prior to calling this - * @note If this buffer is externally synchronized, this will read exclusively from the guest buffer */ void Read(span data, vk::DeviceSize offset); /** * @brief Writes data at the specified offset in the buffer - * @note The buffer **must** be locked prior to calling this - * @note If this buffer is externally synchronized, this will write to the guest buffer and not to the host buffer + * @param skipCleanHostWrite Skip writing to the host buffer if it's clean, assumes the buffer data will be synchronised externally */ - void Write(span data, vk::DeviceSize offset); + void Write(span data, vk::DeviceSize offset, bool skipCleanHostWrite = false); /** * @return A cached or newly created view into this buffer with the supplied attributes @@ -256,15 +245,14 @@ namespace skyline::gpu { /** * @brief Reads data at the specified offset in the view * @note The view **must** be locked prior to calling this - * @note If this buffer is externally synchronized, this will read exclusively from the guest buffer */ void Read(span data, vk::DeviceSize offset) const; /** * @brief Writes data at the specified offset in the view * @note The view **must** be locked prior to calling this - * @note If this buffer is externally synchronized, this will write to the guest buffer and not to the host buffer + * @param skipCleanHostWrite Skip writing to the host buffer if it's clean, assumes the buffer data will be synchronised externally */ - void Write(span data, vk::DeviceSize offset) const; + void Write(span data, vk::DeviceSize offset, bool skipCleanHostWrite = false) const; }; } diff --git a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h index fe7129b9..5f7c86c1 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h @@ -623,6 +623,16 @@ namespace skyline::gpu::interconnect { view.Read(span(object).template cast(), offset); return object; } + + /** + * @brief Writes a span to the supplied offset in the constant buffer + * @note This must only be called when the GuestBuffer is resolved correctly + */ + template + void Write(span buf, size_t offset) { + std::scoped_lock lock{view}; + view.Write(buf.template cast(), offset, true); + } }; ConstantBuffer constantBufferSelector; //!< The constant buffer selector is used to bind a constant buffer to a stage or update data in it @@ -690,6 +700,10 @@ namespace skyline::gpu::interconnect { if (!view) { auto mappings{channelCtx.asCtx->gmmu.TranslateRange(constantBufferSelector.iova, constantBufferSelector.size)}; view = gpu.buffer.FindOrCreate(mappings.front(), executor.cycle); + { + std::scoped_lock lock{*view}; + view->bufferDelegate->buffer->SynchronizeHost(false); + } constantBufferCache.Insert(constantBufferSelector.size, constantBufferSelector.iova, *view); } @@ -700,15 +714,9 @@ namespace skyline::gpu::interconnect { void ConstantBufferUpdate(std::vector data, u32 offset) { auto constantBuffer{GetConstantBufferSelector().value()}; - auto& constantBufferView{constantBuffer.view}; - { - std::scoped_lock lock{constantBufferView}; - executor.AttachBuffer(constantBufferView); - constantBufferView->buffer->MarkExternallySynchronized(); // We want to handle synchronization of updated constant buffers ourselves - constantBufferView.Write(span(data).cast(), offset); - } + constantBuffer.Write(data, offset); - executor.AddOutsideRpCommand([constantBufferView, data = std::move(data), offset](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &cycle, GPU &) { + executor.AddOutsideRpCommand([constantBufferView = constantBuffer.view, data = std::move(data), offset](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &cycle, GPU &) { std::scoped_lock lock{constantBufferView}; commandBuffer.updateBuffer(constantBufferView->buffer->GetBacking(), constantBufferView->view->offset + offset, vk::ArrayProxy(static_cast(data.size()), data.data())); }); @@ -2575,7 +2583,6 @@ namespace skyline::gpu::interconnect { public: template void Draw(u32 count, u32 first, i32 vertexOffset = 0) { - // Draw state validation ValidatePrimitiveRestartState(); // Shader + Binding Setup