Revert "SF: Encapsulate frame targeting"
This reverts commit c183eed053289c242c32c5c35a7071863165a61d.
Reason for revert: Regression in SF performance.
Bug: 281884106
Test: Jank suites
Change-Id: I05adaa57c955a8c09c210943bc06977b48362aa4
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 06d6ef0..736b752 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -77,7 +77,6 @@
#include <processgroup/processgroup.h>
#include <renderengine/RenderEngine.h>
#include <renderengine/impl/ExternalTexture.h>
-#include <scheduler/FrameTargeter.h>
#include <sys/types.h>
#include <ui/ColorSpace.h>
#include <ui/DataspaceUtils.h>
@@ -2140,6 +2139,44 @@
}
}
+bool SurfaceFlinger::wouldPresentEarly(TimePoint frameTime, Period vsyncPeriod) const {
+ const bool isThreeVsyncsAhead = mExpectedPresentTime - frameTime > 2 * vsyncPeriod;
+ return isThreeVsyncsAhead ||
+ getPreviousPresentFence(frameTime, vsyncPeriod)->getSignalTime() !=
+ Fence::SIGNAL_TIME_PENDING;
+}
+
+auto SurfaceFlinger::getPreviousPresentFence(TimePoint frameTime, Period vsyncPeriod) const
+ -> const FenceTimePtr& {
+ const bool isTwoVsyncsAhead = mExpectedPresentTime - frameTime > vsyncPeriod;
+ const size_t i = static_cast<size_t>(isTwoVsyncsAhead);
+ return mPreviousPresentFences[i].fenceTime;
+}
+
+bool SurfaceFlinger::isFencePending(const FenceTimePtr& fence, int graceTimeMs) {
+ ATRACE_CALL();
+ if (fence == FenceTime::NO_FENCE) {
+ return false;
+ }
+
+ const status_t status = fence->wait(graceTimeMs);
+ // This is the same as Fence::Status::Unsignaled, but it saves a getStatus() call,
+ // which calls wait(0) again internally
+ return status == -ETIME;
+}
+
+TimePoint SurfaceFlinger::calculateExpectedPresentTime(TimePoint frameTime) const {
+ const auto& schedule = mScheduler->getVsyncSchedule();
+
+ const TimePoint vsyncDeadline = schedule->vsyncDeadlineAfter(frameTime);
+ if (mScheduler->vsyncModulator().getVsyncConfig().sfOffset > 0) {
+ return vsyncDeadline;
+ }
+
+ // Inflate the expected present time if we're targeting the next vsync.
+ return vsyncDeadline + schedule->period();
+}
+
void SurfaceFlinger::configure() FTL_FAKE_GUARD(kMainThreadContext) {
Mutex::Autolock lock(mStateLock);
if (configureLocked()) {
@@ -2313,15 +2350,75 @@
return mustComposite;
}
-bool SurfaceFlinger::commit(const scheduler::FrameTarget& pacesetterFrameTarget)
+bool SurfaceFlinger::commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expectedVsyncTime)
FTL_FAKE_GUARD(kMainThreadContext) {
- const VsyncId vsyncId = pacesetterFrameTarget.vsyncId();
- ATRACE_NAME(ftl::Concat(__func__, ' ', ftl::to_underlying(vsyncId)).c_str());
+ // The expectedVsyncTime, which was predicted when this frame was scheduled, is normally in the
+ // future relative to frameTime, but may not be for delayed frames. Adjust mExpectedPresentTime
+ // accordingly, but not mScheduledPresentTime.
+ const TimePoint lastScheduledPresentTime = mScheduledPresentTime;
+ mScheduledPresentTime = expectedVsyncTime;
- if (pacesetterFrameTarget.didMissFrame()) {
+ // Calculate the expected present time once and use the cached value throughout this frame to
+ // make sure all layers are seeing this same value.
+ mExpectedPresentTime = expectedVsyncTime >= frameTime ? expectedVsyncTime
+ : calculateExpectedPresentTime(frameTime);
+
+ ATRACE_FORMAT("%s %" PRId64 " vsyncIn %.2fms%s", __func__, ftl::to_underlying(vsyncId),
+ ticks<std::milli, float>(mExpectedPresentTime - TimePoint::now()),
+ mExpectedPresentTime == expectedVsyncTime ? "" : " (adjusted)");
+
+ const Period vsyncPeriod = mScheduler->getVsyncSchedule()->period();
+ const FenceTimePtr& previousPresentFence = getPreviousPresentFence(frameTime, vsyncPeriod);
+
+ // When backpressure propagation is enabled, we want to give a small grace period of 1ms
+ // for the present fence to fire instead of just giving up on this frame to handle cases
+ // where present fence is just about to get signaled.
+ const int graceTimeForPresentFenceMs = static_cast<int>(
+ mBackpressureGpuComposition || !mCompositionCoverage.test(CompositionCoverage::Gpu));
+
+ // Pending frames may trigger backpressure propagation.
+ const TracedOrdinal<bool> framePending = {"PrevFramePending",
+ isFencePending(previousPresentFence,
+ graceTimeForPresentFenceMs)};
+
+ // Frame missed counts for metrics tracking.
+ // A frame is missed if the prior frame is still pending. If no longer pending,
+ // then we still count the frame as missed if the predicted present time
+ // was further in the past than when the fence actually fired.
+
+ // Add some slop to correct for drift. This should generally be
+ // smaller than a typical frame duration, but should not be so small
+ // that it reports reasonable drift as a missed frame.
+ const nsecs_t frameMissedSlop = vsyncPeriod.ns() / 2;
+ const nsecs_t previousPresentTime = previousPresentFence->getSignalTime();
+ const TracedOrdinal<bool> frameMissed = {"PrevFrameMissed",
+ framePending ||
+ (previousPresentTime >= 0 &&
+ (lastScheduledPresentTime.ns() <
+ previousPresentTime - frameMissedSlop))};
+ const TracedOrdinal<bool> hwcFrameMissed = {"PrevHwcFrameMissed",
+ frameMissed &&
+ mCompositionCoverage.test(
+ CompositionCoverage::Hwc)};
+
+ const TracedOrdinal<bool> gpuFrameMissed = {"PrevGpuFrameMissed",
+ frameMissed &&
+ mCompositionCoverage.test(
+ CompositionCoverage::Gpu)};
+
+ if (frameMissed) {
+ mFrameMissedCount++;
mTimeStats->incrementMissedFrames();
}
+ if (hwcFrameMissed) {
+ mHwcFrameMissedCount++;
+ }
+
+ if (gpuFrameMissed) {
+ mGpuFrameMissedCount++;
+ }
+
if (mTracingEnabledChanged) {
mLayerTracingEnabled = mLayerTracing.isEnabled();
mTracingEnabledChanged = false;
@@ -2330,7 +2427,7 @@
// If we are in the middle of a mode change and the fence hasn't
// fired yet just wait for the next commit.
if (mSetActiveModePending) {
- if (pacesetterFrameTarget.isFramePending()) {
+ if (framePending) {
mScheduler->scheduleFrame();
return false;
}
@@ -2344,29 +2441,26 @@
}
}
- if (pacesetterFrameTarget.isFramePending()) {
- if (mBackpressureGpuComposition || pacesetterFrameTarget.didMissHwcFrame()) {
+ if (framePending) {
+ if (mBackpressureGpuComposition || (hwcFrameMissed && !gpuFrameMissed)) {
scheduleCommit(FrameHint::kNone);
return false;
}
}
- const Period vsyncPeriod = mScheduler->getVsyncSchedule()->period();
-
// Save this once per commit + composite to ensure consistency
// TODO (b/240619471): consider removing active display check once AOD is fixed
const auto activeDisplay = FTL_FAKE_GUARD(mStateLock, getDisplayDeviceLocked(mActiveDisplayId));
mPowerHintSessionEnabled = mPowerAdvisor->usePowerHintSession() && activeDisplay &&
activeDisplay->getPowerMode() == hal::PowerMode::ON;
if (mPowerHintSessionEnabled) {
- mPowerAdvisor->setCommitStart(pacesetterFrameTarget.frameBeginTime());
- mPowerAdvisor->setExpectedPresentTime(pacesetterFrameTarget.expectedPresentTime());
+ mPowerAdvisor->setCommitStart(frameTime);
+ mPowerAdvisor->setExpectedPresentTime(mExpectedPresentTime);
// Frame delay is how long we should have minus how long we actually have.
const Duration idealSfWorkDuration =
mScheduler->vsyncModulator().getVsyncConfig().sfWorkDuration;
- const Duration frameDelay =
- idealSfWorkDuration - pacesetterFrameTarget.expectedFrameDuration();
+ const Duration frameDelay = idealSfWorkDuration - (mExpectedPresentTime - frameTime);
mPowerAdvisor->setFrameDelay(frameDelay);
mPowerAdvisor->setTotalFrameTargetWorkDuration(idealSfWorkDuration);
@@ -2383,8 +2477,7 @@
// Composite if transactions were committed, or if requested by HWC.
bool mustComposite = mMustComposite.exchange(false);
{
- mFrameTimeline->setSfWakeUp(ftl::to_underlying(vsyncId),
- pacesetterFrameTarget.frameBeginTime().ns(),
+ mFrameTimeline->setSfWakeUp(ftl::to_underlying(vsyncId), frameTime.ns(),
Fps::fromPeriodNsecs(vsyncPeriod.ns()));
const bool flushTransactions = clearTransactionFlags(eTransactionFlushNeeded);
@@ -2392,11 +2485,10 @@
if (flushTransactions) {
updates = flushLifecycleUpdates();
if (mTransactionTracing) {
- mTransactionTracing
- ->addCommittedTransactions(ftl::to_underlying(vsyncId),
- pacesetterFrameTarget.frameBeginTime().ns(),
- updates, mFrontEndDisplayInfos,
- mFrontEndDisplayInfosChanged);
+ mTransactionTracing->addCommittedTransactions(ftl::to_underlying(vsyncId),
+ frameTime.ns(), updates,
+ mFrontEndDisplayInfos,
+ mFrontEndDisplayInfosChanged);
}
}
bool transactionsAreEmpty;
@@ -2439,7 +2531,7 @@
if (mLayerTracingEnabled && !mLayerTracing.flagIsSet(LayerTracing::TRACE_COMPOSITION)) {
// This will block and tracing should only be enabled for debugging.
- addToLayerTracing(mVisibleRegionsDirty, pacesetterFrameTarget.frameBeginTime(), vsyncId);
+ addToLayerTracing(mVisibleRegionsDirty, frameTime, vsyncId);
}
mLastCommittedVsyncId = vsyncId;
@@ -2448,11 +2540,8 @@
return mustComposite && CC_LIKELY(mBootStage != BootStage::BOOTLOADER);
}
-CompositeResult SurfaceFlinger::composite(scheduler::FrameTargeter& pacesetterFrameTargeter)
+void SurfaceFlinger::composite(TimePoint frameTime, VsyncId vsyncId)
FTL_FAKE_GUARD(kMainThreadContext) {
- const scheduler::FrameTarget& pacesetterFrameTarget = pacesetterFrameTargeter.target();
-
- const VsyncId vsyncId = pacesetterFrameTarget.vsyncId();
ATRACE_NAME(ftl::Concat(__func__, ' ', ftl::to_underlying(vsyncId)).c_str());
compositionengine::CompositionRefreshArgs refreshArgs;
@@ -2460,18 +2549,17 @@
refreshArgs.outputs.reserve(displays.size());
std::vector<DisplayId> displayIds;
for (const auto& [_, display] : displays) {
- displayIds.push_back(display->getId());
- display->tracePowerMode();
-
+ bool dropFrame = false;
if (display->isVirtual()) {
- const Fps refreshRate = display->getAdjustedRefreshRate();
- if (refreshRate.isValid() &&
- !mScheduler->isVsyncInPhase(pacesetterFrameTarget.frameBeginTime(), refreshRate)) {
- continue;
- }
+ Fps refreshRate = display->getAdjustedRefreshRate();
+ using fps_approx_ops::operator>;
+ dropFrame = (refreshRate > 0_Hz) && !mScheduler->isVsyncInPhase(frameTime, refreshRate);
}
-
- refreshArgs.outputs.push_back(display->getCompositionDisplay());
+ if (!dropFrame) {
+ refreshArgs.outputs.push_back(display->getCompositionDisplay());
+ }
+ display->tracePowerMode();
+ displayIds.push_back(display->getId());
}
mPowerAdvisor->setDisplays(displayIds);
@@ -2531,15 +2619,15 @@
if (!getHwComposer().getComposer()->isSupported(
Hwc2::Composer::OptionalFeature::ExpectedPresentTime) &&
- pacesetterFrameTarget.wouldPresentEarly(vsyncPeriod)) {
+ wouldPresentEarly(frameTime, vsyncPeriod)) {
+ const auto prevVsyncTime = mExpectedPresentTime - vsyncPeriod;
const auto hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration;
- refreshArgs.earliestPresentTime =
- pacesetterFrameTarget.pastVsyncTime(vsyncPeriod) - hwcMinWorkDuration;
+ refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration;
}
refreshArgs.scheduledFrameTime = mScheduler->getScheduledFrameTime();
- refreshArgs.expectedPresentTime = pacesetterFrameTarget.expectedPresentTime().ns();
+ refreshArgs.expectedPresentTime = mExpectedPresentTime.ns();
refreshArgs.hasTrustedPresentationListener = mNumTrustedPresentationListeners > 0;
// Store the present time just before calling to the composition engine so we could notify
@@ -2565,12 +2653,13 @@
}
}
- mTimeStats->recordFrameDuration(pacesetterFrameTarget.frameBeginTime().ns(), systemTime());
+ mTimeStats->recordFrameDuration(frameTime.ns(), systemTime());
// Send a power hint hint after presentation is finished
if (mPowerHintSessionEnabled) {
const nsecs_t pastPresentTime =
- pacesetterFrameTarget.presentFenceForPastVsync(vsyncPeriod)->getSignalTime();
+ getPreviousPresentFence(frameTime, vsyncPeriod)->getSignalTime();
+
mPowerAdvisor->setSfPresentTiming(TimePoint::fromNs(pastPresentTime), TimePoint::now());
mPowerAdvisor->reportActualWorkDuration();
}
@@ -2579,7 +2668,7 @@
scheduleComposite(FrameHint::kNone);
}
- postComposition(pacesetterFrameTargeter, presentTime);
+ postComposition(presentTime);
const bool hadGpuComposited = mCompositionCoverage.test(CompositionCoverage::Gpu);
mCompositionCoverage.clear();
@@ -2622,7 +2711,7 @@
mLayersWithQueuedFrames.clear();
if (mLayerTracingEnabled && mLayerTracing.flagIsSet(LayerTracing::TRACE_COMPOSITION)) {
// This will block and should only be used for debugging.
- addToLayerTracing(mVisibleRegionsDirty, pacesetterFrameTarget.frameBeginTime(), vsyncId);
+ addToLayerTracing(mVisibleRegionsDirty, frameTime, vsyncId);
}
if (mVisibleRegionsDirty) mHdrLayerInfoChanged = true;
@@ -2635,8 +2724,6 @@
if (mPowerHintSessionEnabled) {
mPowerAdvisor->setCompositeEnd(TimePoint::now());
}
-
- return {mCompositionCoverage};
}
void SurfaceFlinger::updateLayerGeometry() {
@@ -2720,8 +2807,7 @@
return ui::ROTATION_0;
}
-void SurfaceFlinger::postComposition(scheduler::FrameTargeter& pacesetterFrameTargeter,
- nsecs_t presentStartTime) {
+void SurfaceFlinger::postComposition(nsecs_t callTime) {
ATRACE_CALL();
ALOGV(__func__);
@@ -2738,11 +2824,15 @@
glCompositionDoneFenceTime = FenceTime::NO_FENCE;
}
+ mPreviousPresentFences[1] = mPreviousPresentFences[0];
+
auto presentFence = defaultDisplay
? getHwComposer().getPresentFence(defaultDisplay->getPhysicalId())
: Fence::NO_FENCE;
- auto presentFenceTime = pacesetterFrameTargeter.setPresentFence(presentFence);
+ auto presentFenceTime = std::make_shared<FenceTime>(presentFence);
+ mPreviousPresentFences[0] = {presentFence, presentFenceTime};
+
const TimePoint presentTime = TimePoint::now();
// Set presentation information before calling Layer::releasePendingBuffer, such that jank
@@ -2925,7 +3015,7 @@
if (!layer->hasTrustedPresentationListener()) {
return;
}
- const frontend::LayerSnapshot* snapshot = mLayerLifecycleManagerEnabled
+ const frontend::LayerSnapshot* snapshot = (mLayerLifecycleManagerEnabled)
? mLayerSnapshotBuilder.getSnapshot(layer->sequence)
: layer->getLayerSnapshot();
std::optional<const DisplayDevice*> displayOpt = std::nullopt;
@@ -2934,8 +3024,7 @@
}
const DisplayDevice* display = displayOpt.value_or(nullptr);
layer->updateTrustedPresentationState(display, snapshot,
- nanoseconds_to_milliseconds(presentStartTime),
- false);
+ nanoseconds_to_milliseconds(callTime), false);
});
}
@@ -3847,9 +3936,6 @@
if (display->refreshRateSelector().kernelIdleTimerController()) {
features |= Feature::kKernelIdleTimer;
}
- if (mBackpressureGpuComposition) {
- features |= Feature::kBackpressureGpuComposition;
- }
auto modulatorPtr = sp<VsyncModulator>::make(mVsyncConfiguration->getCurrentConfigs());
@@ -4167,38 +4253,33 @@
TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyTimelineCheck(
const TransactionHandler::TransactionFlushState& flushState) {
- const auto& transaction = *flushState.transaction;
-
- const TimePoint desiredPresentTime = TimePoint::fromNs(transaction.desiredPresentTime);
- const TimePoint expectedPresentTime = mScheduler->pacesetterFrameTarget().expectedPresentTime();
-
using TransactionReadiness = TransactionHandler::TransactionReadiness;
-
+ const auto& transaction = *flushState.transaction;
+ TimePoint desiredPresentTime = TimePoint::fromNs(transaction.desiredPresentTime);
// Do not present if the desiredPresentTime has not passed unless it is more than
// one second in the future. We ignore timestamps more than 1 second in the future
// for stability reasons.
- if (!transaction.isAutoTimestamp && desiredPresentTime >= expectedPresentTime &&
- desiredPresentTime < expectedPresentTime + 1s) {
+ if (!transaction.isAutoTimestamp && desiredPresentTime >= mExpectedPresentTime &&
+ desiredPresentTime < mExpectedPresentTime + 1s) {
ATRACE_FORMAT("not current desiredPresentTime: %" PRId64 " expectedPresentTime: %" PRId64,
- desiredPresentTime, expectedPresentTime);
+ desiredPresentTime, mExpectedPresentTime);
return TransactionReadiness::NotReady;
}
- if (!mScheduler->isVsyncValid(expectedPresentTime, transaction.originUid)) {
- ATRACE_FORMAT("!isVsyncValid expectedPresentTime: %" PRId64 " uid: %d", expectedPresentTime,
- transaction.originUid);
+ if (!mScheduler->isVsyncValid(mExpectedPresentTime, transaction.originUid)) {
+ ATRACE_FORMAT("!isVsyncValid expectedPresentTime: %" PRId64 " uid: %d",
+ mExpectedPresentTime, transaction.originUid);
return TransactionReadiness::NotReady;
}
// If the client didn't specify desiredPresentTime, use the vsyncId to determine the
// expected present time of this transaction.
if (transaction.isAutoTimestamp &&
- frameIsEarly(expectedPresentTime, VsyncId{transaction.frameTimelineInfo.vsyncId})) {
+ frameIsEarly(mExpectedPresentTime, VsyncId{transaction.frameTimelineInfo.vsyncId})) {
ATRACE_FORMAT("frameIsEarly vsyncId: %" PRId64 " expectedPresentTime: %" PRId64,
- transaction.frameTimelineInfo.vsyncId, expectedPresentTime);
+ transaction.frameTimelineInfo.vsyncId, mExpectedPresentTime);
return TransactionReadiness::NotReady;
}
-
return TransactionReadiness::Ready;
}
@@ -5942,6 +6023,10 @@
dumpVsync(result);
result.append("\n");
+ StringAppendF(&result, "Total missed frame count: %u\n", mFrameMissedCount.load());
+ StringAppendF(&result, "HWC missed frame count: %u\n", mHwcFrameMissedCount.load());
+ StringAppendF(&result, "GPU missed frame count: %u\n\n", mGpuFrameMissedCount.load());
+
/*
* Dump the visible layer list
*/