SF: Move present timing to FrameTargeter
Split out from I2c27dc709afd1f33bddbf9c2ca1cd61dd335f66c.
Store earliestPresentTime on the FrameTarget, like expectedPresentTime,
so it can be stored per display.
Add FrameTargeter::computeEarliestPresentTime. This calculation was
previously done in SurfaceFlinger, using data mostly contained in the
FrameTargeter. This will simplify computing this per display.
Move computation of the earliestPresentTime to
FrameTargeter::beginFrame. Add a scheduler::Feature to track whether
ExpectedPresentTime is supported.
Make previousFrameVsyncTime and wouldPresentEarly protected, now that
they are only called by FrameTargeter (subclass) and tests.
Make the test a friend of FrameTarget for accessing the above methods.
Bug: 255601557
Bug: 256196556
Bug: 259132483
Test: atest libscheduler_test:FrameTargeterTest
Test: atest
libscheduler_test:FrameTargeterWithExpectedPresentSupportTest
Change-Id: Ib927935de6ba2b7b8d5037b42eb635ae92019634
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 5f772ac..5f10e79 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -192,7 +192,8 @@
.vsyncId = vsyncId,
// TODO(b/255601557): Calculate per display.
.expectedVsyncTime = expectedVsyncTime,
- .sfWorkDuration = mVsyncModulator->getVsyncConfig().sfWorkDuration};
+ .sfWorkDuration = mVsyncModulator->getVsyncConfig().sfWorkDuration,
+ .hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration};
ftl::NonNull<const Display*> pacesetterPtr = pacesetterPtrLocked();
pacesetterPtr->targeterPtr->beginFrame(beginFrameArgs, *pacesetterPtr->schedulePtr);
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 8c77739..f62f1ba 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -514,9 +514,7 @@
: displayId(displayId),
selectorPtr(std::move(selectorPtr)),
schedulePtr(std::move(schedulePtr)),
- targeterPtr(std::make_unique<
- FrameTargeter>(displayId,
- features.test(Feature::kBackpressureGpuComposition))) {}
+ targeterPtr(std::make_unique<FrameTargeter>(displayId, features)) {}
const PhysicalDisplayId displayId;
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/Features.h b/services/surfaceflinger/Scheduler/include/scheduler/Features.h
index 7c72ac6..52485fb 100644
--- a/services/surfaceflinger/Scheduler/include/scheduler/Features.h
+++ b/services/surfaceflinger/Scheduler/include/scheduler/Features.h
@@ -29,6 +29,7 @@
kTracePredictedVsync = 1 << 3,
kBackpressureGpuComposition = 1 << 4,
kSmallDirtyContentDetection = 1 << 5,
+ kExpectedPresentTime = 1 << 6,
};
using FeatureFlags = ftl::Flags<Feature>;
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
index 70d4846..a5bb6c2 100644
--- a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
+++ b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
@@ -19,11 +19,13 @@
#include <array>
#include <atomic>
#include <memory>
+#include <optional>
#include <ui/DisplayId.h>
#include <ui/Fence.h>
#include <ui/FenceTime.h>
+#include <scheduler/Features.h>
#include <scheduler/Time.h>
#include <scheduler/VsyncId.h>
#include <scheduler/interface/CompositeResult.h>
@@ -49,14 +51,11 @@
TimePoint expectedPresentTime() const { return mExpectedPresentTime; }
+ std::optional<TimePoint> earliestPresentTime() const { return mEarliestPresentTime; }
+
// The time of the VSYNC that preceded this frame. See `presentFenceForPastVsync` for details.
TimePoint pastVsyncTime(Period minFramePeriod) const;
- // Equivalent to `pastVsyncTime` unless running N VSYNCs ahead.
- TimePoint previousFrameVsyncTime(Period minFramePeriod) const {
- return mExpectedPresentTime - minFramePeriod;
- }
-
// The present fence for the frame that had targeted the most recent VSYNC before this frame.
// If the target VSYNC for any given frame is more than `vsyncPeriod` in the future, then the
// VSYNC of at least one previous frame has not yet passed. In other words, this is NOT the
@@ -69,8 +68,6 @@
return mPresentFences.front().fenceTime;
}
- bool wouldPresentEarly(Period minFramePeriod) const;
-
bool isFramePending() const { return mFramePending; }
bool didMissFrame() const { return mFrameMissed; }
bool didMissHwcFrame() const { return mHwcFrameMissed && !mGpuFrameMissed; }
@@ -79,9 +76,17 @@
explicit FrameTarget(const std::string& displayLabel);
~FrameTarget() = default;
+ bool wouldPresentEarly(Period minFramePeriod) const;
+
+ // Equivalent to `pastVsyncTime` unless running N VSYNCs ahead.
+ TimePoint previousFrameVsyncTime(Period minFramePeriod) const {
+ return mExpectedPresentTime - minFramePeriod;
+ }
+
VsyncId mVsyncId;
TimePoint mFrameBeginTime;
TimePoint mExpectedPresentTime;
+ std::optional<TimePoint> mEarliestPresentTime;
TracedOrdinal<bool> mFramePending;
TracedOrdinal<bool> mFrameMissed;
@@ -95,6 +100,8 @@
std::array<FenceWithFenceTime, 2> mPresentFences;
private:
+ friend class FrameTargeterTestBase;
+
template <int N>
inline bool targetsVsyncsAhead(Period minFramePeriod) const {
static_assert(N > 1);
@@ -105,9 +112,10 @@
// Computes a display's per-frame metrics about past/upcoming targeting of present deadlines.
class FrameTargeter final : private FrameTarget {
public:
- FrameTargeter(PhysicalDisplayId displayId, bool backpressureGpuComposition)
+ FrameTargeter(PhysicalDisplayId displayId, FeatureFlags flags)
: FrameTarget(to_string(displayId)),
- mBackpressureGpuComposition(backpressureGpuComposition) {}
+ mBackpressureGpuComposition(flags.test(Feature::kBackpressureGpuComposition)),
+ mSupportsExpectedPresentTime(flags.test(Feature::kExpectedPresentTime)) {}
const FrameTarget& target() const { return *this; }
@@ -116,10 +124,14 @@
VsyncId vsyncId;
TimePoint expectedVsyncTime;
Duration sfWorkDuration;
+ Duration hwcMinWorkDuration;
};
void beginFrame(const BeginFrameArgs&, const IVsyncSource&);
+ std::optional<TimePoint> computeEarliestPresentTime(Period minFramePeriod,
+ Duration hwcMinWorkDuration);
+
// TODO(b/241285191): Merge with FrameTargeter::endFrame.
FenceTimePtr setPresentFence(sp<Fence>);
@@ -128,7 +140,7 @@
void dump(utils::Dumper&) const;
private:
- friend class FrameTargeterTest;
+ friend class FrameTargeterTestBase;
// For tests.
using IsFencePendingFuncPtr = bool (*)(const FenceTimePtr&, int graceTimeMs);
@@ -138,6 +150,7 @@
static bool isFencePending(const FenceTimePtr&, int graceTimeMs);
const bool mBackpressureGpuComposition;
+ const bool mSupportsExpectedPresentTime;
TimePoint mScheduledPresentTime;
CompositionCoverageFlags mCompositionCoverage;
diff --git a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
index e80372b..68c277d 100644
--- a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
+++ b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
@@ -82,6 +82,10 @@
}
}
+ if (!mSupportsExpectedPresentTime) {
+ mEarliestPresentTime = computeEarliestPresentTime(minFramePeriod, args.hwcMinWorkDuration);
+ }
+
ATRACE_FORMAT("%s %" PRId64 " vsyncIn %.2fms%s", __func__, ftl::to_underlying(args.vsyncId),
ticks<std::milli, float>(mExpectedPresentTime - TimePoint::now()),
mExpectedPresentTime == args.expectedVsyncTime ? "" : " (adjusted)");
@@ -121,6 +125,14 @@
if (mGpuFrameMissed) mGpuFrameMissedCount++;
}
+std::optional<TimePoint> FrameTargeter::computeEarliestPresentTime(Period minFramePeriod,
+ Duration hwcMinWorkDuration) {
+ if (wouldPresentEarly(minFramePeriod)) {
+ return previousFrameVsyncTime(minFramePeriod) - hwcMinWorkDuration;
+ }
+ return {};
+}
+
void FrameTargeter::endFrame(const CompositeResult& result) {
mCompositionCoverage = result.compositionCoverage;
}
diff --git a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
index c883385..a9abcaf 100644
--- a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
+++ b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
@@ -44,12 +44,18 @@
} // namespace
-class FrameTargeterTest : public testing::Test {
+class FrameTargeterTestBase : public testing::Test {
public:
+ FrameTargeterTestBase(FeatureFlags flags) : mTargeter(PhysicalDisplayId::fromPort(13), flags) {}
+
const auto& target() const { return mTargeter.target(); }
+ bool wouldPresentEarly(Period minFramePeriod) const {
+ return target().wouldPresentEarly(minFramePeriod);
+ }
+
struct Frame {
- Frame(FrameTargeterTest* testPtr, VsyncId vsyncId, TimePoint& frameBeginTime,
+ Frame(FrameTargeterTestBase* testPtr, VsyncId vsyncId, TimePoint& frameBeginTime,
Duration frameDuration, Fps refreshRate, Fps peakRefreshRate,
FrameTargeter::IsFencePendingFuncPtr isFencePendingFuncPtr = Frame::fenceSignaled,
const ftl::Optional<VsyncSource>& vsyncSourceOpt = std::nullopt)
@@ -61,7 +67,8 @@
.vsyncId = vsyncId,
.expectedVsyncTime =
frameBeginTime + frameDuration,
- .sfWorkDuration = 10ms};
+ .sfWorkDuration = 10ms,
+ .hwcMinWorkDuration = kHwcMinWorkDuration};
testPtr->mTargeter.beginFrame(args,
vsyncSourceOpt
@@ -93,7 +100,7 @@
static bool fencePending(const FenceTimePtr&, int) { return true; }
static bool fenceSignaled(const FenceTimePtr&, int) { return false; }
- FrameTargeterTest* const testPtr;
+ FrameTargeterTestBase* const testPtr;
TimePoint& frameBeginTime;
const Period period;
@@ -102,11 +109,24 @@
bool ended = false;
};
+ static constexpr Duration kHwcMinWorkDuration = std::chrono::nanoseconds(5ns);
+
private:
FenceToFenceTimeMap mFenceMap;
- static constexpr bool kBackpressureGpuComposition = true;
- FrameTargeter mTargeter{PhysicalDisplayId::fromPort(13), kBackpressureGpuComposition};
+ FrameTargeter mTargeter;
+};
+
+class FrameTargeterTest : public FrameTargeterTestBase {
+public:
+ FrameTargeterTest() : FrameTargeterTestBase(Feature::kBackpressureGpuComposition) {}
+};
+
+class FrameTargeterWithExpectedPresentSupportTest : public FrameTargeterTestBase {
+public:
+ FrameTargeterWithExpectedPresentSupportTest()
+ : FrameTargeterTestBase(FeatureFlags(Feature::kBackpressureGpuComposition) |
+ Feature::kExpectedPresentTime) {}
};
TEST_F(FrameTargeterTest, targetsFrames) {
@@ -208,7 +228,7 @@
TEST_F(FrameTargeterTest, doesNotDetectEarlyPresentIfNoFence) {
constexpr Period kPeriod = (60_Hz).getPeriod();
EXPECT_EQ(target().presentFenceForPastVsync(kPeriod), FenceTime::NO_FENCE);
- EXPECT_FALSE(target().wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(wouldPresentEarly(kPeriod));
}
TEST_F(FrameTargeterTest, detectsEarlyPresent) {
@@ -220,7 +240,8 @@
// The target is not early while past present fences are pending.
for (int n = 3; n-- > 0;) {
const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
- EXPECT_FALSE(target().wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(target().earliestPresentTime());
}
// The target is early if the past present fence was signaled.
@@ -228,7 +249,41 @@
const auto fence = frame.end();
fence->signalForTest(frameBeginTime.ns());
- EXPECT_TRUE(target().wouldPresentEarly(kPeriod));
+ Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
+
+ // `finalFrame` would present early, so it has an earliest present time.
+ EXPECT_TRUE(wouldPresentEarly(kPeriod));
+ ASSERT_NE(std::nullopt, target().earliestPresentTime());
+ EXPECT_EQ(*target().earliestPresentTime(),
+ target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration);
+}
+
+// Same as `detectsEarlyPresent`, above, but verifies that we do not set an earliest present time
+// when there is expected present time support.
+TEST_F(FrameTargeterWithExpectedPresentSupportTest, detectsEarlyPresent) {
+ VsyncId vsyncId{333};
+ TimePoint frameBeginTime(3000ms);
+ constexpr Fps kRefreshRate = 60_Hz;
+ constexpr Period kPeriod = kRefreshRate.getPeriod();
+
+ // The target is not early while past present fences are pending.
+ for (int n = 3; n-- > 0;) {
+ const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
+ EXPECT_FALSE(wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(target().earliestPresentTime());
+ }
+
+ // The target is early if the past present fence was signaled.
+ Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
+ const auto fence = frame.end();
+ fence->signalForTest(frameBeginTime.ns());
+
+ Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
+
+ // `finalFrame` would present early, but we have expected present time support, so it has no
+ // earliest present time.
+ EXPECT_TRUE(wouldPresentEarly(kPeriod));
+ ASSERT_EQ(std::nullopt, target().earliestPresentTime());
}
TEST_F(FrameTargeterTest, detectsEarlyPresentTwoVsyncsAhead) {
@@ -240,7 +295,8 @@
// The target is not early while past present fences are pending.
for (int n = 3; n-- > 0;) {
const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
- EXPECT_FALSE(target().wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(target().earliestPresentTime());
}
Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
@@ -248,12 +304,18 @@
fence->signalForTest(frameBeginTime.ns());
// The target is two VSYNCs ahead, so the past present fence is still pending.
- EXPECT_FALSE(target().wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(wouldPresentEarly(kPeriod));
+ EXPECT_FALSE(target().earliestPresentTime());
{ const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); }
+ Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate);
+
// The target is early if the past present fence was signaled.
- EXPECT_TRUE(target().wouldPresentEarly(kPeriod));
+ EXPECT_TRUE(wouldPresentEarly(kPeriod));
+ ASSERT_NE(std::nullopt, target().earliestPresentTime());
+ EXPECT_EQ(*target().earliestPresentTime(),
+ target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration);
}
TEST_F(FrameTargeterTest, detectsEarlyPresentThreeVsyncsAhead) {
@@ -264,7 +326,10 @@
const Frame frame(this, VsyncId{555}, frameBeginTime, 16ms, kRefreshRate, kRefreshRate);
// The target is more than two VSYNCs ahead, but present fences are not tracked that far back.
- EXPECT_TRUE(target().wouldPresentEarly(kPeriod));
+ EXPECT_TRUE(wouldPresentEarly(kPeriod));
+ EXPECT_TRUE(target().earliestPresentTime());
+ EXPECT_EQ(*target().earliestPresentTime(),
+ target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration);
}
TEST_F(FrameTargeterTest, detectsMissedFrames) {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4d04aad..eecfabb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2749,24 +2749,13 @@
refreshArgs.devOptFlashDirtyRegionsDelay = std::chrono::milliseconds(mDebugFlashDelay);
}
- const Period minFramePeriod = mScheduler->getVsyncSchedule()->minFramePeriod();
-
- if (!getHwComposer().getComposer()->isSupported(
- Hwc2::Composer::OptionalFeature::ExpectedPresentTime) &&
- pacesetterTarget.wouldPresentEarly(minFramePeriod)) {
- const auto hwcMinWorkDuration =
- mScheduler->getVsyncConfiguration().getCurrentConfigs().hwcMinWorkDuration;
-
- // TODO(b/255601557): Calculate and pass per-display values for each FrameTarget.
- refreshArgs.earliestPresentTime =
- pacesetterTarget.previousFrameVsyncTime(minFramePeriod) - hwcMinWorkDuration;
- }
-
const TimePoint expectedPresentTime = pacesetterTarget.expectedPresentTime();
// TODO(b/255601557) Update frameInterval per display
refreshArgs.frameInterval = mScheduler->getNextFrameInterval(pacesetterId, expectedPresentTime);
refreshArgs.scheduledFrameTime = mScheduler->getScheduledFrameTime();
refreshArgs.expectedPresentTime = expectedPresentTime.ns();
+ // TODO(b/255601557): Calculate and pass per-display values for each FrameTarget.
+ refreshArgs.earliestPresentTime = pacesetterTarget.earliestPresentTime();
refreshArgs.hasTrustedPresentationListener = mNumTrustedPresentationListeners > 0;
{
auto& notifyExpectedPresentData = mNotifyExpectedPresentMap[pacesetterId];
@@ -4218,6 +4207,10 @@
if (mBackpressureGpuComposition) {
features |= Feature::kBackpressureGpuComposition;
}
+ if (getHwComposer().getComposer()->isSupported(
+ Hwc2::Composer::OptionalFeature::ExpectedPresentTime)) {
+ features |= Feature::kExpectedPresentTime;
+ }
mScheduler = std::make_unique<Scheduler>(static_cast<ICompositor&>(*this),
static_cast<ISchedulerCallback&>(*this), features,
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index 04c1977..fa79956 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -606,7 +606,14 @@
mFlinger->commitTransactions();
mFlinger->flushTransactionQueues(getFuzzedVsyncId(mFdp));
- scheduler::FrameTargeter frameTargeter(displayId, mFdp.ConsumeBool());
+ scheduler::FeatureFlags flags;
+ if (mFdp.ConsumeBool()) {
+ flags |= scheduler::Feature::kBackpressureGpuComposition;
+ }
+ if (mFdp.ConsumeBool()) {
+ flags |= scheduler::Feature::kExpectedPresentTime;
+ }
+ scheduler::FrameTargeter frameTargeter(displayId, flags);
mFlinger->onCompositionPresented(displayId, ftl::init::map(displayId, &frameTargeter),
mFdp.ConsumeIntegral<nsecs_t>());
}
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp
index 0a0533c..8a5500f 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp
@@ -426,7 +426,15 @@
}
void SchedulerFuzzer::fuzzFrameTargeter() {
- scheduler::FrameTargeter frameTargeter(kDisplayId, mFdp.ConsumeBool());
+ scheduler::FeatureFlags flags;
+ if (mFdp.ConsumeBool()) {
+ flags |= scheduler::Feature::kBackpressureGpuComposition;
+ }
+ if (mFdp.ConsumeBool()) {
+ flags |= scheduler::Feature::kExpectedPresentTime;
+ }
+
+ scheduler::FrameTargeter frameTargeter(kDisplayId, flags);
const struct VsyncSource final : scheduler::IVsyncSource {
explicit VsyncSource(FuzzedDataProvider& fuzzer) : fuzzer(fuzzer) {}
@@ -442,7 +450,8 @@
frameTargeter.beginFrame({.frameBeginTime = getFuzzedTimePoint(mFdp),
.vsyncId = getFuzzedVsyncId(mFdp),
.expectedVsyncTime = getFuzzedTimePoint(mFdp),
- .sfWorkDuration = getFuzzedDuration(mFdp)},
+ .sfWorkDuration = getFuzzedDuration(mFdp),
+ .hwcMinWorkDuration = getFuzzedDuration(mFdp)},
vsyncSource);
frameTargeter.setPresentFence(makeFakeFence());
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 28549a6..f00eacc 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -386,13 +386,14 @@
LOG_ALWAYS_FATAL_IF(!displayIdOpt);
const auto displayId = *displayIdOpt;
- constexpr bool kBackpressureGpuComposition = true;
- scheduler::FrameTargeter frameTargeter(displayId, kBackpressureGpuComposition);
+ scheduler::FrameTargeter frameTargeter(displayId,
+ scheduler::Feature::kBackpressureGpuComposition);
frameTargeter.beginFrame({.frameBeginTime = frameTime,
.vsyncId = vsyncId,
.expectedVsyncTime = expectedVsyncTime,
- .sfWorkDuration = 10ms},
+ .sfWorkDuration = 10ms,
+ .hwcMinWorkDuration = 10ms},
*mScheduler->getVsyncSchedule());
scheduler::FrameTargets targets;