Use the HWC caching mechanism to avoid stalls in the ion driver.

HWC supports caching buffers for layers using "slot" assignments.
Use this in VrFlinger to avoid importing a buffer handle every
frame. The avoids periodic stalls we observe in the ion driver
when mapping a buffer into the HWC address space.

Bug: 66459419
Test: Observe systraces no longer have MapBuffer in HWC in steady
      state; system does not drop frames.

Change-Id: Iba4161b33561322bfbccbfafe600b432a6fa7c44

5 files changed, 73 insertions, 28 deletions

diff --git a/libs/vr/libvrflinger/acquired_buffer.cpp b/libs/vr/libvrflinger/acquired_buffer.cpp
index fda9585dc4..9614c6d306 100644
--- a/libs/vr/libvrflinger/acquired_buffer.cpp
+++ b/libs/vr/libvrflinger/acquired_buffer.cpp
@@ -9,8 +9,8 @@ namespace android {
 namespace dvr {
 
 AcquiredBuffer::AcquiredBuffer(const std::shared_ptr<BufferConsumer>& buffer,
-                               LocalHandle acquire_fence)
-    : buffer_(buffer), acquire_fence_(std::move(acquire_fence)) {}
+                               LocalHandle acquire_fence, std::size_t slot)
+    : buffer_(buffer), acquire_fence_(std::move(acquire_fence)), slot_(slot) {}
 
 AcquiredBuffer::AcquiredBuffer(const std::shared_ptr<BufferConsumer>& buffer,
                                int* error) {
@@ -31,18 +31,20 @@ AcquiredBuffer::AcquiredBuffer(const std::shared_ptr<BufferConsumer>& buffer,
   }
 }
 
-AcquiredBuffer::AcquiredBuffer(AcquiredBuffer&& other)
-    : buffer_(std::move(other.buffer_)),
-      acquire_fence_(std::move(other.acquire_fence_)) {}
+AcquiredBuffer::AcquiredBuffer(AcquiredBuffer&& other) {
+  *this = std::move(other);
+}
 
 AcquiredBuffer::~AcquiredBuffer() { Release(LocalHandle(kEmptyFence)); }
 
 AcquiredBuffer& AcquiredBuffer::operator=(AcquiredBuffer&& other) {
   if (this != &other) {
-    Release(LocalHandle(kEmptyFence));
+    Release();
 
-    buffer_ = std::move(other.buffer_);
-    acquire_fence_ = std::move(other.acquire_fence_);
+    using std::swap;
+    swap(buffer_, other.buffer_);
+    swap(acquire_fence_, other.acquire_fence_);
+    swap(slot_, other.slot_);
   }
   return *this;
 }
@@ -81,8 +83,6 @@ int AcquiredBuffer::Release(LocalHandle release_fence) {
   ALOGD_IF(TRACE, "AcquiredBuffer::Release: buffer_id=%d release_fence=%d",
            buffer_ ? buffer_->id() : -1, release_fence.Get());
   if (buffer_) {
-    // Close the release fence since we can't transfer it with an async release.
-    release_fence.Close();
     const int ret = buffer_->ReleaseAsync();
     if (ret < 0) {
       ALOGE("AcquiredBuffer::Release: Failed to release buffer %d: %s",
@@ -92,9 +92,10 @@ int AcquiredBuffer::Release(LocalHandle release_fence) {
     }
 
     buffer_ = nullptr;
-    acquire_fence_.Close();
   }
 
+  acquire_fence_.Close();
+  slot_ = 0;
   return 0;
 }
 
diff --git a/libs/vr/libvrflinger/acquired_buffer.h b/libs/vr/libvrflinger/acquired_buffer.h
index e0dc9f2ac2..32e912a65d 100644
--- a/libs/vr/libvrflinger/acquired_buffer.h
+++ b/libs/vr/libvrflinger/acquired_buffer.h
@@ -21,7 +21,7 @@ class AcquiredBuffer {
   // this constructor; the constructor does not attempt to ACQUIRE the buffer
   // itself.
   AcquiredBuffer(const std::shared_ptr<BufferConsumer>& buffer,
-                 pdx::LocalHandle acquire_fence);
+                 pdx::LocalHandle acquire_fence, std::size_t slot = 0);
 
   // Constructs an AcquiredBuffer from a BufferConsumer. The BufferConsumer MUST
   // be in the POSTED state prior to calling this constructor, as this
@@ -64,13 +64,18 @@ class AcquiredBuffer {
   // to the producer. On success, the BufferConsumer and acquire fence are set
   // to empty state; if release fails, the BufferConsumer and acquire fence are
   // left in place and a negative error code is returned.
-  int Release(pdx::LocalHandle release_fence);
+  int Release(pdx::LocalHandle release_fence = {});
+
+  // Returns the slot in the queue this buffer belongs to. Buffers that are not
+  // part of a queue return 0.
+  std::size_t slot() const { return slot_; }
 
  private:
   std::shared_ptr<BufferConsumer> buffer_;
   // Mutable so that the fence can be closed when it is determined to be
   // signaled during IsAvailable().
   mutable pdx::LocalHandle acquire_fence_;
+  std::size_t slot_{0};
 
   AcquiredBuffer(const AcquiredBuffer&) = delete;
   void operator=(const AcquiredBuffer&) = delete;
diff --git a/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 685378133e..3d132c95ea 100644
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -382,7 +382,7 @@ void DirectDisplaySurface::DequeueBuffersLocked() {
     }
 
     acquired_buffers_.Append(
-        AcquiredBuffer(buffer_consumer, std::move(acquire_fence)));
+        AcquiredBuffer(buffer_consumer, std::move(acquire_fence), slot));
   }
 }
 
diff --git a/libs/vr/libvrflinger/hardware_composer.cpp b/libs/vr/libvrflinger/hardware_composer.cpp
index de6477b1ef..b3da120c74 100644
--- a/libs/vr/libvrflinger/hardware_composer.cpp
+++ b/libs/vr/libvrflinger/hardware_composer.cpp
@@ -605,7 +605,7 @@ int HardwareComposer::PostThreadPollInterruptible(
   } else if (pfd[0].revents != 0) {
     return 0;
   } else if (pfd[1].revents != 0) {
-    ALOGI("VrHwcPost thread interrupted");
+    ALOGI("VrHwcPost thread interrupted: revents=%x", pfd[1].revents);
     return kPostThreadInterrupted;
   } else {
     return 0;
@@ -1069,6 +1069,7 @@ void Layer::Reset() {
   acquire_fence_.Close();
   surface_rect_functions_applied_ = false;
   pending_visibility_settings_ = true;
+  cached_buffer_map_.clear();
 }
 
 Layer::Layer(const std::shared_ptr<DirectDisplaySurface>& surface,
@@ -1112,6 +1113,7 @@ Layer& Layer::operator=(Layer&& other) {
     swap(surface_rect_functions_applied_,
          other.surface_rect_functions_applied_);
     swap(pending_visibility_settings_, other.pending_visibility_settings_);
+    swap(cached_buffer_map_, other.cached_buffer_map_);
   }
   return *this;
 }
@@ -1205,15 +1207,30 @@ void Layer::CommonLayerSetup() {
   UpdateLayerSettings();
 }
 
+bool Layer::CheckAndUpdateCachedBuffer(std::size_t slot, int buffer_id) {
+  auto search = cached_buffer_map_.find(slot);
+  if (search != cached_buffer_map_.end() && search->second == buffer_id)
+    return true;
+
+  // Assign or update the buffer slot.
+  if (buffer_id >= 0)
+    cached_buffer_map_[slot] = buffer_id;
+  return false;
+}
+
 void Layer::Prepare() {
-  int right, bottom;
+  int right, bottom, id;
   sp<GraphicBuffer> handle;
+  std::size_t slot;
 
   // Acquire the next buffer according to the type of source.
   IfAnyOf<SourceSurface, SourceBuffer>::Call(&source_, [&](auto& source) {
-    std::tie(right, bottom, handle, acquire_fence_) = source.Acquire();
+    std::tie(right, bottom, id, handle, acquire_fence_, slot) =
+        source.Acquire();
   });
 
+  TRACE_FORMAT("Layer::Prepare|buffer_id=%d;slot=%zu|", id, slot);
+
   // Update any visibility (blending, z-order) changes that occurred since
   // last prepare.
   UpdateVisibilitySettings();
@@ -1243,10 +1260,15 @@ void Layer::Prepare() {
           composition_type_.cast<Hwc2::IComposerClient::Composition>());
     }
 
+    // See if the HWC cache already has this buffer.
+    const bool cached = CheckAndUpdateCachedBuffer(slot, id);
+    if (cached)
+      handle = nullptr;
+
     HWC::Error error{HWC::Error::None};
     error =
         composer_->setLayerBuffer(HWC_DISPLAY_PRIMARY, hardware_composer_layer_,
-                                  0, handle, acquire_fence_.Get());
+                                  slot, handle, acquire_fence_.Get());
 
     ALOGE_IF(error != HWC::Error::None,
              "Layer::Prepare: Error setting layer buffer: %s",
diff --git a/libs/vr/libvrflinger/hardware_composer.h b/libs/vr/libvrflinger/hardware_composer.h
index 8131e50989..7010db9630 100644
--- a/libs/vr/libvrflinger/hardware_composer.h
+++ b/libs/vr/libvrflinger/hardware_composer.h
@@ -161,6 +161,14 @@ class Layer {
   // Applies visibility settings that may have changed.
   void UpdateVisibilitySettings();
 
+  // Checks whether the buffer, given by id, is associated with the given slot
+  // in the HWC buffer cache. If the slot is not associated with the given
+  // buffer the cache is updated to establish the association and the buffer
+  // should be sent to HWC using setLayerBuffer. Returns true if the association
+  // was already established, false if not. A buffer_id of -1 is never
+  // associated and always returns false.
+  bool CheckAndUpdateCachedBuffer(std::size_t slot, int buffer_id);
+
   // Composer instance shared by all instances of Layer. This must be set
   // whenever a new instance of the Composer is created. This may be set to
   // nullptr as long as there are no instances of Layer that might need to use
@@ -198,19 +206,21 @@ class Layer {
     // the previous buffer is returned or an empty value if no buffer has ever
     // been posted. When a new buffer is acquired the previous buffer's release
     // fence is passed out automatically.
-    std::tuple<int, int, sp<GraphicBuffer>, pdx::LocalHandle> Acquire() {
+    std::tuple<int, int, int, sp<GraphicBuffer>, pdx::LocalHandle, std::size_t>
+    Acquire() {
       if (surface->IsBufferAvailable()) {
         acquired_buffer.Release(std::move(release_fence));
         acquired_buffer = surface->AcquireCurrentBuffer();
         ATRACE_ASYNC_END("BufferPost", acquired_buffer.buffer()->id());
       }
       if (!acquired_buffer.IsEmpty()) {
-        return std::make_tuple(acquired_buffer.buffer()->width(),
-                               acquired_buffer.buffer()->height(),
-                               acquired_buffer.buffer()->buffer()->buffer(),
-                               acquired_buffer.ClaimAcquireFence());
+        return std::make_tuple(
+            acquired_buffer.buffer()->width(),
+            acquired_buffer.buffer()->height(), acquired_buffer.buffer()->id(),
+            acquired_buffer.buffer()->buffer()->buffer(),
+            acquired_buffer.ClaimAcquireFence(), acquired_buffer.slot());
       } else {
-        return std::make_tuple(0, 0, nullptr, pdx::LocalHandle{});
+        return std::make_tuple(0, 0, -1, nullptr, pdx::LocalHandle{}, 0);
       }
     }
 
@@ -242,12 +252,13 @@ class Layer {
   struct SourceBuffer {
     std::shared_ptr<IonBuffer> buffer;
 
-    std::tuple<int, int, sp<GraphicBuffer>, pdx::LocalHandle> Acquire() {
+    std::tuple<int, int, int, sp<GraphicBuffer>, pdx::LocalHandle, std::size_t>
+    Acquire() {
       if (buffer)
-        return std::make_tuple(buffer->width(), buffer->height(),
-                               buffer->buffer(), pdx::LocalHandle{});
+        return std::make_tuple(buffer->width(), buffer->height(), -1,
+                               buffer->buffer(), pdx::LocalHandle{}, 0);
       else
-        return std::make_tuple(0, 0, nullptr, pdx::LocalHandle{});
+        return std::make_tuple(0, 0, -1, nullptr, pdx::LocalHandle{}, 0);
     }
 
     void Finish(pdx::LocalHandle /*fence*/) {}
@@ -266,6 +277,12 @@ class Layer {
   bool surface_rect_functions_applied_ = false;
   bool pending_visibility_settings_ = true;
 
+  // Map of buffer slot assignments that have already been established with HWC:
+  // slot -> buffer_id. When this map contains a matching slot and buffer_id the
+  // buffer argument to setLayerBuffer may be nullptr to avoid the cost of
+  // importing a buffer HWC already knows about.
+  std::map<std::size_t, int> cached_buffer_map_;
+
   Layer(const Layer&) = delete;
   void operator=(const Layer&) = delete;
 };