Attach coalesced Buffer if any source Buffer is attached

A buffer that's attached to a context could be coalesced into a larger buffer which isn't attached, this would break as it wouldn't keep the buffer alive till the end of the associated context. To fix this if any source buffers are attached then the resulting coalesced buffer is also attached now.
This commit is contained in:
PixelyIon 2022-07-16 20:12:09 +05:30
parent 284ac53d88
commit 3ac5ed8c06
No known key found for this signature in database
GPG Key ID: 11BC6C3201BC2C05
5 changed files with 44 additions and 12 deletions

View File

@ -24,7 +24,7 @@ namespace skyline::gpu {
return mutex.try_lock();
}
BufferView BufferManager::FindOrCreate(GuestBuffer guestMapping, ContextTag tag) {
BufferView BufferManager::FindOrCreate(GuestBuffer guestMapping, ContextTag tag, const std::function<void(std::shared_ptr<Buffer>, ContextLock<Buffer> &)> &attachBuffer) {
/*
* We align the buffer to the page boundary to ensure that:
* 1) Any buffer view has the same alignment guarantees as on the guest, this is required for UBOs, SSBOs and Texel buffers
@ -60,9 +60,14 @@ namespace skyline::gpu {
}
auto newBuffer{std::make_shared<Buffer>(gpu, span<u8>{lowestAddress, highestAddress}, tag, overlaps)};
ContextLock newLock{tag, *newBuffer};
bool hasAlreadyAttached{}; //!< If any overlapping view was already attached to the current context
for (auto &overlap : overlaps) {
ContextLock overlapLock{tag, *overlap};
if (!overlapLock.isFirst)
hasAlreadyAttached = true;
buffers.erase(std::find(buffers.begin(), buffers.end(), overlap));
// Transfer all views from the overlapping buffer to the new buffer with the new buffer and updated offset, ensuring pointer stability
@ -94,6 +99,10 @@ namespace skyline::gpu {
newBuffer->delegates.splice(newBuffer->delegates.end(), overlap->delegates);
}
if (hasAlreadyAttached)
// We need to reattach the buffer if any overlapping views were already attached to the current context
attachBuffer(newBuffer, newLock);
buffers.insert(std::lower_bound(buffers.begin(), buffers.end(), newBuffer->guest->end().base(), BufferLessThan), newBuffer);
return newBuffer->GetView(static_cast<vk::DeviceSize>(guestMapping.begin() - newBuffer->guest->begin()) + offset, size);

View File

@ -62,10 +62,11 @@ namespace skyline::gpu {
bool try_lock();
/**
* @param attachBuffer A function that attaches the buffer to the current context, this'll be called when coalesced buffers are merged into the current buffer
* @return A pre-existing or newly created Buffer object which covers the supplied mappings
* @note The buffer manager **must** be locked prior to calling this
*/
BufferView FindOrCreate(GuestBuffer guestMapping, ContextTag tag = {});
BufferView FindOrCreate(GuestBuffer guestMapping, ContextTag tag = {}, const std::function<void(std::shared_ptr<Buffer>, ContextLock<Buffer> &)> &attachBuffer = {});
/**
* @return A dynamically allocated megabuffer which can be used to store buffer modifications allowing them to be replayed in-sequence on the GPU

View File

@ -137,7 +137,7 @@ namespace skyline::gpu::interconnect {
return didLock;
}
void CommandExecutor::AttachLockedBuffer(BufferView &view, ContextLock<BufferView> &lock) {
void CommandExecutor::AttachLockedBufferView(BufferView &view, ContextLock<BufferView> &lock) {
if (!bufferManagerLock)
// See AttachTexture(...)
bufferManagerLock.emplace(gpu.buffer);
@ -151,6 +151,13 @@ namespace skyline::gpu::interconnect {
attachedBufferDelegates.emplace(view.bufferDelegate);
}
void CommandExecutor::AttachLockedBuffer(std::shared_ptr<Buffer> buffer, ContextLock<Buffer> &lock) {
if (lock.isFirst) {
attachedBuffers.emplace_back(std::move(buffer));
lock.isFirst = false;
}
}
void CommandExecutor::AttachDependency(const std::shared_ptr<void> &dependency) {
cycle->AttachObject(dependency);
}

View File

@ -124,7 +124,7 @@ namespace skyline::gpu::interconnect {
BufferManager &AcquireBufferManager();
/**
* @brief Attach the lifetime of a buffer to the command buffer
* @brief Attach the lifetime of a buffer view to the command buffer
* @return If this is the first usage of the backing of this resource within this execution
* @note The supplied buffer will be locked automatically until the command buffer is submitted and must **not** be locked by the caller
* @note This'll automatically handle syncing of the buffer in the most optimal way possible
@ -132,13 +132,20 @@ namespace skyline::gpu::interconnect {
bool AttachBuffer(BufferView &view);
/**
* @brief Attach the lifetime of a buffer that's already locked to the command buffer
* @return If this is the first usage of the backing of this resource within this execution
* @brief Attach the lifetime of a buffer view that's already locked to the command buffer
* @note The supplied buffer **must** be locked with the executor's tag
* @note There must be no other external locks on the buffer aside from the supplied lock
* @note This'll automatically handle syncing of the buffer in the most optimal way possible
*/
void AttachLockedBuffer(BufferView &view, ContextLock<BufferView>& lock);
void AttachLockedBufferView(BufferView &view, ContextLock<BufferView>& lock);
/**
* @brief Attach the lifetime of a buffer object that's already locked to the command buffer
* @note The supplied buffer **must** be locked with the executor's tag
* @note There must be no other external locks on the buffer aside from the supplied lock
* @note This'll automatically handle syncing of the buffer in the most optimal way possible
*/
void AttachLockedBuffer(std::shared_ptr<Buffer> buffer, ContextLock<Buffer>& lock);
/**
* @brief Attach the lifetime of the fence cycle dependency to the command buffer

View File

@ -647,7 +647,7 @@ namespace skyline::gpu::interconnect {
// TODO: see Read()
Logger::Warn("GPU dirty buffer reads for attached buffers are unimplemented");
}, [&megaBuffer, &pExecutor, srcCpuBuf, dstOffset, &view = this->view, &lock]() {
pExecutor.AttachLockedBuffer(view, lock);
pExecutor.AttachLockedBufferView(view, lock);
auto srcGpuOffset{megaBuffer.Push(srcCpuBuf)};
auto srcGpuBuf{megaBuffer.GetBacking()};
@ -727,7 +727,9 @@ namespace skyline::gpu::interconnect {
auto view{constantBufferCache.Lookup(constantBufferSelector.size, constantBufferSelector.iova)};
if (!view) {
auto mappings{channelCtx.asCtx->gmmu.TranslateRange(constantBufferSelector.iova, constantBufferSelector.size)};
view = executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag);
view = executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag, [this](std::shared_ptr<Buffer> buffer, ContextLock<Buffer> &lock) {
executor.AttachLockedBuffer(buffer, lock);
});
constantBufferCache.Insert(constantBufferSelector.size, constantBufferSelector.iova, *view);
}
@ -918,7 +920,9 @@ namespace skyline::gpu::interconnect {
if (mappings.size() != 1)
Logger::Warn("Multiple buffer mappings ({}) are not supported", mappings.size());
return executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag);
return executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag, [this](std::shared_ptr<Buffer> buffer, ContextLock<Buffer> &lock) {
executor.AttachLockedBuffer(buffer, lock);
});
}
/**
@ -1847,7 +1851,9 @@ namespace skyline::gpu::interconnect {
if (mappings.size() != 1)
Logger::Warn("Multiple buffer mappings ({}) are not supported", mappings.size());
vertexBuffer.view = executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag);
vertexBuffer.view = executor.AcquireBufferManager().FindOrCreate(mappings.front(), executor.tag, [this](std::shared_ptr<Buffer> buffer, ContextLock<Buffer> &lock) {
executor.AttachLockedBuffer(buffer, lock);
});
return &vertexBuffer;
}
@ -2602,7 +2608,9 @@ namespace skyline::gpu::interconnect {
Logger::Warn("Multiple buffer mappings ({}) are not supported", mappings.size());
auto mapping{mappings.front()};
indexBuffer.view = executor.AcquireBufferManager().FindOrCreate(span<u8>(mapping.data(), size), executor.tag);
indexBuffer.view = executor.AcquireBufferManager().FindOrCreate(span<u8>(mapping.data(), size), executor.tag, [this](std::shared_ptr<Buffer> buffer, ContextLock<Buffer> &lock) {
executor.AttachLockedBuffer(buffer, lock);
});
return indexBuffer.view;
}