diff --git a/BUILD.gn b/BUILD.gn index b5f41467a5..60bda328fe 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -659,8 +659,6 @@ source_set("libdawn_native_sources") { "src/dawn_native/vulkan/BindGroupLayoutVk.h", "src/dawn_native/vulkan/BindGroupVk.cpp", "src/dawn_native/vulkan/BindGroupVk.h", - "src/dawn_native/vulkan/BufferUploader.cpp", - "src/dawn_native/vulkan/BufferUploader.h", "src/dawn_native/vulkan/BufferVk.cpp", "src/dawn_native/vulkan/BufferVk.h", "src/dawn_native/vulkan/CommandBufferVk.cpp", @@ -692,6 +690,8 @@ source_set("libdawn_native_sources") { "src/dawn_native/vulkan/SamplerVk.h", "src/dawn_native/vulkan/ShaderModuleVk.cpp", "src/dawn_native/vulkan/ShaderModuleVk.h", + "src/dawn_native/vulkan/StagingBufferVk.cpp", + "src/dawn_native/vulkan/StagingBufferVk.h", "src/dawn_native/vulkan/SwapChainVk.cpp", "src/dawn_native/vulkan/SwapChainVk.h", "src/dawn_native/vulkan/TextureVk.cpp", diff --git a/src/dawn_native/vulkan/BufferUploader.cpp b/src/dawn_native/vulkan/BufferUploader.cpp deleted file mode 100644 index e74143f05d..0000000000 --- a/src/dawn_native/vulkan/BufferUploader.cpp +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright 2017 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dawn_native/vulkan/BufferUploader.h" - -#include "dawn_native/vulkan/DeviceVk.h" -#include "dawn_native/vulkan/FencedDeleter.h" -#include "dawn_native/vulkan/MemoryAllocator.h" - -#include - -namespace dawn_native { namespace vulkan { - - BufferUploader::BufferUploader(Device* device) : mDevice(device) { - } - - BufferUploader::~BufferUploader() { - } - - void BufferUploader::BufferSubData(VkBuffer buffer, - VkDeviceSize offset, - VkDeviceSize size, - const void* data) { - // TODO(cwallez@chromium.org): this is soooooo bad. We should use some sort of ring buffer - // for this. - - // Create a staging buffer - VkBufferCreateInfo createInfo; - createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; - createInfo.pNext = nullptr; - createInfo.flags = 0; - createInfo.size = size; - createInfo.usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT; - createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; - createInfo.queueFamilyIndexCount = 0; - createInfo.pQueueFamilyIndices = 0; - - VkBuffer stagingBuffer = VK_NULL_HANDLE; - if (mDevice->fn.CreateBuffer(mDevice->GetVkDevice(), &createInfo, nullptr, - &stagingBuffer) != VK_SUCCESS) { - ASSERT(false); - } - - VkMemoryRequirements requirements; - mDevice->fn.GetBufferMemoryRequirements(mDevice->GetVkDevice(), stagingBuffer, - &requirements); - - DeviceMemoryAllocation allocation; - if (!mDevice->GetMemoryAllocator()->Allocate(requirements, true, &allocation)) { - ASSERT(false); - } - - if (mDevice->fn.BindBufferMemory(mDevice->GetVkDevice(), stagingBuffer, - allocation.GetMemory(), - allocation.GetMemoryOffset()) != VK_SUCCESS) { - ASSERT(false); - } - - // Write to the staging buffer - ASSERT(allocation.GetMappedPointer() != nullptr); - memcpy(allocation.GetMappedPointer(), data, static_cast(size)); - - // Enqueue host write -> transfer src barrier and copy command - VkCommandBuffer commands = mDevice->GetPendingCommandBuffer(); - - VkMemoryBarrier barrier; - barrier.sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER; - barrier.pNext = nullptr; - barrier.srcAccessMask = VK_ACCESS_HOST_WRITE_BIT; - barrier.dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - - mDevice->fn.CmdPipelineBarrier(commands, VK_PIPELINE_STAGE_HOST_BIT, - VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 1, &barrier, 0, nullptr, - 0, nullptr); - - VkBufferCopy copy; - copy.srcOffset = 0; - copy.dstOffset = offset; - copy.size = size; - mDevice->fn.CmdCopyBuffer(commands, stagingBuffer, buffer, 1, ©); - - // TODO(cwallez@chromium.org): Buffers must be deleted before the memory. - // This happens to work for now, but is fragile. - mDevice->GetMemoryAllocator()->Free(&allocation); - mDevice->GetFencedDeleter()->DeleteWhenUnused(stagingBuffer); - } - - void BufferUploader::Tick(Serial) { - } - -}} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/BufferVk.cpp b/src/dawn_native/vulkan/BufferVk.cpp index 4175097402..0080e7668c 100644 --- a/src/dawn_native/vulkan/BufferVk.cpp +++ b/src/dawn_native/vulkan/BufferVk.cpp @@ -14,7 +14,7 @@ #include "dawn_native/vulkan/BufferVk.h" -#include "dawn_native/vulkan/BufferUploader.h" +#include "dawn_native/DynamicUploader.h" #include "dawn_native/vulkan/DeviceVk.h" #include "dawn_native/vulkan/FencedDeleter.h" @@ -199,11 +199,18 @@ namespace dawn_native { namespace vulkan { MaybeError Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) { Device* device = ToBackend(GetDevice()); - VkCommandBuffer commands = device->GetPendingCommandBuffer(); - TransitionUsageNow(commands, dawn::BufferUsageBit::TransferDst); + DynamicUploader* uploader = nullptr; + DAWN_TRY_ASSIGN(uploader, device->GetDynamicUploader()); + + UploadHandle uploadHandle; + DAWN_TRY_ASSIGN(uploadHandle, uploader->Allocate(count, kDefaultAlignment)); + ASSERT(uploadHandle.mappedBuffer != nullptr); + + memcpy(uploadHandle.mappedBuffer, data, count); + + DAWN_TRY(device->CopyFromStagingToBuffer(uploadHandle.stagingBuffer, + uploadHandle.startOffset, this, start, count)); - BufferUploader* uploader = device->GetBufferUploader(); - uploader->BufferSubData(mHandle, start, count, data); return {}; } diff --git a/src/dawn_native/vulkan/BufferVk.h b/src/dawn_native/vulkan/BufferVk.h index ae90b4f499..70636fc7e1 100644 --- a/src/dawn_native/vulkan/BufferVk.h +++ b/src/dawn_native/vulkan/BufferVk.h @@ -46,6 +46,9 @@ namespace dawn_native { namespace vulkan { void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override; void UnmapImpl() override; + // TODO(b-brber): Remove once alignment constraint is added to validation (dawn:73). + static constexpr size_t kDefaultAlignment = 4; // TODO(b-brber): Figure out this value. + VkBuffer mHandle = VK_NULL_HANDLE; DeviceMemoryAllocation mMemoryAllocation; diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 32873c3816..8fcd392055 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -23,7 +23,6 @@ #include "dawn_native/vulkan/BackendVk.h" #include "dawn_native/vulkan/BindGroupLayoutVk.h" #include "dawn_native/vulkan/BindGroupVk.h" -#include "dawn_native/vulkan/BufferUploader.h" #include "dawn_native/vulkan/BufferVk.h" #include "dawn_native/vulkan/CommandBufferVk.h" #include "dawn_native/vulkan/ComputePipelineVk.h" @@ -36,6 +35,7 @@ #include "dawn_native/vulkan/RenderPipelineVk.h" #include "dawn_native/vulkan/SamplerVk.h" #include "dawn_native/vulkan/ShaderModuleVk.h" +#include "dawn_native/vulkan/StagingBufferVk.h" #include "dawn_native/vulkan/SwapChainVk.h" #include "dawn_native/vulkan/TextureVk.h" #include "dawn_native/vulkan/VulkanError.h" @@ -61,11 +61,11 @@ namespace dawn_native { namespace vulkan { DAWN_TRY(functions->LoadDeviceProcs(mVkDevice, mDeviceInfo)); GatherQueueFromDevice(); - - mBufferUploader = std::make_unique(this); mDeleter = std::make_unique(this); mMapRequestTracker = std::make_unique(this); mMemoryAllocator = std::make_unique(this); + mDynamicUploader = std::make_unique(this); + mRenderPassCache = std::make_unique(this); return {}; @@ -116,7 +116,12 @@ namespace dawn_native { namespace vulkan { mUnusedFences.clear(); // Free services explicitly so that they can free Vulkan objects before vkDestroyDevice - mBufferUploader = nullptr; + mDynamicUploader = nullptr; + + // Releasing the uploader enqueues buffers to be deleted. + // Call Tick() again to allow the deleter to clear them prior to being released. + mDeleter->Tick(mCompletedSerial); + mDeleter = nullptr; mMapRequestTracker = nullptr; mMemoryAllocator = nullptr; @@ -205,7 +210,7 @@ namespace dawn_native { namespace vulkan { RecycleCompletedCommands(); mMapRequestTracker->Tick(mCompletedSerial); - mBufferUploader->Tick(mCompletedSerial); + mDynamicUploader->Tick(mCompletedSerial); mMemoryAllocator->Tick(mCompletedSerial); mDeleter->Tick(mCompletedSerial); @@ -247,10 +252,6 @@ namespace dawn_native { namespace vulkan { return mMemoryAllocator.get(); } - BufferUploader* Device::GetBufferUploader() const { - return mBufferUploader.get(); - } - FencedDeleter* Device::GetFencedDeleter() const { return mDeleter.get(); } @@ -497,7 +498,9 @@ namespace dawn_native { namespace vulkan { } ResultOrError> Device::CreateStagingBuffer(size_t size) { - return DAWN_UNIMPLEMENTED_ERROR("Device unable to create staging buffer."); + std::unique_ptr stagingBuffer = + std::make_unique(size, this); + return std::move(stagingBuffer); } MaybeError Device::CopyFromStagingToBuffer(StagingBufferBase* source, @@ -505,7 +508,35 @@ namespace dawn_native { namespace vulkan { BufferBase* destination, uint32_t destinationOffset, uint32_t size) { - return DAWN_UNIMPLEMENTED_ERROR("Device unable to copy from staging buffer."); + // Insert memory barrier to ensure host write operations are made visible before + // copying from the staging buffer. However, this barrier can be removed (see note below). + // + // Note: Depending on the spec understanding, an explicit barrier may not be required when + // used with HOST_COHERENT as vkQueueSubmit does an implicit barrier between host and + // device. See "Availability, Visibility, and Domain Operations" in Vulkan spec for details. + + // Insert pipeline barrier to ensure correct ordering with previous memory operations on the + // buffer. + ToBackend(destination) + ->TransitionUsageNow(GetPendingCommandBuffer(), dawn::BufferUsageBit::TransferDst); + + VkBufferCopy copy; + copy.srcOffset = sourceOffset; + copy.dstOffset = destinationOffset; + copy.size = size; + + this->fn.CmdCopyBuffer(GetPendingCommandBuffer(), ToBackend(source)->GetBufferHandle(), + ToBackend(destination)->GetHandle(), 1, ©); + + return {}; + } + + ResultOrError Device::GetDynamicUploader() const { + // TODO(b-brber): Refactor this into device init once moved into DeviceBase. + if (mDynamicUploader->IsEmpty()) { + DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer(kDefaultUploadBufferSize)); + } + return mDynamicUploader.get(); } }} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h index aefd00dcfc..ffa2a0d295 100644 --- a/src/dawn_native/vulkan/DeviceVk.h +++ b/src/dawn_native/vulkan/DeviceVk.h @@ -81,6 +81,8 @@ namespace dawn_native { namespace vulkan { uint32_t destinationOffset, uint32_t size) override; + ResultOrError GetDynamicUploader() const; + private: ResultOrError CreateBindGroupImpl( const BindGroupDescriptor* descriptor) override; @@ -114,7 +116,7 @@ namespace dawn_native { namespace vulkan { uint32_t mQueueFamily = 0; VkQueue mQueue = VK_NULL_HANDLE; - std::unique_ptr mBufferUploader; + std::unique_ptr mDynamicUploader; std::unique_ptr mDeleter; std::unique_ptr mMapRequestTracker; std::unique_ptr mMemoryAllocator; @@ -145,6 +147,9 @@ namespace dawn_native { namespace vulkan { std::vector mUnusedCommands; CommandPoolAndBuffer mPendingCommands; std::vector mWaitSemaphores; + + static constexpr size_t kDefaultUploadBufferSize = + 64000; // TODO(b-brber): Figure out this value. }; }} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/Forward.h b/src/dawn_native/vulkan/Forward.h index aade94e10b..5e6cac66de 100644 --- a/src/dawn_native/vulkan/Forward.h +++ b/src/dawn_native/vulkan/Forward.h @@ -33,6 +33,7 @@ namespace dawn_native { namespace vulkan { class RenderPipeline; class Sampler; class ShaderModule; + class StagingBuffer; class SwapChain; class Texture; class TextureView; @@ -52,6 +53,7 @@ namespace dawn_native { namespace vulkan { using RenderPipelineType = RenderPipeline; using SamplerType = Sampler; using ShaderModuleType = ShaderModule; + using StagingBufferType = StagingBuffer; using SwapChainType = SwapChain; using TextureType = Texture; using TextureViewType = TextureView; diff --git a/src/dawn_native/vulkan/MemoryAllocator.cpp b/src/dawn_native/vulkan/MemoryAllocator.cpp index e71adcb319..6d7cf07d52 100644 --- a/src/dawn_native/vulkan/MemoryAllocator.cpp +++ b/src/dawn_native/vulkan/MemoryAllocator.cpp @@ -60,6 +60,12 @@ namespace dawn_native { namespace vulkan { continue; } + // Mappable must also be host coherent. + if (mappable && + (info.memoryTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) == 0) { + continue; + } + // Found the first candidate memory type if (bestType == -1) { bestType = static_cast(i); diff --git a/src/dawn_native/vulkan/StagingBufferVk.cpp b/src/dawn_native/vulkan/StagingBufferVk.cpp new file mode 100644 index 0000000000..4e96f85f9b --- /dev/null +++ b/src/dawn_native/vulkan/StagingBufferVk.cpp @@ -0,0 +1,72 @@ +// Copyright 2018 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/vulkan/StagingBufferVk.h" +#include "dawn_native/vulkan/DeviceVk.h" +#include "dawn_native/vulkan/FencedDeleter.h" +#include "dawn_native/vulkan/MemoryAllocator.h" + +namespace dawn_native { namespace vulkan { + + StagingBuffer::StagingBuffer(size_t size, Device* device) + : StagingBufferBase(size), mDevice(device) { + } + + MaybeError StagingBuffer::Initialize() { + VkBufferCreateInfo createInfo; + createInfo.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; + createInfo.pNext = nullptr; + createInfo.flags = 0; + createInfo.size = GetSize(); + createInfo.usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT; + createInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + createInfo.queueFamilyIndexCount = 0; + createInfo.pQueueFamilyIndices = 0; + + if (mDevice->fn.CreateBuffer(mDevice->GetVkDevice(), &createInfo, nullptr, &mBuffer) != + VK_SUCCESS) { + return DAWN_CONTEXT_LOST_ERROR("Unable to create staging buffer."); + } + + VkMemoryRequirements requirements; + mDevice->fn.GetBufferMemoryRequirements(mDevice->GetVkDevice(), mBuffer, &requirements); + + if (!mDevice->GetMemoryAllocator()->Allocate(requirements, true, &mAllocation)) { + return DAWN_CONTEXT_LOST_ERROR("Unable to allocate memory for staging buffer."); + } + + if (mDevice->fn.BindBufferMemory(mDevice->GetVkDevice(), mBuffer, mAllocation.GetMemory(), + mAllocation.GetMemoryOffset()) != VK_SUCCESS) { + return DAWN_CONTEXT_LOST_ERROR("Unable to attach memory to the staging buffer."); + } + + mMappedPointer = mAllocation.GetMappedPointer(); + if (mMappedPointer == nullptr) { + return DAWN_CONTEXT_LOST_ERROR("Unable to map staging buffer."); + } + + return {}; + } + + StagingBuffer::~StagingBuffer() { + mMappedPointer = nullptr; + mDevice->GetFencedDeleter()->DeleteWhenUnused(mBuffer); + mDevice->GetMemoryAllocator()->Free(&mAllocation); + } + + VkBuffer StagingBuffer::GetBufferHandle() const { + return mBuffer; + } + +}} // namespace dawn_native::vulkan \ No newline at end of file diff --git a/src/dawn_native/vulkan/BufferUploader.h b/src/dawn_native/vulkan/StagingBufferVk.h similarity index 54% rename from src/dawn_native/vulkan/BufferUploader.h rename to src/dawn_native/vulkan/StagingBufferVk.h index 37ff0d7d6e..618c5ed7ff 100644 --- a/src/dawn_native/vulkan/BufferUploader.h +++ b/src/dawn_native/vulkan/StagingBufferVk.h @@ -1,4 +1,4 @@ -// Copyright 2017 The Dawn Authors +// Copyright 2018 The Dawn Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,32 +12,30 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_ -#define DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_ +#ifndef DAWNNATIVE_STAGINGBUFFERVK_H_ +#define DAWNNATIVE_STAGINGBUFFERVK_H_ -#include "common/SerialQueue.h" -#include "common/vulkan_platform.h" +#include "dawn_native/StagingBuffer.h" +#include "dawn_native/vulkan/MemoryAllocator.h" namespace dawn_native { namespace vulkan { class Device; - class BufferUploader { + class StagingBuffer : public StagingBufferBase { public: - BufferUploader(Device* device); - ~BufferUploader(); + StagingBuffer(size_t size, Device* device); + ~StagingBuffer(); - void BufferSubData(VkBuffer buffer, - VkDeviceSize offset, - VkDeviceSize size, - const void* data); + VkBuffer GetBufferHandle() const; - void Tick(Serial completedSerial); + MaybeError Initialize() override; private: - Device* mDevice = nullptr; + Device* mDevice; + VkBuffer mBuffer; + DeviceMemoryAllocation mAllocation; }; - }} // namespace dawn_native::vulkan -#endif // DAWNNATIVE_VULKAN_BUFFERUPLOADER_H_ +#endif // DAWNNATIVE_STAGINGBUFFERVK_H_ diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp index 70dfac8346..399994fe52 100644 --- a/src/tests/end2end/BufferTests.cpp +++ b/src/tests/end2end/BufferTests.cpp @@ -222,7 +222,7 @@ TEST_P(BufferSetSubDataTests, SmallDataAtOffset) { TEST_P(BufferSetSubDataTests, ManySetSubData) { // TODO(cwallez@chromium.org): Use ringbuffers for SetSubData on explicit APIs. // otherwise this creates too many resources and can take freeze the driver(?) - DAWN_SKIP_TEST_IF(IsMetal() || IsVulkan()); + DAWN_SKIP_TEST_IF(IsMetal()); // Note: Increasing the size of the buffer will likely cause timeout issues. // In D3D12, timeout detection occurs when the GPU scheduler tries but cannot preempt the task