[SF] Move the notifyExpectedPresentHint call to SF
This moves the notifyExpectedPresent call off the HWComposer,
HWComposer should only be access with mStateLock or from the main
thread, and moving this to SF achieves that.
Schedule the HWComposer::notifyExpectedPresent
call on the main thread once the decision to
send the hint is made
BUG: 311300327
Test: atest
Change-Id: Ia5f92546028ce104e391364c6696415c29760232
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index dea0194..c75f902 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -106,6 +106,7 @@
"SurfaceFlinger_HdrOutputControlTest.cpp",
"SurfaceFlinger_HotplugTest.cpp",
"SurfaceFlinger_InitializeDisplaysTest.cpp",
+ "SurfaceFlinger_NotifyExpectedPresentTest.cpp",
"SurfaceFlinger_NotifyPowerBoostTest.cpp",
"SurfaceFlinger_PowerHintTest.cpp",
"SurfaceFlinger_SetDisplayStateTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index d3ce4f2..c9edb84 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -82,15 +82,6 @@
EXPECT_CALL(*mHal, setVsyncEnabled(hwcDisplayId, Hwc2::IComposerClient::Vsync::DISABLE));
EXPECT_CALL(*mHal, onHotplugConnect(hwcDisplayId));
}
-
- void setDisplayData(HalDisplayId displayId, TimePoint lastExpectedPresentTimestamp,
- Fps lastFrameInterval) {
- ASSERT_TRUE(mHwc.mDisplayData.find(displayId) != mHwc.mDisplayData.end());
- auto& displayData = mHwc.mDisplayData.at(displayId);
- std::scoped_lock lock{displayData.expectedPresentLock};
- displayData.lastExpectedPresentTimestamp = lastExpectedPresentTimestamp;
- displayData.lastFrameInterval = lastFrameInterval;
- }
};
TEST_F(HWComposerTest, isHeadless) {
@@ -417,144 +408,6 @@
EXPECT_FALSE(displayIdOpt);
}
-TEST_F(HWComposerTest, notifyExpectedPresentTimeout) {
- constexpr hal::HWDisplayId kHwcDisplayId = 2;
- expectHotplugConnect(kHwcDisplayId);
- const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
- ASSERT_TRUE(info);
-
- auto expectedPresentTime = systemTime() + ms2ns(10);
- static constexpr Fps Fps60Hz = 60_Hz;
- static constexpr int32_t kFrameInterval5HzNs = static_cast<Fps>(5_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameInterval60HzNs = Fps60Hz.getPeriodNsecs();
- static constexpr int32_t kFrameInterval120HzNs = static_cast<Fps>(120_Hz).getPeriodNsecs();
- static constexpr Period kVsyncPeriod =
- Period::fromNs(static_cast<Fps>(240_Hz).getPeriodNsecs());
- static constexpr Period kTimeoutNs = Period::fromNs(kFrameInterval5HzNs);
- static constexpr auto kLastExpectedPresentTimestamp = TimePoint::fromNs(0);
-
- ASSERT_NO_FATAL_FAILURE(setDisplayData(info->id, kLastExpectedPresentTimestamp, Fps60Hz));
-
- {
- // Very first ExpectedPresent after idle, no previous timestamp
- EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
- .WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- kTimeoutNs);
- }
- {
- // Absent timeoutNs
- expectedPresentTime += 2 * kFrameInterval5HzNs;
- EXPECT_CALL(*mHal, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0);
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- /*timeoutOpt*/ std::nullopt);
- }
- {
- // Timeout is 0
- expectedPresentTime += kFrameInterval60HzNs;
- EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
- .WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- Period::fromNs(0));
- }
- {
- // ExpectedPresent is after the timeoutNs
- expectedPresentTime += 2 * kFrameInterval5HzNs;
- EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
- .WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- kTimeoutNs);
- }
- {
- // ExpectedPresent has not changed
- EXPECT_CALL(*mHal, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0);
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- kTimeoutNs);
- }
- {
- // ExpectedPresent is after the last reported ExpectedPresent.
- expectedPresentTime += kFrameInterval60HzNs;
- EXPECT_CALL(*mHal, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0);
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- kTimeoutNs);
- }
- {
- // ExpectedPresent is before the last reported ExpectedPresent but after the timeoutNs,
- // representing we changed our decision and want to present earlier than previously
- // reported.
- expectedPresentTime -= kFrameInterval120HzNs;
- EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
- .WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime), Fps60Hz,
- kTimeoutNs);
- }
-}
-
-TEST_F(HWComposerTest, notifyExpectedPresentRenderRateChanged) {
- constexpr hal::HWDisplayId kHwcDisplayId = 2;
- expectHotplugConnect(kHwcDisplayId);
- const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
- ASSERT_TRUE(info);
-
- const auto now = systemTime();
- auto expectedPresentTime = now;
- static constexpr Period kTimeoutNs = Period::fromNs(static_cast<Fps>(1_Hz).getPeriodNsecs());
-
- ASSERT_NO_FATAL_FAILURE(setDisplayData(info->id, TimePoint::fromNs(now), Fps::fromValue(0)));
- static constexpr int32_t kFrameIntervalNs120Hz = static_cast<Fps>(120_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs96Hz = static_cast<Fps>(96_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs80Hz = static_cast<Fps>(80_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs60Hz = static_cast<Fps>(60_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs40Hz = static_cast<Fps>(40_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs30Hz = static_cast<Fps>(30_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs24Hz = static_cast<Fps>(24_Hz).getPeriodNsecs();
- static constexpr int32_t kFrameIntervalNs20Hz = static_cast<Fps>(20_Hz).getPeriodNsecs();
- static constexpr Period kVsyncPeriod =
- Period::fromNs(static_cast<Fps>(240_Hz).getPeriodNsecs());
-
- struct FrameRateIntervalTestData {
- int32_t frameIntervalNs;
- bool callExpectedPresent;
- };
- const std::vector<FrameRateIntervalTestData> frameIntervals = {
- {kFrameIntervalNs60Hz, true}, {kFrameIntervalNs96Hz, true},
- {kFrameIntervalNs80Hz, true}, {kFrameIntervalNs120Hz, true},
- {kFrameIntervalNs80Hz, true}, {kFrameIntervalNs60Hz, true},
- {kFrameIntervalNs60Hz, false}, {kFrameIntervalNs30Hz, false},
- {kFrameIntervalNs24Hz, true}, {kFrameIntervalNs40Hz, true},
- {kFrameIntervalNs20Hz, false}, {kFrameIntervalNs60Hz, true},
- {kFrameIntervalNs20Hz, false}, {kFrameIntervalNs120Hz, true},
- };
-
- for (const auto& [frameIntervalNs, callExpectedPresent] : frameIntervals) {
- {
- expectedPresentTime += frameIntervalNs;
- if (callExpectedPresent) {
- EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime,
- frameIntervalNs))
- .WillOnce(Return(HalError::NONE));
- } else {
- EXPECT_CALL(*mHal, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0);
- }
- mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
- TimePoint::fromNs(expectedPresentTime),
- Fps::fromPeriodNsecs(frameIntervalNs), kTimeoutNs);
- }
- }
-}
-
struct MockHWC2ComposerCallback final : StrictMock<HWC2::ComposerCallback> {
MOCK_METHOD(void, onComposerHalHotplugEvent, (hal::HWDisplayId, DisplayHotplugEvent),
(override));
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp
new file mode 100644
index 0000000..7206e29
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp
@@ -0,0 +1,180 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "LibSurfaceFlingerUnittests"
+
+#include "DisplayTransactionTestHelpers.h"
+
+namespace android {
+
+using FakeHwcDisplayInjector = TestableSurfaceFlinger::FakeHwcDisplayInjector;
+
+class NotifyExpectedPresentTest : public DisplayTransactionTest {
+public:
+ void SetUp() override {
+ mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this).inject();
+ FakeHwcDisplayInjector(mDisplay->getPhysicalId(), hal::DisplayType::PHYSICAL, kIsPrimary)
+ .setPowerMode(hal::PowerMode::ON)
+ .inject(&mFlinger, mComposer);
+ }
+
+protected:
+ sp<DisplayDevice> mDisplay;
+ static constexpr bool kIsPrimary = true;
+ static constexpr hal::HWDisplayId HWC_DISPLAY_ID =
+ FakeHwcDisplayInjector::DEFAULT_HWC_DISPLAY_ID;
+};
+
+TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentTimeout) {
+ const auto physicDisplayId = mDisplay->getPhysicalId();
+ auto expectedPresentTime = systemTime() + ms2ns(10);
+ static constexpr Fps kFps60Hz = 60_Hz;
+ static constexpr int32_t kFrameInterval5HzNs = static_cast<Fps>(5_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameInterval60HzNs = kFps60Hz.getPeriodNsecs();
+ static constexpr int32_t kFrameInterval120HzNs = static_cast<Fps>(120_Hz).getPeriodNsecs();
+ static constexpr Period kVsyncPeriod =
+ Period::fromNs(static_cast<Fps>(240_Hz).getPeriodNsecs());
+ static constexpr Period kTimeoutNs = Period::fromNs(kFrameInterval5HzNs);
+ static constexpr auto kLastExpectedPresentTimestamp = TimePoint::fromNs(0);
+
+ ASSERT_NO_FATAL_FAILURE(mFlinger.setNotifyExpectedPresentData(physicDisplayId,
+ kLastExpectedPresentTimestamp,
+ kFps60Hz));
+
+ {
+ // Very first ExpectedPresent after idle, no previous timestamp
+ EXPECT_CALL(*mComposer,
+ notifyExpectedPresent(HWC_DISPLAY_ID, expectedPresentTime,
+ kFrameInterval60HzNs))
+ .WillOnce(Return(Error::NONE));
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ kTimeoutNs);
+ }
+ {
+ // Absent timeoutNs
+ expectedPresentTime += 2 * kFrameInterval5HzNs;
+ EXPECT_CALL(*mComposer, notifyExpectedPresent(HWC_DISPLAY_ID, _, _)).Times(0);
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ /*timeoutOpt*/ std::nullopt);
+ }
+ {
+ // Timeout is 0
+ expectedPresentTime += kFrameInterval60HzNs;
+ EXPECT_CALL(*mComposer,
+ notifyExpectedPresent(HWC_DISPLAY_ID, expectedPresentTime,
+ kFrameInterval60HzNs))
+ .WillOnce(Return(Error::NONE));
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ Period::fromNs(0));
+ }
+ {
+ // ExpectedPresent is after the timeoutNs
+ expectedPresentTime += 2 * kFrameInterval5HzNs;
+ EXPECT_CALL(*mComposer,
+ notifyExpectedPresent(HWC_DISPLAY_ID, expectedPresentTime,
+ kFrameInterval60HzNs))
+ .WillOnce(Return(Error::NONE));
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ kTimeoutNs);
+ }
+ {
+ // ExpectedPresent has not changed
+ EXPECT_CALL(*mComposer, notifyExpectedPresent(HWC_DISPLAY_ID, _, _)).Times(0);
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ kTimeoutNs);
+ }
+ {
+ // ExpectedPresent is after the last reported ExpectedPresent.
+ expectedPresentTime += kFrameInterval60HzNs;
+ EXPECT_CALL(*mComposer, notifyExpectedPresent(HWC_DISPLAY_ID, _, _)).Times(0);
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ kTimeoutNs);
+ }
+ {
+ // ExpectedPresent is before the last reported ExpectedPresent but after the timeoutNs,
+ // representing we changed our decision and want to present earlier than previously
+ // reported.
+ expectedPresentTime -= kFrameInterval120HzNs;
+ EXPECT_CALL(*mComposer,
+ notifyExpectedPresent(HWC_DISPLAY_ID, expectedPresentTime,
+ kFrameInterval60HzNs))
+ .WillOnce(Return(Error::NONE));
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), kFps60Hz,
+ kTimeoutNs);
+ }
+}
+
+TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentRenderRateChanged) {
+ const auto physicDisplayId = mDisplay->getPhysicalId();
+ const auto now = systemTime();
+ auto expectedPresentTime = now;
+ static constexpr Period kTimeoutNs = Period::fromNs(static_cast<Fps>(1_Hz).getPeriodNsecs());
+
+ ASSERT_NO_FATAL_FAILURE(mFlinger.setNotifyExpectedPresentData(physicDisplayId,
+ TimePoint::fromNs(now),
+ Fps::fromValue(0)));
+ static constexpr int32_t kFrameIntervalNs120Hz = static_cast<Fps>(120_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs96Hz = static_cast<Fps>(96_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs80Hz = static_cast<Fps>(80_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs60Hz = static_cast<Fps>(60_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs40Hz = static_cast<Fps>(40_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs30Hz = static_cast<Fps>(30_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs24Hz = static_cast<Fps>(24_Hz).getPeriodNsecs();
+ static constexpr int32_t kFrameIntervalNs20Hz = static_cast<Fps>(20_Hz).getPeriodNsecs();
+ static constexpr Period kVsyncPeriod =
+ Period::fromNs(static_cast<Fps>(240_Hz).getPeriodNsecs());
+
+ struct FrameRateIntervalTestData {
+ int32_t frameIntervalNs;
+ bool callExpectedPresent;
+ };
+ const std::vector<FrameRateIntervalTestData> frameIntervals = {
+ {kFrameIntervalNs60Hz, true}, {kFrameIntervalNs96Hz, true},
+ {kFrameIntervalNs80Hz, true}, {kFrameIntervalNs120Hz, true},
+ {kFrameIntervalNs80Hz, true}, {kFrameIntervalNs60Hz, true},
+ {kFrameIntervalNs60Hz, false}, {kFrameIntervalNs30Hz, false},
+ {kFrameIntervalNs24Hz, true}, {kFrameIntervalNs40Hz, true},
+ {kFrameIntervalNs20Hz, false}, {kFrameIntervalNs60Hz, true},
+ {kFrameIntervalNs20Hz, false}, {kFrameIntervalNs120Hz, true},
+ };
+
+ for (const auto& [frameIntervalNs, callExpectedPresent] : frameIntervals) {
+ {
+ expectedPresentTime += frameIntervalNs;
+ if (callExpectedPresent) {
+ EXPECT_CALL(*mComposer,
+ notifyExpectedPresent(HWC_DISPLAY_ID, expectedPresentTime,
+ frameIntervalNs))
+ .WillOnce(Return(Error::NONE));
+ } else {
+ EXPECT_CALL(*mComposer, notifyExpectedPresent(HWC_DISPLAY_ID, _, _)).Times(0);
+ }
+ mFlinger.notifyExpectedPresentIfRequired(physicDisplayId, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime),
+ Fps::fromPeriodNsecs(frameIntervalNs),
+ kTimeoutNs);
+ }
+ }
+}
+} // namespace android
\ No newline at end of file
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 0909178..fd578a2 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -694,6 +694,21 @@
mFlinger->mLegacyFrontEndEnabled = false;
}
+ void notifyExpectedPresentIfRequired(PhysicalDisplayId displayId, Period vsyncPeriod,
+ TimePoint expectedPresentTime, Fps frameInterval,
+ std::optional<Period> timeoutOpt) {
+ mFlinger->notifyExpectedPresentIfRequired(displayId, vsyncPeriod, expectedPresentTime,
+ frameInterval, timeoutOpt);
+ }
+
+ void setNotifyExpectedPresentData(PhysicalDisplayId displayId,
+ TimePoint lastExpectedPresentTimestamp,
+ Fps lastFrameInterval) {
+ auto& displayData = mFlinger->mNotifyExpectedPresentMap[displayId];
+ displayData.lastExpectedPresentTimestamp = lastExpectedPresentTimestamp;
+ displayData.lastFrameInterval = lastFrameInterval;
+ }
+
~TestableSurfaceFlinger() {
// All these pointer and container clears help ensure that GMock does
// not report a leaked object, since the SurfaceFlinger instance may