Merge "SF: avoid skipping waiting for the earliest time to present" into udc-dev

10 files changed, 53 insertions, 62 deletions

diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index a8322d8572..d93e25e4ca 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -83,12 +83,9 @@ struct CompositionRefreshArgs {
     // If set, causes the dirty regions to flash with the delay
     std::optional<std::chrono::microseconds> devOptFlashDirtyRegionsDelay;
 
-    // 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;
+    // Optional.
+    // The earliest time to send the present command to the HAL.
+    std::optional<std::chrono::steady_clock::time_point> earliestPresentTime;
 
     // The expected time for the next present
     nsecs_t expectedPresentTime{0};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index c29165210d..a3fda61ecb 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -122,12 +122,9 @@ struct OutputCompositionState {
 
     bool previousDeviceRequestedSuccess = false;
 
+    // Optional.
     // 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;
+    std::optional<std::chrono::steady_clock::time_point> earliestPresentTime;
 
     // The expected time for the next present
     nsecs_t expectedPresentTime{0};
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index d50a768474..85fc09549b 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -263,7 +263,6 @@ bool Display::chooseCompositionStrategy(
     if (status_t result =
                 hwc.getDeviceCompositionChanges(*halDisplayId, requiresClientComposition,
                                                 getState().earliestPresentTime,
-                                                getState().previousPresentFence,
                                                 getState().expectedPresentTime, outChanges);
         result != NO_ERROR) {
         ALOGE("chooseCompositionStrategy failed for %s: %d (%s)", getName().c_str(), result,
@@ -380,16 +379,11 @@ compositionengine::Output::FrameFences Display::presentAndGetFrameFences() {
 
     const TimePoint startTime = TimePoint::now();
 
-    if (isPowerHintSessionEnabled()) {
-        if (!getCompositionEngine().getHwComposer().getComposer()->isSupported(
-                    Hwc2::Composer::OptionalFeature::ExpectedPresentTime) &&
-            getState().previousPresentFence->getSignalTime() != Fence::SIGNAL_TIME_PENDING) {
-            mPowerAdvisor->setHwcPresentDelayedTime(mId, getState().earliestPresentTime);
-        }
+    if (isPowerHintSessionEnabled() && getState().earliestPresentTime) {
+        mPowerAdvisor->setHwcPresentDelayedTime(mId, *getState().earliestPresentTime);
     }
 
-    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime,
-                                   getState().previousPresentFence);
+    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime);
 
     if (isPowerHintSessionEnabled()) {
         mPowerAdvisor->setHwcPresentTiming(mId, startTime, TimePoint::now());
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 175dd1d825..e720af5a33 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -842,7 +842,6 @@ void Output::writeCompositionState(const compositionengine::CompositionRefreshAr
     }
 
     editState().earliestPresentTime = refreshArgs.earliestPresentTime;
-    editState().previousPresentFence = refreshArgs.previousPresentFence;
     editState().expectedPresentTime = refreshArgs.expectedPresentTime;
 
     compositionengine::OutputLayer* peekThroughLayer = nullptr;
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index 0756c1becd..9be6bc2075 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -595,7 +595,7 @@ TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutIfGpuDisplay) {
 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));
 
     chooseCompositionStrategy(mDisplay.get());
@@ -619,8 +619,8 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperation) {
             .WillOnce(Return(false));
 
     EXPECT_CALL(mHwComposer,
-                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _, _))
-            .WillOnce(testing::DoAll(testing::SetArgPointee<5>(mDeviceRequestedChanges),
+                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
+            .WillOnce(testing::DoAll(testing::SetArgPointee<4>(mDeviceRequestedChanges),
                                      Return(NO_ERROR)));
     EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(mDeviceRequestedChanges.changedTypes))
             .Times(1);
@@ -672,8 +672,8 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperationWithChanges) {
             .WillOnce(Return(false));
 
     EXPECT_CALL(mHwComposer,
-                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _, _))
-            .WillOnce(DoAll(SetArgPointee<5>(mDeviceRequestedChanges), Return(NO_ERROR)));
+                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))
+            .WillOnce(DoAll(SetArgPointee<4>(mDeviceRequestedChanges), Return(NO_ERROR)));
     EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(mDeviceRequestedChanges.changedTypes))
             .Times(1);
     EXPECT_CALL(*mDisplay, applyDisplayRequests(mDeviceRequestedChanges.displayRequests)).Times(1);
@@ -901,7 +901,7 @@ TEST_F(DisplayPresentAndGetFrameFencesTest, returnsPresentAndLayerFences) {
     sp<Fence> layer1Fence = sp<Fence>::make();
     sp<Fence> layer2Fence = sp<Fence>::make();
 
-    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));
@@ -1078,7 +1078,7 @@ TEST_F(DisplayFunctionalTest, postFramebufferCriticalCallsAreOrdered) {
 
     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 1a56ab751f..67b94ee749 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -54,16 +54,14 @@ public:
     MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
 
     MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
-    MOCK_METHOD6(getDeviceCompositionChanges,
-                 status_t(HalDisplayId, bool, std::chrono::steady_clock::time_point,
-                          const std::shared_ptr<FenceTime>&, nsecs_t,
-                          std::optional<android::HWComposer::DeviceRequestedChanges>*));
+    MOCK_METHOD5(getDeviceCompositionChanges,
+                 status_t(HalDisplayId, bool, std::optional<std::chrono::steady_clock::time_point>,
+                          nsecs_t, std::optional<android::HWComposer::DeviceRequestedChanges>*));
     MOCK_METHOD5(setClientTarget,
                  status_t(HalDisplayId, uint32_t, const sp<Fence>&, const sp<GraphicBuffer>&,
                           ui::Dataspace));
-    MOCK_METHOD3(presentAndGetReleaseFences,
-                 status_t(HalDisplayId, std::chrono::steady_clock::time_point,
-                          const std::shared_ptr<FenceTime>&));
+    MOCK_METHOD2(presentAndGetReleaseFences,
+                 status_t(HalDisplayId, std::optional<std::chrono::steady_clock::time_point>));
     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 28148ac1dc..f350eba7ca 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -395,8 +395,8 @@ status_t HWComposer::setClientTarget(HalDisplayId displayId, uint32_t slot,
 
 status_t HWComposer::getDeviceCompositionChanges(
         HalDisplayId displayId, bool frameUsesClientComposition,
-        std::chrono::steady_clock::time_point earliestPresentTime,
-        const std::shared_ptr<FenceTime>& previousPresentFence, nsecs_t expectedPresentTime,
+        std::optional<std::chrono::steady_clock::time_point> earliestPresentTime,
+        nsecs_t expectedPresentTime,
         std::optional<android::HWComposer::DeviceRequestedChanges>* outChanges) {
     ATRACE_CALL();
 
@@ -426,14 +426,13 @@ status_t HWComposer::getDeviceCompositionChanges(
 
         // If composer supports getting the expected present time, we can skip
         // as composer will make sure to prevent early presentation
-        if (mComposer->isSupported(Hwc2::Composer::OptionalFeature::ExpectedPresentTime)) {
+        if (!earliestPresentTime) {
             return true;
         }
 
         // composer doesn't support getting the expected present time. We can only
         // skip validate if we know that we are not going to present early.
-        return std::chrono::steady_clock::now() >= earliestPresentTime ||
-                previousPresentFence->getSignalTime() == Fence::SIGNAL_TIME_PENDING;
+        return std::chrono::steady_clock::now() >= *earliestPresentTime;
     }();
 
     displayData.validateWasSkipped = false;
@@ -508,8 +507,8 @@ sp<Fence> HWComposer::getLayerReleaseFence(HalDisplayId displayId, HWC2::Layer*
 }
 
 status_t HWComposer::presentAndGetReleaseFences(
-        HalDisplayId displayId, std::chrono::steady_clock::time_point earliestPresentTime,
-        const std::shared_ptr<FenceTime>& previousPresentFence) {
+        HalDisplayId displayId,
+        std::optional<std::chrono::steady_clock::time_point> earliestPresentTime) {
     ATRACE_CALL();
 
     RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
@@ -525,13 +524,9 @@ status_t HWComposer::presentAndGetReleaseFences(
         return NO_ERROR;
     }
 
-    const bool waitForEarliestPresent =
-            !mComposer->isSupported(Hwc2::Composer::OptionalFeature::ExpectedPresentTime) &&
-            previousPresentFence->getSignalTime() != Fence::SIGNAL_TIME_PENDING;
-
-    if (waitForEarliestPresent) {
+    if (earliestPresentTime) {
         ATRACE_NAME("wait for earliest present time");
-        std::this_thread::sleep_until(earliestPresentTime);
+        std::this_thread::sleep_until(*earliestPresentTime);
     }
 
     auto error = hwcDisplay->present(&displayData.lastPresentFence);
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 7a3f41cc1c..3702c62b65 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -143,17 +143,16 @@ public:
     // expected.
     virtual status_t getDeviceCompositionChanges(
             HalDisplayId, bool frameUsesClientComposition,
-            std::chrono::steady_clock::time_point earliestPresentTime,
-            const std::shared_ptr<FenceTime>& previousPresentFence, nsecs_t expectedPresentTime,
-            std::optional<DeviceRequestedChanges>* outChanges) = 0;
+            std::optional<std::chrono::steady_clock::time_point> earliestPresentTime,
+            nsecs_t expectedPresentTime, std::optional<DeviceRequestedChanges>* outChanges) = 0;
 
     virtual status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
                                      const sp<GraphicBuffer>& target, ui::Dataspace) = 0;
 
     // Present layers to the display and read releaseFences.
     virtual status_t presentAndGetReleaseFences(
-            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
-            const std::shared_ptr<FenceTime>& previousPresentFence) = 0;
+            HalDisplayId,
+            std::optional<std::chrono::steady_clock::time_point> earliestPresentTime) = 0;
 
     // set power mode
     virtual status_t setPowerMode(PhysicalDisplayId, hal::PowerMode) = 0;
@@ -339,8 +338,8 @@ public:
 
     status_t getDeviceCompositionChanges(
             HalDisplayId, bool frameUsesClientComposition,
-            std::chrono::steady_clock::time_point earliestPresentTime,
-            const std::shared_ptr<FenceTime>& previousPresentFence, nsecs_t expectedPresentTime,
+            std::optional<std::chrono::steady_clock::time_point> earliestPresentTime,
+            nsecs_t expectedPresentTime,
             std::optional<DeviceRequestedChanges>* outChanges) override;
 
     status_t setClientTarget(HalDisplayId, uint32_t slot, const sp<Fence>& acquireFence,
@@ -348,8 +347,8 @@ public:
 
     // Present layers to the display and read releaseFences.
     status_t presentAndGetReleaseFences(
-            HalDisplayId, std::chrono::steady_clock::time_point earliestPresentTime,
-            const std::shared_ptr<FenceTime>& previousPresentFence) override;
+            HalDisplayId,
+            std::optional<std::chrono::steady_clock::time_point> earliestPresentTime) 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 a31f681b38..6a2e3475b9 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2621,8 +2621,21 @@ void SurfaceFlinger::composite(TimePoint frameTime, VsyncId vsyncId)
     const auto prevVsyncTime = mExpectedPresentTime - mScheduler->getVsyncSchedule()->period();
     const auto hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration;
 
-    refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration;
-    refreshArgs.previousPresentFence = mPreviousPresentFences[0].fenceTime;
+    const Period vsyncPeriod = mScheduler->getVsyncSchedule()->period();
+    const bool threeVsyncsAhead = mExpectedPresentTime - frameTime > 2 * vsyncPeriod;
+
+    // We should wait for the earliest present time if HWC doesn't support ExpectedPresentTime,
+    // and the next vsync is not already taken by the previous frame.
+    const bool waitForEarliestPresent =
+            !getHwComposer().getComposer()->isSupported(
+                    Hwc2::Composer::OptionalFeature::ExpectedPresentTime) &&
+            (threeVsyncsAhead ||
+             mPreviousPresentFences[0].fenceTime->getSignalTime() != Fence::SIGNAL_TIME_PENDING);
+
+    if (waitForEarliestPresent) {
+        refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration;
+    }
+
     refreshArgs.scheduledFrameTime = mScheduler->getScheduledFrameTime();
     refreshArgs.expectedPresentTime = mExpectedPresentTime.ns();
     refreshArgs.hasTrustedPresentationListener = mNumTrustedPresentationListeners > 0;
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
index 8a6af10f58..a9247fe903 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
@@ -222,7 +222,7 @@ void DisplayHardwareFuzzer::getDeviceCompositionChanges(HalDisplayId halDisplayI
     std::optional<impl::HWComposer::DeviceRequestedChanges> outChanges;
     mHwc.getDeviceCompositionChanges(halDisplayID,
                                      mFdp.ConsumeBool() /*frameUsesClientComposition*/,
-                                     std::chrono::steady_clock::now(), FenceTime::NO_FENCE,
+                                     std::chrono::steady_clock::now(),
                                      mFdp.ConsumeIntegral<nsecs_t>(), &outChanges);
 }
 
@@ -555,8 +555,7 @@ void DisplayHardwareFuzzer::invokeComposer() {
     mHwc.setClientTarget(halDisplayID, mFdp.ConsumeIntegral<uint32_t>(), Fence::NO_FENCE,
                          sp<GraphicBuffer>::make(), mFdp.PickValueInArray(kDataspaces));
 
-    mHwc.presentAndGetReleaseFences(halDisplayID, std::chrono::steady_clock::now(),
-                                    FenceTime::NO_FENCE);
+    mHwc.presentAndGetReleaseFences(halDisplayID, std::chrono::steady_clock::now());
 
     mHwc.setPowerMode(mPhysicalDisplayId, mFdp.PickValueInArray(kPowerModes));