commit f01a6f1491ec94dca77d80d3c76c473e806226da
author Vishnu Nair <vishnun@google.com> Mon Apr 03 22:34:17 2023 +0000
committer Vishnu Nair <vishnun@google.com> Tue Apr 04 00:07:45 2023 +0000
tree 5c1793a4a0248f468851f6acc2a74aea3d84d793
parent f9c1b98e54aa996d2a4412916a09e6fe504be560

[sf] Fix latch unsignaled issues

With LatchUnsignaledConfig::AutoSingleLayer flag, we should only
apply a transaction with an unsignaled fence if its the only transaction.
If we pass an unsignaled fence to HWC, HWC might miss presenting
the frame if the fence does not fire in time. If we apply another
transaction we may penalize the other transaction unfairly.

With LatchUnsignaledConfig::Always flag, respect backpressure flag
and take the opportunity to simplify the logic by treating the flag as
a signal to ignore the fence when applying transactions.

Test: surfaceflinger unit tests
Bug: 276229937
Change-Id: Ib5cd8205d62fdf8912b7f5c2cad312a01cae4300

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a271083..4be4d59 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4266,20 +4266,23 @@
             return TraverseBuffersReturnValues::STOP_TRAVERSAL;
         }
 
-        // check fence status
-        const bool allowLatchUnsignaled = shouldLatchUnsignaled(layer, s, transaction.states.size(),
-                                                                flushState.firstTransaction);
-        ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),
-                      allowLatchUnsignaled ? "true" : "false");
-
-        const bool acquireFenceChanged = s.bufferData &&
+        // ignore the acquire fence if LatchUnsignaledConfig::Always is set.
+        const bool checkAcquireFence = enableLatchUnsignaledConfig != LatchUnsignaledConfig::Always;
+        const bool acquireFenceAvailable = s.bufferData &&
                 s.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged) &&
                 s.bufferData->acquireFence;
-        const bool fenceSignaled =
-                (!acquireFenceChanged ||
-                 s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled);
+        const bool fenceSignaled = !checkAcquireFence || !acquireFenceAvailable ||
+                s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled;
         if (!fenceSignaled) {
-            if (!allowLatchUnsignaled) {
+            // check fence status
+            const bool allowLatchUnsignaled =
+                    shouldLatchUnsignaled(layer, s, transaction.states.size(),
+                                          flushState.firstTransaction);
+            ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(),
+                          allowLatchUnsignaled ? "true" : "false");
+            if (allowLatchUnsignaled) {
+                ready = TransactionReadiness::NotReadyUnsignaled;
+            } else {
                 ready = TransactionReadiness::NotReady;
                 auto& listener = s.bufferData->releaseBufferListener;
                 if (listener &&
@@ -4292,10 +4295,6 @@
                 }
                 return TraverseBuffersReturnValues::STOP_TRAVERSAL;
             }
-
-            ready = enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer
-                    ? TransactionReadiness::ReadyUnsignaledSingle
-                    : TransactionReadiness::ReadyUnsignaled;
         }
         return TraverseBuffersReturnValues::CONTINUE_TRAVERSAL;
     });