summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Marin Shalamanov <shalamanov@google.com> 2021-06-08 19:50:10 +0200
committer Marin Shalamanov <shalamanov@google.com> 2021-06-14 10:47:02 +0200
commit2cde1001345b0b3bac2441ced1d059b65a5d7980 (patch)
tree065412d4d08e0cfeb1d7a45983948d7b38348915
parent993ddf4afb45b389e950a16237de720783fad485 (diff)
SF: Always create LayerHistory in Scheduler
Currently if Scheduler is created when there is only one supported display mode, LayerHistory is not created. This optimization assumes that the list of supported modes is never going to change but this is not the case for TV devices. The typical use case is when a TV dongle or set-top box is first connected to a display after it boots. In this cas some devices will first report only one display mode and later update to the full list of supported modes. This CL changes the behaviour to always have an instance of LayerHistory. If there's only one display mode recordLayerHistory() and chooseRefreshRateForContent() are noops. The alternative to create LayerHistory on demand is more cumbersome because we also would need to register all already created layers. Bug: 188872896 Test: presubmit Change-Id: Ia236fd4a81cc736c172220ff60762ddbc3cbb83a
-rw-r--r--services/surfaceflinger/Scheduler/Scheduler.cpp28
-rw-r--r--services/surfaceflinger/Scheduler/Scheduler.h2
-rw-r--r--services/surfaceflinger/tests/unittests/SchedulerTest.cpp38
-rw-r--r--services/surfaceflinger/tests/unittests/TestableScheduler.h5
4 files changed, 42 insertions, 31 deletions
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index fac2c654cb..857071c3f1 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -194,8 +194,6 @@ Scheduler::VsyncSchedule Scheduler::createVsyncSchedule(bool supportKernelTimer)
std::unique_ptr<LayerHistory> Scheduler::createLayerHistory(
const scheduler::RefreshRateConfigs& configs) {
- if (!configs.canSwitch()) return nullptr;
-
return std::make_unique<scheduler::LayerHistory>(configs);
}
@@ -579,8 +577,6 @@ void Scheduler::setIgnorePresentFences(bool ignore) {
}
void Scheduler::registerLayer(Layer* layer) {
- if (!mLayerHistory) return;
-
scheduler::LayerHistory::LayerVoteType voteType;
if (!mOptions.useContentDetection ||
@@ -600,26 +596,22 @@ void Scheduler::registerLayer(Layer* layer) {
}
void Scheduler::deregisterLayer(Layer* layer) {
- if (mLayerHistory) {
- mLayerHistory->deregisterLayer(layer);
- }
+ mLayerHistory->deregisterLayer(layer);
}
void Scheduler::recordLayerHistory(Layer* layer, nsecs_t presentTime,
LayerHistory::LayerUpdateType updateType) {
- if (mLayerHistory) {
+ if (mRefreshRateConfigs.canSwitch()) {
mLayerHistory->record(layer, presentTime, systemTime(), updateType);
}
}
void Scheduler::setModeChangePending(bool pending) {
- if (mLayerHistory) {
- mLayerHistory->setModeChangePending(pending);
- }
+ mLayerHistory->setModeChangePending(pending);
}
void Scheduler::chooseRefreshRateForContent() {
- if (!mLayerHistory) return;
+ if (!mRefreshRateConfigs.canSwitch()) return;
ATRACE_CALL();
@@ -691,9 +683,7 @@ void Scheduler::setDisplayPowerState(bool normal) {
// Display Power event will boost the refresh rate to performance.
// Clear Layer History to get fresh FPS detection
- if (mLayerHistory) {
- mLayerHistory->clear();
- }
+ mLayerHistory->clear();
}
void Scheduler::kernelIdleTimerCallback(TimerState state) {
@@ -732,9 +722,7 @@ void Scheduler::touchTimerCallback(TimerState state) {
// NOTE: Instead of checking all the layers, we should be checking the layer
// that is currently on top. b/142507166 will give us this capability.
if (handleTimerStateChanged(&mFeatures.touch, touch)) {
- if (mLayerHistory) {
- mLayerHistory->clear();
- }
+ mLayerHistory->clear();
}
ATRACE_INT("TouchState", static_cast<int>(touch));
}
@@ -908,9 +896,7 @@ void Scheduler::onDisplayRefreshed(nsecs_t timestamp) {
}
void Scheduler::onPrimaryDisplayAreaChanged(uint32_t displayArea) {
- if (mLayerHistory) {
- mLayerHistory->setDisplayArea(displayArea);
- }
+ mLayerHistory->setDisplayArea(displayArea);
}
void Scheduler::setPreferredRefreshRateForUid(FrameRateOverride frameRateOverride) {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 49d3d93f36..30a32537ad 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -268,7 +268,7 @@ private:
VsyncSchedule mVsyncSchedule;
// Used to choose refresh rate if content detection is enabled.
- const std::unique_ptr<LayerHistory> mLayerHistory;
+ std::unique_ptr<LayerHistory> mLayerHistory;
// Timer that records time between requests for next vsync.
std::optional<scheduler::OneShotTimer> mIdleTimer;
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 38e503fc81..8ad8ea4299 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -51,9 +51,18 @@ protected:
SchedulerTest();
- const scheduler::RefreshRateConfigs
- mConfigs{{DisplayMode::Builder(0).setVsyncPeriod(16'666'667).setGroup(0).build()},
- DisplayModeId(0)};
+ const DisplayModePtr mode60 = DisplayMode::Builder(0)
+ .setId(DisplayModeId(0))
+ .setVsyncPeriod(Fps(60.f).getPeriodNsecs())
+ .setGroup(0)
+ .build();
+ const DisplayModePtr mode120 = DisplayMode::Builder(1)
+ .setId(DisplayModeId(1))
+ .setVsyncPeriod(Fps(120.f).getPeriodNsecs())
+ .setGroup(0)
+ .build();
+
+ scheduler::RefreshRateConfigs mConfigs{{mode60}, mode60->getId()};
mock::SchedulerCallback mSchedulerCallback;
@@ -149,15 +158,14 @@ TEST_F(SchedulerTest, validConnectionHandle) {
EXPECT_EQ(kEventConnections, mScheduler->getEventThreadConnectionCount(mConnectionHandle));
}
-TEST_F(SchedulerTest, noLayerHistory) {
- // Layer history should not be created if there is a single config.
- ASSERT_FALSE(mScheduler->hasLayerHistory());
-
+TEST_F(SchedulerTest, chooseRefreshRateForContentIsNoopWhenModeSwitchingIsNotSupported) {
+ // The layer is registered at creation time and deregistered at destruction time.
sp<mock::MockLayer> layer = sp<mock::MockLayer>::make(mFlinger.flinger());
- // Content detection should be no-op.
- mScheduler->registerLayer(layer.get());
+ // recordLayerHistory should be a noop
+ ASSERT_EQ(static_cast<size_t>(0), mScheduler->getNumActiveLayers());
mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer);
+ ASSERT_EQ(static_cast<size_t>(0), mScheduler->getNumActiveLayers());
constexpr bool kPowerStateNormal = true;
mScheduler->setDisplayPowerState(kPowerStateNormal);
@@ -169,6 +177,18 @@ TEST_F(SchedulerTest, noLayerHistory) {
mScheduler->chooseRefreshRateForContent();
}
+TEST_F(SchedulerTest, updateDisplayModes) {
+ ASSERT_EQ(static_cast<size_t>(0), mScheduler->layerHistorySize());
+ sp<mock::MockLayer> layer = sp<mock::MockLayer>::make(mFlinger.flinger());
+ ASSERT_EQ(static_cast<size_t>(1), mScheduler->layerHistorySize());
+
+ mConfigs.updateDisplayModes({mode60, mode120}, /* activeMode */ mode60->getId());
+
+ ASSERT_EQ(static_cast<size_t>(0), mScheduler->getNumActiveLayers());
+ mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer);
+ ASSERT_EQ(static_cast<size_t>(1), mScheduler->getNumActiveLayers());
+}
+
TEST_F(SchedulerTest, testDispatchCachedReportedMode) {
// If the optional fields are cleared, the function should return before
// onModeChange is called.
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 3f9dd01588..41fd6e316f 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -64,6 +64,11 @@ public:
return mutableLayerHistory()->mLayerInfos.size();
}
+ size_t getNumActiveLayers() NO_THREAD_SAFETY_ANALYSIS {
+ if (!mLayerHistory) return 0;
+ return mutableLayerHistory()->mActiveLayersEnd;
+ }
+
void replaceTouchTimer(int64_t millis) {
if (mTouchTimer) {
mTouchTimer.reset();