Merge "SF: check previous present fence to avoid early presentation" into sc-dev
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index 29937fb..554e2f4 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -24,6 +24,7 @@
 #include <compositionengine/LayerFE.h>
 #include <compositionengine/OutputColorSetting.h>
 #include <math/mat4.h>
+#include <ui/FenceTime.h>
 #include <ui/Transform.h>
 
 namespace android::compositionengine {
@@ -83,6 +84,10 @@
     // The earliest time to send the present command to the HAL
     std::chrono::steady_clock::time_point earliestPresentTime;
 
+    // The previous present fence. Used together with earliestPresentTime
+    // to prevent an early presentation of a frame.
+    std::shared_ptr<FenceTime> previousPresentFence;
+
     // The predicted next invalidation time
     std::optional<std::chrono::steady_clock::time_point> nextInvalidateTime;
 };
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index d41c2dd..f34cb94 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -19,6 +19,7 @@
 #include <cstdint>
 
 #include <math/mat4.h>
+#include <ui/FenceTime.h>
 
 // TODO(b/129481165): remove the #pragma below and fix conversion issues
 #pragma clang diagnostic push
@@ -118,6 +119,10 @@
     // The earliest time to send the present command to the HAL
     std::chrono::steady_clock::time_point earliestPresentTime;
 
+    // The previous present fence. Used together with earliestPresentTime
+    // to prevent an early presentation of a frame.
+    std::shared_ptr<FenceTime> previousPresentFence;
+
     // Current display brightness
     float displayBrightnessNits{-1.f};
 
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index ae1336e..2f2c686 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -229,7 +229,8 @@
     auto& hwc = getCompositionEngine().getHwComposer();
     if (status_t result =
                 hwc.getDeviceCompositionChanges(*halDisplayId, anyLayersRequireClientComposition(),
-                                                getState().earliestPresentTime, &changes);
+                                                getState().earliestPresentTime,
+                                                getState().previousPresentFence, &changes);
         result != NO_ERROR) {
         ALOGE("chooseCompositionStrategy failed for %s: %d (%s)", getName().c_str(), result,
               strerror(-result));
@@ -330,7 +331,8 @@
     }
 
     auto& hwc = getCompositionEngine().getHwComposer();
-    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime);
+    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime,
+                                   getState().previousPresentFence);
 
     fences.presentFence = hwc.getPresentFence(*halDisplayIdOpt);
 
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 67bb149..cafcb40 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -729,6 +729,7 @@
     }
 
     editState().earliestPresentTime = refreshArgs.earliestPresentTime;
+    editState().previousPresentFence = refreshArgs.previousPresentFence;
 
     compositionengine::OutputLayer* peekThroughLayer = nullptr;
     sp<GraphicBuffer> previousOverride = nullptr;
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index db9437b..c037cc6 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -579,7 +579,7 @@
 TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutOnHwcError) {
     EXPECT_CALL(*mDisplay, anyLayersRequireClientComposition()).WillOnce(Return(false));
     EXPECT_CALL(mHwComposer,
-                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _))
+                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _, _))
             .WillOnce(Return(INVALID_OPERATION));
 
     mDisplay->chooseCompositionStrategy();
@@ -602,7 +602,7 @@
             .WillOnce(Return(false));
 
     EXPECT_CALL(mHwComposer,
-                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))
+                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
             .WillOnce(Return(NO_ERROR));
     EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false));
 
@@ -633,8 +633,8 @@
             .WillOnce(Return(false));
 
     EXPECT_CALL(mHwComposer,
-                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))
-            .WillOnce(DoAll(SetArgPointee<3>(changes), Return(NO_ERROR)));
+                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
+            .WillOnce(DoAll(SetArgPointee<4>(changes), Return(NO_ERROR)));
     EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(changes.changedTypes)).Times(1);
     EXPECT_CALL(*mDisplay, applyDisplayRequests(changes.displayRequests)).Times(1);
     EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(changes.layerRequests)).Times(1);
@@ -844,7 +844,7 @@
     sp<Fence> layer1Fence = new Fence();
     sp<Fence> layer2Fence = new Fence();
 
-    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _))
+    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _, _))
             .Times(1);
     EXPECT_CALL(mHwComposer, getPresentFence(HalDisplayId(DEFAULT_DISPLAY_ID)))
             .WillOnce(Return(presentFence));
@@ -1020,7 +1020,7 @@
 
     mDisplay->editState().isEnabled = true;
 
-    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _));
+    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _, _));
     EXPECT_CALL(*mDisplaySurface, onFrameCommitted());
 
     mDisplay->postFramebuffer();
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 64cbea9..a195e58 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -52,14 +52,16 @@
                       std::optional<PhysicalDisplayId>));
     MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
     MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
-    MOCK_METHOD4(getDeviceCompositionChanges,
+    MOCK_METHOD5(getDeviceCompositionChanges,
                  status_t(HalDisplayId, bool, std::chrono::steady_clock::time_point,
+                          const std::shared_ptr<FenceTime>&,
                           std::optional<android::HWComposer::DeviceRequestedChanges>*));
     MOCK_METHOD5(setClientTarget,
                  status_t(HalDisplayId, uint32_t, const sp<Fence>&, const sp<GraphicBuffer>&,
                           ui::Dataspace));
-    MOCK_METHOD2(presentAndGetReleaseFences,
-                 status_t(HalDisplayId, std::chrono::steady_clock::time_point));
+    MOCK_METHOD3(presentAndGetReleaseFences,
+                 status_t(HalDisplayId, std::chrono::steady_clock::time_point,
+                          const std::shared_ptr<FenceTime>&));
     MOCK_METHOD2(setPowerMode, status_t(PhysicalDisplayId, hal::PowerMode));
     MOCK_METHOD2(setActiveConfig, status_t(HalDisplayId, size_t));
     MOCK_METHOD2(setColorTransform, status_t(HalDisplayId, const mat4&));
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 32f04e5..7e45dab 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -471,6 +471,7 @@
 status_t HWComposer::getDeviceCompositionChanges(
         HalDisplayId displayId, bool frameUsesClientComposition,
         std::chrono::steady_clock::time_point earliestPresentTime,
+        const std::shared_ptr<FenceTime>& previousPresentFence,
         std::optional<android::HWComposer::DeviceRequestedChanges>* outChanges) {
     ATRACE_CALL();
 
@@ -487,12 +488,16 @@
 
     hal::Error error = hal::Error::NONE;
 
-    // First try to skip validate altogether when we passed the earliest time
-    // to present and there is no client. Otherwise, we may present a frame too
-    // early or in case of client composition we first need to render the
+    // First try to skip validate altogether. We can do that when
+    // 1. The previous frame has not been presented yet or already passed the
+    // earliest time to present. Otherwise, we may present a frame too early.
+    // 2. There is no client composition. Otherwise, we first need to render the
     // client target buffer.
-    const bool canSkipValidate =
-            std::chrono::steady_clock::now() >= earliestPresentTime && !frameUsesClientComposition;
+    const bool prevFencePending =
+            previousPresentFence->getSignalTime() == Fence::SIGNAL_TIME_PENDING;
+    const bool canPresentEarly =
+            !prevFencePending && std::chrono::steady_clock::now() < earliestPresentTime;
+    const bool canSkipValidate = !canPresentEarly && !frameUsesClientComposition;
     displayData.validateWasSkipped = false;
     if (canSkipValidate) {
         sp<Fence> outPresentFence;
@@ -559,7 +564,8 @@
 }
 
 status_t HWComposer::presentAndGetReleaseFences(
-        HalDisplayId displayId, std::chrono::steady_clock::time_point earliestPresentTime) {
+        HalDisplayId displayId, std::chrono::steady_clock::time_point earliestPresentTime,
+        const std::shared_ptr<FenceTime>& previousPresentFence) {
     ATRACE_CALL();
 
     RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
@@ -575,7 +581,9 @@
         return NO_ERROR;
     }
 
-    {
+    const bool previousFramePending =
+            previousPresentFence->getSignalTime() == Fence::SIGNAL_TIME_PENDING;
+    if (!previousFramePending) {
         ATRACE_NAME("wait for earliest present time");
         std::this_thread::sleep_until(earliestPresentTime);
     }
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index cd6f9f5..b1849e8 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -27,7 +27,7 @@
 #include <vector>
 
 #include <android-base/thread_annotations.h>
-#include <ui/Fence.h>
+#include <ui/FenceTime.h>
 
 // TODO(b/129481165): remove the #pragma below and fix conversion issues
 #pragma clang diagnostic push
@@ -134,6 +134,7 @@
     virtual status_t getDeviceCompositionChanges(
             HalDisplayId, bool frameUsesClientComposition,
             std::chrono::steady_clock::time_point earliestPresentTime,
+            const std::shared_ptr<FenceTime>& previousPresentFence,
             std::optional<DeviceRequestedChanges>* outChanges) = 0;
 
     virtual status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
@@ -141,7 +142,8 @@
 
     // Present layers to the display and read releaseFences.
     virtual status_t presentAndGetReleaseFences(
-            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime) = 0;
+            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
+            const std::shared_ptr<FenceTime>& previousPresentFence) = 0;
 
     // set power mode
     virtual status_t setPowerMode(PhysicalDisplayId, hal::PowerMode) = 0;
@@ -275,6 +277,7 @@
     status_t getDeviceCompositionChanges(
             HalDisplayId, bool frameUsesClientComposition,
             std::chrono::steady_clock::time_point earliestPresentTime,
+            const std::shared_ptr<FenceTime>& previousPresentFence,
             std::optional<DeviceRequestedChanges>* outChanges) override;
 
     status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
@@ -282,7 +285,8 @@
 
     // Present layers to the display and read releaseFences.
     status_t presentAndGetReleaseFences(
-            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime) override;
+            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
+            const std::shared_ptr<FenceTime>& previousPresentFence) override;
 
     // set power mode
     status_t setPowerMode(PhysicalDisplayId, hal::PowerMode mode) override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e2f3ebb..813eebe 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2070,6 +2070,7 @@
     const auto prevVsyncTime = mScheduler->getPreviousVsyncFrom(mExpectedPresentTime);
     const auto hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration;
     refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration;
+    refreshArgs.previousPresentFence = mPreviousPresentFences[0].fenceTime;
     refreshArgs.nextInvalidateTime = mEventQueue->nextExpectedInvalidate();
 
     mGeometryInvalid = false;