From 815ec0d7d2774757a454276463584a0df7501251 Mon Sep 17 00:00:00 2001
From: Greg Daniel <egdaniel@google.com>
Date: Mon, 25 Jan 2021 14:09:27 -0500
Subject: [PATCH] Always use coherent memory on linux vulkan.

Bug: skia:11215

Change-Id: Id30276f674f83faada744ae88f8a5e457c80f5a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358531
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
---
 src/gpu/vk/GrVkAMDMemoryAllocator.cpp | 13 ++++++++++---
 src/gpu/vk/GrVkAMDMemoryAllocator.h   |  8 +++++++-
 src/gpu/vk/GrVkCaps.cpp               | 14 ++++++++++++++
 src/gpu/vk/GrVkCaps.h                 |  6 ++++++
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/gpu/vk/GrVkAMDMemoryAllocator.cpp b/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
index 30efc9f3a0..5bae6fe952 100644
--- a/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
+++ b/src/gpu/vk/GrVkAMDMemoryAllocator.cpp
@@ -87,13 +87,15 @@ sk_sp<GrVkMemoryAllocator> GrVkAMDMemoryAllocator::Make(VkInstance instance,
     vmaCreateAllocator(&info, &allocator);
 
     return sk_sp<GrVkAMDMemoryAllocator>(new GrVkAMDMemoryAllocator(
-            allocator, std::move(interface)));
+            allocator, std::move(interface), caps->mustUseCoherentHostVisibleMemory()));
 }
 
 GrVkAMDMemoryAllocator::GrVkAMDMemoryAllocator(VmaAllocator allocator,
-                                               sk_sp<const GrVkInterface> interface)
+                                               sk_sp<const GrVkInterface> interface,
+                                               bool mustUseCoherentHostVisibleMemory)
         : fAllocator(allocator)
-        , fInterface(std::move(interface)) {}
+        , fInterface(std::move(interface))
+        , fMustUseCoherentHostVisibleMemory(mustUseCoherentHostVisibleMemory) {}
 
 GrVkAMDMemoryAllocator::~GrVkAMDMemoryAllocator() {
     vmaDestroyAllocator(fAllocator);
@@ -176,6 +178,11 @@ VkResult GrVkAMDMemoryAllocator::allocateBufferMemory(VkBuffer buffer, BufferUsa
             break;
     }
 
+    if (fMustUseCoherentHostVisibleMemory &&
+        (info.requiredFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT)) {
+        info.requiredFlags |= VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
+    }
+
     if (AllocationPropertyFlags::kDedicatedAllocation & flags) {
         info.flags |= VMA_ALLOCATION_CREATE_DEDICATED_MEMORY_BIT;
     }
diff --git a/src/gpu/vk/GrVkAMDMemoryAllocator.h b/src/gpu/vk/GrVkAMDMemoryAllocator.h
index 057e31dfd6..9f3693ae4f 100644
--- a/src/gpu/vk/GrVkAMDMemoryAllocator.h
+++ b/src/gpu/vk/GrVkAMDMemoryAllocator.h
@@ -63,7 +63,8 @@ public:
     uint64_t totalAllocatedMemory() const override;
 
 private:
-    GrVkAMDMemoryAllocator(VmaAllocator allocator, sk_sp<const GrVkInterface> interface);
+    GrVkAMDMemoryAllocator(VmaAllocator allocator, sk_sp<const GrVkInterface> interface,
+                           bool mustUseCoherentHostVisibleMemory);
 
     VmaAllocator fAllocator;
 
@@ -72,6 +73,11 @@ private:
     // vulkan calls.
     sk_sp<const GrVkInterface> fInterface;
 
+    // For host visible allocations do we require they are coherent or not. All devices are required
+    // to support a host visible and coherent memory type. This is used to work around bugs for
+    // devices that don't handle non coherent memory correctly.
+    bool fMustUseCoherentHostVisibleMemory;
+
     using INHERITED = GrVkMemoryAllocator;
 };
 
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 095e926b04..42497ce998 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -538,6 +538,20 @@ void GrVkCaps::applyDriverCorrectnessWorkarounds(const VkPhysicalDevicePropertie
         fMustLoadFullImageWithDiscardableMSAA = true;
     }
 
+#ifdef SK_BUILD_FOR_UNIX
+    if (kIntel_VkVendor == properties.vendorID) {
+        // At least on our linux Debug Intel HD405 bot we are seeing issues doing read pixels with
+        // non-conherent memory. It seems like the device is not properly honoring the
+        // vkInvalidateMappedMemoryRanges calls correctly. Other linux intel devices seem to work
+        // okay. However, since I'm not sure how to target a specific intel devices or driver
+        // version I am going to stop all intel linux from using non-coherent memory. Currently we
+        // are not shipping anything on these platforms and the only real thing that will regress is
+        // read backs. If we find later we do care about this performance we can come back to figure
+        // out how to do a more narrow workaround.
+        fMustUseCoherentHostVisibleMemory = true;
+    }
+#endif
+
     ////////////////////////////////////////////////////////////////////////////
     // GrCaps workarounds
     ////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index c8956559a0..6c2fcf54fa 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -160,6 +160,10 @@ public:
         return fMustInvalidatePrimaryCmdBufferStateAfterClearAttachments;
     }
 
+    // For host visible allocations, this returns true if we require that they are coherent. This
+    // is used to work around bugs for devices that don't handle non-coherent memory correctly.
+    bool mustUseCoherentHostVisibleMemory() const { return fMustUseCoherentHostVisibleMemory; }
+
     // The max draw count that can be passed into indirect draw calls.
     uint32_t  maxDrawIndirectDrawCount() const { return fMaxDrawIndirectDrawCount; }
 
@@ -358,6 +362,8 @@ private:
     bool fPreferPrimaryOverSecondaryCommandBuffers = true;
     bool fMustInvalidatePrimaryCmdBufferStateAfterClearAttachments = false;
 
+    bool fMustUseCoherentHostVisibleMemory = false;
+
     // We default this to 100 since we already cap the max render tasks at 100 before doing a
     // submission in the GrDrawingManager, so we shouldn't be going over 100 secondary command
     // buffers per primary anyways.