drm_hwcomposer: Fix HwcLayer::GetReleaseFences()

GetReleaseFences() should return release fence for the prior buffer
(not for the one assigned to layer at the moment of GetReleaseFences call).

Once not provided, old front buffer can be damaged before new buffer
presented. (Such issues start to appear once we started using
non-blocking DRM/KMS commits).

Using present (out) fence is a perfect solution, since it is
signaled once old buffer replaced with the new one.

Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>

7 files changed, 46 insertions, 24 deletions

diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index 5e160d4..0f73f1f 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -18,7 +18,6 @@
 
 #include "DrmDevice.h"
 
-#include <fcntl.h>
 #include <xf86drm.h>
 #include <xf86drmMode.h>
 
diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp
index 53e98dc..dbf0993 100644
--- a/drm/ResourceManager.cpp
+++ b/drm/ResourceManager.cpp
@@ -18,7 +18,6 @@
 
 #include "ResourceManager.h"
 
-#include <fcntl.h>
 #include <sys/stat.h>
 
 #include <ctime>
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 3956fc9..d94c9bc 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -391,6 +391,9 @@ HWC2::Error HwcDisplay::GetHdrCapabilities(uint32_t *num_types,
 
 /* Find API details at:
  * https://cs.android.com/android/platform/superproject/+/android-11.0.0_r3:hardware/libhardware/include/hardware/hwcomposer2.h;l=1767
+ *
+ * Called after PresentDisplay(), CLIENT is expecting release fence for the
+ * prior buffer (not the one assigned to the layer at the moment).
  */
 HWC2::Error HwcDisplay::GetReleaseFences(uint32_t *num_elements,
                                          hwc2_layer_t *layers,
@@ -402,8 +405,13 @@ HWC2::Error HwcDisplay::GetReleaseFences(uint32_t *num_elements,
 
   uint32_t num_layers = 0;
 
-  for (std::pair<const hwc2_layer_t, HwcLayer> &l : layers_) {
+  for (auto &l : layers_) {
+    if (!l.second.GetPriorBufferScanOutFlag() || !present_fence_) {
+      continue;
+    }
+
     ++num_layers;
+
     if (layers == nullptr || fences == nullptr)
       continue;
 
@@ -413,9 +421,10 @@ HWC2::Error HwcDisplay::GetReleaseFences(uint32_t *num_elements,
     }
 
     layers[num_layers - 1] = l.first;
-    fences[num_layers - 1] = l.second.GetReleaseFence().Release();
+    fences[num_layers - 1] = UniqueFd::Dup(present_fence_.Get()).Release();
   }
   *num_elements = num_layers;
+
   return HWC2::Error::None;
 }
 
@@ -520,9 +529,9 @@ HWC2::Error HwcDisplay::CreateComposition(AtomicCommitArgs &a_args) {
 /* Find API details at:
  * https://cs.android.com/android/platform/superproject/+/android-11.0.0_r3:hardware/libhardware/include/hardware/hwcomposer2.h;l=1805
  */
-HWC2::Error HwcDisplay::PresentDisplay(int32_t *present_fence) {
+HWC2::Error HwcDisplay::PresentDisplay(int32_t *out_present_fence) {
   if (IsInHeadlessMode()) {
-    *present_fence = -1;
+    *out_present_fence = -1;
     return HWC2::Error::None;
   }
   HWC2::Error ret{};
@@ -537,13 +546,14 @@ HWC2::Error HwcDisplay::PresentDisplay(int32_t *present_fence) {
 
   if (ret == HWC2::Error::BadLayer) {
     // Can we really have no client or device layers?
-    *present_fence = -1;
+    *out_present_fence = -1;
     return HWC2::Error::None;
   }
   if (ret != HWC2::Error::None)
     return ret;
 
-  *present_fence = a_args.out_fence.Release();
+  this->present_fence_ = UniqueFd::Dup(a_args.out_fence.Get());
+  *out_present_fence = a_args.out_fence.Release();
 
   ++frame_no_;
   return HWC2::Error::None;
@@ -690,6 +700,16 @@ HWC2::Error HwcDisplay::ValidateDisplay(uint32_t *num_types,
     *num_types = *num_requests = 0;
     return HWC2::Error::None;
   }
+
+  /* In current drm_hwc design in case previous frame layer was not validated as
+   * a CLIENT, it is used by display controller (Front buffer). We have to store
+   * this state to provide the CLIENT with the release fences for such buffers.
+   */
+  for (auto &l : layers_) {
+    l.second.SetPriorBufferScanOutFlag(l.second.GetValidatedType() !=
+                                       HWC2::Composition::Client);
+  }
+
   return backend_->ValidateDisplay(this, num_types, num_requests);
 }
 
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index 8ff9a92..cd33ebe 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -106,7 +106,7 @@ class HwcDisplay {
                                  float *min_luminance);
   HWC2::Error GetReleaseFences(uint32_t *num_elements, hwc2_layer_t *layers,
                                int32_t *fences);
-  HWC2::Error PresentDisplay(int32_t *present_fence);
+  HWC2::Error PresentDisplay(int32_t *out_present_fence);
   HWC2::Error SetActiveConfig(hwc2_config_t config);
   HWC2::Error ChosePreferredConfig();
   HWC2::Error SetClientTarget(buffer_handle_t target, int32_t acquire_fence,
@@ -199,6 +199,8 @@ class HwcDisplay {
 
   DrmHwcTwo *const hwc2_;
 
+  UniqueFd present_fence_;
+
   std::optional<DrmMode> staged_mode_;
   int64_t staged_mode_change_time_{};
   uint32_t staged_mode_config_id_{};
diff --git a/hwc2_device/HwcLayer.cpp b/hwc2_device/HwcLayer.cpp
index 66babda..cc535d7 100644
--- a/hwc2_device/HwcLayer.cpp
+++ b/hwc2_device/HwcLayer.cpp
@@ -18,8 +18,6 @@
 
 #include "HwcLayer.h"
 
-#include <fcntl.h>
-
 #include "utils/log.h"
 
 namespace android {
@@ -167,7 +165,7 @@ HWC2::Error HwcLayer::SetLayerZOrder(uint32_t order) {
 void HwcLayer::PopulateDrmLayer(DrmHwcLayer *layer) {
   layer->sf_handle = buffer_;
   // TODO(rsglobal): Avoid extra fd duplication
-  layer->acquire_fence = UniqueFd(fcntl(acquire_fence_.Get(), F_DUPFD_CLOEXEC));
+  layer->acquire_fence = UniqueFd::Dup(acquire_fence_.Get());
   layer->display_frame = display_frame_;
   layer->alpha = std::lround(alpha_ * UINT16_MAX);
   layer->blending = blending_;
diff --git a/hwc2_device/HwcLayer.h b/hwc2_device/HwcLayer.h
index df4ce6d..ad94129 100644
--- a/hwc2_device/HwcLayer.h
+++ b/hwc2_device/HwcLayer.h
@@ -43,6 +43,14 @@ class HwcLayer {
     return sf_type_ != validated_type_;
   }
 
+  bool GetPriorBufferScanOutFlag() const {
+    return prior_buffer_scanout_flag_;
+  }
+
+  void SetPriorBufferScanOutFlag(bool state) {
+    prior_buffer_scanout_flag_ = state;
+  }
+
   uint32_t GetZOrder() const {
     return z_order_;
   }
@@ -55,10 +63,6 @@ class HwcLayer {
     return display_frame_;
   }
 
-  UniqueFd GetReleaseFence() {
-    return std::move(release_fence_);
-  }
-
   void PopulateDrmLayer(DrmHwcLayer *layer);
 
   bool RequireScalingOrPhasing() const {
@@ -107,15 +111,9 @@ class HwcLayer {
   DrmHwcColorSpace color_space_ = DrmHwcColorSpace::kUndefined;
   DrmHwcSampleRange sample_range_ = DrmHwcSampleRange::kUndefined;
 
-  UniqueFd acquire_fence_;
+  bool prior_buffer_scanout_flag_{};
 
-  /*
-   * Release fence is not used.
-   * There is no release fence support available in the DRM/KMS. In case no
-   * release fence provided application will use this buffer for writing when
-   * the next frame present fence is signaled.
-   */
-  UniqueFd release_fence_;
+  UniqueFd acquire_fence_;
 };
 
 }  // namespace android
diff --git a/utils/UniqueFd.h b/utils/UniqueFd.h
index 1a390ba..d747a7f 100644
--- a/utils/UniqueFd.h
+++ b/utils/UniqueFd.h
@@ -17,6 +17,7 @@
 #ifndef UNIQUEFD_H_
 #define UNIQUEFD_H_
 
+#include <fcntl.h>
 #include <unistd.h>
 
 #include <memory>
@@ -73,6 +74,11 @@ class UniqueFd {
     return fd_;
   }
 
+  static auto Dup(int fd) {
+    // NOLINTNEXTLINE(android-cloexec-dup): fcntl has issue (see issue #63)
+    return UniqueFd(dup(fd));
+  }
+
   explicit operator bool() const {
     return fd_ != kEmptyFd;
   }