SF: Decouple VsyncModulator from Scheduler
Let VsyncModulator return phase offsets to SurfaceFlinger, and let the
latter propagate them to Scheduler.
Clean up identifiers and comments for consistency and regularity, and
fix uninitialized atomics.
Parametrize clock to avoid sleeping in tests, and minimize boilerplate
in test cases.
Bug: 160012986
Test: systrace with debug.sf.vsync_trace_detailed_info
Test: libsurfaceflinger_unittest
Change-Id: I05a4279592d38fdd933aad48118b2de0135cd0ea
diff --git a/services/surfaceflinger/Scheduler/PhaseOffsets.h b/services/surfaceflinger/Scheduler/PhaseOffsets.h
index e395b42..0ae9fef 100644
--- a/services/surfaceflinger/Scheduler/PhaseOffsets.h
+++ b/services/surfaceflinger/Scheduler/PhaseOffsets.h
@@ -31,7 +31,7 @@
*/
class PhaseConfiguration {
public:
- using Offsets = VSyncModulator::OffsetsConfig;
+ using Offsets = VsyncModulator::OffsetsConfig;
virtual ~PhaseConfiguration();
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 6a5082a..4e7a9a1 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -60,28 +60,13 @@
~ISchedulerCallback() = default;
};
-struct IPhaseOffsetControl {
- virtual void setPhaseOffset(scheduler::ConnectionHandle, nsecs_t phaseOffset) = 0;
-
-protected:
- ~IPhaseOffsetControl() = default;
-};
-
-class Scheduler : public IPhaseOffsetControl {
+class Scheduler {
public:
using RefreshRate = scheduler::RefreshRateConfigs::RefreshRate;
using ConfigEvent = scheduler::RefreshRateConfigEvent;
- // Indicates whether to start the transaction early, or at vsync time.
- enum class TransactionStart {
- Early, // DEPRECATED. Start the transaction early. Times out on its own
- EarlyStart, // Start the transaction early and keep this config until EarlyEnd
- EarlyEnd, // End the early config started at EarlyStart
- Normal // Start the transaction at the normal time
- };
-
Scheduler(const scheduler::RefreshRateConfigs&, ISchedulerCallback&);
- virtual ~Scheduler();
+ ~Scheduler();
DispSync& getPrimaryDispSync();
@@ -104,7 +89,7 @@
void onScreenReleased(ConnectionHandle);
// Modifies phase offset in the event thread.
- void setPhaseOffset(ConnectionHandle, nsecs_t phaseOffset) override;
+ void setPhaseOffset(ConnectionHandle, nsecs_t phaseOffset);
void getDisplayStatInfo(DisplayStatInfo* stats);
diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.cpp b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
index 96d47ad..7a1b7e4 100644
--- a/services/surfaceflinger/Scheduler/VsyncModulator.cpp
+++ b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
@@ -14,55 +14,51 @@
* limitations under the License.
*/
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
+#undef LOG_TAG
+#define LOG_TAG "VsyncModulator"
+
#include "VsyncModulator.h"
-#include <cutils/properties.h>
+#include <android-base/properties.h>
+#include <log/log.h>
#include <utils/Trace.h>
#include <chrono>
#include <cinttypes>
#include <mutex>
+using namespace std::chrono_literals;
+
namespace android::scheduler {
-VSyncModulator::VSyncModulator(IPhaseOffsetControl& phaseOffsetControl,
- Scheduler::ConnectionHandle appConnectionHandle,
- Scheduler::ConnectionHandle sfConnectionHandle,
- const OffsetsConfig& config)
- : mPhaseOffsetControl(phaseOffsetControl),
- mAppConnectionHandle(appConnectionHandle),
- mSfConnectionHandle(sfConnectionHandle),
- mOffsetsConfig(config) {
- char value[PROPERTY_VALUE_MAX];
- property_get("debug.sf.vsync_trace_detailed_info", value, "0");
- mTraceDetailedInfo = atoi(value);
-}
+const std::chrono::nanoseconds VsyncModulator::MIN_EARLY_TRANSACTION_TIME = 1ms;
-void VSyncModulator::setPhaseOffsets(const OffsetsConfig& config) {
+VsyncModulator::VsyncModulator(const OffsetsConfig& config, Now now)
+ : mOffsetsConfig(config),
+ mNow(now),
+ mTraceDetailedInfo(base::GetBoolProperty("debug.sf.vsync_trace_detailed_info", false)) {}
+
+VsyncModulator::Offsets VsyncModulator::setPhaseOffsets(const OffsetsConfig& config) {
std::lock_guard<std::mutex> lock(mMutex);
mOffsetsConfig = config;
- updateOffsetsLocked();
+ return updateOffsetsLocked();
}
-void VSyncModulator::setTransactionStart(Scheduler::TransactionStart transactionStart) {
- switch (transactionStart) {
- case Scheduler::TransactionStart::EarlyStart:
- ALOGW_IF(mExplicitEarlyWakeup, "Already in TransactionStart::EarlyStart");
+VsyncModulator::OffsetsOpt VsyncModulator::setTransactionSchedule(TransactionSchedule schedule) {
+ switch (schedule) {
+ case Schedule::EarlyStart:
+ ALOGW_IF(mExplicitEarlyWakeup, "%s: Duplicate EarlyStart", __FUNCTION__);
mExplicitEarlyWakeup = true;
break;
- case Scheduler::TransactionStart::EarlyEnd:
- ALOGW_IF(!mExplicitEarlyWakeup, "Not in TransactionStart::EarlyStart");
+ case Schedule::EarlyEnd:
+ ALOGW_IF(!mExplicitEarlyWakeup, "%s: Unexpected EarlyEnd", __FUNCTION__);
mExplicitEarlyWakeup = false;
break;
- case Scheduler::TransactionStart::Normal:
- case Scheduler::TransactionStart::Early:
- // Non explicit don't change the explicit early wakeup state
+ case Schedule::Early:
+ case Schedule::Late:
+ // No change to mExplicitEarlyWakeup for non-explicit states.
break;
}
@@ -70,114 +66,98 @@
ATRACE_INT("mExplicitEarlyWakeup", mExplicitEarlyWakeup);
}
- if (!mExplicitEarlyWakeup &&
- (transactionStart == Scheduler::TransactionStart::Early ||
- transactionStart == Scheduler::TransactionStart::EarlyEnd)) {
- mRemainingEarlyFrameCount = MIN_EARLY_FRAME_COUNT_TRANSACTION;
- mEarlyTxnStartTime = std::chrono::steady_clock::now();
+ if (!mExplicitEarlyWakeup && (schedule == Schedule::Early || schedule == Schedule::EarlyEnd)) {
+ mEarlyTransactionFrames = MIN_EARLY_TRANSACTION_FRAMES;
+ mEarlyTransactionStartTime = mNow();
}
// An early transaction stays an early transaction.
- if (transactionStart == mTransactionStart ||
- mTransactionStart == Scheduler::TransactionStart::EarlyEnd) {
- return;
+ if (schedule == mTransactionSchedule || mTransactionSchedule == Schedule::EarlyEnd) {
+ return std::nullopt;
}
- mTransactionStart = transactionStart;
- updateOffsets();
+ mTransactionSchedule = schedule;
+ return updateOffsets();
}
-void VSyncModulator::onTransactionHandled() {
- mTxnAppliedTime = std::chrono::steady_clock::now();
- if (mTransactionStart == Scheduler::TransactionStart::Normal) return;
- mTransactionStart = Scheduler::TransactionStart::Normal;
- updateOffsets();
+VsyncModulator::OffsetsOpt VsyncModulator::onTransactionCommit() {
+ mLastTransactionCommitTime = mNow();
+ if (mTransactionSchedule == Schedule::Late) return std::nullopt;
+ mTransactionSchedule = Schedule::Late;
+ return updateOffsets();
}
-void VSyncModulator::onRefreshRateChangeInitiated() {
- if (mRefreshRateChangePending) {
- return;
- }
+VsyncModulator::OffsetsOpt VsyncModulator::onRefreshRateChangeInitiated() {
+ if (mRefreshRateChangePending) return std::nullopt;
mRefreshRateChangePending = true;
- updateOffsets();
+ return updateOffsets();
}
-void VSyncModulator::onRefreshRateChangeCompleted() {
- if (!mRefreshRateChangePending) {
- return;
- }
+VsyncModulator::OffsetsOpt VsyncModulator::onRefreshRateChangeCompleted() {
+ if (!mRefreshRateChangePending) return std::nullopt;
mRefreshRateChangePending = false;
- updateOffsets();
+ return updateOffsets();
}
-void VSyncModulator::onRefreshed(bool usedRenderEngine) {
+VsyncModulator::OffsetsOpt VsyncModulator::onDisplayRefresh(bool usedGpuComposition) {
bool updateOffsetsNeeded = false;
- // Apply a margin to account for potential data races
- // This might make us stay in early offsets for one
- // additional frame but it's better to be conservative here.
- if ((mEarlyTxnStartTime.load() + MARGIN_FOR_TX_APPLY) < mTxnAppliedTime.load()) {
- if (mRemainingEarlyFrameCount > 0) {
- mRemainingEarlyFrameCount--;
+ if (mEarlyTransactionStartTime.load() + MIN_EARLY_TRANSACTION_TIME <=
+ mLastTransactionCommitTime.load()) {
+ if (mEarlyTransactionFrames > 0) {
+ mEarlyTransactionFrames--;
updateOffsetsNeeded = true;
}
}
- if (usedRenderEngine) {
- mRemainingRenderEngineUsageCount = MIN_EARLY_GL_FRAME_COUNT_TRANSACTION;
+ if (usedGpuComposition) {
+ mEarlyGpuFrames = MIN_EARLY_GPU_FRAMES;
updateOffsetsNeeded = true;
- } else if (mRemainingRenderEngineUsageCount > 0) {
- mRemainingRenderEngineUsageCount--;
+ } else if (mEarlyGpuFrames > 0) {
+ mEarlyGpuFrames--;
updateOffsetsNeeded = true;
}
- if (updateOffsetsNeeded) {
- updateOffsets();
- }
+
+ if (!updateOffsetsNeeded) return std::nullopt;
+ return updateOffsets();
}
-VSyncModulator::Offsets VSyncModulator::getOffsets() const {
+VsyncModulator::Offsets VsyncModulator::getOffsets() const {
std::lock_guard<std::mutex> lock(mMutex);
return mOffsets;
}
-const VSyncModulator::Offsets& VSyncModulator::getNextOffsets() const {
+const VsyncModulator::Offsets& VsyncModulator::getNextOffsets() const {
// Early offsets are used if we're in the middle of a refresh rate
// change, or if we recently begin a transaction.
- if (mExplicitEarlyWakeup || mTransactionStart == Scheduler::TransactionStart::EarlyEnd ||
- mRemainingEarlyFrameCount > 0 || mRefreshRateChangePending) {
+ if (mExplicitEarlyWakeup || mTransactionSchedule == Schedule::EarlyEnd ||
+ mEarlyTransactionFrames > 0 || mRefreshRateChangePending) {
return mOffsetsConfig.early;
- } else if (mRemainingRenderEngineUsageCount > 0) {
- return mOffsetsConfig.earlyGl;
+ } else if (mEarlyGpuFrames > 0) {
+ return mOffsetsConfig.earlyGpu;
} else {
return mOffsetsConfig.late;
}
}
-void VSyncModulator::updateOffsets() {
+VsyncModulator::Offsets VsyncModulator::updateOffsets() {
std::lock_guard<std::mutex> lock(mMutex);
- updateOffsetsLocked();
+ return updateOffsetsLocked();
}
-void VSyncModulator::updateOffsetsLocked() {
+VsyncModulator::Offsets VsyncModulator::updateOffsetsLocked() {
const Offsets& offsets = getNextOffsets();
-
- mPhaseOffsetControl.setPhaseOffset(mSfConnectionHandle, offsets.sf);
- mPhaseOffsetControl.setPhaseOffset(mAppConnectionHandle, offsets.app);
-
mOffsets = offsets;
- if (!mTraceDetailedInfo) {
- return;
+ if (mTraceDetailedInfo) {
+ const bool isEarly = &offsets == &mOffsetsConfig.early;
+ const bool isEarlyGpu = &offsets == &mOffsetsConfig.earlyGpu;
+ const bool isLate = &offsets == &mOffsetsConfig.late;
+
+ ATRACE_INT("Vsync-EarlyOffsetsOn", isEarly);
+ ATRACE_INT("Vsync-EarlyGpuOffsetsOn", isEarlyGpu);
+ ATRACE_INT("Vsync-LateOffsetsOn", isLate);
}
- const bool isEarly = &offsets == &mOffsetsConfig.early;
- const bool isEarlyGl = &offsets == &mOffsetsConfig.earlyGl;
- const bool isLate = &offsets == &mOffsetsConfig.late;
-
- ATRACE_INT("Vsync-EarlyOffsetsOn", isEarly);
- ATRACE_INT("Vsync-EarlyGLOffsetsOn", isEarlyGl);
- ATRACE_INT("Vsync-LateOffsetsOn", isLate);
+ return offsets;
}
} // namespace android::scheduler
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion"
diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.h b/services/surfaceflinger/Scheduler/VsyncModulator.h
index ab678c9..f920bd2 100644
--- a/services/surfaceflinger/Scheduler/VsyncModulator.h
+++ b/services/surfaceflinger/Scheduler/VsyncModulator.h
@@ -18,108 +18,106 @@
#include <chrono>
#include <mutex>
+#include <optional>
-#include "Scheduler.h"
+#include <android-base/thread_annotations.h>
+#include <utils/Timers.h>
namespace android::scheduler {
-/*
- * Modulates the vsync-offsets depending on current SurfaceFlinger state.
- */
-class VSyncModulator {
-private:
- // Number of frames we'll keep the early phase offsets once they are activated for a
- // transaction. This acts as a low-pass filter in case the client isn't quick enough in
- // sending new transactions.
- static constexpr int MIN_EARLY_FRAME_COUNT_TRANSACTION = 2;
+// State machine controlled by transaction flags. VsyncModulator switches to early phase offsets
+// when a transaction is flagged EarlyStart or Early, lasting until an EarlyEnd transaction or a
+// fixed number of frames, respectively.
+enum class TransactionSchedule {
+ Late, // Default.
+ Early, // Deprecated.
+ EarlyStart,
+ EarlyEnd
+};
- // Number of frames we'll keep the early gl phase offsets once they are activated.
- // This acts as a low-pass filter to avoid scenarios where we rapidly
- // switch in and out of gl composition.
- static constexpr int MIN_EARLY_GL_FRAME_COUNT_TRANSACTION = 2;
-
- // Margin used to account for potential data races
- static const constexpr std::chrono::nanoseconds MARGIN_FOR_TX_APPLY = 1ms;
-
+// Modulates VSYNC phase depending on transaction schedule and refresh rate changes.
+class VsyncModulator {
public:
- // Wrapper for a collection of surfaceflinger/app offsets for a particular
- // configuration.
+ // Number of frames to keep early offsets after an early transaction or GPU composition.
+ // This acts as a low-pass filter in case subsequent transactions are delayed, or if the
+ // composition strategy alternates on subsequent frames.
+ static constexpr int MIN_EARLY_TRANSACTION_FRAMES = 2;
+ static constexpr int MIN_EARLY_GPU_FRAMES = 2;
+
+ // Duration to delay the MIN_EARLY_TRANSACTION_FRAMES countdown after an early transaction.
+ // This may keep early offsets for an extra frame, but avoids a race with transaction commit.
+ static const std::chrono::nanoseconds MIN_EARLY_TRANSACTION_TIME;
+
+ // Phase offsets for SF and app deadlines from VSYNC.
struct Offsets {
nsecs_t sf;
nsecs_t app;
bool operator==(const Offsets& other) const { return sf == other.sf && app == other.app; }
-
bool operator!=(const Offsets& other) const { return !(*this == other); }
};
+ using OffsetsOpt = std::optional<Offsets>;
+
struct OffsetsConfig {
- Offsets early; // For transactions with the eEarlyWakeup flag.
- Offsets earlyGl; // As above but while compositing with GL.
- Offsets late; // Default.
+ Offsets early; // Used for early transactions, and during refresh rate change.
+ Offsets earlyGpu; // Used during GPU composition.
+ Offsets late; // Default.
bool operator==(const OffsetsConfig& other) const {
- return early == other.early && earlyGl == other.earlyGl && late == other.late;
+ return early == other.early && earlyGpu == other.earlyGpu && late == other.late;
}
bool operator!=(const OffsetsConfig& other) const { return !(*this == other); }
};
- VSyncModulator(IPhaseOffsetControl&, ConnectionHandle appConnectionHandle,
- ConnectionHandle sfConnectionHandle, const OffsetsConfig&);
+ using Clock = std::chrono::steady_clock;
+ using TimePoint = Clock::time_point;
+ using Now = TimePoint (*)();
- void setPhaseOffsets(const OffsetsConfig&) EXCLUDES(mMutex);
+ explicit VsyncModulator(const OffsetsConfig&, Now = Clock::now);
- // Signals that a transaction has started, and changes offsets accordingly.
- void setTransactionStart(Scheduler::TransactionStart transactionStart);
+ Offsets getOffsets() const EXCLUDES(mMutex);
- // Signals that a transaction has been completed, so that we can finish
- // special handling for a transaction.
- void onTransactionHandled();
+ [[nodiscard]] Offsets setPhaseOffsets(const OffsetsConfig&) EXCLUDES(mMutex);
+
+ // Changes offsets in response to transaction flags or commit.
+ [[nodiscard]] OffsetsOpt setTransactionSchedule(TransactionSchedule);
+ [[nodiscard]] OffsetsOpt onTransactionCommit();
// Called when we send a refresh rate change to hardware composer, so that
// we can move into early offsets.
- void onRefreshRateChangeInitiated();
+ [[nodiscard]] OffsetsOpt onRefreshRateChangeInitiated();
- // Called when we detect from vsync signals that the refresh rate changed.
+ // Called when we detect from VSYNC signals that the refresh rate changed.
// This way we can move out of early offsets if no longer necessary.
- void onRefreshRateChangeCompleted();
+ [[nodiscard]] OffsetsOpt onRefreshRateChangeCompleted();
- // Called when the display is presenting a new frame. usedRenderEngine
- // should be set to true if RenderEngine was involved with composing the new
- // frame.
- void onRefreshed(bool usedRenderEngine);
-
- // Returns the offsets that we are currently using
- Offsets getOffsets() const EXCLUDES(mMutex);
+ [[nodiscard]] OffsetsOpt onDisplayRefresh(bool usedGpuComposition);
private:
- friend class VSyncModulatorTest;
- // Returns the next offsets that we should be using
const Offsets& getNextOffsets() const REQUIRES(mMutex);
- // Updates offsets and persists them into the scheduler framework.
- void updateOffsets() EXCLUDES(mMutex);
- void updateOffsetsLocked() REQUIRES(mMutex);
-
- IPhaseOffsetControl& mPhaseOffsetControl;
- const ConnectionHandle mAppConnectionHandle;
- const ConnectionHandle mSfConnectionHandle;
+ [[nodiscard]] Offsets updateOffsets() EXCLUDES(mMutex);
+ [[nodiscard]] Offsets updateOffsetsLocked() REQUIRES(mMutex);
mutable std::mutex mMutex;
OffsetsConfig mOffsetsConfig GUARDED_BY(mMutex);
Offsets mOffsets GUARDED_BY(mMutex){mOffsetsConfig.late};
- std::atomic<Scheduler::TransactionStart> mTransactionStart =
- Scheduler::TransactionStart::Normal;
- std::atomic<bool> mRefreshRateChangePending = false;
+ using Schedule = TransactionSchedule;
+ std::atomic<Schedule> mTransactionSchedule = Schedule::Late;
std::atomic<bool> mExplicitEarlyWakeup = false;
- std::atomic<int> mRemainingEarlyFrameCount = 0;
- std::atomic<int> mRemainingRenderEngineUsageCount = 0;
- std::atomic<std::chrono::steady_clock::time_point> mEarlyTxnStartTime = {};
- std::atomic<std::chrono::steady_clock::time_point> mTxnAppliedTime = {};
- bool mTraceDetailedInfo = false;
+ std::atomic<bool> mRefreshRateChangePending = false;
+
+ std::atomic<int> mEarlyTransactionFrames = 0;
+ std::atomic<int> mEarlyGpuFrames = 0;
+ std::atomic<TimePoint> mEarlyTransactionStartTime = TimePoint();
+ std::atomic<TimePoint> mLastTransactionCommitTime = TimePoint();
+
+ const Now mNow;
+ const bool mTraceDetailedInfo;
};
} // namespace android::scheduler