commit c56eede3396a4a8aa26126e20bf6d174a6a75f7c
author Matteo Franchin <matteo.franchin@arm.com> Tue Dec 03 17:10:38 2019 +0000
committer John Stultz <john.stultz@linaro.org> Tue Dec 03 17:10:38 2019 +0000
tree 0feac6a020c41028fb6b16cecfc283fa5a28e10a
parent d6a6e6eadd783365013fdc493c00994054f3e6aa

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>

drmhwctwo.cpp

diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
index ab81f61..ffc5fcd 100644
--- a/drmhwctwo.cpp
+++ b/drmhwctwo.cpp
@@ -547,16 +547,16 @@
   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 @@
   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 @@
   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;