SurfaceFlinger: Scheduler: move reset callback to scheduler thread
Dispatch timer reset callback from Scheduler's thread. This would
make both Scheduler callbacks (idle timer and reset timer) to be dispatched
from the same context and avoid any potential deadlocks.
Test: surfaceflinger unit tests
Change-Id: Iad7d10b85db090662f9b7c4ffb45ed9110035ea4
diff --git a/services/surfaceflinger/Scheduler/IdleTimer.cpp b/services/surfaceflinger/Scheduler/IdleTimer.cpp
index e975e65..b28b1aa 100644
--- a/services/surfaceflinger/Scheduler/IdleTimer.cpp
+++ b/services/surfaceflinger/Scheduler/IdleTimer.cpp
@@ -22,8 +22,9 @@
namespace android {
namespace scheduler {
-IdleTimer::IdleTimer(const Interval& interval, const TimeoutCallback& timeoutCallback)
- : mInterval(interval), mTimeoutCallback(timeoutCallback) {}
+IdleTimer::IdleTimer(const Interval& interval, const ResetCallback& resetCallback,
+ const TimeoutCallback& timeoutCallback)
+ : mInterval(interval), mResetCallback(resetCallback), mTimeoutCallback(timeoutCallback) {}
IdleTimer::~IdleTimer() {
stop();
@@ -49,11 +50,34 @@
}
void IdleTimer::loop() {
- std::lock_guard<std::mutex> lock(mMutex);
- while (mState != TimerState::STOPPED) {
- if (mState == TimerState::IDLE) {
- mCondition.wait(mMutex);
- } else if (mState == TimerState::RESET) {
+ while (true) {
+ bool triggerReset = false;
+ bool triggerTimeout = false;
+ {
+ std::lock_guard<std::mutex> lock(mMutex);
+ if (mState == TimerState::STOPPED) {
+ break;
+ }
+
+ if (mState == TimerState::IDLE) {
+ mCondition.wait(mMutex);
+ continue;
+ }
+
+ if (mState == TimerState::RESET) {
+ triggerReset = true;
+ }
+ }
+ if (triggerReset && mResetCallback) {
+ mResetCallback();
+ }
+
+ { // lock the mutex again. someone might have called stop meanwhile
+ std::lock_guard<std::mutex> lock(mMutex);
+ if (mState == TimerState::STOPPED) {
+ break;
+ }
+
auto triggerTime = std::chrono::steady_clock::now() + mInterval;
mState = TimerState::WAITING;
while (mState == TimerState::WAITING) {
@@ -62,14 +86,14 @@
if (waitTime > zero) mCondition.wait_for(mMutex, waitTime);
if (mState == TimerState::WAITING &&
(triggerTime - std::chrono::steady_clock::now()) <= zero) {
- if (mTimeoutCallback) {
- mTimeoutCallback();
- }
-
+ triggerTimeout = true;
mState = TimerState::IDLE;
}
}
}
+ if (triggerTimeout && mTimeoutCallback) {
+ mTimeoutCallback();
+ }
}
} // namespace scheduler
diff --git a/services/surfaceflinger/Scheduler/IdleTimer.h b/services/surfaceflinger/Scheduler/IdleTimer.h
index aee3fa3..19f1267 100644
--- a/services/surfaceflinger/Scheduler/IdleTimer.h
+++ b/services/surfaceflinger/Scheduler/IdleTimer.h
@@ -32,9 +32,11 @@
class IdleTimer {
public:
using Interval = std::chrono::milliseconds;
+ using ResetCallback = std::function<void()>;
using TimeoutCallback = std::function<void()>;
- IdleTimer(const Interval& interval, const TimeoutCallback& timeoutCallback);
+ IdleTimer(const Interval& interval, const ResetCallback& resetCallback,
+ const TimeoutCallback& timeoutCallback);
~IdleTimer();
void start();
@@ -62,6 +64,9 @@
// Interval after which timer expires.
const Interval mInterval;
+ // Callback that happens when timer resets.
+ const ResetCallback mResetCallback;
+
// Callback that happens when timer expires.
const TimeoutCallback mTimeoutCallback;
};
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 5268c8c..7fd7762 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -75,6 +75,7 @@
if (mSetIdleTimerMs > 0) {
mIdleTimer =
std::make_unique<scheduler::IdleTimer>(std::chrono::milliseconds(mSetIdleTimerMs),
+ [this] { resetTimerCallback(); },
[this] { expiredTimerCallback(); });
mIdleTimer->start();
}
@@ -87,7 +88,6 @@
sp<Scheduler::ConnectionHandle> Scheduler::createConnection(
const char* connectionName, int64_t phaseOffsetNs, ResyncCallback resyncCallback,
- ResetIdleTimerCallback resetIdleTimerCallback,
impl::EventThread::InterceptVSyncsCallback interceptCallback) {
const int64_t id = sNextId++;
ALOGV("Creating a connection handle with ID: %" PRId64 "\n", id);
@@ -97,8 +97,7 @@
std::move(interceptCallback));
auto eventThreadConnection =
- createConnectionInternal(eventThread.get(), std::move(resyncCallback),
- std::move(resetIdleTimerCallback));
+ createConnectionInternal(eventThread.get(), std::move(resyncCallback));
mConnections.emplace(id,
std::make_unique<Connection>(new ConnectionHandle(id),
eventThreadConnection,
@@ -115,26 +114,17 @@
std::move(interceptCallback), connectionName);
}
-sp<EventThreadConnection> Scheduler::createConnectionInternal(
- EventThread* eventThread, ResyncCallback&& resyncCallback,
- ResetIdleTimerCallback&& resetIdleTimerCallback) {
+sp<EventThreadConnection> Scheduler::createConnectionInternal(EventThread* eventThread,
+ ResyncCallback&& resyncCallback) {
return eventThread->createEventConnection(std::move(resyncCallback),
- [this,
- resetIdleTimerCallback =
- std::move(resetIdleTimerCallback)] {
- resetIdleTimer();
- if (resetIdleTimerCallback) {
- resetIdleTimerCallback();
- }
- });
+ [this] { resetIdleTimer(); });
}
sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection(
- const sp<Scheduler::ConnectionHandle>& handle, ResyncCallback resyncCallback,
- ResetIdleTimerCallback resetIdleTimerCallback) {
+ const sp<Scheduler::ConnectionHandle>& handle, ResyncCallback resyncCallback) {
RETURN_VALUE_IF_INVALID(nullptr);
return createConnectionInternal(mConnections[handle->id]->thread.get(),
- std::move(resyncCallback), std::move(resetIdleTimerCallback));
+ std::move(resyncCallback));
}
EventThread* Scheduler::getEventThread(const sp<Scheduler::ConnectionHandle>& handle) {
@@ -263,6 +253,11 @@
mExpiredTimerCallback = expiredTimerCallback;
}
+void Scheduler::setResetIdleTimerCallback(const ResetIdleTimerCallback& resetTimerCallback) {
+ std::lock_guard<std::mutex> lock(mCallbackLock);
+ mResetTimerCallback = resetTimerCallback;
+}
+
void Scheduler::updateFrameSkipping(const int64_t skipCount) {
ATRACE_INT("FrameSkipCount", skipCount);
if (mSkipCount != skipCount) {
@@ -351,6 +346,13 @@
void Scheduler::resetIdleTimer() {
if (mIdleTimer) {
mIdleTimer->reset();
+ }
+}
+
+void Scheduler::resetTimerCallback() {
+ std::lock_guard<std::mutex> lock(mCallbackLock);
+ if (mResetTimerCallback) {
+ mResetTimerCallback();
ATRACE_INT("ExpiredIdleTimer", 0);
}
}
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 089d579..03f2879 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -37,6 +37,7 @@
class Scheduler {
public:
using ExpiredIdleTimerCallback = std::function<void()>;
+ using ResetIdleTimerCallback = std::function<void()>;
// Enum to indicate whether to start the transaction early, or at vsync time.
enum class TransactionStart { EARLY, NORMAL };
@@ -72,12 +73,11 @@
/** Creates an EventThread connection. */
sp<ConnectionHandle> createConnection(const char* connectionName, int64_t phaseOffsetNs,
- ResyncCallback, ResetIdleTimerCallback,
+ ResyncCallback,
impl::EventThread::InterceptVSyncsCallback);
sp<IDisplayEventConnection> createDisplayEventConnection(const sp<ConnectionHandle>& handle,
- ResyncCallback,
- ResetIdleTimerCallback);
+ ResyncCallback);
// Getter methods.
EventThread* getEventThread(const sp<ConnectionHandle>& handle);
@@ -117,6 +117,8 @@
void incrementFrameCounter();
// Callback that gets invoked once the idle timer expires.
void setExpiredIdleTimerCallback(const ExpiredIdleTimerCallback& expiredTimerCallback);
+ // Callback that gets invoked once the idle timer is reset.
+ void setResetIdleTimerCallback(const ResetIdleTimerCallback& resetTimerCallback);
// Returns relevant information about Scheduler for dumpsys purposes.
std::string doDump();
@@ -127,8 +129,7 @@
private:
// Creates a connection on the given EventThread and forwards the given callbacks.
- sp<EventThreadConnection> createConnectionInternal(EventThread*, ResyncCallback&&,
- ResetIdleTimerCallback&&);
+ sp<EventThreadConnection> createConnectionInternal(EventThread*, ResyncCallback&&);
nsecs_t calculateAverage() const;
void updateFrameSkipping(const int64_t skipCount);
@@ -140,10 +141,11 @@
void determineTimestampAverage(bool isAutoTimestamp, const nsecs_t framePresentTime);
// Function that resets the idle timer.
void resetIdleTimer();
+ // Function that is called when the timer resets.
+ void resetTimerCallback();
// Function that is called when the timer expires.
void expiredTimerCallback();
-
// If fences from sync Framework are supported.
const bool mHasSyncFramework;
@@ -184,6 +186,7 @@
std::mutex mCallbackLock;
ExpiredIdleTimerCallback mExpiredTimerCallback GUARDED_BY(mCallbackLock);
+ ExpiredIdleTimerCallback mResetTimerCallback GUARDED_BY(mCallbackLock);
};
} // namespace android