Merge "SF: avoid skipping waiting for the earliest time to present" into udc-dev
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index a8322d8..d93e25e 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -83,12 +83,9 @@
// 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 c291652..a3fda61 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -122,12 +122,9 @@
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 d50a768..85fc095 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -263,7 +263,6 @@
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 @@
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 175dd1d..e720af5 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -842,7 +842,6 @@
}
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 0756c1b..9be6bc2 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -595,7 +595,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));
chooseCompositionStrategy(mDisplay.get());
@@ -619,8 +619,8 @@
.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 @@
.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 @@
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 @@
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 1a56ab7..67b94ee 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -54,16 +54,14 @@
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 28148ac..f350eba 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -395,8 +395,8 @@
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 @@
// 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 @@
}
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 @@
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 7a3f41c..3702c62 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -143,17 +143,16 @@
// 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 @@
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 @@
// 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 a31f681..6a2e347 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2621,8 +2621,21 @@
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 8a6af10..a9247fe 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_displayhardware_fuzzer.cpp
@@ -222,7 +222,7 @@
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 @@
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));