From 2c3217576fda79c0fc8113f157fa7f32ff81d844 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Wed, 13 Dec 2017 13:46:40 -0800 Subject: surfaceflinger: remove vsync hint The vsync hint isn't actually useful. Test: used taimen bug 69982006 Change-Id: Ic64577901f95f31beb50e96b7a0570e4068a33b4 --- services/surfaceflinger/EventThread.h | 6 ------ 1 file changed, 6 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 6a59fbbf42..0823839ee7 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -29,7 +29,6 @@ #include #include "DisplayDevice.h" -#include "DisplayHardware/PowerHAL.h" // --------------------------------------------------------------------------- namespace android { @@ -99,7 +98,6 @@ public: DisplayEventReceiver::Event* event); void dump(String8& result) const; - void sendVsyncHintOff(); void setPhaseOffset(nsecs_t phaseOffset); @@ -112,11 +110,9 @@ private: void removeDisplayEventConnection(const wp& connection); void enableVSyncLocked(); void disableVSyncLocked(); - void sendVsyncHintOnLocked(); // constants sp mVSyncSource; - PowerHAL mPowerHAL; SurfaceFlinger& mFlinger; mutable Mutex mLock; @@ -132,9 +128,7 @@ private: // for debugging bool mDebugVsyncEnabled; - bool mVsyncHintSent; const bool mInterceptVSyncs; - timer_t mTimerId; }; // --------------------------------------------------------------------------- -- cgit v1.2.3-59-g8ed1b From 78ce418ea76033a19663dcc0905e0390d21e5baf Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Wed, 31 Jan 2018 16:39:51 -0800 Subject: SF: Clang format selected sources Bug: None Test: None Change-Id: Icef36ab31252e0e785d1088cbde2aaa0cf356fdf --- services/surfaceflinger/DispSync.cpp | 118 ++++++++++--------------- services/surfaceflinger/DispSync.h | 17 ++-- services/surfaceflinger/EventControlThread.cpp | 17 ++-- services/surfaceflinger/EventThread.cpp | 79 +++++++---------- services/surfaceflinger/EventThread.h | 21 ++--- services/surfaceflinger/MessageQueue.cpp | 41 ++++----- services/surfaceflinger/MessageQueue.h | 29 +++--- 7 files changed, 127 insertions(+), 195 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/DispSync.cpp b/services/surfaceflinger/DispSync.cpp index bef12ea50f..b5fc365470 100644 --- a/services/surfaceflinger/DispSync.cpp +++ b/services/surfaceflinger/DispSync.cpp @@ -33,8 +33,8 @@ #include #include "DispSync.h" -#include "SurfaceFlinger.h" #include "EventLog/EventLog.h" +#include "SurfaceFlinger.h" using std::max; using std::min; @@ -53,15 +53,14 @@ static const bool kEnableZeroPhaseTracer = false; // needed to re-synchronize the software vsync model with the hardware. The // error metric used is the mean of the squared difference between each // present time and the nearest software-predicted vsync. -static const nsecs_t kErrorThreshold = 160000000000; // 400 usec squared +static const nsecs_t kErrorThreshold = 160000000000; // 400 usec squared #undef LOG_TAG #define LOG_TAG "DispSyncThread" -class DispSyncThread: public Thread { +class DispSyncThread : public Thread { public: - - explicit DispSyncThread(const char* name): - mName(name), + explicit DispSyncThread(const char* name) + : mName(name), mStop(false), mPeriod(0), mPhase(0), @@ -78,8 +77,8 @@ public: mPhase = phase; mReferenceTime = referenceTime; ALOGV("[%s] updateModel: mPeriod = %" PRId64 ", mPhase = %" PRId64 - " mReferenceTime = %" PRId64, mName, ns2us(mPeriod), - ns2us(mPhase), ns2us(mReferenceTime)); + " mReferenceTime = %" PRId64, + mName, ns2us(mPeriod), ns2us(mPhase), ns2us(mReferenceTime)); mCond.signal(); } @@ -115,8 +114,7 @@ public: if (mPeriod == 0) { err = mCond.wait(mMutex); if (err != NO_ERROR) { - ALOGE("error waiting for new events: %s (%d)", - strerror(-err), err); + ALOGE("error waiting for new events: %s (%d)", strerror(-err), err); return false; } continue; @@ -133,16 +131,14 @@ public: ALOGV("[%s] Waiting forever", mName); err = mCond.wait(mMutex); } else { - ALOGV("[%s] Waiting until %" PRId64, mName, - ns2us(targetTime)); + ALOGV("[%s] Waiting until %" PRId64, mName, ns2us(targetTime)); err = mCond.waitRelative(mMutex, targetTime - now); } if (err == TIMED_OUT) { isWakeup = true; } else if (err != NO_ERROR) { - ALOGE("error waiting for next event: %s (%d)", - strerror(-err), err); + ALOGE("error waiting for next event: %s (%d)", strerror(-err), err); return false; } } @@ -153,8 +149,7 @@ public: static const nsecs_t kMaxWakeupLatency = us2ns(1500); if (isWakeup) { - mWakeupLatency = ((mWakeupLatency * 63) + - (now - targetTime)) / 64; + mWakeupLatency = ((mWakeupLatency * 63) + (now - targetTime)) / 64; mWakeupLatency = min(mWakeupLatency, kMaxWakeupLatency); if (kTraceDetailedInfo) { ATRACE_INT64("DispSync:WakeupLat", now - targetTime); @@ -174,7 +169,7 @@ public: } status_t addEventListener(const char* name, nsecs_t phase, - const sp& callback) { + const sp& callback) { if (kTraceDetailedInfo) ATRACE_CALL(); Mutex::Autolock lock(mMutex); @@ -191,8 +186,7 @@ public: // We want to allow the firstmost future event to fire without // allowing any past events to fire - listener.mLastEventTime = systemTime() - mPeriod / 2 + mPhase - - mWakeupLatency; + listener.mLastEventTime = systemTime() - mPeriod / 2 + mPhase - mWakeupLatency; mEventListeners.push(listener); @@ -225,7 +219,6 @@ public: } private: - struct EventListener { const char* mName; nsecs_t mPhase; @@ -243,8 +236,7 @@ private: ALOGV("[%s] computeNextEventTimeLocked", mName); nsecs_t nextEventTime = INT64_MAX; for (size_t i = 0; i < mEventListeners.size(); i++) { - nsecs_t t = computeListenerNextEventTimeLocked(mEventListeners[i], - now); + nsecs_t t = computeListenerNextEventTimeLocked(mEventListeners[i], now); if (t < nextEventTime) { nextEventTime = t; @@ -257,22 +249,19 @@ private: Vector gatherCallbackInvocationsLocked(nsecs_t now) { if (kTraceDetailedInfo) ATRACE_CALL(); - ALOGV("[%s] gatherCallbackInvocationsLocked @ %" PRId64, mName, - ns2us(now)); + ALOGV("[%s] gatherCallbackInvocationsLocked @ %" PRId64, mName, ns2us(now)); Vector callbackInvocations; nsecs_t onePeriodAgo = now - mPeriod; for (size_t i = 0; i < mEventListeners.size(); i++) { - nsecs_t t = computeListenerNextEventTimeLocked(mEventListeners[i], - onePeriodAgo); + nsecs_t t = computeListenerNextEventTimeLocked(mEventListeners[i], onePeriodAgo); if (t < now) { CallbackInvocation ci; ci.mCallback = mEventListeners[i].mCallback; ci.mEventTime = t; - ALOGV("[%s] [%s] Preparing to fire", mName, - mEventListeners[i].mName); + ALOGV("[%s] [%s] Preparing to fire", mName, mEventListeners[i].mName); callbackInvocations.push(ci); mEventListeners.editItemAt(i).mLastEventTime = t; } @@ -281,18 +270,16 @@ private: return callbackInvocations; } - nsecs_t computeListenerNextEventTimeLocked(const EventListener& listener, - nsecs_t baseTime) { + nsecs_t computeListenerNextEventTimeLocked(const EventListener& listener, nsecs_t baseTime) { if (kTraceDetailedInfo) ATRACE_CALL(); - ALOGV("[%s] [%s] computeListenerNextEventTimeLocked(%" PRId64 ")", - mName, listener.mName, ns2us(baseTime)); + ALOGV("[%s] [%s] computeListenerNextEventTimeLocked(%" PRId64 ")", mName, listener.mName, + ns2us(baseTime)); nsecs_t lastEventTime = listener.mLastEventTime + mWakeupLatency; ALOGV("[%s] lastEventTime: %" PRId64, mName, ns2us(lastEventTime)); if (baseTime < lastEventTime) { baseTime = lastEventTime; - ALOGV("[%s] Clamping baseTime to lastEventTime -> %" PRId64, mName, - ns2us(baseTime)); + ALOGV("[%s] Clamping baseTime to lastEventTime -> %" PRId64, mName, ns2us(baseTime)); } baseTime -= mReferenceTime; @@ -374,11 +361,8 @@ private: bool mParity; }; -DispSync::DispSync(const char* name) : - mName(name), - mRefreshSkipCount(0), - mThread(new DispSyncThread(name)) { -} +DispSync::DispSync(const char* name) + : mName(name), mRefreshSkipCount(0), mThread(new DispSyncThread(name)) {} DispSync::~DispSync() {} @@ -451,8 +435,8 @@ bool DispSync::addResyncSample(nsecs_t timestamp) { mPhase = 0; mReferenceTime = timestamp; ALOGV("[%s] First resync sample: mPeriod = %" PRId64 ", mPhase = 0, " - "mReferenceTime = %" PRId64, mName, ns2us(mPeriod), - ns2us(mReferenceTime)); + "mReferenceTime = %" PRId64, + mName, ns2us(mPeriod), ns2us(mReferenceTime)); mThread->updateModel(mPeriod, mPhase, mReferenceTime); } @@ -480,16 +464,13 @@ bool DispSync::addResyncSample(nsecs_t timestamp) { // Check against kErrorThreshold / 2 to add some hysteresis before having to // resync again bool modelLocked = mModelUpdated && mError < (kErrorThreshold / 2); - ALOGV("[%s] addResyncSample returning %s", mName, - modelLocked ? "locked" : "unlocked"); + ALOGV("[%s] addResyncSample returning %s", mName, modelLocked ? "locked" : "unlocked"); return !modelLocked; } -void DispSync::endResync() { -} +void DispSync::endResync() {} -status_t DispSync::addEventListener(const char* name, nsecs_t phase, - const sp& callback) { +status_t DispSync::addEventListener(const char* name, nsecs_t phase, const sp& callback) { Mutex::Autolock lock(mMutex); return mThread->addEventListener(name, phase, callback); } @@ -597,8 +578,7 @@ void DispSync::updateErrorLocked() { // call getSignalTime() periodically so the cache is updated when the // fence signals. nsecs_t time = mPresentFences[i]->getCachedSignalTime(); - if (time == Fence::SIGNAL_TIME_PENDING || - time == Fence::SIGNAL_TIME_INVALID) { + if (time == Fence::SIGNAL_TIME_PENDING || time == Fence::SIGNAL_TIME_INVALID) { continue; } @@ -622,9 +602,8 @@ void DispSync::updateErrorLocked() { mError = 0; // Use mod ACCEPTABLE_ZERO_ERR_SAMPLES_COUNT to avoid log spam. mZeroErrSamplesCount++; - ALOGE_IF( - (mZeroErrSamplesCount % ACCEPTABLE_ZERO_ERR_SAMPLES_COUNT) == 0, - "No present times for model error."); + ALOGE_IF((mZeroErrSamplesCount % ACCEPTABLE_ZERO_ERR_SAMPLES_COUNT) == 0, + "No present times for model error."); } if (kTraceDetailedInfo) { @@ -650,17 +629,14 @@ nsecs_t DispSync::computeNextRefresh(int periodOffset) const { void DispSync::dump(String8& result) const { Mutex::Autolock lock(mMutex); - result.appendFormat("present fences are %s\n", - mIgnorePresentFences ? "ignored" : "used"); - result.appendFormat("mPeriod: %" PRId64 " ns (%.3f fps; skipCount=%d)\n", - mPeriod, 1000000000.0 / mPeriod, mRefreshSkipCount); + result.appendFormat("present fences are %s\n", mIgnorePresentFences ? "ignored" : "used"); + result.appendFormat("mPeriod: %" PRId64 " ns (%.3f fps; skipCount=%d)\n", mPeriod, + 1000000000.0 / mPeriod, mRefreshSkipCount); result.appendFormat("mPhase: %" PRId64 " ns\n", mPhase); - result.appendFormat("mError: %" PRId64 " ns (sqrt=%.1f)\n", - mError, sqrt(mError)); + result.appendFormat("mError: %" PRId64 " ns (sqrt=%.1f)\n", mError, sqrt(mError)); result.appendFormat("mNumResyncSamplesSincePresent: %d (limit %d)\n", - mNumResyncSamplesSincePresent, MAX_RESYNC_SAMPLES_WITHOUT_PRESENT); - result.appendFormat("mNumResyncSamples: %zd (max %d)\n", - mNumResyncSamples, MAX_RESYNC_SAMPLES); + mNumResyncSamplesSincePresent, MAX_RESYNC_SAMPLES_WITHOUT_PRESENT); + result.appendFormat("mNumResyncSamples: %zd (max %d)\n", mNumResyncSamples, MAX_RESYNC_SAMPLES); result.appendFormat("mResyncSamples:\n"); nsecs_t previous = -1; @@ -670,14 +646,13 @@ void DispSync::dump(String8& result) const { if (i == 0) { result.appendFormat(" %" PRId64 "\n", sampleTime); } else { - result.appendFormat(" %" PRId64 " (+%" PRId64 ")\n", - sampleTime, sampleTime - previous); + result.appendFormat(" %" PRId64 " (+%" PRId64 ")\n", sampleTime, + sampleTime - previous); } previous = sampleTime; } - result.appendFormat("mPresentFences [%d]:\n", - NUM_PRESENT_SAMPLES); + result.appendFormat("mPresentFences [%d]:\n", NUM_PRESENT_SAMPLES); nsecs_t now = systemTime(SYSTEM_TIME_MONOTONIC); previous = Fence::SIGNAL_TIME_INVALID; for (size_t i = 0; i < NUM_PRESENT_SAMPLES; i++) { @@ -685,17 +660,16 @@ void DispSync::dump(String8& result) const { nsecs_t presentTime = mPresentFences[idx]->getSignalTime(); if (presentTime == Fence::SIGNAL_TIME_PENDING) { result.appendFormat(" [unsignaled fence]\n"); - } else if(presentTime == Fence::SIGNAL_TIME_INVALID) { + } else if (presentTime == Fence::SIGNAL_TIME_INVALID) { result.appendFormat(" [invalid fence]\n"); } else if (previous == Fence::SIGNAL_TIME_PENDING || - previous == Fence::SIGNAL_TIME_INVALID) { + previous == Fence::SIGNAL_TIME_INVALID) { result.appendFormat(" %" PRId64 " (%.3f ms ago)\n", presentTime, - (now - presentTime) / 1000000.0); + (now - presentTime) / 1000000.0); } else { - result.appendFormat(" %" PRId64 " (+%" PRId64 " / %.3f) (%.3f ms ago)\n", - presentTime, presentTime - previous, - (presentTime - previous) / (double) mPeriod, - (now - presentTime) / 1000000.0); + result.appendFormat(" %" PRId64 " (+%" PRId64 " / %.3f) (%.3f ms ago)\n", presentTime, + presentTime - previous, (presentTime - previous) / (double)mPeriod, + (now - presentTime) / 1000000.0); } previous = presentTime; } diff --git a/services/surfaceflinger/DispSync.h b/services/surfaceflinger/DispSync.h index 880a24d6ad..d066d55107 100644 --- a/services/surfaceflinger/DispSync.h +++ b/services/surfaceflinger/DispSync.h @@ -20,8 +20,8 @@ #include #include -#include #include +#include #include @@ -47,12 +47,10 @@ class DispSyncThread; // false to indicate that a resynchronization (via addResyncSample) is not // needed. class DispSync { - public: - - class Callback: public virtual RefBase { + class Callback : public virtual RefBase { public: - virtual ~Callback() {}; + virtual ~Callback(){}; virtual void onDispSyncEvent(nsecs_t when) = 0; }; @@ -108,8 +106,7 @@ public: // given phase offset from the hardware vsync events. The callback is // called from a separate thread and it should return reasonably quickly // (i.e. within a few hundred microseconds). - status_t addEventListener(const char* name, nsecs_t phase, - const sp& callback); + status_t addEventListener(const char* name, nsecs_t phase, const sp& callback); // removeEventListener removes an already-registered event callback. Once // this method returns that callback will no longer be called by the @@ -126,7 +123,6 @@ public: void dump(String8& result) const; private: - void updateModelLocked(); void updateErrorLocked(); void resetErrorLocked(); @@ -174,8 +170,7 @@ private: // These member variables store information about the present fences used // to validate the currently computed model. - std::shared_ptr - mPresentFences[NUM_PRESENT_SAMPLES] {FenceTime::NO_FENCE}; + std::shared_ptr mPresentFences[NUM_PRESENT_SAMPLES]{FenceTime::NO_FENCE}; size_t mPresentSampleOffset; int mRefreshSkipCount; @@ -195,6 +190,6 @@ private: bool mIgnorePresentFences; }; -} +} // namespace android #endif // ANDROID_DISPSYNC_H diff --git a/services/surfaceflinger/EventControlThread.cpp b/services/surfaceflinger/EventControlThread.cpp index 02eea4769b..6fd4cdf28e 100644 --- a/services/surfaceflinger/EventControlThread.cpp +++ b/services/surfaceflinger/EventControlThread.cpp @@ -19,10 +19,8 @@ namespace android { -EventControlThread::EventControlThread(const sp& flinger): - mFlinger(flinger), - mVsyncEnabled(false) { -} +EventControlThread::EventControlThread(const sp& flinger) + : mFlinger(flinger), mVsyncEnabled(false) {} void EventControlThread::setVsyncEnabled(bool enabled) { Mutex::Autolock lock(mMutex); @@ -31,24 +29,21 @@ void EventControlThread::setVsyncEnabled(bool enabled) { } bool EventControlThread::threadLoop() { - enum class VsyncState {Unset, On, Off}; + enum class VsyncState { Unset, On, Off }; auto currentVsyncState = VsyncState::Unset; while (true) { auto requestedVsyncState = VsyncState::On; { Mutex::Autolock lock(mMutex); - requestedVsyncState = - mVsyncEnabled ? VsyncState::On : VsyncState::Off; + requestedVsyncState = mVsyncEnabled ? VsyncState::On : VsyncState::Off; while (currentVsyncState == requestedVsyncState) { status_t err = mCond.wait(mMutex); if (err != NO_ERROR) { - ALOGE("error waiting for new events: %s (%d)", - strerror(-err), err); + ALOGE("error waiting for new events: %s (%d)", strerror(-err), err); return false; } - requestedVsyncState = - mVsyncEnabled ? VsyncState::On : VsyncState::Off; + requestedVsyncState = mVsyncEnabled ? VsyncState::On : VsyncState::Off; } } diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 5c0e3b37f3..6b92cc8a0e 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -21,8 +21,8 @@ #include -#include #include +#include #include #include @@ -36,18 +36,17 @@ namespace android { // --------------------------------------------------------------------------- EventThread::EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs) - : mVSyncSource(src), - mFlinger(flinger), - mUseSoftwareVSync(false), - mVsyncEnabled(false), - mDebugVsyncEnabled(false), - mInterceptVSyncs(interceptVSyncs) { - - for (int32_t i=0 ; i& connection) { +void EventThread::removeDisplayEventConnection(const wp& connection) { Mutex::Autolock _l(mLock); mDisplayEventConnections.remove(connection); } -void EventThread::setVsyncRate(uint32_t count, - const sp& connection) { +void EventThread::setVsyncRate(uint32_t count, const sp& connection) { if (int32_t(count) >= 0) { // server must protect against bad params Mutex::Autolock _l(mLock); const int32_t new_count = (count == 0) ? -1 : count; @@ -90,8 +87,7 @@ void EventThread::setVsyncRate(uint32_t count, } } -void EventThread::requestNextVsync( - const sp& connection) { +void EventThread::requestNextVsync(const sp& connection) { Mutex::Autolock _l(mLock); mFlinger.resyncWithRateLimit(); @@ -131,7 +127,7 @@ void EventThread::onVSyncEvent(nsecs_t timestamp) { void EventThread::onHotplugReceived(int type, bool connected) { ALOGE_IF(type >= DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES, - "received hotplug event for an invalid display (id=%d)", type); + "received hotplug event for an invalid display (id=%d)", type); Mutex::Autolock _l(mLock); if (type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) { @@ -147,12 +143,12 @@ void EventThread::onHotplugReceived(int type, bool connected) { bool EventThread::threadLoop() { DisplayEventReceiver::Event event; - Vector< sp > signalConnections; + Vector > signalConnections; signalConnections = waitForEvent(&event); // dispatch events to listeners... const size_t count = signalConnections.size(); - for (size_t i=0 ; i& conn(signalConnections[i]); // now see if we still need to report this event status_t err = conn->postEvent(event); @@ -162,8 +158,8 @@ bool EventThread::threadLoop() { // FIXME: Note that some events cannot be dropped and would have // to be re-sent later. // Right-now we don't have the ability to do this. - ALOGW("EventThread: dropping event (%08x) for connection %p", - event.header.type, conn.get()); + ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type, + conn.get()); } else if (err < 0) { // handle any other error on the pipe as fatal. the only // reasonable thing to do is to clean-up this connection. @@ -176,11 +172,9 @@ bool EventThread::threadLoop() { // This will return when (1) a vsync event has been received, and (2) there was // at least one connection interested in receiving it when we started waiting. -Vector< sp > EventThread::waitForEvent( - DisplayEventReceiver::Event* event) -{ +Vector > EventThread::waitForEvent(DisplayEventReceiver::Event* event) { Mutex::Autolock _l(mLock); - Vector< sp > signalConnections; + Vector > signalConnections; do { bool eventPending = false; @@ -188,7 +182,7 @@ Vector< sp > EventThread::waitForEvent( size_t vsyncCount = 0; nsecs_t timestamp = 0; - for (int32_t i=0 ; i > EventThread::waitForEvent( // find out connections waiting for events size_t count = mDisplayEventConnections.size(); - for (size_t i=0 ; i connection(mDisplayEventConnections[i].promote()); if (connection != nullptr) { bool added = false; @@ -231,7 +225,7 @@ Vector< sp > EventThread::waitForEvent( signalConnections.add(connection); added = true; } else if (connection->count == 1 || - (vsyncCount % connection->count) == 0) { + (vsyncCount % connection->count) == 0) { // continuous event, and time to report it signalConnections.add(connection); added = true; @@ -335,28 +329,22 @@ void EventThread::disableVSyncLocked() { void EventThread::dump(String8& result) const { Mutex::Autolock _l(mLock); - result.appendFormat("VSYNC state: %s\n", - mDebugVsyncEnabled?"enabled":"disabled"); - result.appendFormat(" soft-vsync: %s\n", - mUseSoftwareVSync?"enabled":"disabled"); + result.appendFormat("VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled"); + result.appendFormat(" soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled"); result.appendFormat(" numListeners=%zu,\n events-delivered: %u\n", - mDisplayEventConnections.size(), - mVSyncEvent[DisplayDevice::DISPLAY_PRIMARY].vsync.count); - for (size_t i=0 ; i connection = - mDisplayEventConnections.itemAt(i).promote(); - result.appendFormat(" %p: count=%d\n", - connection.get(), connection != nullptr ? connection->count : 0); + mDisplayEventConnections.size(), + mVSyncEvent[DisplayDevice::DISPLAY_PRIMARY].vsync.count); + for (size_t i = 0; i < mDisplayEventConnections.size(); i++) { + sp connection = mDisplayEventConnections.itemAt(i).promote(); + result.appendFormat(" %p: count=%d\n", connection.get(), + connection != nullptr ? connection->count : 0); } } // --------------------------------------------------------------------------- -EventThread::Connection::Connection( - const sp& eventThread) - : count(-1), mEventThread(eventThread), mChannel(gui::BitTube::DefaultSize) -{ -} +EventThread::Connection::Connection(const sp& eventThread) + : count(-1), mEventThread(eventThread), mChannel(gui::BitTube::DefaultSize) {} EventThread::Connection::~Connection() { // do nothing here -- clean-up will happen automatically @@ -382,8 +370,7 @@ void EventThread::Connection::requestNextVsync() { mEventThread->requestNextVsync(this); } -status_t EventThread::Connection::postEvent( - const DisplayEventReceiver::Event& event) { +status_t EventThread::Connection::postEvent(const DisplayEventReceiver::Event& event) { ssize_t size = DisplayEventReceiver::sendEvents(&mChannel, &event, 1); return size < 0 ? status_t(size) : status_t(NO_ERROR); } diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 0823839ee7..b4d718cd17 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -20,13 +20,13 @@ #include #include -#include #include #include +#include #include -#include #include +#include #include "DisplayDevice.h" @@ -39,10 +39,9 @@ class String8; // --------------------------------------------------------------------------- - class VSyncSource : public virtual RefBase { public: - class Callback: public virtual RefBase { + class Callback : public virtual RefBase { public: virtual ~Callback() {} virtual void onVSyncEvent(nsecs_t when) = 0; @@ -70,13 +69,12 @@ class EventThread : public Thread, private VSyncSource::Callback { virtual void onFirstRef(); status_t stealReceiveChannel(gui::BitTube* outChannel) override; status_t setVsyncRate(uint32_t count) override; - void requestNextVsync() override; // asynchronous + void requestNextVsync() override; // asynchronous sp const mEventThread; gui::BitTube mChannel; }; public: - EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs); sp createEventConnection() const; @@ -94,16 +92,15 @@ public: // called when receiving a hotplug event void onHotplugReceived(int type, bool connected); - Vector< sp > waitForEvent( - DisplayEventReceiver::Event* event); + Vector > waitForEvent(DisplayEventReceiver::Event* event); void dump(String8& result) const; void setPhaseOffset(nsecs_t phaseOffset); private: - virtual bool threadLoop(); - virtual void onFirstRef(); + virtual bool threadLoop(); + virtual void onFirstRef(); virtual void onVSyncEvent(nsecs_t timestamp); @@ -119,8 +116,8 @@ private: mutable Condition mCondition; // protected by mLock - SortedVector< wp > mDisplayEventConnections; - Vector< DisplayEventReceiver::Event > mPendingEvents; + SortedVector > mDisplayEventConnections; + Vector mPendingEvents; DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES]; bool mUseSoftwareVSync; bool mVsyncEnabled; diff --git a/services/surfaceflinger/MessageQueue.cpp b/services/surfaceflinger/MessageQueue.cpp index 0b1199c2d0..c9c398969c 100644 --- a/services/surfaceflinger/MessageQueue.cpp +++ b/services/surfaceflinger/MessageQueue.cpp @@ -14,32 +14,29 @@ * limitations under the License. */ -#include #include +#include #include #include -#include -#include #include +#include +#include #include -#include "MessageQueue.h" #include "EventThread.h" +#include "MessageQueue.h" #include "SurfaceFlinger.h" namespace android { // --------------------------------------------------------------------------- -MessageBase::MessageBase() - : MessageHandler() { -} +MessageBase::MessageBase() : MessageHandler() {} -MessageBase::~MessageBase() { -} +MessageBase::~MessageBase() {} void MessageBase::handleMessage(const Message&) { this->handler(); @@ -75,22 +72,17 @@ void MessageQueue::Handler::handleMessage(const Message& message) { // --------------------------------------------------------------------------- -MessageQueue::MessageQueue() -{ -} +MessageQueue::MessageQueue() {} -MessageQueue::~MessageQueue() { -} +MessageQueue::~MessageQueue() {} -void MessageQueue::init(const sp& flinger) -{ +void MessageQueue::init(const sp& flinger) { mFlinger = flinger; mLooper = new Looper(true); mHandler = new Handler(*this); } -void MessageQueue::setEventThread(const sp& eventThread) -{ +void MessageQueue::setEventThread(const sp& eventThread) { if (mEventThread == eventThread) { return; } @@ -102,8 +94,8 @@ void MessageQueue::setEventThread(const sp& eventThread) mEventThread = eventThread; mEvents = eventThread->createEventConnection(); mEvents->stealReceiveChannel(&mEventTube); - mLooper->addFd(mEventTube.getFd(), 0, Looper::EVENT_INPUT, - MessageQueue::cb_eventReceiver, this); + mLooper->addFd(mEventTube.getFd(), 0, Looper::EVENT_INPUT, MessageQueue::cb_eventReceiver, + this); } void MessageQueue::waitMessage() { @@ -128,9 +120,7 @@ void MessageQueue::waitMessage() { } while (true); } -status_t MessageQueue::postMessage( - const sp& messageHandler, nsecs_t relTime) -{ +status_t MessageQueue::postMessage(const sp& messageHandler, nsecs_t relTime) { const Message dummyMessage; if (relTime > 0) { mLooper->sendMessageDelayed(relTime, messageHandler, dummyMessage); @@ -140,7 +130,6 @@ status_t MessageQueue::postMessage( return NO_ERROR; } - void MessageQueue::invalidate() { mEvents->requestNextVsync(); } @@ -150,7 +139,7 @@ void MessageQueue::refresh() { } int MessageQueue::cb_eventReceiver(int fd, int events, void* data) { - MessageQueue* queue = reinterpret_cast(data); + MessageQueue* queue = reinterpret_cast(data); return queue->eventReceiver(fd, events); } @@ -158,7 +147,7 @@ int MessageQueue::eventReceiver(int /*fd*/, int /*events*/) { ssize_t n; DisplayEventReceiver::Event buffer[8]; while ((n = DisplayEventReceiver::getEvents(&mEventTube, buffer, 8)) > 0) { - for (int i=0 ; idispatchInvalidate(); break; diff --git a/services/surfaceflinger/MessageQueue.h b/services/surfaceflinger/MessageQueue.h index 14f50bbdce..fe1bf081b8 100644 --- a/services/surfaceflinger/MessageQueue.h +++ b/services/surfaceflinger/MessageQueue.h @@ -17,16 +17,16 @@ #ifndef ANDROID_MESSAGE_QUEUE_H #define ANDROID_MESSAGE_QUEUE_H -#include #include +#include #include -#include -#include #include +#include +#include -#include #include +#include #include "Barrier.h" @@ -40,11 +40,10 @@ class SurfaceFlinger; // --------------------------------------------------------------------------- -class MessageBase : public MessageHandler -{ +class MessageBase : public MessageHandler { public: MessageBase(); - + // return true if message has a handler virtual bool handler() = 0; @@ -79,15 +78,12 @@ private: class MessageQueue { class Handler : public MessageHandler { - enum { - eventMaskInvalidate = 0x1, - eventMaskRefresh = 0x2, - eventMaskTransaction = 0x4 - }; + enum { eventMaskInvalidate = 0x1, eventMaskRefresh = 0x2, eventMaskTransaction = 0x4 }; MessageQueue& mQueue; int32_t mEventMask; + public: - explicit Handler(MessageQueue& queue) : mQueue(queue), mEventMask(0) { } + explicit Handler(MessageQueue& queue) : mQueue(queue), mEventMask(0) {} virtual void handleMessage(const Message& message); void dispatchRefresh(); void dispatchInvalidate(); @@ -102,14 +98,13 @@ class MessageQueue { gui::BitTube mEventTube; sp mHandler; - static int cb_eventReceiver(int fd, int events, void* data); int eventReceiver(int fd, int events); public: enum { - INVALIDATE = 0, - REFRESH = 1, + INVALIDATE = 0, + REFRESH = 1, }; MessageQueue(); @@ -118,7 +113,7 @@ public: void setEventThread(const sp& events); void waitMessage(); - status_t postMessage(const sp& message, nsecs_t reltime=0); + status_t postMessage(const sp& message, nsecs_t reltime = 0); // sends INVALIDATE message at next VSYNC void invalidate(); -- cgit v1.2.3-59-g8ed1b From 46a46b317153f26e3361c9c74158fc6414bff7da Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Wed, 31 Jan 2018 19:01:18 -0800 Subject: SF: Cleanup EventThread Part 1 The cleanups in this CL are primarily about switching from android::Thread to std::thread. 1) Convert from android::Thread to std::thread, along with using std::mutex and std::condition_variable to keep consistency. 2) Switch the header to #pragma once. 3) Added Clang thread annotations and enabled the corresponding warning. 4) Added proper thread shutdown handling (invoked by dtor). Test: No issues observed on Pixel XL Bug: None Change-Id: I2ef0ad18ef75e139f70d856031991f87d187efe6 --- services/surfaceflinger/EventThread.cpp | 161 +++++++++++++++++------------ services/surfaceflinger/EventThread.h | 60 ++++++----- services/surfaceflinger/SurfaceFlinger.cpp | 17 +-- 3 files changed, 131 insertions(+), 107 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 6b92cc8a0e..2cbc59edb4 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -16,10 +16,14 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include +#include +#include #include +#include +#include #include +#include #include #include @@ -31,105 +35,126 @@ #include "EventThread.h" #include "SurfaceFlinger.h" +using namespace std::chrono_literals; + // --------------------------------------------------------------------------- + namespace android { + // --------------------------------------------------------------------------- -EventThread::EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs) - : mVSyncSource(src), - mFlinger(flinger), - mUseSoftwareVSync(false), - mVsyncEnabled(false), - mDebugVsyncEnabled(false), - mInterceptVSyncs(interceptVSyncs) { - for (int32_t i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; i++) { - mVSyncEvent[i].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; - mVSyncEvent[i].header.id = 0; - mVSyncEvent[i].header.timestamp = 0; - mVSyncEvent[i].vsync.count = 0; +EventThread::EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs, + const char* threadName) + : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) { + for (auto& event : mVSyncEvent) { + event.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; + event.header.id = 0; + event.header.timestamp = 0; + event.vsync.count = 0; + } + + mThread = std::thread(&EventThread::threadMain, this); + + pthread_setname_np(mThread.native_handle(), threadName); + + pid_t tid = pthread_gettid_np(mThread.native_handle()); + + // Use SCHED_FIFO to minimize jitter + constexpr int EVENT_THREAD_PRIORITY = 2; + struct sched_param param = {0}; + param.sched_priority = EVENT_THREAD_PRIORITY; + if (pthread_setschedparam(mThread.native_handle(), SCHED_FIFO, ¶m) != 0) { + ALOGE("Couldn't set SCHED_FIFO for EventThread"); + } + + set_sched_policy(tid, SP_FOREGROUND); +} + +EventThread::~EventThread() { + { + std::lock_guard lock(mMutex); + mKeepRunning = false; + mCondition.notify_all(); } + mThread.join(); } void EventThread::setPhaseOffset(nsecs_t phaseOffset) { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); mVSyncSource->setPhaseOffset(phaseOffset); } -void EventThread::onFirstRef() { - run("EventThread", PRIORITY_URGENT_DISPLAY + PRIORITY_MORE_FAVORABLE); -} - sp EventThread::createEventConnection() const { return new Connection(const_cast(this)); } status_t EventThread::registerDisplayEventConnection( const sp& connection) { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); mDisplayEventConnections.add(connection); - mCondition.broadcast(); + mCondition.notify_all(); return NO_ERROR; } void EventThread::removeDisplayEventConnection(const wp& connection) { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); mDisplayEventConnections.remove(connection); } void EventThread::setVsyncRate(uint32_t count, const sp& connection) { if (int32_t(count) >= 0) { // server must protect against bad params - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); const int32_t new_count = (count == 0) ? -1 : count; if (connection->count != new_count) { connection->count = new_count; - mCondition.broadcast(); + mCondition.notify_all(); } } } void EventThread::requestNextVsync(const sp& connection) { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); mFlinger.resyncWithRateLimit(); if (connection->count < 0) { connection->count = 0; - mCondition.broadcast(); + mCondition.notify_all(); } } void EventThread::onScreenReleased() { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); if (!mUseSoftwareVSync) { // disable reliance on h/w vsync mUseSoftwareVSync = true; - mCondition.broadcast(); + mCondition.notify_all(); } } void EventThread::onScreenAcquired() { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); if (mUseSoftwareVSync) { // resume use of h/w vsync mUseSoftwareVSync = false; - mCondition.broadcast(); + mCondition.notify_all(); } } void EventThread::onVSyncEvent(nsecs_t timestamp) { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; mVSyncEvent[0].header.id = 0; mVSyncEvent[0].header.timestamp = timestamp; mVSyncEvent[0].vsync.count++; - mCondition.broadcast(); + mCondition.notify_all(); } void EventThread::onHotplugReceived(int type, bool connected) { ALOGE_IF(type >= DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES, "received hotplug event for an invalid display (id=%d)", type); - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); if (type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) { DisplayEventReceiver::Event event; event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG; @@ -137,46 +162,48 @@ void EventThread::onHotplugReceived(int type, bool connected) { event.header.timestamp = systemTime(); event.hotplug.connected = connected; mPendingEvents.add(event); - mCondition.broadcast(); + mCondition.notify_all(); } } -bool EventThread::threadLoop() { - DisplayEventReceiver::Event event; - Vector > signalConnections; - signalConnections = waitForEvent(&event); - - // dispatch events to listeners... - const size_t count = signalConnections.size(); - for (size_t i = 0; i < count; i++) { - const sp& conn(signalConnections[i]); - // now see if we still need to report this event - status_t err = conn->postEvent(event); - if (err == -EAGAIN || err == -EWOULDBLOCK) { - // The destination doesn't accept events anymore, it's probably - // full. For now, we just drop the events on the floor. - // FIXME: Note that some events cannot be dropped and would have - // to be re-sent later. - // Right-now we don't have the ability to do this. - ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type, - conn.get()); - } else if (err < 0) { - // handle any other error on the pipe as fatal. the only - // reasonable thing to do is to clean-up this connection. - // The most common error we'll get here is -EPIPE. - removeDisplayEventConnection(signalConnections[i]); +void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS { + std::unique_lock lock(mMutex); + while (mKeepRunning) { + DisplayEventReceiver::Event event; + Vector > signalConnections; + signalConnections = waitForEventLocked(&lock, &event); + + // dispatch events to listeners... + const size_t count = signalConnections.size(); + for (size_t i = 0; i < count; i++) { + const sp& conn(signalConnections[i]); + // now see if we still need to report this event + status_t err = conn->postEvent(event); + if (err == -EAGAIN || err == -EWOULDBLOCK) { + // The destination doesn't accept events anymore, it's probably + // full. For now, we just drop the events on the floor. + // FIXME: Note that some events cannot be dropped and would have + // to be re-sent later. + // Right-now we don't have the ability to do this. + ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type, + conn.get()); + } else if (err < 0) { + // handle any other error on the pipe as fatal. the only + // reasonable thing to do is to clean-up this connection. + // The most common error we'll get here is -EPIPE. + removeDisplayEventConnection(signalConnections[i]); + } } } - return true; } // This will return when (1) a vsync event has been received, and (2) there was // at least one connection interested in receiving it when we started waiting. -Vector > EventThread::waitForEvent(DisplayEventReceiver::Event* event) { - Mutex::Autolock _l(mLock); +Vector > EventThread::waitForEventLocked( + std::unique_lock* lock, DisplayEventReceiver::Event* event) { Vector > signalConnections; - do { + while (signalConnections.isEmpty() && mKeepRunning) { bool eventPending = false; bool waitForVSync = false; @@ -279,8 +306,8 @@ Vector > EventThread::waitForEvent(DisplayEventRecei // use a (long) timeout when waiting for h/w vsync, and // generate fake events when necessary. bool softwareSync = mUseSoftwareVSync; - nsecs_t timeout = softwareSync ? ms2ns(16) : ms2ns(1000); - if (mCondition.waitRelative(mLock, timeout) == TIMED_OUT) { + auto timeout = softwareSync ? 16ms : 1000ms; + if (mCondition.wait_for(*lock, timeout) == std::cv_status::timeout) { if (!softwareSync) { ALOGW("Timed out waiting for hw vsync; faking it"); } @@ -296,10 +323,10 @@ Vector > EventThread::waitForEvent(DisplayEventRecei // h/w vsync should be disabled, so this will wait until we // get a new connection, or an existing connection becomes // interested in receiving vsync again. - mCondition.wait(mLock); + mCondition.wait(*lock); } } - } while (signalConnections.isEmpty()); + } // here we're guaranteed to have a timestamp and some connections to signal // (The connections might have dropped out of mDisplayEventConnections @@ -328,7 +355,7 @@ void EventThread::disableVSyncLocked() { } void EventThread::dump(String8& result) const { - Mutex::Autolock _l(mLock); + std::lock_guard lock(mMutex); result.appendFormat("VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled"); result.appendFormat(" soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled"); result.appendFormat(" numListeners=%zu,\n events-delivered: %u\n", @@ -377,4 +404,4 @@ status_t EventThread::Connection::postEvent(const DisplayEventReceiver::Event& e // --------------------------------------------------------------------------- -}; // namespace android +} // namespace android diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index b4d718cd17..63cdb542f3 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -14,19 +14,23 @@ * limitations under the License. */ -#ifndef ANDROID_SURFACE_FLINGER_EVENT_THREAD_H -#define ANDROID_SURFACE_FLINGER_EVENT_THREAD_H +#pragma once #include #include +#include +#include +#include + +#include #include #include #include #include +#include #include -#include #include "DisplayDevice.h" @@ -53,7 +57,7 @@ public: virtual void setPhaseOffset(nsecs_t phaseOffset) = 0; }; -class EventThread : public Thread, private VSyncSource::Callback { +class EventThread : public virtual RefBase, private VSyncSource::Callback { class Connection : public BnDisplayEventConnection { public: explicit Connection(const sp& eventThread); @@ -75,7 +79,9 @@ class EventThread : public Thread, private VSyncSource::Callback { }; public: - EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs); + EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs, + const char* threadName); + ~EventThread(); sp createEventConnection() const; status_t registerDisplayEventConnection(const sp& connection); @@ -92,46 +98,46 @@ public: // called when receiving a hotplug event void onHotplugReceived(int type, bool connected); - Vector > waitForEvent(DisplayEventReceiver::Event* event); - void dump(String8& result) const; void setPhaseOffset(nsecs_t phaseOffset); private: - virtual bool threadLoop(); - virtual void onFirstRef(); - - virtual void onVSyncEvent(nsecs_t timestamp); + void threadMain(); + Vector> waitForEventLocked(std::unique_lock* lock, + DisplayEventReceiver::Event* event) + REQUIRES(mMutex); void removeDisplayEventConnection(const wp& connection); - void enableVSyncLocked(); - void disableVSyncLocked(); + void enableVSyncLocked() REQUIRES(mMutex); + void disableVSyncLocked() REQUIRES(mMutex); + + // Implements VSyncSource::Callback + void onVSyncEvent(nsecs_t timestamp) override; // constants - sp mVSyncSource; + sp mVSyncSource GUARDED_BY(mMutex); SurfaceFlinger& mFlinger; - mutable Mutex mLock; - mutable Condition mCondition; + std::thread mThread; + mutable std::mutex mMutex; + mutable std::condition_variable mCondition; // protected by mLock - SortedVector > mDisplayEventConnections; - Vector mPendingEvents; - DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES]; - bool mUseSoftwareVSync; - bool mVsyncEnabled; + SortedVector> mDisplayEventConnections GUARDED_BY(mMutex); + Vector mPendingEvents GUARDED_BY(mMutex); + DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES] GUARDED_BY( + mMutex); + bool mUseSoftwareVSync GUARDED_BY(mMutex) = false; + bool mVsyncEnabled GUARDED_BY(mMutex) = false; + bool mKeepRunning GUARDED_BY(mMutex) = true; // for debugging - bool mDebugVsyncEnabled; + bool mDebugVsyncEnabled GUARDED_BY(mMutex) = false; - const bool mInterceptVSyncs; + const bool mInterceptVSyncs = false; }; // --------------------------------------------------------------------------- }; // namespace android - -// --------------------------------------------------------------------------- - -#endif /* ANDROID_SURFACE_FLINGER_EVENT_THREAD_H */ diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 70a282db17..3774ab1e4c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -579,22 +579,12 @@ void SurfaceFlinger::init() { // start the EventThread sp vsyncSrc = new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app"); - mEventThread = new EventThread(vsyncSrc, *this, false); + mEventThread = new EventThread(vsyncSrc, *this, false, "appEventThread"); sp sfVsyncSrc = new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); - mSFEventThread = new EventThread(sfVsyncSrc, *this, true); + mSFEventThread = new EventThread(sfVsyncSrc, *this, true, "sfEventThread"); mEventQueue.setEventThread(mSFEventThread); - // set EventThread and SFEventThread to SCHED_FIFO to minimize jitter - struct sched_param param = {0}; - param.sched_priority = 2; - if (sched_setscheduler(mSFEventThread->getTid(), SCHED_FIFO, ¶m) != 0) { - ALOGE("Couldn't set SCHED_FIFO for SFEventThread"); - } - if (sched_setscheduler(mEventThread->getTid(), SCHED_FIFO, ¶m) != 0) { - ALOGE("Couldn't set SCHED_FIFO for EventThread"); - } - // Get a RenderEngine for the given display / config (can't fail) getBE().mRenderEngine = RenderEngine::create(HAL_PIXEL_FORMAT_RGBA_8888, hasWideColorDisplay ? RenderEngine::WIDE_COLOR_SUPPORT : 0); @@ -1074,7 +1064,8 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { ALOGV("VSync Injections enabled"); if (mVSyncInjector.get() == nullptr) { mVSyncInjector = new InjectVSyncSource(); - mInjectorEventThread = new EventThread(mVSyncInjector, *this, false); + mInjectorEventThread = new EventThread(mVSyncInjector, *this, false, + "injEventThread"); } mEventQueue.setEventThread(mInjectorEventThread); } else { -- cgit v1.2.3-59-g8ed1b From e83f93151800f4f7999f7e0c3b727de9267a5f5f Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Thu, 1 Feb 2018 12:53:17 -0800 Subject: SF: Cleanup EventThread Part 2 De-refbase EventThread and a bunch of related classes. Convert from usign StrongPointer to std::unique_ptr to hold owned references, or bare pointers for unowned references. I did not see any need for using std::shared_ptr, or anything else, as SurfaceFlinger appeared to own all the objects, and they were created once and not really destroyed afterwards. Test: Things seem to still work on a Pixel XL Bug: None Change-Id: Ifff32118d31bc1bb51e38df695ebe5cf86d2bb6d --- services/surfaceflinger/DispSync.cpp | 16 ++++---- services/surfaceflinger/DispSync.h | 8 ++-- services/surfaceflinger/EventThread.cpp | 6 +-- services/surfaceflinger/EventThread.h | 17 ++++----- services/surfaceflinger/MessageQueue.cpp | 2 +- services/surfaceflinger/MessageQueue.h | 4 +- services/surfaceflinger/SurfaceFlinger.cpp | 59 +++++++++++++++--------------- services/surfaceflinger/SurfaceFlinger.h | 10 +++-- 8 files changed, 63 insertions(+), 59 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/DispSync.cpp b/services/surfaceflinger/DispSync.cpp index b5fc365470..9e01fd0d8d 100644 --- a/services/surfaceflinger/DispSync.cpp +++ b/services/surfaceflinger/DispSync.cpp @@ -168,8 +168,7 @@ public: return false; } - status_t addEventListener(const char* name, nsecs_t phase, - const sp& callback) { + status_t addEventListener(const char* name, nsecs_t phase, DispSync::Callback* callback) { if (kTraceDetailedInfo) ATRACE_CALL(); Mutex::Autolock lock(mMutex); @@ -195,7 +194,7 @@ public: return NO_ERROR; } - status_t removeEventListener(const sp& callback) { + status_t removeEventListener(DispSync::Callback* callback) { if (kTraceDetailedInfo) ATRACE_CALL(); Mutex::Autolock lock(mMutex); @@ -223,11 +222,11 @@ private: const char* mName; nsecs_t mPhase; nsecs_t mLastEventTime; - sp mCallback; + DispSync::Callback* mCallback; }; struct CallbackInvocation { - sp mCallback; + DispSync::Callback* mCallback; nsecs_t mEventTime; }; @@ -388,7 +387,8 @@ void DispSync::init(bool hasSyncFramework, int64_t dispSyncPresentTimeOffset) { // not needed because any time there is an event registered we will // turn on the HW vsync events. if (!mIgnorePresentFences && kEnableZeroPhaseTracer) { - addEventListener("ZeroPhaseTracer", 0, new ZeroPhaseTracer()); + mZeroPhaseTracer = std::make_unique(); + addEventListener("ZeroPhaseTracer", 0, mZeroPhaseTracer.get()); } } } @@ -470,7 +470,7 @@ bool DispSync::addResyncSample(nsecs_t timestamp) { void DispSync::endResync() {} -status_t DispSync::addEventListener(const char* name, nsecs_t phase, const sp& callback) { +status_t DispSync::addEventListener(const char* name, nsecs_t phase, Callback* callback) { Mutex::Autolock lock(mMutex); return mThread->addEventListener(name, phase, callback); } @@ -482,7 +482,7 @@ void DispSync::setRefreshSkipCount(int count) { updateModelLocked(); } -status_t DispSync::removeEventListener(const sp& callback) { +status_t DispSync::removeEventListener(Callback* callback) { Mutex::Autolock lock(mMutex); return mThread->removeEventListener(callback); } diff --git a/services/surfaceflinger/DispSync.h b/services/surfaceflinger/DispSync.h index d066d55107..9336f4dd86 100644 --- a/services/surfaceflinger/DispSync.h +++ b/services/surfaceflinger/DispSync.h @@ -48,7 +48,7 @@ class DispSyncThread; // needed. class DispSync { public: - class Callback : public virtual RefBase { + class Callback { public: virtual ~Callback(){}; virtual void onDispSyncEvent(nsecs_t when) = 0; @@ -106,12 +106,12 @@ public: // given phase offset from the hardware vsync events. The callback is // called from a separate thread and it should return reasonably quickly // (i.e. within a few hundred microseconds). - status_t addEventListener(const char* name, nsecs_t phase, const sp& callback); + status_t addEventListener(const char* name, nsecs_t phase, Callback* callback); // removeEventListener removes an already-registered event callback. Once // this method returns that callback will no longer be called by the // DispSync object. - status_t removeEventListener(const sp& callback); + status_t removeEventListener(Callback* callback); // computeNextRefresh computes when the next refresh is expected to begin. // The periodOffset value can be used to move forward or backward; an @@ -188,6 +188,8 @@ private: // Ignore present (retire) fences if the device doesn't have support for the // sync framework bool mIgnorePresentFences; + + std::unique_ptr mZeroPhaseTracer; }; } // namespace android diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 2cbc59edb4..53d95e21ec 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -43,7 +43,7 @@ namespace android { // --------------------------------------------------------------------------- -EventThread::EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs, +EventThread::EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs, const char* threadName) : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) { for (auto& event : mVSyncEvent) { @@ -339,7 +339,7 @@ void EventThread::enableVSyncLocked() { // never enable h/w VSYNC when screen is off if (!mVsyncEnabled) { mVsyncEnabled = true; - mVSyncSource->setCallback(static_cast(this)); + mVSyncSource->setCallback(this); mVSyncSource->setVSyncEnabled(true); } } @@ -370,7 +370,7 @@ void EventThread::dump(String8& result) const { // --------------------------------------------------------------------------- -EventThread::Connection::Connection(const sp& eventThread) +EventThread::Connection::Connection(EventThread* eventThread) : count(-1), mEventThread(eventThread), mChannel(gui::BitTube::DefaultSize) {} EventThread::Connection::~Connection() { diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 63cdb542f3..9ae8fb25b0 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -29,7 +29,6 @@ #include #include -#include #include #include "DisplayDevice.h" @@ -43,9 +42,9 @@ class String8; // --------------------------------------------------------------------------- -class VSyncSource : public virtual RefBase { +class VSyncSource { public: - class Callback : public virtual RefBase { + class Callback { public: virtual ~Callback() {} virtual void onVSyncEvent(nsecs_t when) = 0; @@ -53,14 +52,14 @@ public: virtual ~VSyncSource() {} virtual void setVSyncEnabled(bool enable) = 0; - virtual void setCallback(const sp& callback) = 0; + virtual void setCallback(Callback* callback) = 0; virtual void setPhaseOffset(nsecs_t phaseOffset) = 0; }; -class EventThread : public virtual RefBase, private VSyncSource::Callback { +class EventThread : private VSyncSource::Callback { class Connection : public BnDisplayEventConnection { public: - explicit Connection(const sp& eventThread); + explicit Connection(EventThread* eventThread); status_t postEvent(const DisplayEventReceiver::Event& event); // count >= 1 : continuous event. count is the vsync rate @@ -74,12 +73,12 @@ class EventThread : public virtual RefBase, private VSyncSource::Callback { status_t stealReceiveChannel(gui::BitTube* outChannel) override; status_t setVsyncRate(uint32_t count) override; void requestNextVsync() override; // asynchronous - sp const mEventThread; + EventThread* const mEventThread; gui::BitTube mChannel; }; public: - EventThread(const sp& src, SurfaceFlinger& flinger, bool interceptVSyncs, + EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs, const char* threadName); ~EventThread(); @@ -116,7 +115,7 @@ private: void onVSyncEvent(nsecs_t timestamp) override; // constants - sp mVSyncSource GUARDED_BY(mMutex); + VSyncSource* mVSyncSource GUARDED_BY(mMutex) = nullptr; SurfaceFlinger& mFlinger; std::thread mThread; diff --git a/services/surfaceflinger/MessageQueue.cpp b/services/surfaceflinger/MessageQueue.cpp index c9c398969c..5a6ff4dd98 100644 --- a/services/surfaceflinger/MessageQueue.cpp +++ b/services/surfaceflinger/MessageQueue.cpp @@ -82,7 +82,7 @@ void MessageQueue::init(const sp& flinger) { mHandler = new Handler(*this); } -void MessageQueue::setEventThread(const sp& eventThread) { +void MessageQueue::setEventThread(EventThread* eventThread) { if (mEventThread == eventThread) { return; } diff --git a/services/surfaceflinger/MessageQueue.h b/services/surfaceflinger/MessageQueue.h index fe1bf081b8..dcfc716524 100644 --- a/services/surfaceflinger/MessageQueue.h +++ b/services/surfaceflinger/MessageQueue.h @@ -93,7 +93,7 @@ class MessageQueue { sp mFlinger; sp mLooper; - sp mEventThread; + EventThread* mEventThread; sp mEvents; gui::BitTube mEventTube; sp mHandler; @@ -110,7 +110,7 @@ public: MessageQueue(); ~MessageQueue(); void init(const sp& flinger); - void setEventThread(const sp& events); + void setEventThread(EventThread* events); void waitMessage(); status_t postMessage(const sp& message, nsecs_t reltime = 0); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 3774ab1e4c..1054c32043 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -424,7 +424,7 @@ void SurfaceFlinger::deleteTextureAsync(uint32_t texture) { postMessageAsync(new MessageDestroyGLTexture(getRenderEngine(), texture)); } -class DispSyncSource : public VSyncSource, private DispSync::Callback { +class DispSyncSource final : public VSyncSource, private DispSync::Callback { public: DispSyncSource(DispSync* dispSync, nsecs_t phaseOffset, bool traceVsync, const char* name) : @@ -435,14 +435,13 @@ public: mVsyncEventLabel(String8::format("VSYNC-%s", name)), mDispSync(dispSync), mCallbackMutex(), - mCallback(), mVsyncMutex(), mPhaseOffset(phaseOffset), mEnabled(false) {} - virtual ~DispSyncSource() {} + ~DispSyncSource() override = default; - virtual void setVSyncEnabled(bool enable) { + void setVSyncEnabled(bool enable) override { Mutex::Autolock lock(mVsyncMutex); if (enable) { status_t err = mDispSync->addEventListener(mName, mPhaseOffset, @@ -464,12 +463,12 @@ public: mEnabled = enable; } - virtual void setCallback(const sp& callback) { + void setCallback(VSyncSource::Callback* callback) override{ Mutex::Autolock lock(mCallbackMutex); mCallback = callback; } - virtual void setPhaseOffset(nsecs_t phaseOffset) { + void setPhaseOffset(nsecs_t phaseOffset) override { Mutex::Autolock lock(mVsyncMutex); // Normalize phaseOffset to [0, period) @@ -506,7 +505,7 @@ public: private: virtual void onDispSyncEvent(nsecs_t when) { - sp callback; + VSyncSource::Callback* callback; { Mutex::Autolock lock(mCallbackMutex); callback = mCallback; @@ -533,37 +532,36 @@ private: DispSync* mDispSync; Mutex mCallbackMutex; // Protects the following - sp mCallback; + VSyncSource::Callback* mCallback = nullptr; Mutex mVsyncMutex; // Protects the following nsecs_t mPhaseOffset; bool mEnabled; }; -class InjectVSyncSource : public VSyncSource { +class InjectVSyncSource final : public VSyncSource { public: - InjectVSyncSource() {} + InjectVSyncSource() = default; + ~InjectVSyncSource() override = default; - virtual ~InjectVSyncSource() {} - - virtual void setCallback(const sp& callback) { + void setCallback(VSyncSource::Callback* callback) override { std::lock_guard lock(mCallbackMutex); mCallback = callback; } - virtual void onInjectSyncEvent(nsecs_t when) { + void onInjectSyncEvent(nsecs_t when) { std::lock_guard lock(mCallbackMutex); if (mCallback) { mCallback->onVSyncEvent(when); } } - virtual void setVSyncEnabled(bool) {} - virtual void setPhaseOffset(nsecs_t) {} + void setVSyncEnabled(bool) override {} + void setPhaseOffset(nsecs_t) override {} private: std::mutex mCallbackMutex; // Protects the following - sp mCallback; + VSyncSource::Callback* mCallback = nullptr; }; // Do not call property_set on main thread which will be blocked by init @@ -577,13 +575,16 @@ void SurfaceFlinger::init() { Mutex::Autolock _l(mStateLock); // start the EventThread - sp vsyncSrc = - new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app"); - mEventThread = new EventThread(vsyncSrc, *this, false, "appEventThread"); - sp sfVsyncSrc = - new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); - mSFEventThread = new EventThread(sfVsyncSrc, *this, true, "sfEventThread"); - mEventQueue.setEventThread(mSFEventThread); + + mEventThreadSource = std::make_unique( + &mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app"); + mEventThread = std::make_unique( + mEventThreadSource.get(), *this, false, "sfEventThread"); + mSfEventThreadSource = std::make_unique( + &mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); + mSFEventThread = std::make_unique( + mSfEventThreadSource.get(), *this, true, "appEventThread"); + mEventQueue.setEventThread(mSFEventThread.get()); // Get a RenderEngine for the given display / config (can't fail) getBE().mRenderEngine = RenderEngine::create(HAL_PIXEL_FORMAT_RGBA_8888, @@ -1063,14 +1064,14 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { if (enable) { ALOGV("VSync Injections enabled"); if (mVSyncInjector.get() == nullptr) { - mVSyncInjector = new InjectVSyncSource(); - mInjectorEventThread = new EventThread(mVSyncInjector, *this, false, - "injEventThread"); + mVSyncInjector = std::make_unique(); + mInjectorEventThread = std::make_unique( + mVSyncInjector.get(), *this, false, "injEvThread"); } - mEventQueue.setEventThread(mInjectorEventThread); + mEventQueue.setEventThread(mInjectorEventThread.get()); } else { ALOGV("VSync Injections disabled"); - mEventQueue.setEventThread(mSFEventThread); + mEventQueue.setEventThread(mSFEventThread.get()); } mInjectVSyncs = enable; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 89f3930a04..19dd059e97 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -719,10 +719,12 @@ private: // constant members (no synchronization needed for access) nsecs_t mBootTime; bool mGpuToCpuSupported; - sp mEventThread; - sp mSFEventThread; - sp mInjectorEventThread; - sp mVSyncInjector; + std::unique_ptr mEventThread; + std::unique_ptr mSFEventThread; + std::unique_ptr mInjectorEventThread; + std::unique_ptr mEventThreadSource; + std::unique_ptr mSfEventThreadSource; + std::unique_ptr mVSyncInjector; std::unique_ptr mEventControlThread; sp mBuiltinDisplays[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES]; -- cgit v1.2.3-59-g8ed1b From 0fcde1b23edcb8105b944df70bf1113cac8f0c15 Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Wed, 20 Dec 2017 16:50:21 -0800 Subject: SF: Separate EventThread into interface and impl This allows the normal EventThread to be substituted by a GMock for unit tests. The EventThread is now the abstract interface. impl::EventThread is the normal implementation. Test: Builds Bug: None Change-Id: I2c6234a10849f7d34a215d53e5f601895738a5ae --- services/surfaceflinger/EventThread.cpp | 7 +++++- services/surfaceflinger/EventThread.h | 39 ++++++++++++++++++++++++------ services/surfaceflinger/SurfaceFlinger.cpp | 24 +++++++++--------- services/surfaceflinger/SurfaceFlinger.h | 6 ++++- 4 files changed, 55 insertions(+), 21 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 53d95e21ec..90aab506c8 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -43,6 +43,10 @@ namespace android { // --------------------------------------------------------------------------- +EventThread::~EventThread() = default; + +namespace impl { + EventThread::EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs, const char* threadName) : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) { @@ -84,7 +88,7 @@ void EventThread::setPhaseOffset(nsecs_t phaseOffset) { mVSyncSource->setPhaseOffset(phaseOffset); } -sp EventThread::createEventConnection() const { +sp EventThread::createEventConnection() const { return new Connection(const_cast(this)); } @@ -404,4 +408,5 @@ status_t EventThread::Connection::postEvent(const DisplayEventReceiver::Event& e // --------------------------------------------------------------------------- +} // namespace impl } // namespace android diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 9ae8fb25b0..708806a22d 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -56,7 +56,29 @@ public: virtual void setPhaseOffset(nsecs_t phaseOffset) = 0; }; -class EventThread : private VSyncSource::Callback { +class EventThread { +public: + virtual ~EventThread(); + + virtual sp createEventConnection() const = 0; + + // called before the screen is turned off from main thread + virtual void onScreenReleased() = 0; + + // called after the screen is turned on from main thread + virtual void onScreenAcquired() = 0; + + // called when receiving a hotplug event + virtual void onHotplugReceived(int type, bool connected) = 0; + + virtual void dump(String8& result) const = 0; + + virtual void setPhaseOffset(nsecs_t phaseOffset) = 0; +}; + +namespace impl { + +class EventThread : public android::EventThread, private VSyncSource::Callback { class Connection : public BnDisplayEventConnection { public: explicit Connection(EventThread* eventThread); @@ -82,24 +104,24 @@ public: const char* threadName); ~EventThread(); - sp createEventConnection() const; + sp createEventConnection() const override; status_t registerDisplayEventConnection(const sp& connection); void setVsyncRate(uint32_t count, const sp& connection); void requestNextVsync(const sp& connection); // called before the screen is turned off from main thread - void onScreenReleased(); + void onScreenReleased() override; // called after the screen is turned on from main thread - void onScreenAcquired(); + void onScreenAcquired() override; // called when receiving a hotplug event - void onHotplugReceived(int type, bool connected); + void onHotplugReceived(int type, bool connected) override; - void dump(String8& result) const; + void dump(String8& result) const override; - void setPhaseOffset(nsecs_t phaseOffset); + void setPhaseOffset(nsecs_t phaseOffset) override; private: void threadMain(); @@ -139,4 +161,5 @@ private: // --------------------------------------------------------------------------- -}; // namespace android +} // namespace impl +} // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 7ad13e3ef2..03f6bdc583 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -574,15 +574,16 @@ void SurfaceFlinger::init() { Mutex::Autolock _l(mStateLock); // start the EventThread - - mEventThreadSource = std::make_unique( - &mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app"); - mEventThread = std::make_unique( - mEventThreadSource.get(), *this, false, "sfEventThread"); - mSfEventThreadSource = std::make_unique( - &mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); - mSFEventThread = std::make_unique( - mSfEventThreadSource.get(), *this, true, "appEventThread"); + mEventThreadSource = + std::make_unique(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, + true, "app"); + mEventThread = std::make_unique(mEventThreadSource.get(), *this, false, + "appEventThread"); + mSfEventThreadSource = + std::make_unique(&mPrimaryDispSync, + SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); + mSFEventThread = std::make_unique(mSfEventThreadSource.get(), *this, true, + "sfEventThread"); mEventQueue.setEventThread(mSFEventThread.get()); // Get a RenderEngine for the given display / config (can't fail) @@ -1068,8 +1069,9 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { ALOGV("VSync Injections enabled"); if (mVSyncInjector.get() == nullptr) { mVSyncInjector = std::make_unique(); - mInjectorEventThread = std::make_unique( - mVSyncInjector.get(), *this, false, "injEvThread"); + mInjectorEventThread = + std::make_unique(mVSyncInjector.get(), *this, false, + "injEventThread"); } mEventQueue.setEventThread(mInjectorEventThread.get()); } else { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index a17eb70882..b7ebb1bcf3 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -93,6 +93,10 @@ class Surface; class SurfaceFlingerBE; class VSyncSource; +namespace impl { +class EventThread; +} // namespace impl + namespace RE { class RenderEngine; } @@ -312,7 +316,7 @@ public: private: friend class Client; friend class DisplayEventConnection; - friend class EventThread; + friend class impl::EventThread; friend class Layer; friend class BufferLayer; friend class MonitoredProducer; -- cgit v1.2.3-59-g8ed1b From 9cbe4dad35ae986420a3dda164fb602ee6b48923 Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Tue, 13 Feb 2018 18:51:55 -0800 Subject: SF: Fix an EventThread deadlock EventThread::removeDisplayEventConnection tried to acquire the mutex, however the caller (eventThread::threadMain()) already had it. The fix was to not acquire the mutex, since threadMain() was the only caller. Bug: 73256320 Test: Can toggle "Show taps" from dev options without lockup. Change-Id: I855b19b3d0ac9e0ea05604cc7b077f4a5030a00b --- services/surfaceflinger/EventThread.cpp | 5 ++--- services/surfaceflinger/EventThread.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 90aab506c8..1f4f5a53f4 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -100,8 +100,7 @@ status_t EventThread::registerDisplayEventConnection( return NO_ERROR; } -void EventThread::removeDisplayEventConnection(const wp& connection) { - std::lock_guard lock(mMutex); +void EventThread::removeDisplayEventConnectionLocked(const wp& connection) { mDisplayEventConnections.remove(connection); } @@ -195,7 +194,7 @@ void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS { // handle any other error on the pipe as fatal. the only // reasonable thing to do is to clean-up this connection. // The most common error we'll get here is -EPIPE. - removeDisplayEventConnection(signalConnections[i]); + removeDisplayEventConnectionLocked(signalConnections[i]); } } } diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 708806a22d..97f0a359e2 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -129,7 +129,7 @@ private: DisplayEventReceiver::Event* event) REQUIRES(mMutex); - void removeDisplayEventConnection(const wp& connection); + void removeDisplayEventConnectionLocked(const wp& connection) REQUIRES(mMutex); void enableVSyncLocked() REQUIRES(mMutex); void disableVSyncLocked() REQUIRES(mMutex); -- cgit v1.2.3-59-g8ed1b From 24b0a485fb58afd16a42de6378c0b743d7aca58a Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Fri, 9 Mar 2018 18:52:26 -0800 Subject: SF: Test coverage for EventThread Add a unit test to cover EventThread.cpp Test: atest libsurfaceflinger_unittest Bug: 74827900 Change-Id: If9479cd9deedff836068cb53e7da2cb64041aea1 --- services/surfaceflinger/EventThread.cpp | 18 +- services/surfaceflinger/EventThread.h | 22 +- services/surfaceflinger/SurfaceFlinger.cpp | 21 +- services/surfaceflinger/tests/unittests/Android.bp | 1 + .../tests/unittests/AsyncCallRecorder.h | 165 ++++++++ .../tests/unittests/EventThreadTest.cpp | 422 +++++++++++++++++++++ 6 files changed, 627 insertions(+), 22 deletions(-) create mode 100644 services/surfaceflinger/tests/unittests/AsyncCallRecorder.h create mode 100644 services/surfaceflinger/tests/unittests/EventThreadTest.cpp (limited to 'services/surfaceflinger/EventThread.h') diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index bb9c0703ed..bc271c8ec5 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -26,14 +26,12 @@ #include #include -#include #include #include #include #include "EventThread.h" -#include "SurfaceFlinger.h" using namespace std::chrono_literals; @@ -47,9 +45,11 @@ EventThread::~EventThread() = default; namespace impl { -EventThread::EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs, - const char* threadName) - : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) { +EventThread::EventThread(VSyncSource* src, ResyncWithRateLimitCallback resyncWithRateLimitCallback, + InterceptVSyncsCallback interceptVSyncsCallback, const char* threadName) + : mVSyncSource(src), + mResyncWithRateLimitCallback(resyncWithRateLimitCallback), + mInterceptVSyncsCallback(interceptVSyncsCallback) { for (auto& event : mVSyncEvent) { event.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; event.header.id = 0; @@ -118,7 +118,9 @@ void EventThread::setVsyncRate(uint32_t count, const sp void EventThread::requestNextVsync(const sp& connection) { std::lock_guard lock(mMutex); - mFlinger.resyncWithRateLimit(); + if (mResyncWithRateLimitCallback) { + mResyncWithRateLimitCallback(); + } if (connection->count < 0) { connection->count = 0; @@ -216,8 +218,8 @@ Vector > EventThread::waitForEventLocked( timestamp = mVSyncEvent[i].header.timestamp; if (timestamp) { // we have a vsync event to dispatch - if (mInterceptVSyncs) { - mFlinger.mInterceptor->saveVSyncEvent(timestamp); + if (mInterceptVSyncsCallback) { + mInterceptVSyncsCallback(timestamp); } *event = mVSyncEvent[i]; mVSyncEvent[i].header.timestamp = 0; diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 97f0a359e2..9c13ed2755 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -37,6 +37,7 @@ namespace android { // --------------------------------------------------------------------------- +class EventThreadTest; class SurfaceFlinger; class String8; @@ -82,7 +83,9 @@ class EventThread : public android::EventThread, private VSyncSource::Callback { class Connection : public BnDisplayEventConnection { public: explicit Connection(EventThread* eventThread); - status_t postEvent(const DisplayEventReceiver::Event& event); + virtual ~Connection(); + + virtual status_t postEvent(const DisplayEventReceiver::Event& event); // count >= 1 : continuous event. count is the vsync rate // count == 0 : one-shot event that has not fired @@ -90,7 +93,6 @@ class EventThread : public android::EventThread, private VSyncSource::Callback { int32_t count; private: - virtual ~Connection(); virtual void onFirstRef(); status_t stealReceiveChannel(gui::BitTube* outChannel) override; status_t setVsyncRate(uint32_t count) override; @@ -100,8 +102,11 @@ class EventThread : public android::EventThread, private VSyncSource::Callback { }; public: - EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs, - const char* threadName); + using ResyncWithRateLimitCallback = std::function; + using InterceptVSyncsCallback = std::function; + + EventThread(VSyncSource* src, ResyncWithRateLimitCallback resyncWithRateLimitCallback, + InterceptVSyncsCallback interceptVSyncsCallback, const char* threadName); ~EventThread(); sp createEventConnection() const override; @@ -124,6 +129,8 @@ public: void setPhaseOffset(nsecs_t phaseOffset) override; private: + friend EventThreadTest; + void threadMain(); Vector> waitForEventLocked(std::unique_lock* lock, DisplayEventReceiver::Event* event) @@ -137,8 +144,9 @@ private: void onVSyncEvent(nsecs_t timestamp) override; // constants - VSyncSource* mVSyncSource GUARDED_BY(mMutex) = nullptr; - SurfaceFlinger& mFlinger; + VSyncSource* const mVSyncSource GUARDED_BY(mMutex) = nullptr; + const ResyncWithRateLimitCallback mResyncWithRateLimitCallback; + const InterceptVSyncsCallback mInterceptVSyncsCallback; std::thread mThread; mutable std::mutex mMutex; @@ -155,8 +163,6 @@ private: // for debugging bool mDebugVsyncEnabled GUARDED_BY(mMutex) = false; - - const bool mInterceptVSyncs = false; }; // --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 588d24c45f..a837d01d80 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -636,14 +636,21 @@ void SurfaceFlinger::init() { mEventThreadSource = std::make_unique(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app"); - mEventThread = std::make_unique(mEventThreadSource.get(), *this, false, + mEventThread = std::make_unique(mEventThreadSource.get(), + [this]() { resyncWithRateLimit(); }, + impl::EventThread::InterceptVSyncsCallback(), "appEventThread"); mSfEventThreadSource = std::make_unique(&mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf"); - mSFEventThread = std::make_unique(mSfEventThreadSource.get(), *this, true, - "sfEventThread"); + mSFEventThread = + std::make_unique(mSfEventThreadSource.get(), + [this]() { resyncWithRateLimit(); }, + [this](nsecs_t timestamp) { + mInterceptor->saveVSyncEvent(timestamp); + }, + "sfEventThread"); mEventQueue->setEventThread(mSFEventThread.get()); mVsyncModulator.setEventThread(mSFEventThread.get()); @@ -1132,9 +1139,11 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) { ALOGV("VSync Injections enabled"); if (mVSyncInjector.get() == nullptr) { mVSyncInjector = std::make_unique(); - mInjectorEventThread = - std::make_unique(mVSyncInjector.get(), *this, false, - "injEventThread"); + mInjectorEventThread = std::make_unique< + impl::EventThread>(mVSyncInjector.get(), + [this]() { resyncWithRateLimit(); }, + impl::EventThread::InterceptVSyncsCallback(), + "injEventThread"); } mEventQueue->setEventThread(mInjectorEventThread.get()); } else { diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index bcabe0dffc..3e9f6a425c 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -20,6 +20,7 @@ cc_test { srcs: [ ":libsurfaceflinger_sources", "DisplayTransactionTest.cpp", + "EventThreadTest.cpp", "mock/DisplayHardware/MockComposer.cpp", "mock/DisplayHardware/MockDisplaySurface.cpp", "mock/gui/MockGraphicBufferConsumer.cpp", diff --git a/services/surfaceflinger/tests/unittests/AsyncCallRecorder.h b/services/surfaceflinger/tests/unittests/AsyncCallRecorder.h new file mode 100644 index 0000000000..2245ee1a8a --- /dev/null +++ b/services/surfaceflinger/tests/unittests/AsyncCallRecorder.h @@ -0,0 +1,165 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace android { + +// This class helps record calls made by another thread when they are made +// asynchronously, with no other way for the tests to verify that the calls have +// been made. +// +// A normal Google Mock recorder, while thread safe, does not allow you to wait +// for asynchronous calls to be made. +// +// Usage: +// +// In the test, use a Google Mock expectation to invoke an instance of the +// recorder: +// +// AsyncCallRecorder recorder; +// +// EXPECT_CALL(someMock, someFunction(_)). +// .WillRepeatedly(Invoke(recorder.getInvocable())); +// +// Then you can invoke the functionality being tested: +// +// threadUnderTest.doSomethingAsync() +// +// And afterwards make a number of assertions using the recorder: +// +// // Wait for one call (with reasonable default timeout), and get the args +// // as a std::tuple inside a std::optional. +// auto args = recorder.waitForCall(); +// // The returned std::optional will have a value if the recorder function +// // was called. +// ASSERT_TRUE(args.has_value()); +// // The arguments can be checked if needed using standard tuple +// // operations. +// EXPECT_EQ(123, std::get<0>(args.value())); +// +// Alternatively maybe you want to assert that a call was not made. +// +// EXPECT_FALSE(recorder.waitForUnexpectedCall().has_value()); +// +// However this check uses a really short timeout so as not to block the test +// unnecessarily. And it could be possible for the check to return false and +// then the recorder could observe a call being made after. +template +class AsyncCallRecorder; + +template +class AsyncCallRecorder { +public: + // For the tests, we expect the wait for an expected change to be signaled + // to be much shorter than this. + static constexpr std::chrono::milliseconds DEFAULT_CALL_EXPECTED_TIMEOUT{10}; + + // The wait here is tricky. We don't expect a change, but we don't want to + // wait forever (or for longer than the typical test function runtime). As + // even the simplest Google Test can take 1ms (1000us) to run, we wait for + // half that time. + static constexpr std::chrono::microseconds UNEXPECTED_CALL_TIMEOUT{500}; + + using ArgTuple = std::tuple>...>; + + void recordCall(Args... args) { + std::lock_guard lock(mMutex); + mCalls.emplace_back(std::make_tuple(args...)); + mCondition.notify_all(); + } + + // Returns a functor which can be used with the Google Mock Invoke() + // function, or as a std::function to record calls. + auto getInvocable() { + return [this](Args... args) { recordCall(args...); }; + } + + // Returns a set of arguments as a std::optional> for the + // oldest call, waiting for the given timeout if necessary if there are no + // arguments in the FIFO. + std::optional waitForCall( + std::chrono::microseconds timeout = DEFAULT_CALL_EXPECTED_TIMEOUT) + NO_THREAD_SAFETY_ANALYSIS { + std::unique_lock lock(mMutex); + + // Wait if necessary for us to have a record from a call. + mCondition.wait_for(lock, timeout, + [this]() NO_THREAD_SAFETY_ANALYSIS { return !mCalls.empty(); }); + + // Return the arguments from the oldest call, if one was made + bool called = !mCalls.empty(); + std::optional result; + if (called) { + result.emplace(std::move(mCalls.front())); + mCalls.pop_front(); + } + return result; + } + + // Waits using a small default timeout for when a call is not expected to be + // made. The returned std::optional> should not have a value + // except if a set of arguments was unexpectedly received because a call was + // actually made. + // + // Note this function uses a small timeout to not block test execution, and + // it is possible the code under test could make the call AFTER the timeout + // expires. + std::optional waitForUnexpectedCall() { return waitForCall(UNEXPECTED_CALL_TIMEOUT); } + +private: + std::mutex mMutex; + std::condition_variable mCondition; + std::deque mCalls GUARDED_BY(mMutex); +}; + +// Like AsyncCallRecorder, but for when the function being invoked +// asynchronously is expected to return a value. +// +// This helper allows a single constant return value to be set to be returned by +// all calls that were made. +template +class AsyncCallRecorderWithCannedReturn; + +template +class AsyncCallRecorderWithCannedReturn + : public AsyncCallRecorder { +public: + explicit AsyncCallRecorderWithCannedReturn(Ret returnvalue) : mReturnValue(returnvalue) {} + + auto getInvocable() { + return [this](Args... args) { + this->recordCall(args...); + return mReturnValue; + }; + } + +private: + const Ret mReturnValue; +}; + +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp new file mode 100644 index 0000000000..80fdb80264 --- /dev/null +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -0,0 +1,422 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#undef LOG_TAG +#define LOG_TAG "LibSurfaceFlingerUnittests" + +#include +#include + +#include + +#include + +#include "AsyncCallRecorder.h" +#include "EventThread.h" + +using namespace std::chrono_literals; +using namespace std::placeholders; + +using testing::_; +using testing::Invoke; + +namespace android { +namespace { + +class MockVSyncSource : public VSyncSource { +public: + MOCK_METHOD1(setVSyncEnabled, void(bool)); + MOCK_METHOD1(setCallback, void(VSyncSource::Callback*)); + MOCK_METHOD1(setPhaseOffset, void(nsecs_t)); +}; + +} // namespace + +class EventThreadTest : public testing::Test { +protected: + class MockEventThreadConnection : public android::impl::EventThread::Connection { + public: + explicit MockEventThreadConnection(android::impl::EventThread* eventThread) + : android::impl::EventThread::Connection(eventThread) {} + MOCK_METHOD1(postEvent, status_t(const DisplayEventReceiver::Event& event)); + }; + + using ConnectionEventRecorder = + AsyncCallRecorderWithCannedReturn; + + EventThreadTest(); + ~EventThreadTest() override; + + void createThread(); + sp createConnection(ConnectionEventRecorder& recorder); + + void expectVSyncSetEnabledCallReceived(bool expectedState); + void expectVSyncSetPhaseOffsetCallReceived(nsecs_t expectedPhaseOffset); + VSyncSource::Callback* expectVSyncSetCallbackCallReceived(); + void expectInterceptCallReceived(nsecs_t expectedTimestamp); + void expectVsyncEventReceivedByConnection(const char* name, + ConnectionEventRecorder& connectionEventRecorder, + nsecs_t expectedTimestamp, unsigned expectedCount); + void expectVsyncEventReceivedByConnection(nsecs_t expectedTimestamp, unsigned expectedCount); + void expectHotplugEventReceivedByConnection(int expectedDisplayType, bool expectedConnected); + + AsyncCallRecorder mVSyncSetEnabledCallRecorder; + AsyncCallRecorder mVSyncSetCallbackCallRecorder; + AsyncCallRecorder mVSyncSetPhaseOffsetCallRecorder; + AsyncCallRecorder mResyncCallRecorder; + AsyncCallRecorder mInterceptVSyncCallRecorder; + ConnectionEventRecorder mConnectionEventCallRecorder{0}; + + MockVSyncSource mVSyncSource; + std::unique_ptr mThread; + sp mConnection; +}; + +EventThreadTest::EventThreadTest() { + const ::testing::TestInfo* const test_info = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name()); + + EXPECT_CALL(mVSyncSource, setVSyncEnabled(_)) + .WillRepeatedly(Invoke(mVSyncSetEnabledCallRecorder.getInvocable())); + + EXPECT_CALL(mVSyncSource, setCallback(_)) + .WillRepeatedly(Invoke(mVSyncSetCallbackCallRecorder.getInvocable())); + + EXPECT_CALL(mVSyncSource, setPhaseOffset(_)) + .WillRepeatedly(Invoke(mVSyncSetPhaseOffsetCallRecorder.getInvocable())); + + createThread(); + mConnection = createConnection(mConnectionEventCallRecorder); +} + +EventThreadTest::~EventThreadTest() { + const ::testing::TestInfo* const test_info = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name()); +} + +void EventThreadTest::createThread() { + mThread = + std::make_unique(&mVSyncSource, + mResyncCallRecorder.getInvocable(), + mInterceptVSyncCallRecorder.getInvocable(), + "unit-test-event-thread"); +} + +sp EventThreadTest::createConnection( + ConnectionEventRecorder& recorder) { + sp connection = new MockEventThreadConnection(mThread.get()); + EXPECT_CALL(*connection, postEvent(_)).WillRepeatedly(Invoke(recorder.getInvocable())); + return connection; +} + +void EventThreadTest::expectVSyncSetEnabledCallReceived(bool expectedState) { + auto args = mVSyncSetEnabledCallRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()); + EXPECT_EQ(expectedState, std::get<0>(args.value())); +} + +void EventThreadTest::expectVSyncSetPhaseOffsetCallReceived(nsecs_t expectedPhaseOffset) { + auto args = mVSyncSetPhaseOffsetCallRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()); + EXPECT_EQ(expectedPhaseOffset, std::get<0>(args.value())); +} + +VSyncSource::Callback* EventThreadTest::expectVSyncSetCallbackCallReceived() { + auto callbackSet = mVSyncSetCallbackCallRecorder.waitForCall(); + return callbackSet.has_value() ? std::get<0>(callbackSet.value()) : nullptr; +} + +void EventThreadTest::expectInterceptCallReceived(nsecs_t expectedTimestamp) { + auto args = mInterceptVSyncCallRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()); + EXPECT_EQ(expectedTimestamp, std::get<0>(args.value())); +} + +void EventThreadTest::expectVsyncEventReceivedByConnection( + const char* name, ConnectionEventRecorder& connectionEventRecorder, + nsecs_t expectedTimestamp, unsigned expectedCount) { + auto args = connectionEventRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()) << name << " did not receive an event for timestamp " + << expectedTimestamp; + const auto& event = std::get<0>(args.value()); + EXPECT_EQ(DisplayEventReceiver::DISPLAY_EVENT_VSYNC, event.header.type) + << name << " did not get the correct event for timestamp " << expectedTimestamp; + EXPECT_EQ(expectedTimestamp, event.header.timestamp) + << name << " did not get the expected timestamp for timestamp " << expectedTimestamp; + EXPECT_EQ(expectedCount, event.vsync.count) + << name << " did not get the expected count for timestamp " << expectedTimestamp; +} + +void EventThreadTest::expectVsyncEventReceivedByConnection(nsecs_t expectedTimestamp, + unsigned expectedCount) { + expectVsyncEventReceivedByConnection("mConnectionEventCallRecorder", + mConnectionEventCallRecorder, expectedTimestamp, + expectedCount); +} + +void EventThreadTest::expectHotplugEventReceivedByConnection(int expectedDisplayType, + bool expectedConnected) { + auto args = mConnectionEventCallRecorder.waitForCall(); + ASSERT_TRUE(args.has_value()); + const auto& event = std::get<0>(args.value()); + EXPECT_EQ(DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, event.header.type); + EXPECT_EQ(static_cast(expectedDisplayType), event.header.id); + EXPECT_EQ(expectedConnected, event.hotplug.connected); +} + +namespace { + +/* ------------------------------------------------------------------------ + * Test cases + */ + +TEST_F(EventThreadTest, canCreateAndDestroyThreadWithNoEventsSent) { + EXPECT_FALSE(mVSyncSetEnabledCallRecorder.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mVSyncSetCallbackCallRecorder.waitForCall(0us).has_value()); + EXPECT_FALSE(mVSyncSetPhaseOffsetCallRecorder.waitForCall(0us).has_value()); + EXPECT_FALSE(mResyncCallRecorder.waitForCall(0us).has_value()); + EXPECT_FALSE(mInterceptVSyncCallRecorder.waitForCall(0us).has_value()); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForCall(0us).has_value()); +} + +TEST_F(EventThreadTest, requestNextVsyncPostsASingleVSyncEventToTheConnection) { + // Signal that we want the next vsync event to be posted to the connection + mThread->requestNextVsync(mConnection); + + // EventThread should immediately request a resync. + EXPECT_TRUE(mResyncCallRecorder.waitForCall().has_value()); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // Use the received callback to signal a first vsync event. + // The interceptor should receive the event, as well as the connection. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + expectVsyncEventReceivedByConnection(123, 1u); + + // Use the received callback to signal a second vsync event. + // The interceptor should receive the event, but the the connection should + // not as it was only interested in the first. + callback->onVSyncEvent(456); + expectInterceptCallReceived(456); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); + + // EventThread should also detect that at this point that it does not need + // any more vsync events, and should disable their generation. + expectVSyncSetEnabledCallReceived(false); +} + +TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { + // Create a first connection, register it, and request a vsync rate of zero. + ConnectionEventRecorder firstConnectionEventRecorder{0}; + sp firstConnection = createConnection(firstConnectionEventRecorder); + mThread->setVsyncRate(0, firstConnection); + + // By itself, this should not enable vsync events + EXPECT_FALSE(mVSyncSetEnabledCallRecorder.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mVSyncSetCallbackCallRecorder.waitForCall(0us).has_value()); + + // However if there is another connection which wants events at a nonzero rate..... + ConnectionEventRecorder secondConnectionEventRecorder{0}; + sp secondConnection = + createConnection(secondConnectionEventRecorder); + mThread->setVsyncRate(1, secondConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // Send a vsync event. EventThread should then make a call to the + // interceptor, and the second connection. The first connection should not + // get the event. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + EXPECT_FALSE(firstConnectionEventRecorder.waitForUnexpectedCall().has_value()); + expectVsyncEventReceivedByConnection("secondConnection", secondConnectionEventRecorder, 123, + 1u); +} + +TEST_F(EventThreadTest, setVsyncRateOnePostsAllEventsToThatConnection) { + mThread->setVsyncRate(1, mConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // Send a vsync event. EventThread should then make a call to the + // interceptor, and the connection. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + expectVsyncEventReceivedByConnection(123, 1u); + + // A second event should go to the same places. + callback->onVSyncEvent(456); + expectInterceptCallReceived(456); + expectVsyncEventReceivedByConnection(456, 2u); + + // A third event should go to the same places. + callback->onVSyncEvent(789); + expectInterceptCallReceived(789); + expectVsyncEventReceivedByConnection(789, 3u); +} + +TEST_F(EventThreadTest, setVsyncRateTwoPostsEveryOtherEventToThatConnection) { + mThread->setVsyncRate(2, mConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // The first event will be seen by the interceptor, and not the connection. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); + + // The second event will be seen by the interceptor and the connection. + callback->onVSyncEvent(456); + expectInterceptCallReceived(456); + expectVsyncEventReceivedByConnection(456, 2u); + + // The third event will be seen by the interceptor, and not the connection. + callback->onVSyncEvent(789); + expectInterceptCallReceived(789); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); + + // The fourth event will be seen by the interceptor and the connection. + callback->onVSyncEvent(101112); + expectInterceptCallReceived(101112); + expectVsyncEventReceivedByConnection(101112, 4u); +} + +TEST_F(EventThreadTest, connectionsRemovedIfInstanceDestroyed) { + mThread->setVsyncRate(1, mConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // Destroy the only (strong) reference to the connection. + mConnection = nullptr; + + // The first event will be seen by the interceptor, and not the connection. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); + + // EventThread should disable vsync callbacks + expectVSyncSetEnabledCallReceived(false); +} + +TEST_F(EventThreadTest, connectionsRemovedIfEventDeliveryError) { + ConnectionEventRecorder errorConnectionEventRecorder{NO_MEMORY}; + sp errorConnection = createConnection(errorConnectionEventRecorder); + mThread->setVsyncRate(1, errorConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // The first event will be seen by the interceptor, and by the connection, + // which then returns an error. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u); + + // A subsequent event will be seen by the interceptor and not by the + // connection. + callback->onVSyncEvent(456); + expectInterceptCallReceived(456); + EXPECT_FALSE(errorConnectionEventRecorder.waitForUnexpectedCall().has_value()); + + // EventThread should disable vsync callbacks with the second event + expectVSyncSetEnabledCallReceived(false); +} + +TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) { + ConnectionEventRecorder errorConnectionEventRecorder{WOULD_BLOCK}; + sp errorConnection = createConnection(errorConnectionEventRecorder); + mThread->setVsyncRate(1, errorConnection); + + // EventThread should enable vsync callbacks, and set a callback interface + // pointer to use them with the VSync source. + expectVSyncSetEnabledCallReceived(true); + auto callback = expectVSyncSetCallbackCallReceived(); + ASSERT_TRUE(callback); + + // The first event will be seen by the interceptor, and by the connection, + // which then returns an non-fatal error. + callback->onVSyncEvent(123); + expectInterceptCallReceived(123); + expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u); + + // A subsequent event will be seen by the interceptor, and by the connection, + // which still then returns an non-fatal error. + callback->onVSyncEvent(456); + expectInterceptCallReceived(456); + expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 456, 2u); + + // EventThread will not disable vsync callbacks as the errors are non-fatal. + EXPECT_FALSE(mVSyncSetEnabledCallRecorder.waitForUnexpectedCall().has_value()); +} + +TEST_F(EventThreadTest, setPhaseOffsetForwardsToVSyncSource) { + mThread->setPhaseOffset(321); + expectVSyncSetPhaseOffsetCallReceived(321); +} + +TEST_F(EventThreadTest, postHotplugPrimaryDisconnect) { + mThread->onHotplugReceived(DisplayDevice::DISPLAY_PRIMARY, false); + expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_PRIMARY, false); +} + +TEST_F(EventThreadTest, postHotplugPrimaryConnect) { + mThread->onHotplugReceived(DisplayDevice::DISPLAY_PRIMARY, true); + expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_PRIMARY, true); +} + +TEST_F(EventThreadTest, postHotplugExternalDisconnect) { + mThread->onHotplugReceived(DisplayDevice::DISPLAY_EXTERNAL, false); + expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_EXTERNAL, false); +} + +TEST_F(EventThreadTest, postHotplugExternalConnect) { + mThread->onHotplugReceived(DisplayDevice::DISPLAY_EXTERNAL, true); + expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_EXTERNAL, true); +} + +TEST_F(EventThreadTest, postHotplugVirtualDisconnectIsFilteredOut) { + mThread->onHotplugReceived(DisplayDevice::DISPLAY_VIRTUAL, false); + EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); +} + +} // namespace +} // namespace android -- cgit v1.2.3-59-g8ed1b