buffer_cache: Small performance improvement

Remove redundant call to UnmarkRegionAsGpuModified; ForEachDownloadRange can do its operation while looping over pages. Move stalling download operation after download page gathering, that might avoid some cases of stalling unnecessarily

Author: raphaelthegreat

src/video_core/amdgpu/liverpool.h

@@ -87,7 +87,7 @@ struct Liverpool {
         }
     };
 
-    static const BinaryInfo& SearchBinaryInfo(const u32* code, size_t search_limit = 0x1000) {
+    static const BinaryInfo& SearchBinaryInfo(const u32* code, size_t search_limit = 0x2000) {
         constexpr u32 token_mov_vcchi = 0xBEEB03FF;
 
         if (code[0] == token_mov_vcchi) {

src/video_core/buffer_cache/buffer_cache.cpp

@@ -46,34 +46,16 @@ void BufferCache::InvalidateMemory(VAddr device_addr, u64 size) {
     if (!IsRegionRegistered(device_addr, size)) {
         return;
     }
-    if (memory_tracker->IsRegionGpuModified(device_addr, size)) {
-        ReadMemory(device_addr, size);
-    }
+    DownloadMemory(device_addr, size);
     memory_tracker->MarkRegionAsCpuModified(device_addr, size);
 }
 
-void BufferCache::ReadMemory(VAddr device_addr, u64 size) {
-    if (std::this_thread::get_id() != liverpool->gpu_id) {
-        std::binary_semaphore command_wait{0};
-        liverpool->SendCommand([this, &command_wait, device_addr, size] {
-            Buffer& buffer = slot_buffers[FindBuffer(device_addr, size)];
-            DownloadBufferMemory(buffer, device_addr, size);
-            command_wait.release();
-        });
-        command_wait.acquire();
-    } else {
-        Buffer& buffer = slot_buffers[FindBuffer(device_addr, size)];
-        DownloadBufferMemory(buffer, device_addr, size);
-    }
-    memory_tracker->UnmarkRegionAsGpuModified(device_addr, size);
-}
-
-void BufferCache::DownloadBufferMemory(Buffer& buffer, VAddr device_addr, u64 size) {
+void BufferCache::DownloadMemory(VAddr device_addr, u64 size) {
     boost::container::small_vector<vk::BufferCopy, 1> copies;
     u64 total_size_bytes = 0;
+    const VAddr buffer_addr = PageManager::GetPageAddr(device_addr);
     memory_tracker->ForEachDownloadRange<true>(
         device_addr, size, [&](u64 device_addr_out, u64 range_size) {
-            const VAddr buffer_addr = buffer.CpuAddr();
             const auto add_download = [&](VAddr start, VAddr end) {
                 const u64 new_offset = start - buffer_addr;
                 const u64 new_size = end - start;
@@ -93,22 +75,36 @@ void BufferCache::DownloadBufferMemory(Buffer& buffer, VAddr device_addr, u64 si
     if (total_size_bytes == 0) {
         return;
     }
-    const auto [staging, offset] = staging_buffer.Map(total_size_bytes);
-    for (auto& copy : copies) {
-        // Modify copies to have the staging offset in mind
-        copy.dstOffset += offset;
-    }
-    staging_buffer.Commit();
-    scheduler.EndRendering();
-    const auto cmdbuf = scheduler.CommandBuffer();
-    cmdbuf.copyBuffer(buffer.buffer, staging_buffer.Handle(), copies);
-    scheduler.Finish();
-    auto* memory = Core::Memory::Instance();
-    for (const auto& copy : copies) {
-        const VAddr copy_device_addr = buffer.CpuAddr() + copy.srcOffset;
-        const u64 dst_offset = copy.dstOffset - offset;
-        memory->TryWriteBacking(std::bit_cast<u8*>(copy_device_addr), staging + dst_offset,
-                                copy.size);
+    std::binary_semaphore command_wait{0};
+    auto do_download = [this, &command_wait, buffer_addr, device_addr,
+                        size, total_size_bytes, &copies]() mutable {
+        const BufferId buffer_id = FindBuffer(device_addr, size);
+        auto& buffer = slot_buffers[buffer_id];
+        const auto [staging, offset] = staging_buffer.Map(total_size_bytes);
+        for (auto& copy : copies) {
+            // Modify copies to have the staging offset in mind
+            copy.srcOffset += buffer.Offset(buffer_addr);
+            copy.dstOffset += offset;
+        }
+        staging_buffer.Commit();
+        scheduler.EndRendering();
+        const auto cmdbuf = scheduler.CommandBuffer();
+        cmdbuf.copyBuffer(buffer.buffer, staging_buffer.Handle(), copies);
+        scheduler.Finish();
+        auto* memory = Core::Memory::Instance();
+        for (const auto& copy : copies) {
+            const VAddr copy_device_addr = buffer.CpuAddr() + copy.srcOffset;
+            const u64 dst_offset = copy.dstOffset - offset;
+            memory->TryWriteBacking(std::bit_cast<u8*>(copy_device_addr), staging + dst_offset,
+                                    copy.size);
+        }
+        command_wait.release();
+    };
+    if (std::this_thread::get_id() != liverpool->gpu_id) {
+        liverpool->SendCommand(std::move(do_download));
+        command_wait.acquire();
+    } else {
+        do_download();
     }
 }
 

src/video_core/buffer_cache/buffer_cache.h

@@ -82,7 +82,7 @@ class BufferCache {
     void InvalidateMemory(VAddr device_addr, u64 size);
 
     /// Waits on pending downloads in the logical page range.
-    void ReadMemory(VAddr device_addr, u64 size);
+    void DownloadMemory(VAddr device_addr, u64 size);
 
     /// Binds host vertex buffers for the current draw.
     void BindVertexBuffers(const Vulkan::GraphicsPipeline& pipeline);
@@ -134,8 +134,6 @@ class BufferCache {
         }
     }
 
-    void DownloadBufferMemory(Buffer& buffer, VAddr device_addr, u64 size);
-
     [[nodiscard]] OverlapResult ResolveOverlaps(VAddr device_addr, u32 wanted_size);
 
     void JoinOverlap(BufferId new_buffer_id, BufferId overlap_id, bool accumulate_stream_score);

src/video_core/buffer_cache/word_manager.h

@@ -223,7 +223,8 @@ class RegionManager {
      * @param size   Size in bytes of the region to query for modifications
      */
     template <Type type>
-    [[nodiscard]] bool IsRegionModified(u64 offset, u64 size) const noexcept {
+    [[nodiscard]] bool IsRegionModified(u64 offset, u64 size) noexcept {
+        std::scoped_lock lk{lock};
         const std::span<const u64> state_words = Span<type>();
         bool result = false;
         IterateWords(offset, size, [&](size_t index, u64 mask) {

src/video_core/page_manager.cpp

@@ -153,7 +153,7 @@ struct PageManager::Impl {
         if (Common::IsWriteError(context)) {
             return rasterizer->InvalidateMemory(addr, 1);
         } else {
-            return rasterizer->ReadMemory(addr, 1);
+            return rasterizer->DownloadMemory(addr, 1);
         }
         return false;
     }

src/video_core/renderer_vulkan/vk_rasterizer.cpp

@@ -929,12 +929,8 @@ bool Rasterizer::InvalidateMemory(VAddr addr, u64 size) {
     return true;
 }
 
-bool Rasterizer::ReadMemory(VAddr addr, u64 size) {
-    if (!IsMapped(addr, size)) {
-        // Not GPU mapped memory, can skip invalidation logic entirely.
-        return false;
-    }
-    buffer_cache.ReadMemory(addr, size);
+bool Rasterizer::DownloadMemory(VAddr addr, u64 size) {
+    buffer_cache.DownloadMemory(addr, size);
     return true;
 }
 

src/video_core/renderer_vulkan/vk_rasterizer.h

@@ -57,7 +57,7 @@ class Rasterizer {
     void CopyBuffer(VAddr dst, VAddr src, u32 num_bytes, bool dst_gds, bool src_gds);
     u32 ReadDataFromGds(u32 gsd_offset);
     bool InvalidateMemory(VAddr addr, u64 size);
-    bool ReadMemory(VAddr addr, u64 size);
+    bool DownloadMemory(VAddr addr, u64 size);
     bool IsMapped(VAddr addr, u64 size);
     void MapMemory(VAddr addr, u64 size);
     void UnmapMemory(VAddr addr, u64 size);

src/video_core/texture_cache/texture_cache.cpp

@@ -436,9 +436,9 @@ ImageView& TextureCache::FindDepthTarget(BaseDesc& desc) {
     const ImageId image_id = FindImage(desc);
     Image& image = slot_images[image_id];
     image.flags |= ImageFlagBits::GpuModified;
-    image.flags &= ~ImageFlagBits::Dirty;
     image.usage.depth_target = 1u;
     image.usage.stencil = image.info.HasStencil();
+    UpdateImage(image_id);
 
     // Register meta data for this depth buffer
     if (!(image.flags & ImageFlagBits::MetaRegistered)) {