SF: Clean up emitting of mode change event
Clarify the special case of emitting despite the unchanged mode. Rename
functions and state for consistency with DisplayModeRequest::emitEvent.
Remove Cycle parameters for which the argument is always Cycle::Render.
This does not change behavior.
Bug: 255635821
Flag: EXEMPT refactor
Test: SchedulerTest.emitModeChangeEvent
Change-Id: I99debb33ba84dff707d6da0a92dacc4e275fc5ce
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
index 998b1b8..a398c01 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
@@ -210,6 +210,8 @@
// within the timeout of DisplayPowerTimer.
bool powerOnImminent = false;
+ bool shouldEmitEvent() const { return !idle; }
+
bool operator==(GlobalSignals other) const {
return touch == other.touch && idle == other.idle &&
powerOnImminent == other.powerOnImminent;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 84584a4..b711027 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -424,50 +424,43 @@
eventThreadFor(cycle).onHdcpLevelsChanged(displayId, connectedLevel, maxLevel);
}
-void Scheduler::onPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) {
+void Scheduler::onPrimaryDisplayModeChanged(const FrameRateMode& mode) {
{
std::lock_guard<std::mutex> lock(mPolicyLock);
- // Cache the last reported modes for primary display.
- mPolicy.cachedModeChangedParams = {cycle, mode};
+ mPolicy.emittedModeOpt = mode;
// Invalidate content based refresh rate selection so it could be calculated
// again for the new refresh rate.
mPolicy.contentRequirements.clear();
}
- onNonPrimaryDisplayModeChanged(cycle, mode);
+ onNonPrimaryDisplayModeChanged(mode);
}
-void Scheduler::dispatchCachedReportedMode() {
- // Check optional fields first.
- if (!mPolicy.modeOpt) {
- ALOGW("No mode ID found, not dispatching cached mode.");
- return;
- }
- if (!mPolicy.cachedModeChangedParams) {
- ALOGW("No mode changed params found, not dispatching cached mode.");
+void Scheduler::emitModeChangeIfNeeded() {
+ if (!mPolicy.modeOpt || !mPolicy.emittedModeOpt) {
+ ALOGW("No mode change to emit");
return;
}
- // If the mode is not the current mode, this means that a
- // mode change is in progress. In that case we shouldn't dispatch an event
- // as it will be dispatched when the current mode changes.
- if (pacesetterSelectorPtr()->getActiveMode() != mPolicy.modeOpt) {
+ const auto& mode = *mPolicy.modeOpt;
+
+ if (mode != pacesetterSelectorPtr()->getActiveMode()) {
+ // A mode change is pending. The event will be emitted when the mode becomes active.
return;
}
- // If there is no change from cached mode, there is no need to dispatch an event
- if (*mPolicy.modeOpt == mPolicy.cachedModeChangedParams->mode) {
+ if (mode == *mPolicy.emittedModeOpt) {
+ // The event was already emitted.
return;
}
- mPolicy.cachedModeChangedParams->mode = *mPolicy.modeOpt;
- onNonPrimaryDisplayModeChanged(mPolicy.cachedModeChangedParams->cycle,
- mPolicy.cachedModeChangedParams->mode);
+ mPolicy.emittedModeOpt = mode;
+ onNonPrimaryDisplayModeChanged(mode);
}
-void Scheduler::onNonPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) {
+void Scheduler::onNonPrimaryDisplayModeChanged(const FrameRateMode& mode) {
if (hasEventThreads()) {
- eventThreadFor(cycle).onModeChanged(mode);
+ eventThreadFor(Cycle::Render).onModeChanged(mode);
}
}
@@ -1139,7 +1132,8 @@
for (auto& [id, choice] : modeChoices) {
modeRequests.emplace_back(
display::DisplayModeRequest{.mode = std::move(choice.mode),
- .emitEvent = !choice.consideredSignals.idle});
+ .emitEvent = choice.consideredSignals
+ .shouldEmitEvent()});
}
if (!FlagManager::getInstance().vrr_bugfix_dropped_frame()) {
@@ -1149,12 +1143,10 @@
if (mPolicy.modeOpt != modeOpt) {
mPolicy.modeOpt = modeOpt;
refreshRateChanged = true;
- } else {
- // We don't need to change the display mode, but we might need to send an event
- // about a mode change, since it was suppressed if previously considered idle.
- if (!consideredSignals.idle) {
- dispatchCachedReportedMode();
- }
+ } else if (consideredSignals.shouldEmitEvent()) {
+ // The mode did not change, but we may need to emit if DisplayModeRequest::emitEvent was
+ // previously false.
+ emitModeChangeIfNeeded();
}
}
if (refreshRateChanged) {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index e1f4d20..fc22cc3 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -154,8 +154,8 @@
void dispatchHotplugError(int32_t errorCode);
- void onPrimaryDisplayModeChanged(Cycle, const FrameRateMode&) EXCLUDES(mPolicyLock);
- void onNonPrimaryDisplayModeChanged(Cycle, const FrameRateMode&);
+ void onPrimaryDisplayModeChanged(const FrameRateMode&) EXCLUDES(mPolicyLock);
+ void onNonPrimaryDisplayModeChanged(const FrameRateMode&);
void enableSyntheticVsync(bool = true) REQUIRES(kMainThreadContext);
@@ -458,7 +458,7 @@
void updateAttachedChoreographersFrameRate(const surfaceflinger::frontend::RequestedLayerState&,
Fps fps) EXCLUDES(mChoreographerLock);
- void dispatchCachedReportedMode() REQUIRES(mPolicyLock) EXCLUDES(mDisplayLock);
+ void emitModeChangeIfNeeded() REQUIRES(mPolicyLock) EXCLUDES(mDisplayLock);
// IEventThreadCallback overrides
bool throttleVsync(TimePoint, uid_t) override;
@@ -584,13 +584,8 @@
// Chosen display mode.
ftl::Optional<FrameRateMode> modeOpt;
- struct ModeChangedParams {
- Cycle cycle;
- FrameRateMode mode;
- };
-
- // Parameters for latest dispatch of mode change event.
- std::optional<ModeChangedParams> cachedModeChangedParams;
+ // Display mode of latest emitted event.
+ std::optional<FrameRateMode> emittedModeOpt;
} mPolicy GUARDED_BY(mPolicyLock);
std::mutex mChoreographerLock;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e7d802a..c616a70 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3596,7 +3596,7 @@
? &scheduler::Scheduler::onPrimaryDisplayModeChanged
: &scheduler::Scheduler::onNonPrimaryDisplayModeChanged;
- ((*mScheduler).*onDisplayModeChanged)(scheduler::Cycle::Render, mode);
+ ((*mScheduler).*onDisplayModeChanged)(mode);
}
sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
@@ -7962,10 +7962,10 @@
// TODO(b/140204874): Leave the event in until we do proper testing with all apps that might
// be depending in this callback.
if (const auto activeMode = selector.getActiveMode(); displayId == mActiveDisplayId) {
- mScheduler->onPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode);
+ mScheduler->onPrimaryDisplayModeChanged(activeMode);
toggleKernelIdleTimer();
} else {
- mScheduler->onNonPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode);
+ mScheduler->onNonPrimaryDisplayModeChanged(activeMode);
}
auto preferredModeOpt = getPreferredDisplayMode(displayId, currentPolicy.defaultMode);
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index d4792d5..5b5d6ae 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -203,7 +203,7 @@
const auto selectorPtr =
std::make_shared<RefreshRateSelector>(kDisplay1Modes, kDisplay1Mode120->getId());
mScheduler->registerDisplay(kDisplayId1, selectorPtr);
- mScheduler->onPrimaryDisplayModeChanged(Cycle::Render, kDisplay1Mode120_120);
+ mScheduler->onPrimaryDisplayModeChanged(kDisplay1Mode120_120);
mScheduler->setContentRequirements({kLayer});
@@ -231,7 +231,7 @@
EXPECT_CALL(*mEventThread, onModeChanged(kDisplay1Mode120_120)).Times(1);
mScheduler->touchTimerCallback(TimerState::Reset);
- mScheduler->onPrimaryDisplayModeChanged(Cycle::Render, kDisplay1Mode120_120);
+ mScheduler->onPrimaryDisplayModeChanged(kDisplay1Mode120_120);
}
TEST_F(SchedulerTest, calculateMaxAcquiredBufferCount) {