SF: Remove ConnectionHandle concept in Scheduler
This abstraction never made sense, and only incurs pointless locking and
hash map lookups. This is a step toward replacing the two EventThreads
with a single EventDispatcher.
Start the Scheduler's timers after initialization. This solves the same
bug as Ib2617b914145bc4180cc7ca27203c59dbd625c94, which avoided crashes
without fixing the root cause of the race.
In a few cases, initScheduler indirectly calls Scheduler functions that
attempt EventThread dispatch before EventThread creation, so skip those
EventThread accesses using hasEventThreads for now. Further refactoring
will untangle circular dependencies and remove hasEventThreads.
Bug: 241285191
Bug: 241285945
Test: presubmit
Change-Id: I89b93b47540ab24bb5675edc79370f38bc48114f
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index edba50b..4344494 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2092,12 +2092,11 @@
sp<IDisplayEventConnection> SurfaceFlinger::createDisplayEventConnection(
gui::ISurfaceComposer::VsyncSource vsyncSource, EventRegistrationFlags eventRegistration,
const sp<IBinder>& layerHandle) {
- const auto& handle =
- vsyncSource == gui::ISurfaceComposer::VsyncSource::eVsyncSourceSurfaceFlinger
- ? mSfConnectionHandle
- : mAppConnectionHandle;
+ const auto cycle = vsyncSource == gui::ISurfaceComposer::VsyncSource::eVsyncSourceSurfaceFlinger
+ ? scheduler::Cycle::LastComposite
+ : scheduler::Cycle::Render;
- return mScheduler->createDisplayEventConnection(handle, eventRegistration, layerHandle);
+ return mScheduler->createDisplayEventConnection(cycle, eventRegistration, layerHandle);
}
void SurfaceFlinger::scheduleCommit(FrameHint hint) {
@@ -2139,7 +2138,7 @@
const int32_t hotplugErrorCode = static_cast<int32_t>(-timestamp);
ALOGD("SurfaceFlinger got hotplugErrorCode=%d for display %" PRIu64, hotplugErrorCode,
hwcDisplayId);
- mScheduler->onHotplugConnectionError(mAppConnectionHandle, hotplugErrorCode);
+ mScheduler->onHotplugConnectionError(scheduler::Cycle::Render, hotplugErrorCode);
return;
}
@@ -2190,7 +2189,7 @@
if (FlagManager::getInstance().hotplug2()) {
ALOGD("SurfaceFlinger got hotplug event=%d", static_cast<int32_t>(event));
// TODO(b/311403559): use enum type instead of int
- mScheduler->onHotplugConnectionError(mAppConnectionHandle, static_cast<int32_t>(event));
+ mScheduler->onHotplugConnectionError(scheduler::Cycle::Render, static_cast<int32_t>(event));
}
}
@@ -3480,7 +3479,7 @@
if (!activeMode) {
ALOGE("Failed to hotplug display %s", to_string(displayId).c_str());
if (FlagManager::getInstance().hotplug2()) {
- mScheduler->onHotplugConnectionError(mAppConnectionHandle,
+ mScheduler->onHotplugConnectionError(scheduler::Cycle::Render,
static_cast<int32_t>(
DisplayHotplugEvent::ERROR_UNKNOWN));
}
@@ -3533,8 +3532,8 @@
}
void SurfaceFlinger::dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected) {
- mScheduler->onHotplugReceived(mAppConnectionHandle, displayId, connected);
- mScheduler->onHotplugReceived(mSfConnectionHandle, displayId, connected);
+ mScheduler->onHotplugReceived(scheduler::Cycle::Render, displayId, connected);
+ mScheduler->onHotplugReceived(scheduler::Cycle::LastComposite, displayId, connected);
}
void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId,
@@ -3544,7 +3543,7 @@
? &scheduler::Scheduler::onPrimaryDisplayModeChanged
: &scheduler::Scheduler::onNonPrimaryDisplayModeChanged;
- ((*mScheduler).*onDisplayModeChanged)(mAppConnectionHandle, mode);
+ ((*mScheduler).*onDisplayModeChanged)(scheduler::Cycle::Render, mode);
}
sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
@@ -4196,7 +4195,7 @@
return getDefaultDisplayDeviceLocked()->getPhysicalId();
}();
- mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId);
+ mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId);
}
void SurfaceFlinger::notifyCpuLoadUp() {
@@ -4361,24 +4360,22 @@
mScheduler = std::make_unique<Scheduler>(static_cast<ICompositor&>(*this),
static_cast<ISchedulerCallback&>(*this), features,
getFactory(), activeRefreshRate, *mTimeStats);
+
+ // The pacesetter must be registered before EventThread creation below.
mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector());
if (FlagManager::getInstance().vrr_config()) {
mScheduler->setRenderRate(display->getPhysicalId(), activeMode.fps);
}
- mScheduler->startTimers();
const auto configs = mScheduler->getVsyncConfiguration().getCurrentConfigs();
- mAppConnectionHandle =
- mScheduler->createEventThread(Scheduler::Cycle::Render,
- mFrameTimeline->getTokenManager(),
- /* workDuration */ configs.late.appWorkDuration,
- /* readyDuration */ configs.late.sfWorkDuration);
- mSfConnectionHandle =
- mScheduler->createEventThread(Scheduler::Cycle::LastComposite,
- mFrameTimeline->getTokenManager(),
- /* workDuration */ activeRefreshRate.getPeriod(),
- /* readyDuration */ configs.late.sfWorkDuration);
+ mScheduler->createEventThread(scheduler::Cycle::Render, mFrameTimeline->getTokenManager(),
+ /* workDuration */ configs.late.appWorkDuration,
+ /* readyDuration */ configs.late.sfWorkDuration);
+ mScheduler->createEventThread(scheduler::Cycle::LastComposite,
+ mFrameTimeline->getTokenManager(),
+ /* workDuration */ activeRefreshRate.getPeriod(),
+ /* readyDuration */ configs.late.sfWorkDuration);
mScheduler->initVsync(*mFrameTimeline->getTokenManager(), configs.late.sfWorkDuration);
@@ -4386,6 +4383,9 @@
sp<RegionSamplingThread>::make(*this,
RegionSamplingThread::EnvironmentTimingTunables());
mFpsReporter = sp<FpsReporter>::make(*mFrameTimeline);
+
+ // Timer callbacks may fire, so do this last.
+ mScheduler->startTimers();
}
void SurfaceFlinger::doCommitTransactions() {
@@ -6322,7 +6322,7 @@
}
void SurfaceFlinger::dumpEvents(std::string& result) const {
- mScheduler->dump(mAppConnectionHandle, result);
+ mScheduler->dump(scheduler::Cycle::Render, result);
}
void SurfaceFlinger::dumpVsync(std::string& result) const {
@@ -7094,14 +7094,15 @@
mForceFullDamage = n != 0;
return NO_ERROR;
}
- case 1018: { // Modify Choreographer's duration
+ case 1018: { // Set the render deadline as a duration until VSYNC.
n = data.readInt32();
- mScheduler->setDuration(mAppConnectionHandle, std::chrono::nanoseconds(n), 0ns);
+ mScheduler->setDuration(scheduler::Cycle::Render, std::chrono::nanoseconds(n), 0ns);
return NO_ERROR;
}
- case 1019: { // Modify SurfaceFlinger's duration
+ case 1019: { // Set the deadline of the last composite as a duration until VSYNC.
n = data.readInt32();
- mScheduler->setDuration(mSfConnectionHandle, std::chrono::nanoseconds(n), 0ns);
+ mScheduler->setDuration(scheduler::Cycle::LastComposite,
+ std::chrono::nanoseconds(n), 0ns);
return NO_ERROR;
}
case 1020: { // Unused
@@ -7333,7 +7334,7 @@
auto inUid = static_cast<uid_t>(data.readInt32());
const auto refreshRate = data.readFloat();
mScheduler->setPreferredRefreshRateForUid(FrameRateOverride{inUid, refreshRate});
- mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId);
+ mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId);
return NO_ERROR;
}
// Toggle caching feature
@@ -8473,10 +8474,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(mAppConnectionHandle, activeMode);
+ mScheduler->onPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode);
toggleKernelIdleTimer();
} else {
- mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode);
+ mScheduler->onNonPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode);
}
auto preferredModeOpt = getPreferredDisplayMode(displayId, currentPolicy.defaultMode);
@@ -8664,7 +8665,7 @@
}();
mScheduler->setGameModeFrameRateForUid(FrameRateOverride{static_cast<uid_t>(uid), frameRate});
- mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId);
+ mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId);
return NO_ERROR;
}
@@ -8926,7 +8927,8 @@
Mutex::Autolock lock(mStateLock);
display->setSecure(connectedLevel >= 2 /* HDCP_V1 */);
}
- mScheduler->onHdcpLevelsChanged(mAppConnectionHandle, displayId, connectedLevel, maxLevel);
+ mScheduler->onHdcpLevelsChanged(scheduler::Cycle::Render, displayId, connectedLevel,
+ maxLevel);
}));
}