SurfaceFlinger: no app vsyncs during config switch
Do not dipatch vsync callbacks to applications during a config
switch as applications will get choreographer callbacks at uneven
times and will stuff the queue to surface flinger.
Test: test google/perf/jank/UIBench/UIBench-Trace:com.android.uibench.janktests.UiBenchJankTests#testClippedListView -v
Change-Id: I0537933c27af83a9186bcff54c3596d612c748ea
diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.cpp b/services/surfaceflinger/Scheduler/DispSyncSource.cpp
index 697d634..d848c97 100644
--- a/services/surfaceflinger/Scheduler/DispSyncSource.cpp
+++ b/services/surfaceflinger/Scheduler/DispSyncSource.cpp
@@ -85,7 +85,19 @@
}
}
+void DispSyncSource::pauseVsyncCallback(bool pause) {
+ std::lock_guard lock(mVsyncMutex);
+ mCallbackPaused = pause;
+}
+
void DispSyncSource::onDispSyncEvent(nsecs_t when) {
+ {
+ std::lock_guard lock(mVsyncMutex);
+ if (mCallbackPaused) {
+ return;
+ }
+ }
+
VSyncSource::Callback* callback;
{
std::lock_guard lock(mCallbackMutex);
diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.h b/services/surfaceflinger/Scheduler/DispSyncSource.h
index 0fd84a2..5e3d181 100644
--- a/services/surfaceflinger/Scheduler/DispSyncSource.h
+++ b/services/surfaceflinger/Scheduler/DispSyncSource.h
@@ -33,6 +33,7 @@
void setVSyncEnabled(bool enable) override;
void setCallback(VSyncSource::Callback* callback) override;
void setPhaseOffset(nsecs_t phaseOffset) override;
+ void pauseVsyncCallback(bool pause) override;
private:
// The following method is the implementation of the DispSync::Callback.
@@ -53,6 +54,7 @@
std::mutex mVsyncMutex;
nsecs_t mPhaseOffset GUARDED_BY(mVsyncMutex);
bool mEnabled GUARDED_BY(mVsyncMutex) = false;
+ bool mCallbackPaused GUARDED_BY(mVsyncMutex) = false;
};
} // namespace android
\ No newline at end of file
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 5d9cfde..4b500f1 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -203,6 +203,12 @@
mVSyncSource->setPhaseOffset(phaseOffset);
}
+void EventThread::pauseVsyncCallback(bool pause) {
+ std::lock_guard<std::mutex> lock(mMutex);
+ ATRACE_INT("vsyncPaused", pause);
+ mVSyncSource->pauseVsyncCallback(pause);
+}
+
sp<EventThreadConnection> EventThread::createEventConnection(
ResyncCallback resyncCallback, ResetIdleTimerCallback resetIdleTimerCallback) const {
return new EventThreadConnection(const_cast<EventThread*>(this), std::move(resyncCallback),
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index b275183..3411438 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -66,6 +66,9 @@
virtual void setVSyncEnabled(bool enable) = 0;
virtual void setCallback(Callback* callback) = 0;
virtual void setPhaseOffset(nsecs_t phaseOffset) = 0;
+
+ // pause/resume vsync callback generation
+ virtual void pauseVsyncCallback(bool pause) = 0;
};
class EventThreadConnection : public BnDisplayEventConnection {
@@ -119,6 +122,8 @@
// Requests the next vsync. If resetIdleTimer is set to true, it resets the idle timer.
virtual void requestNextVsync(const sp<EventThreadConnection>& connection,
bool resetIdleTimer) = 0;
+
+ virtual void pauseVsyncCallback(bool pause) = 0;
};
namespace impl {
@@ -152,6 +157,8 @@
void setPhaseOffset(nsecs_t phaseOffset) override;
+ void pauseVsyncCallback(bool pause) override;
+
private:
friend EventThreadTest;
diff --git a/services/surfaceflinger/Scheduler/InjectVSyncSource.h b/services/surfaceflinger/Scheduler/InjectVSyncSource.h
index a0e1447..90609af 100644
--- a/services/surfaceflinger/Scheduler/InjectVSyncSource.h
+++ b/services/surfaceflinger/Scheduler/InjectVSyncSource.h
@@ -44,6 +44,7 @@
void setVSyncEnabled(bool) override {}
void setPhaseOffset(nsecs_t) override {}
+ void pauseVsyncCallback(bool) {}
private:
std::mutex mCallbackMutex;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 30fba3c..8ed3958 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -163,6 +163,12 @@
mConnections[handle->id]->thread->setPhaseOffset(phaseOffset);
}
+void Scheduler::pauseVsyncCallback(const android::sp<android::Scheduler::ConnectionHandle>& handle,
+ bool pause) {
+ RETURN_IF_INVALID();
+ mConnections[handle->id]->thread->pauseVsyncCallback(pause);
+}
+
void Scheduler::getDisplayStatInfo(DisplayStatInfo* stats) {
stats->vsyncTime = mPrimaryDispSync->computeNextRefresh(0);
stats->vsyncPeriod = mPrimaryDispSync->getPeriod();
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 7b5278c..ce5c87e 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -111,6 +111,9 @@
// Offers ability to modify phase offset in the event thread.
void setPhaseOffset(const sp<ConnectionHandle>& handle, nsecs_t phaseOffset);
+ // pause/resume vsync callback generation to avoid sending vsync callbacks during config switch
+ void pauseVsyncCallback(const sp<ConnectionHandle>& handle, bool pause);
+
void getDisplayStatInfo(DisplayStatInfo* stats);
void enableHardwareVsync();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3bec6dc..e1e3dfb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -949,12 +949,18 @@
// Don't check against the current mode yet. Worst case we set the desired
// config twice.
- {
- std::lock_guard<std::mutex> lock(mActiveConfigLock);
- mDesiredActiveConfig = ActiveConfigInfo{mode, displayToken};
+ std::lock_guard<std::mutex> lock(mActiveConfigLock);
+ mDesiredActiveConfig = ActiveConfigInfo{mode, displayToken};
+
+ if (!mDesiredActiveConfigChanged) {
+ // This is the first time we set the desired
+ mScheduler->pauseVsyncCallback(mAppConnectionHandle, true);
+
+ // This will trigger HWC refresh without resetting the idle timer.
+ repaintEverythingForHWC();
}
- // This will trigger HWC refresh without resetting the idle timer.
- repaintEverythingForHWC();
+ mDesiredActiveConfigChanged = true;
+ ATRACE_INT("DesiredActiveConfigChanged", mDesiredActiveConfigChanged);
}
status_t SurfaceFlinger::setActiveConfig(const sp<IBinder>& displayToken, int mode) {
@@ -964,23 +970,6 @@
return NO_ERROR;
}
-void SurfaceFlinger::setActiveConfigInHWC() {
- ATRACE_CALL();
-
- const auto display = getDisplayDevice(mUpcomingActiveConfig.displayToken);
- if (!display) {
- return;
- }
-
- const auto displayId = display->getId();
- LOG_ALWAYS_FATAL_IF(!displayId);
-
- ATRACE_INT("ActiveConfigModeHWC", mUpcomingActiveConfig.configId);
- getHwComposer().setActiveConfig(*displayId, mUpcomingActiveConfig.configId);
- mSetActiveConfigState = SetActiveConfigState::NOTIFIED_HWC;
- ATRACE_INT("SetActiveConfigState", mSetActiveConfigState);
-}
-
void SurfaceFlinger::setActiveConfigInternal() {
ATRACE_CALL();
@@ -990,51 +979,68 @@
const auto display = getDisplayDeviceLocked(mUpcomingActiveConfig.displayToken);
display->setActiveConfig(mUpcomingActiveConfig.configId);
- mSetActiveConfigState = SetActiveConfigState::NONE;
- ATRACE_INT("SetActiveConfigState", mSetActiveConfigState);
-
mScheduler->resyncToHardwareVsync(true, getVsyncPeriod());
ATRACE_INT("ActiveConfigMode", mUpcomingActiveConfig.configId);
}
-bool SurfaceFlinger::updateSetActiveConfigStateMachine() NO_THREAD_SAFETY_ANALYSIS {
+bool SurfaceFlinger::performSetActiveConfig() NO_THREAD_SAFETY_ANALYSIS {
+ // we may be in the process of changing the active state
+ if (mWaitForNextInvalidate) {
+ mWaitForNextInvalidate = false;
+
+ repaintEverythingForHWC();
+ // We do not want to give another frame to HWC while we are transitioning.
+ return true;
+ }
+
+ if (mCheckPendingFence) {
+ if (mPreviousPresentFence != Fence::NO_FENCE &&
+ (mPreviousPresentFence->getStatus() == Fence::Status::Unsignaled)) {
+ // fence has not signaled yet. wait for the next invalidate
+ repaintEverythingForHWC();
+ return true;
+ }
+
+ // We received the present fence from the HWC, so we assume it successfully updated
+ // the config, hence we update SF.
+ mCheckPendingFence = false;
+ setActiveConfigInternal();
+ }
+
// Store the local variable to release the lock.
ActiveConfigInfo desiredActiveConfig;
{
std::lock_guard<std::mutex> lock(mActiveConfigLock);
+ if (!mDesiredActiveConfigChanged) {
+ return false;
+ }
desiredActiveConfig = mDesiredActiveConfig;
}
const auto display = getDisplayDevice(desiredActiveConfig.displayToken);
- if (display) {
- if (mSetActiveConfigState == SetActiveConfigState::NONE &&
- display->getActiveConfig() != desiredActiveConfig.configId) {
- // Step 1) Desired active config was set, it is different than the
- // config currently in use. Notify HWC.
- mUpcomingActiveConfig = desiredActiveConfig;
- setActiveConfigInHWC();
- } else if (mSetActiveConfigState == SetActiveConfigState::NOTIFIED_HWC) {
- // Step 2) HWC was notified and we received refresh from it.
- mSetActiveConfigState = SetActiveConfigState::REFRESH_RECEIVED;
- ATRACE_INT("SetActiveConfigState", mSetActiveConfigState);
- repaintEverythingForHWC();
- // We do not want to give another frame to HWC while we are transitioning.
- return false;
- } else if (mSetActiveConfigState == SetActiveConfigState::REFRESH_RECEIVED &&
- !(mPreviousPresentFence != Fence::NO_FENCE &&
- (mPreviousPresentFence->getStatus() == Fence::Status::Unsignaled))) {
- // Step 3) We received the present fence from the HWC, so we assume it
- // successfully updated the config, hence we update SF.
- setActiveConfigInternal();
- // If the config changed again while we were transitioning, restart the
- // process.
- if (desiredActiveConfig != mUpcomingActiveConfig) {
- mUpcomingActiveConfig = desiredActiveConfig;
- setActiveConfigInHWC(); // sets the state to NOTIFY_HWC
- }
- }
+ if (!display || display->getActiveConfig() == desiredActiveConfig.configId) {
+ // display is not valid or we are already in the requested mode
+ // on both cases there is nothing left to do
+ std::lock_guard<std::mutex> lock(mActiveConfigLock);
+ mScheduler->pauseVsyncCallback(mAppConnectionHandle, false);
+ mDesiredActiveConfigChanged = false;
+ ATRACE_INT("DesiredActiveConfigChanged", mDesiredActiveConfigChanged);
+ return false;
}
- return true;
+
+ // Desired active config was set, it is different than the config currently in use. Notify HWC.
+ mUpcomingActiveConfig = desiredActiveConfig;
+ const auto displayId = display->getId();
+ LOG_ALWAYS_FATAL_IF(!displayId);
+
+ ATRACE_INT("ActiveConfigModeHWC", mUpcomingActiveConfig.configId);
+ getHwComposer().setActiveConfig(*displayId, mUpcomingActiveConfig.configId);
+
+ // we need to submit an empty frame to HWC to start the process
+ mWaitForNextInvalidate = true;
+ mCheckPendingFence = true;
+
+ return false;
}
status_t SurfaceFlinger::getDisplayColorModes(const sp<IBinder>& displayToken,
@@ -1575,7 +1581,7 @@
ATRACE_CALL();
switch (what) {
case MessageQueue::INVALIDATE: {
- if (!updateSetActiveConfigStateMachine()) {
+ if (performSetActiveConfig()) {
break;
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index b1f0992..220e664 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -511,16 +511,14 @@
void onInitializeDisplays() REQUIRES(mStateLock);
// Sets the desired active config bit. It obtains the lock, and sets mDesiredActiveConfig.
void setDesiredActiveConfig(const sp<IBinder>& displayToken, int mode) REQUIRES(mStateLock);
- // Calls setActiveConfig in HWC.
- void setActiveConfigInHWC();
// Once HWC has returned the present fence, this sets the active config and a new refresh
// rate in SF. It also triggers HWC vsync.
void setActiveConfigInternal() REQUIRES(mStateLock);
// Active config is updated on INVALIDATE call in a state machine-like manner. When the
- // desired config was set, HWC needs to update the pannel on the next refresh, and when
+ // desired config was set, HWC needs to update the panel on the next refresh, and when
// we receive the fence back, we know that the process was complete. It returns whether
- // the invalidate process should continue.
- bool updateSetActiveConfigStateMachine();
+ // we need to wait for the next invalidate
+ bool performSetActiveConfig();
// called on the main thread in response to setPowerMode()
void setPowerModeInternal(const sp<DisplayDevice>& display, int mode) REQUIRES(mStateLock);
@@ -1082,15 +1080,6 @@
sp<Scheduler::ConnectionHandle> mSfConnectionHandle;
std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats;
- // The following structs are used for configuring active config state at a desired time,
- // which is once per vsync at invalidate time.
- enum SetActiveConfigState {
- NONE, // not in progress
- NOTIFIED_HWC, // call to HWC has been made
- REFRESH_RECEIVED // onRefresh was received from HWC
- };
- std::atomic<SetActiveConfigState> mSetActiveConfigState = SetActiveConfigState::NONE;
-
struct ActiveConfigInfo {
int configId;
sp<IBinder> displayToken;
@@ -1110,6 +1099,11 @@
// This bit can be set at any point in time when the system wants the new config.
ActiveConfigInfo mDesiredActiveConfig GUARDED_BY(mActiveConfigLock);
+ // below flags are set by main thread only
+ bool mDesiredActiveConfigChanged GUARDED_BY(mActiveConfigLock) = false;
+ bool mWaitForNextInvalidate = false;
+ bool mCheckPendingFence = false;
+
/* ------------------------------------------------------------------------ */
sp<IInputFlinger> mInputFlinger;
diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
index 3a7cfba..e499ff5 100644
--- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
+++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
@@ -44,6 +44,7 @@
MOCK_METHOD1(setVSyncEnabled, void(bool));
MOCK_METHOD1(setCallback, void(VSyncSource::Callback*));
MOCK_METHOD1(setPhaseOffset, void(nsecs_t));
+ MOCK_METHOD1(pauseVsyncCallback, void(bool));
};
} // namespace
@@ -71,6 +72,7 @@
void expectVSyncSetEnabledCallReceived(bool expectedState);
void expectVSyncSetPhaseOffsetCallReceived(nsecs_t expectedPhaseOffset);
+ void expectVSyncPauseVsyncCallbackCallReceived(bool expectedPause);
VSyncSource::Callback* expectVSyncSetCallbackCallReceived();
void expectInterceptCallReceived(nsecs_t expectedTimestamp);
void expectVsyncEventReceivedByConnection(const char* name,
@@ -83,6 +85,7 @@
AsyncCallRecorder<void (*)(bool)> mVSyncSetEnabledCallRecorder;
AsyncCallRecorder<void (*)(VSyncSource::Callback*)> mVSyncSetCallbackCallRecorder;
AsyncCallRecorder<void (*)(nsecs_t)> mVSyncSetPhaseOffsetCallRecorder;
+ AsyncCallRecorder<void (*)(bool)> mVSyncPauseVsyncCallbackCallRecorder;
AsyncCallRecorder<void (*)()> mResyncCallRecorder;
AsyncCallRecorder<void (*)()> mResetIdleTimerCallRecorder;
AsyncCallRecorder<void (*)(nsecs_t)> mInterceptVSyncCallRecorder;
@@ -108,6 +111,9 @@
EXPECT_CALL(mVSyncSource, setPhaseOffset(_))
.WillRepeatedly(Invoke(mVSyncSetPhaseOffsetCallRecorder.getInvocable()));
+ EXPECT_CALL(mVSyncSource, pauseVsyncCallback(_))
+ .WillRepeatedly(Invoke(mVSyncPauseVsyncCallbackCallRecorder.getInvocable()));
+
createThread();
mConnection = createConnection(mConnectionEventCallRecorder);
@@ -157,6 +163,12 @@
EXPECT_EQ(expectedPhaseOffset, std::get<0>(args.value()));
}
+void EventThreadTest::expectVSyncPauseVsyncCallbackCallReceived(bool expectedPause) {
+ auto args = mVSyncPauseVsyncCallbackCallRecorder.waitForCall();
+ ASSERT_TRUE(args.has_value());
+ EXPECT_EQ(expectedPause, std::get<0>(args.value()));
+}
+
VSyncSource::Callback* EventThreadTest::expectVSyncSetCallbackCallReceived() {
auto callbackSet = mVSyncSetCallbackCallRecorder.waitForCall();
return callbackSet.has_value() ? std::get<0>(callbackSet.value()) : nullptr;
@@ -406,6 +418,16 @@
expectVSyncSetPhaseOffsetCallReceived(321);
}
+TEST_F(EventThreadTest, pauseVsyncCallbackForwardsToVSyncSource) {
+ mThread->pauseVsyncCallback(true);
+ expectVSyncPauseVsyncCallbackCallReceived(true);
+}
+
+TEST_F(EventThreadTest, resumeVsyncCallbackForwardsToVSyncSource) {
+ mThread->pauseVsyncCallback(false);
+ expectVSyncPauseVsyncCallbackCallReceived(false);
+}
+
TEST_F(EventThreadTest, postHotplugInternalDisconnect) {
mThread->onHotplugReceived(INTERNAL_DISPLAY_ID, false);
expectHotplugEventReceivedByConnection(INTERNAL_DISPLAY_ID, false);
diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
index 589237d..5edee6e 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
@@ -39,6 +39,7 @@
status_t(const sp<android::EventThreadConnection> &));
MOCK_METHOD2(setVsyncRate, void(uint32_t, const sp<android::EventThreadConnection> &));
MOCK_METHOD2(requestNextVsync, void(const sp<android::EventThreadConnection> &, bool));
+ MOCK_METHOD1(pauseVsyncCallback, void(bool));
};
} // namespace mock