From 776da59abc836d2262382d79d7605d2ffc77dec3 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sun, 4 Jul 2021 18:08:49 +0200 Subject: Buffer Cache: Fix High Downloads and don't predownload on Extreme. --- src/video_core/buffer_cache/buffer_base.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/video_core/buffer_cache/buffer_base.h') diff --git a/src/video_core/buffer_cache/buffer_base.h b/src/video_core/buffer_cache/buffer_base.h index b121d36a3c..a56b4c3a8d 100644 --- a/src/video_core/buffer_cache/buffer_base.h +++ b/src/video_core/buffer_cache/buffer_base.h @@ -226,19 +226,19 @@ public: /// Call 'func' for each CPU modified range and unmark those pages as CPU modified template void ForEachUploadRange(VAddr query_cpu_range, u64 size, Func&& func) { - ForEachModifiedRange(query_cpu_range, size, func); + ForEachModifiedRange(query_cpu_range, size, true, func); } /// Call 'func' for each GPU modified range and unmark those pages as GPU modified template - void ForEachDownloadRange(VAddr query_cpu_range, u64 size, Func&& func) { - ForEachModifiedRange(query_cpu_range, size, func); + void ForEachDownloadRange(VAddr query_cpu_range, u64 size, bool clear, Func&& func) { + ForEachModifiedRange(query_cpu_range, size, clear, func); } /// Call 'func' for each GPU modified range and unmark those pages as GPU modified template void ForEachDownloadRange(Func&& func) { - ForEachModifiedRange(cpu_addr, SizeBytes(), func); + ForEachModifiedRange(cpu_addr, SizeBytes(), true, func); } /// Mark buffer as picked @@ -415,7 +415,7 @@ private: * @param func Function to call for each turned off region */ template - void ForEachModifiedRange(VAddr query_cpu_range, s64 size, Func&& func) { + void ForEachModifiedRange(VAddr query_cpu_range, s64 size, bool clear, Func&& func) { static_assert(type != Type::Untracked); const s64 difference = query_cpu_range - cpu_addr; @@ -467,7 +467,9 @@ private: bits = (bits << left_offset) >> left_offset; const u64 current_word = state_words[word_index] & bits; - state_words[word_index] &= ~bits; + if (clear) { + state_words[word_index] &= ~bits; + } if constexpr (type == Type::CPU) { const u64 current_bits = untracked_words[word_index] & bits; -- cgit v1.2.3 From 907da84f022b591c3dc26ff56f49dfeaeefe4d91 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sun, 4 Jul 2021 18:28:20 +0200 Subject: Videocore: Address Feedback & CLANG Format. --- src/video_core/buffer_cache/buffer_base.h | 2 +- src/video_core/buffer_cache/buffer_cache.h | 151 ++++++++++++++--------------- 2 files changed, 75 insertions(+), 78 deletions(-) (limited to 'src/video_core/buffer_cache/buffer_base.h') diff --git a/src/video_core/buffer_cache/buffer_base.h b/src/video_core/buffer_cache/buffer_base.h index a56b4c3a8d..9e39858c86 100644 --- a/src/video_core/buffer_cache/buffer_base.h +++ b/src/video_core/buffer_cache/buffer_base.h @@ -468,7 +468,7 @@ private: const u64 current_word = state_words[word_index] & bits; if (clear) { - state_words[word_index] &= ~bits; + state_words[word_index] &= ~bits; } if constexpr (type == Type::CPU) { diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h index d28930e807..dc2b1f4473 100644 --- a/src/video_core/buffer_cache/buffer_cache.h +++ b/src/video_core/buffer_cache/buffer_cache.h @@ -14,8 +14,8 @@ #include #include -#include #include +#include #include "common/common_types.h" #include "common/div_ceil.h" @@ -333,10 +333,7 @@ private: std::vector cached_write_buffer_ids; - // TODO: This data structure is not optimal and it should be reworked - IntervalSet uncommitted_ranges; - std::deque committed_ranges; - std::deque> pending_downloads; + IntervalSet uncommitted_ranges; size_t immediate_buffer_capacity = 0; std::unique_ptr immediate_buffer_alloc; @@ -564,74 +561,75 @@ bool BufferCache

::ShouldWaitAsyncFlushes() const noexcept { template void BufferCache

::CommitAsyncFlushesHigh() { - const IntervalSet& intervals = uncommitted_ranges; - if (intervals.empty()) { - return; - } - MICROPROFILE_SCOPE(GPU_DownloadMemory); - - boost::container::small_vector, 1> downloads; - u64 total_size_bytes = 0; - u64 largest_copy = 0; - for (auto& interval : intervals) { - const std::size_t size = interval.upper() - interval.lower(); - const VAddr cpu_addr = interval.lower(); - const VAddr cpu_addr_end = interval.upper(); - ForEachBufferInRange(cpu_addr, size, [&](BufferId buffer_id, Buffer& buffer) { - boost::container::small_vector copies; - buffer.ForEachDownloadRange(cpu_addr, size, false, [&](u64 range_offset, u64 range_size) { - VAddr cpu_addr_base = buffer.CpuAddr() + range_offset; - VAddr cpu_addr_end2 = cpu_addr_base + range_size; - const s64 difference = s64(cpu_addr_end2 - cpu_addr_end); - cpu_addr_end2 -= u64(std::max(difference, 0)); - const s64 difference2 = s64(cpu_addr - cpu_addr_base); - cpu_addr_base += u64(std::max(difference2, 0)); - const u64 new_size = cpu_addr_end2 - cpu_addr_base; - const u64 new_offset = cpu_addr_base - buffer.CpuAddr(); - ASSERT(!IsRegionCpuModified(cpu_addr_base, new_size)); - downloads.push_back({ - BufferCopy{ - .src_offset = new_offset, - .dst_offset = total_size_bytes, - .size = new_size, - }, - buffer_id, - }); - total_size_bytes += new_size; - buffer.UnmarkRegionAsGpuModified(cpu_addr_base, new_size); - largest_copy = std::max(largest_copy, new_size); - }); - }); - } - if (downloads.empty()) { - return; - } - if constexpr (USE_MEMORY_MAPS) { - auto download_staging = runtime.DownloadStagingBuffer(total_size_bytes); - for (auto& [copy, buffer_id] : downloads) { - // Have in mind the staging buffer offset for the copy - copy.dst_offset += download_staging.offset; - const std::array copies{copy}; - runtime.CopyBuffer(download_staging.buffer, slot_buffers[buffer_id], copies); - } - runtime.Finish(); - for (const auto& [copy, buffer_id] : downloads) { - const Buffer& buffer = slot_buffers[buffer_id]; - const VAddr cpu_addr = buffer.CpuAddr() + copy.src_offset; - // Undo the modified offset - const u64 dst_offset = copy.dst_offset - download_staging.offset; - const u8* read_mapped_memory = download_staging.mapped_span.data() + dst_offset; - cpu_memory.WriteBlockUnsafe(cpu_addr, read_mapped_memory, copy.size); - } - } else { - const std::span immediate_buffer = ImmediateBuffer(largest_copy); - for (const auto& [copy, buffer_id] : downloads) { - Buffer& buffer = slot_buffers[buffer_id]; - buffer.ImmediateDownload(copy.src_offset, immediate_buffer.subspan(0, copy.size)); - const VAddr cpu_addr = buffer.CpuAddr() + copy.src_offset; - cpu_memory.WriteBlockUnsafe(cpu_addr, immediate_buffer.data(), copy.size); - } - } + const IntervalSet& intervals = uncommitted_ranges; + if (intervals.empty()) { + return; + } + MICROPROFILE_SCOPE(GPU_DownloadMemory); + + boost::container::small_vector, 1> downloads; + u64 total_size_bytes = 0; + u64 largest_copy = 0; + for (auto& interval : intervals) { + const std::size_t size = interval.upper() - interval.lower(); + const VAddr cpu_addr = interval.lower(); + const VAddr cpu_addr_end = interval.upper(); + ForEachBufferInRange(cpu_addr, size, [&](BufferId buffer_id, Buffer& buffer) { + boost::container::small_vector copies; + buffer.ForEachDownloadRange( + cpu_addr, size, false, [&](u64 range_offset, u64 range_size) { + VAddr cpu_addr_base = buffer.CpuAddr() + range_offset; + VAddr cpu_addr_end2 = cpu_addr_base + range_size; + const s64 difference = s64(cpu_addr_end2 - cpu_addr_end); + cpu_addr_end2 -= u64(std::max(difference, 0)); + const s64 difference2 = s64(cpu_addr - cpu_addr_base); + cpu_addr_base += u64(std::max(difference2, 0)); + const u64 new_size = cpu_addr_end2 - cpu_addr_base; + const u64 new_offset = cpu_addr_base - buffer.CpuAddr(); + ASSERT(!IsRegionCpuModified(cpu_addr_base, new_size)); + downloads.push_back({ + BufferCopy{ + .src_offset = new_offset, + .dst_offset = total_size_bytes, + .size = new_size, + }, + buffer_id, + }); + total_size_bytes += new_size; + buffer.UnmarkRegionAsGpuModified(cpu_addr_base, new_size); + largest_copy = std::max(largest_copy, new_size); + }); + }); + } + if (downloads.empty()) { + return; + } + if constexpr (USE_MEMORY_MAPS) { + auto download_staging = runtime.DownloadStagingBuffer(total_size_bytes); + for (auto& [copy, buffer_id] : downloads) { + // Have in mind the staging buffer offset for the copy + copy.dst_offset += download_staging.offset; + const std::array copies{copy}; + runtime.CopyBuffer(download_staging.buffer, slot_buffers[buffer_id], copies); + } + runtime.Finish(); + for (const auto& [copy, buffer_id] : downloads) { + const Buffer& buffer = slot_buffers[buffer_id]; + const VAddr cpu_addr = buffer.CpuAddr() + copy.src_offset; + // Undo the modified offset + const u64 dst_offset = copy.dst_offset - download_staging.offset; + const u8* read_mapped_memory = download_staging.mapped_span.data() + dst_offset; + cpu_memory.WriteBlockUnsafe(cpu_addr, read_mapped_memory, copy.size); + } + } else { + const std::span immediate_buffer = ImmediateBuffer(largest_copy); + for (const auto& [copy, buffer_id] : downloads) { + Buffer& buffer = slot_buffers[buffer_id]; + buffer.ImmediateDownload(copy.src_offset, immediate_buffer.subspan(0, copy.size)); + const VAddr cpu_addr = buffer.CpuAddr() + copy.src_offset; + cpu_memory.WriteBlockUnsafe(cpu_addr, immediate_buffer.data(), copy.size); + } + } } template @@ -644,9 +642,7 @@ void BufferCache

::CommitAsyncFlushes() { } template -void BufferCache

::PopAsyncFlushes() { - -} +void BufferCache

::PopAsyncFlushes() {} template bool BufferCache

::IsRegionGpuModified(VAddr addr, size_t size) { @@ -1055,7 +1051,8 @@ void BufferCache

::MarkWrittenBuffer(BufferId buffer_id, VAddr cpu_addr, u32 s Buffer& buffer = slot_buffers[buffer_id]; buffer.MarkRegionAsGpuModified(cpu_addr, size); - const bool is_accuracy_high = Settings::values.gpu_accuracy.GetValue() == Settings::GPUAccuracy::High; + const bool is_accuracy_high = + Settings::values.gpu_accuracy.GetValue() == Settings::GPUAccuracy::High; const bool is_async = Settings::values.use_asynchronous_gpu_emulation.GetValue(); if (!is_async && !is_accuracy_high) { return; -- cgit v1.2.3 From 1be1828db1abeec12064944aa887ec987b724e5e Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sat, 10 Jul 2021 21:15:21 +0200 Subject: Buffer Cache: Address Feedback. --- src/tests/video_core/buffer_base.cpp | 2 +- src/video_core/buffer_cache/buffer_base.h | 5 +++++ src/video_core/buffer_cache/buffer_cache.h | 8 ++++---- 3 files changed, 10 insertions(+), 5 deletions(-) (limited to 'src/video_core/buffer_cache/buffer_base.h') diff --git a/src/tests/video_core/buffer_base.cpp b/src/tests/video_core/buffer_base.cpp index cfcdc22536..9f5a54de4a 100644 --- a/src/tests/video_core/buffer_base.cpp +++ b/src/tests/video_core/buffer_base.cpp @@ -536,7 +536,7 @@ TEST_CASE("BufferBase: Cached write downloads") { REQUIRE(rasterizer.Count() == 63); buffer.MarkRegionAsGpuModified(c + PAGE, PAGE); int num = 0; - buffer.ForEachDownloadRange(c, WORD, true, [&](u64 offset, u64 size) { ++num; }); + buffer.ForEachDownloadRangeAndClear(c, WORD, [&](u64 offset, u64 size) { ++num; }); buffer.ForEachUploadRange(c, WORD, [&](u64 offset, u64 size) { ++num; }); REQUIRE(num == 0); REQUIRE(!buffer.IsRegionCpuModified(c + PAGE, PAGE)); diff --git a/src/video_core/buffer_cache/buffer_base.h b/src/video_core/buffer_cache/buffer_base.h index 9e39858c86..c3318095c4 100644 --- a/src/video_core/buffer_cache/buffer_base.h +++ b/src/video_core/buffer_cache/buffer_base.h @@ -235,6 +235,11 @@ public: ForEachModifiedRange(query_cpu_range, size, clear, func); } + template + void ForEachDownloadRangeAndClear(VAddr query_cpu_range, u64 size, Func&& func) { + ForEachModifiedRange(query_cpu_range, size, true, func); + } + /// Call 'func' for each GPU modified range and unmark those pages as GPU modified template void ForEachDownloadRange(Func&& func) { diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h index 910909201a..502feddba5 100644 --- a/src/video_core/buffer_cache/buffer_cache.h +++ b/src/video_core/buffer_cache/buffer_cache.h @@ -167,7 +167,7 @@ public: /// Return true when a CPU region is modified from the GPU [[nodiscard]] bool IsRegionGpuModified(VAddr addr, size_t size); - /// Return true when a CPU region is modified from the GPU + /// Return true when a CPU region is modified from the CPU [[nodiscard]] bool IsRegionCpuModified(VAddr addr, size_t size); std::mutex mutex; @@ -593,8 +593,8 @@ void BufferCache

::CommitAsyncFlushesHigh() { const VAddr cpu_addr = interval.lower(); ForEachBufferInRange(cpu_addr, size, [&](BufferId buffer_id, Buffer& buffer) { boost::container::small_vector copies; - buffer.ForEachDownloadRange( - cpu_addr, size, true, [&](u64 range_offset, u64 range_size) { + buffer.ForEachDownloadRangeAndClear( + cpu_addr, size, [&](u64 range_offset, u64 range_size) { const VAddr buffer_addr = buffer.CpuAddr(); const auto add_download = [&](VAddr start, VAddr end) { const u64 new_offset = start - buffer_addr; @@ -1331,7 +1331,7 @@ void BufferCache

::DownloadBufferMemory(Buffer& buffer, VAddr cpu_addr, u64 si boost::container::small_vector copies; u64 total_size_bytes = 0; u64 largest_copy = 0; - buffer.ForEachDownloadRange(cpu_addr, size, true, [&](u64 range_offset, u64 range_size) { + buffer.ForEachDownloadRangeAndClear(cpu_addr, size, [&](u64 range_offset, u64 range_size) { const VAddr buffer_addr = buffer.CpuAddr(); const auto add_download = [&](VAddr start, VAddr end) { const u64 new_offset = start - buffer_addr; -- cgit v1.2.3