[SF] Adds notifyExpectedPresent call for frame rate change
BUG: 296636253
BUG: 284845445
Test: atest HWComposerTest
Change-Id: Id9445a2765fa546eca64d9084eddc06cdbd090eb
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 6807c8e..a44e4be 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -148,7 +148,7 @@
getOverlaySupport, (), (const, override));
MOCK_METHOD(status_t, setRefreshRateChangedCallbackDebugEnabled, (PhysicalDisplayId, bool));
MOCK_METHOD(status_t, notifyExpectedPresentIfRequired,
- (PhysicalDisplayId, nsecs_t, int32_t, int32_t));
+ (PhysicalDisplayId, Period, TimePoint, Fps, std::optional<Period>));
};
} // namespace mock
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index fb6089d..1d9f9ce 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -78,6 +78,59 @@
using aidl::android::hardware::graphics::composer3::DisplayCapability;
namespace hal = android::hardware::graphics::composer::hal;
+namespace {
+bool isFrameIntervalOnCadence(android::TimePoint expectedPresentTime,
+ android::TimePoint lastExpectedPresentTimestamp,
+ android::Fps lastFrameInterval, android::Period timeout,
+ android::Duration threshold) {
+ if (lastFrameInterval.getPeriodNsecs() == 0) {
+ return false;
+ }
+
+ const auto expectedPresentTimeDeltaNs =
+ expectedPresentTime.ns() - lastExpectedPresentTimestamp.ns();
+
+ if (expectedPresentTimeDeltaNs > timeout.ns()) {
+ return false;
+ }
+
+ const auto expectedPresentPeriods = static_cast<nsecs_t>(
+ std::round(static_cast<float>(expectedPresentTimeDeltaNs) /
+ static_cast<float>(lastFrameInterval.getPeriodNsecs())));
+ const auto calculatedPeriodsOutNs = lastFrameInterval.getPeriodNsecs() * expectedPresentPeriods;
+ const auto calculatedExpectedPresentTimeNs =
+ lastExpectedPresentTimestamp.ns() + calculatedPeriodsOutNs;
+ const auto presentTimeDelta =
+ std::abs(expectedPresentTime.ns() - calculatedExpectedPresentTimeNs);
+ return presentTimeDelta < threshold.ns();
+}
+
+bool isExpectedPresentWithinTimeout(android::TimePoint expectedPresentTime,
+ android::TimePoint lastExpectedPresentTimestamp,
+ std::optional<android::Period> timeoutOpt,
+ android::Duration threshold) {
+ if (!timeoutOpt) {
+ // Always within timeout if timeoutOpt is absent and don't send hint
+ // for the timeout
+ return true;
+ }
+
+ if (timeoutOpt->ns() == 0) {
+ // Always outside timeout if timeoutOpt is 0 and always send
+ // the hint for the timeout.
+ return false;
+ }
+
+ if (expectedPresentTime.ns() < lastExpectedPresentTimestamp.ns() + timeoutOpt->ns()) {
+ return true;
+ }
+
+ // Check if within the threshold as it can be just outside the timeout
+ return std::abs(expectedPresentTime.ns() -
+ (lastExpectedPresentTimestamp.ns() + timeoutOpt->ns())) < threshold.ns();
+}
+} // namespace
+
namespace android {
HWComposer::~HWComposer() = default;
@@ -485,7 +538,12 @@
}();
displayData.validateWasSkipped = false;
- displayData.lastExpectedPresentTimestamp = expectedPresentTime;
+ {
+ std::scoped_lock lock{displayData.expectedPresentLock};
+ displayData.lastExpectedPresentTimestamp = TimePoint::fromNs(expectedPresentTime);
+ // TODO(b/296636176) Update displayData.lastFrameInterval for present display commands
+ }
+
if (canSkipValidate) {
sp<Fence> outPresentFence;
uint32_t state = UINT32_MAX;
@@ -879,21 +937,44 @@
}
status_t HWComposer::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId,
- nsecs_t expectedPresentTime,
- int32_t frameIntervalNs, int32_t timeoutNs) {
+ Period vsyncPeriod,
+ TimePoint expectedPresentTime,
+ Fps frameInterval,
+ std::optional<Period> timeoutOpt) {
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
-
auto& displayData = mDisplayData[displayId];
- if (expectedPresentTime >= displayData.lastExpectedPresentTimestamp &&
- expectedPresentTime < displayData.lastExpectedPresentTimestamp + timeoutNs) {
- return NO_ERROR;
- }
+ {
+ std::scoped_lock lock{displayData.expectedPresentLock};
+ const auto lastExpectedPresentTimestamp = displayData.lastExpectedPresentTimestamp;
+ const auto lastFrameInterval = displayData.lastFrameInterval;
+ displayData.lastFrameInterval = frameInterval;
+ const auto threshold = Duration::fromNs(vsyncPeriod.ns() / 2);
- displayData.lastExpectedPresentTimestamp = expectedPresentTime;
+ const constexpr nsecs_t kOneSecondNs =
+ std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();
+ const bool frameIntervalIsOnCadence =
+ isFrameIntervalOnCadence(expectedPresentTime, lastExpectedPresentTimestamp,
+ lastFrameInterval,
+ Period::fromNs(timeoutOpt && timeoutOpt->ns() > 0
+ ? timeoutOpt->ns()
+ : kOneSecondNs),
+ threshold);
+
+ const bool expectedPresentWithinTimeout =
+ isExpectedPresentWithinTimeout(expectedPresentTime, lastExpectedPresentTimestamp,
+ timeoutOpt, threshold);
+
+ if (expectedPresentWithinTimeout && frameIntervalIsOnCadence) {
+ return NO_ERROR;
+ }
+
+ displayData.lastExpectedPresentTimestamp = expectedPresentTime;
+ }
ATRACE_FORMAT("%s ExpectedPresentTime %" PRId64 " frameIntervalNs %d", __func__,
- expectedPresentTime, frameIntervalNs);
+ expectedPresentTime, frameInterval.getPeriodNsecs());
const auto error = mComposer->notifyExpectedPresent(displayData.hwcDisplay->getId(),
- expectedPresentTime, frameIntervalNs);
+ expectedPresentTime.ns(),
+ frameInterval.getPeriodNsecs());
if (error != hal::Error::NONE) {
ALOGE("Error in notifyExpectedPresent call %s", to_string(error).c_str());
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 726a8ea..51e9319 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -301,9 +301,10 @@
aidl::android::hardware::graphics::common::HdrConversionStrategy,
aidl::android::hardware::graphics::common::Hdr*) = 0;
virtual status_t setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId, bool enabled) = 0;
- virtual status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, nsecs_t expectedPresentTime,
- int32_t frameIntervalNs,
- int32_t timeoutNs) = 0;
+ virtual status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod,
+ TimePoint expectedPresentTime,
+ Fps frameInterval,
+ std::optional<Period> timeoutOpt) = 0;
};
static inline bool operator==(const android::HWComposer::DeviceRequestedChanges& lhs,
@@ -462,8 +463,9 @@
aidl::android::hardware::graphics::common::HdrConversionStrategy,
aidl::android::hardware::graphics::common::Hdr*) override;
status_t setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId, bool enabled) override;
- status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, nsecs_t expectedPresentTime,
- int32_t frameIntervalNs, int32_t timeoutNs) override;
+ status_t notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod,
+ TimePoint expectedPresentTime, Fps frameInterval,
+ std::optional<Period> timeoutOpt) override;
// for debugging ----------------------------------------------------------
void dump(std::string& out) const override;
@@ -497,7 +499,10 @@
sp<Fence> lastPresentFence = Fence::NO_FENCE; // signals when the last set op retires
nsecs_t lastPresentTimestamp = 0;
- nsecs_t lastExpectedPresentTimestamp = 0;
+ std::mutex expectedPresentLock;
+ TimePoint lastExpectedPresentTimestamp GUARDED_BY(expectedPresentLock) =
+ TimePoint::fromNs(0);
+ Fps lastFrameInterval GUARDED_BY(expectedPresentLock) = Fps::fromValue(0);
std::unordered_map<HWC2::Layer*, sp<Fence>> releaseFences;
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 4f545a9..58d7a40 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -79,10 +79,13 @@
EXPECT_CALL(*mHal, onHotplugConnect(hwcDisplayId));
}
- void setDisplayData(HalDisplayId displayId, nsecs_t lastExpectedPresentTimestamp) {
+ 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;
}
};
@@ -322,48 +325,137 @@
ASSERT_TRUE(info);
auto expectedPresentTime = systemTime() + ms2ns(10);
- const int32_t frameIntervalNs = static_cast<Fps>(60_Hz).getPeriodNsecs();
- static constexpr nsecs_t kTimeoutNs = ms2ns(30);
+ 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, /* lastExpectedPresentTimestamp= */ 0));
+ ASSERT_NO_FATAL_FAILURE(setDisplayData(info->id, kLastExpectedPresentTimestamp, Fps60Hz));
{
// Very first ExpectedPresent after idle, no previous timestamp
EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, frameIntervalNs))
+ notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
.WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, expectedPresentTime, frameIntervalNs,
+ mHwc.notifyExpectedPresentIfRequired(info->id, kVsyncPeriod,
+ TimePoint::fromNs(expectedPresentTime), Fps60Hz,
kTimeoutNs);
}
{
- // ExpectedPresent is after the timeoutNs
- expectedPresentTime += ms2ns(50);
+ // 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, frameIntervalNs))
+ notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
.WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, expectedPresentTime, frameIntervalNs,
+ 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 += ms2ns(10);
+ expectedPresentTime += kFrameInterval60HzNs;
EXPECT_CALL(*mHal, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0);
- mHwc.notifyExpectedPresentIfRequired(info->id, expectedPresentTime, frameIntervalNs,
+ 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 -= ms2ns(20);
+ expectedPresentTime -= kFrameInterval120HzNs;
EXPECT_CALL(*mHal,
- notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, frameIntervalNs))
+ notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs))
.WillOnce(Return(HalError::NONE));
- mHwc.notifyExpectedPresentIfRequired(info->id, expectedPresentTime, frameIntervalNs,
+ 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_METHOD2(onComposerHalHotplug, void(hal::HWDisplayId, hal::Connection));
MOCK_METHOD1(onComposerHalRefresh, void(hal::HWDisplayId));