drm_hwcomposer: Fix returned fence in PresentDisplay

DrmHwcTwo::HwcDisplay::PresentDisplay was always returning -1 as the
present fence. This commits ensures the fence fd is correctly retrieved
after doing the commit-frame operation. It also updates outdated logic
that caused PresentDisplay to return the retire fence rather than the
present fence.

DrmHwcTwo::HwcDisplay::AddFenceToPresentFence is also changed so that
it assumes it is given ownership of the file descriptor it receives as
argument. This function was indeed called consistently with this
behaviour, which meant the dup led to leakage of file descriptors.

With the changes above this patch fixes a failure in the CTS test
dEQP-VK.wsi.android.display_timing.fifo.display_timing (for example
running Android 10 on HiKey960). The test failed with the error
"Unexpectedly received invalid timestamp." reported multiple times in
the logcat output.

Change-Id: If662e5239895b8b0e2ea31fd99747855f901a427
Signed-off-by: Matteo Franchin <matteo.franchin@arm.com>

4 files changed, 18 insertions, 21 deletions

diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
index ab81f61..ffc5fcd 100644
--- a/drmhwctwo.cpp
+++ b/drmhwctwo.cpp
@@ -547,16 +547,16 @@ HWC2::Error DrmHwcTwo::HwcDisplay::GetReleaseFences(uint32_t *num_elements,
   return HWC2::Error::None;
 }
 
-void DrmHwcTwo::HwcDisplay::AddFenceToRetireFence(int fd) {
-  supported(__func__);
+void DrmHwcTwo::HwcDisplay::AddFenceToPresentFence(int fd) {
   if (fd < 0)
     return;
 
-  if (next_retire_fence_.get() >= 0) {
-    int old_fence = next_retire_fence_.get();
-    next_retire_fence_.Set(sync_merge("dc_retire", old_fence, fd));
+  if (present_fence_.get() >= 0) {
+    int old_fence = present_fence_.get();
+    present_fence_.Set(sync_merge("dc_present", old_fence, fd));
+    close(fd);
   } else {
-    next_retire_fence_.Set(dup(fd));
+    present_fence_.Set(fd);
   }
 }
 
@@ -642,8 +642,8 @@ HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) {
   if (test) {
     ret = compositor_.TestComposition(composition.get());
   } else {
-    AddFenceToRetireFence(composition->take_out_fence());
     ret = compositor_.ApplyComposition(std::move(composition));
+    AddFenceToPresentFence(compositor_.TakeOutFence());
   }
   if (ret) {
     if (!test)
@@ -653,23 +653,20 @@ HWC2::Error DrmHwcTwo::HwcDisplay::CreateComposition(bool test) {
   return HWC2::Error::None;
 }
 
-HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) {
+HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *present_fence) {
   supported(__func__);
   HWC2::Error ret;
 
   ret = CreateComposition(false);
   if (ret == HWC2::Error::BadLayer) {
     // Can we really have no client or device layers?
-    *retire_fence = -1;
+    *present_fence = -1;
     return HWC2::Error::None;
   }
   if (ret != HWC2::Error::None)
     return ret;
 
-  // The retire fence returned here is for the last frame, so return it and
-  // promote the next retire fence
-  *retire_fence = retire_fence_.Release();
-  retire_fence_ = std::move(next_retire_fence_);
+  *present_fence = present_fence_.Release();
 
   ++frame_no_;
   return HWC2::Error::None;
diff --git a/include/drmdisplaycompositor.h b/include/drmdisplaycompositor.h
index 1005598..477f226 100644
--- a/include/drmdisplaycompositor.h
+++ b/include/drmdisplaycompositor.h
@@ -56,6 +56,11 @@ class DrmDisplayCompositor {
   void Dump(std::ostringstream *out) const;
   void Vsync(int display, int64_t timestamp);
   void ClearDisplay();
+  int TakeOutFence() {
+    if (!active_composition_)
+      return -1;
+    return active_composition_->take_out_fence();
+  }
 
   std::tuple<uint32_t, uint32_t, int> GetActiveModeResolution();
 
diff --git a/include/drmhwcomposer.h b/include/drmhwcomposer.h
index 2af7e6e..69313d9 100644
--- a/include/drmhwcomposer.h
+++ b/include/drmhwcomposer.h
@@ -158,10 +158,6 @@ struct DrmHwcLayer {
   }
 };
 
-struct DrmHwcDisplayContents {
-  OutputFd retire_fence;
-  std::vector<DrmHwcLayer> layers;
-};
 }  // namespace android
 
 #endif
diff --git a/include/drmhwctwo.h b/include/drmhwctwo.h
index f675429..c6ce640 100644
--- a/include/drmhwctwo.h
+++ b/include/drmhwctwo.h
@@ -172,7 +172,7 @@ class DrmHwcTwo : public hwc2_device_t {
                                    float *min_luminance);
     HWC2::Error GetReleaseFences(uint32_t *num_elements, hwc2_layer_t *layers,
                                  int32_t *fences);
-    HWC2::Error PresentDisplay(int32_t *retire_fence);
+    HWC2::Error PresentDisplay(int32_t *present_fence);
     HWC2::Error SetActiveConfig(hwc2_config_t config);
     HWC2::Error ChosePreferredConfig();
     HWC2::Error SetClientTarget(buffer_handle_t target, int32_t acquire_fence,
@@ -192,7 +192,7 @@ class DrmHwcTwo : public hwc2_device_t {
 
    private:
     HWC2::Error CreateComposition(bool test);
-    void AddFenceToRetireFence(int fd);
+    void AddFenceToPresentFence(int fd);
     bool HardwareSupportsLayerType(HWC2::Composition comp_type);
 
     ResourceManager *resource_manager_;
@@ -212,8 +212,7 @@ class DrmHwcTwo : public hwc2_device_t {
     uint32_t layer_idx_ = 0;
     std::map<hwc2_layer_t, HwcLayer> layers_;
     HwcLayer client_layer_;
-    UniqueFd retire_fence_;
-    UniqueFd next_retire_fence_;
+    UniqueFd present_fence_;
     int32_t color_mode_;
 
     uint32_t frame_no_ = 0;