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 ++++++++++++++--------------------- 1 file changed, 46 insertions(+), 72 deletions(-) (limited to 'services/surfaceflinger/DispSync.cpp') 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; } -- 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/DispSync.cpp') 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 2713c30843816d3511b39b85a2c268a2b7682047 Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Wed, 28 Mar 2018 17:07:36 -0700 Subject: Early wake-up for transitions (1/2) On some devices it's very likely that we fall into GL comp during app transitions. However, SF offsets are chosen in a way such that the time to finish a frame is just too tight to be completely jank free when hitting GL composition in SurfaceFlinger. Thus, we introduce the concept of a separate early offset, and wakeup SurfaceFlinger at that time if we think that hitting GL comp is likely, or we already hit GL comp in the last frame. Test: Open app, check vsync offsets in systrace Test: Open many dialogs/apps to fall into GPU comp. Bug: 75985430 Change-Id: Ie17e30c4575359fa11bb8912f68dcafe3e569ddb Merged-In: Ie17e30c4575359fa11bb8912f68dcafe3e569ddb --- libs/gui/SurfaceComposerClient.cpp | 11 ++- libs/gui/include/gui/ISurfaceComposer.h | 5 + libs/gui/include/gui/SurfaceComposerClient.h | 2 + services/surfaceflinger/BufferLayer.cpp | 1 + services/surfaceflinger/BufferLayerConsumer.cpp | 1 + services/surfaceflinger/DispSync.cpp | 27 ++++++ services/surfaceflinger/DispSync.h | 5 + .../RenderEngine/GLES20RenderEngine.cpp | 1 + services/surfaceflinger/SurfaceFlinger.cpp | 41 +++++---- services/surfaceflinger/SurfaceFlinger.h | 6 ++ services/surfaceflinger/VSyncModulator.h | 101 +++++++++++++++++++++ 11 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 services/surfaceflinger/VSyncModulator.h (limited to 'services/surfaceflinger/DispSync.cpp') diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index bbf681ea90..63560c4b89 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -101,7 +101,8 @@ void ComposerService::composerServiceDied() SurfaceComposerClient::Transaction::Transaction(const Transaction& other) : mForceSynchronous(other.mForceSynchronous), mTransactionNestCount(other.mTransactionNestCount), - mAnimation(other.mAnimation) { + mAnimation(other.mAnimation), + mEarlyWakeup(other.mEarlyWakeup) { mDisplayStates = other.mDisplayStates; mComposerStates = other.mComposerStates; } @@ -157,9 +158,13 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { if (mAnimation) { flags |= ISurfaceComposer::eAnimation; } + if (mEarlyWakeup) { + flags |= ISurfaceComposer::eEarlyWakeup; + } mForceSynchronous = false; mAnimation = false; + mEarlyWakeup = false; sf->setTransactionState(composerStates, displayStates, flags); mStatus = NO_ERROR; @@ -185,6 +190,10 @@ void SurfaceComposerClient::Transaction::setAnimationTransaction() { mAnimation = true; } +void SurfaceComposerClient::Transaction::setEarlyWakeup() { + mEarlyWakeup = true; +} + layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp& sc) { if (mComposerStates.count(sc) == 0) { // we don't have it, add an initialized layer_state to our list diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 3591090172..e40157206d 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -61,6 +61,11 @@ public: enum { eSynchronous = 0x01, eAnimation = 0x02, + + // Indicates that this transaction will likely result in a lot of layers being composed, and + // thus, SurfaceFlinger should wake-up earlier to avoid missing frame deadlines. In this + // case SurfaceFlinger will wake up at (sf vsync offset - debug.sf.early_phase_offset_ns) + eEarlyWakeup = 0x04 }; enum { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index ffc22f6437..377fe68c41 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -154,6 +154,7 @@ public: uint32_t mForceSynchronous = 0; uint32_t mTransactionNestCount = 0; bool mAnimation = false; + bool mEarlyWakeup = false; int mStatus = NO_ERROR; @@ -273,6 +274,7 @@ public: const Rect& displayRect); void setDisplaySize(const sp& token, uint32_t width, uint32_t height); void setAnimationTransaction(); + void setEarlyWakeup(); }; status_t destroySurface(const sp& id); diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index 82300e679f..2aa4cd3578 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -778,6 +778,7 @@ bool BufferLayer::getOpacityForFormat(uint32_t format) { } void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityTransform) const { + ATRACE_CALL(); const State& s(getDrawingState()); computeGeometry(renderArea, getBE().mMesh, useIdentityTransform); diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index 6b4f5dbfb0..87333d0ffd 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -356,6 +356,7 @@ status_t BufferLayerConsumer::updateAndReleaseLocked(const BufferItem& item, } status_t BufferLayerConsumer::bindTextureImageLocked() { + ATRACE_CALL(); mRE.checkErrors(); if (mCurrentTexture == BufferQueue::INVALID_BUFFER_SLOT && mCurrentTextureImage == nullptr) { diff --git a/services/surfaceflinger/DispSync.cpp b/services/surfaceflinger/DispSync.cpp index 9e01fd0d8d..7acbd11bc5 100644 --- a/services/surfaceflinger/DispSync.cpp +++ b/services/surfaceflinger/DispSync.cpp @@ -209,6 +209,28 @@ public: return BAD_VALUE; } + status_t changePhaseOffset(DispSync::Callback* callback, nsecs_t phase) { + if (kTraceDetailedInfo) ATRACE_CALL(); + Mutex::Autolock lock(mMutex); + + for (size_t i = 0; i < mEventListeners.size(); i++) { + if (mEventListeners[i].mCallback == callback) { + EventListener& listener = mEventListeners.editItemAt(i); + const nsecs_t oldPhase = listener.mPhase; + listener.mPhase = phase; + + // Pretend that the last time this event was handled at the same frame but with the + // new offset to allow for a seamless offset change without double-firing or + // skipping. + listener.mLastEventTime -= (oldPhase - phase); + mCond.signal(); + return NO_ERROR; + } + } + + return BAD_VALUE; + } + // This method is only here to handle the !SurfaceFlinger::hasSyncFramework // case. bool hasAnyEventListeners() { @@ -487,6 +509,11 @@ status_t DispSync::removeEventListener(Callback* callback) { return mThread->removeEventListener(callback); } +status_t DispSync::changePhaseOffset(Callback* callback, nsecs_t phase) { + Mutex::Autolock lock(mMutex); + return mThread->changePhaseOffset(callback, phase); +} + void DispSync::setPeriod(nsecs_t period) { Mutex::Autolock lock(mMutex); mPeriod = period; diff --git a/services/surfaceflinger/DispSync.h b/services/surfaceflinger/DispSync.h index 9336f4dd86..077256ac11 100644 --- a/services/surfaceflinger/DispSync.h +++ b/services/surfaceflinger/DispSync.h @@ -113,6 +113,11 @@ public: // DispSync object. status_t removeEventListener(Callback* callback); + // changePhaseOffset changes the phase offset of an already-registered event callback. The + // method will make sure that there is no skipping or double-firing on the listener per frame, + // even when changing the offsets multiple times. + status_t changePhaseOffset(Callback* callback, nsecs_t phase); + // computeNextRefresh computes when the next refresh is expected to begin. // The periodOffset value can be used to move forward or backward; an // offset of zero is the next refresh, -1 is the previous refresh, 1 is diff --git a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp index 1fc310025d..0fb3d28d05 100644 --- a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp +++ b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp @@ -293,6 +293,7 @@ void GLES20RenderEngine::setupFillWithColor(float r, float g, float b, float a) } void GLES20RenderEngine::drawMesh(const Mesh& mesh) { + ATRACE_CALL(); if (mesh.getTexCoordsSize()) { glEnableVertexAttribArray(Program::texCoords); glVertexAttribPointer(Program::texCoords, mesh.getTexCoordsSize(), GL_FLOAT, GL_FALSE, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 5be7951972..183c1eb07c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -296,6 +296,12 @@ SurfaceFlinger::SurfaceFlinger() : SurfaceFlinger(SkipInitialization) { auto listSize = property_get_int32("debug.sf.max_igbp_list_size", int32_t(defaultListSize)); mMaxGraphicBufferProducerListSize = (listSize > 0) ? size_t(listSize) : defaultListSize; + property_get("debug.sf.early_phase_offset_ns", value, "0"); + const int earlyWakeupOffsetOffsetNs = atoi(value); + ALOGI_IF(earlyWakeupOffsetOffsetNs != 0, "Enabling separate early offset"); + mVsyncModulator.setPhaseOffsets(sfVsyncPhaseOffsetNs - earlyWakeupOffsetOffsetNs, + sfVsyncPhaseOffsetNs); + // We should be reading 'persist.sys.sf.color_saturation' here // but since /data may be encrypted, we need to wait until after vold // comes online to attempt to read the property. The property is @@ -522,19 +528,10 @@ public: return; } - // Remove the listener with the old offset - status_t err = mDispSync->removeEventListener( - static_cast(this)); + status_t err = mDispSync->changePhaseOffset(static_cast(this), + mPhaseOffset); if (err != NO_ERROR) { - ALOGE("error unregistering vsync callback: %s (%d)", - strerror(-err), err); - } - - // Add a listener with the new offset - err = mDispSync->addEventListener(mName, mPhaseOffset, - static_cast(this)); - if (err != NO_ERROR) { - ALOGE("error registering vsync callback: %s (%d)", + ALOGE("error changing vsync offset: %s (%d)", strerror(-err), err); } } @@ -623,6 +620,7 @@ void SurfaceFlinger::init() { mSFEventThread = std::make_unique(mSfEventThreadSource.get(), *this, true, "sfEventThread"); mEventQueue->setEventThread(mSFEventThread.get()); + mVsyncModulator.setEventThread(mSFEventThread.get()); // Get a RenderEngine for the given display / config (can't fail) getBE().mRenderEngine = @@ -1498,6 +1496,7 @@ void SurfaceFlinger::handleMessageRefresh() { mHadClientComposition = mHadClientComposition || getBE().mHwc->hasClientComposition(displayDevice->getHwcDisplayId()); } + mVsyncModulator.setLastFrameUsedRenderEngine(mHadClientComposition); mLayersWithQueuedFrames.clear(); } @@ -2122,6 +2121,7 @@ void SurfaceFlinger::handleTransaction(uint32_t transactionFlags) // with mStateLock held to guarantee that mCurrentState won't change // until the transaction is committed. + mVsyncModulator.setTransactionStart(VSyncModulator::TransactionStart::NORMAL); transactionFlags = getTransactionFlags(eTransactionMask); handleTransactionLocked(transactionFlags); @@ -3070,7 +3070,13 @@ uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags) { } uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags) { + return setTransactionFlags(flags, VSyncModulator::TransactionStart::NORMAL); +} + +uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags, + VSyncModulator::TransactionStart transactionStart) { uint32_t old = android_atomic_or(flags, &mTransactionFlags); + mVsyncModulator.setTransactionStart(transactionStart); if ((old & flags)==0) { // wake the server up signalTransaction(); } @@ -3159,7 +3165,10 @@ void SurfaceFlinger::setTransactionState( } // this triggers the transaction - setTransactionFlags(transactionFlags); + const auto start = (flags & eEarlyWakeup) + ? VSyncModulator::TransactionStart::EARLY + : VSyncModulator::TransactionStart::NORMAL; + setTransactionFlags(transactionFlags, start); // if this is a synchronous transaction, wait for it to take effect // before returning. @@ -4099,9 +4108,9 @@ void SurfaceFlinger::dumpAllLocked(const Vector& args, size_t& index, colorizer.bold(result); result.append("DispSync configuration: "); colorizer.reset(result); - result.appendFormat("app phase %" PRId64 " ns, sf phase %" PRId64 " ns, " - "present offset %" PRId64 " ns (refresh %" PRId64 " ns)", - vsyncPhaseOffsetNs, sfVsyncPhaseOffsetNs, + result.appendFormat("app phase %" PRId64 " ns, sf phase %" PRId64 " ns, early sf phase %" PRId64 + " ns, present offset %" PRId64 " ns (refresh %" PRId64 " ns)", + vsyncPhaseOffsetNs, sfVsyncPhaseOffsetNs, mVsyncModulator.getEarlyPhaseOffset(), dispSyncPresentTimeOffset, activeConfig->getVsyncPeriod()); result.append("\n"); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 89c9cfda19..a29d1d7475 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -54,6 +55,7 @@ #include "Barrier.h" #include "DisplayDevice.h" #include "DispSync.h" +#include "EventThread.h" #include "FrameTracker.h" #include "LayerStats.h" #include "LayerVector.h" @@ -61,6 +63,7 @@ #include "SurfaceInterceptor.h" #include "SurfaceTracing.h" #include "StartPropertySetThread.h" +#include "VSyncModulator.h" #include "DisplayHardware/HWC2.h" #include "DisplayHardware/HWComposer.h" @@ -492,6 +495,7 @@ private: uint32_t peekTransactionFlags(); // Can only be called from the main thread or with mStateLock held uint32_t setTransactionFlags(uint32_t flags); + uint32_t setTransactionFlags(uint32_t flags, VSyncModulator::TransactionStart transactionStart); void commitTransaction(); bool containsAnyInvalidClientState(const Vector& states); uint32_t setClientStateLocked(const ComposerState& composerState); @@ -765,6 +769,8 @@ private: std::unique_ptr mEventControlThread; sp mBuiltinDisplays[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES]; + VSyncModulator mVsyncModulator; + // Can only accessed from the main thread, these members // don't need synchronization State mDrawingState{LayerVector::StateSet::Drawing}; diff --git a/services/surfaceflinger/VSyncModulator.h b/services/surfaceflinger/VSyncModulator.h new file mode 100644 index 0000000000..3126debed4 --- /dev/null +++ b/services/surfaceflinger/VSyncModulator.h @@ -0,0 +1,101 @@ +/* + * Copyright 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 + +using namespace android::surfaceflinger; + +namespace android { + +/* + * Modulates the vsync-offsets depending on current SurfaceFlinger state. + */ +class VSyncModulator { +public: + + enum TransactionStart { + EARLY, + NORMAL + }; + + // Sets the phase offsets + // + // early: the phase offset when waking up early. May be the same as late, in which case we don't + // shift offsets. + // late: the regular sf phase offset. + void setPhaseOffsets(nsecs_t early, nsecs_t late) { + mEarlyPhaseOffset = early; + mLatePhaseOffset = late; + mPhaseOffset = late; + } + + nsecs_t getEarlyPhaseOffset() const { + return mEarlyPhaseOffset; + } + + void setEventThread(EventThread* eventThread) { + mEventThread = eventThread; + } + + void setTransactionStart(TransactionStart transactionStart) { + if (transactionStart == mTransactionStart) return; + mTransactionStart = transactionStart; + updatePhaseOffsets(); + } + + void setLastFrameUsedRenderEngine(bool re) { + if (re == mLastFrameUsedRenderEngine) return; + mLastFrameUsedRenderEngine = re; + updatePhaseOffsets(); + } + +private: + + void updatePhaseOffsets() { + + // Do not change phase offsets if disabled. + if (mEarlyPhaseOffset == mLatePhaseOffset) return; + + if (mTransactionStart == TransactionStart::EARLY || mLastFrameUsedRenderEngine) { + if (mPhaseOffset != mEarlyPhaseOffset) { + if (mEventThread) { + mEventThread->setPhaseOffset(mEarlyPhaseOffset); + } + mPhaseOffset = mEarlyPhaseOffset; + } + } else { + if (mPhaseOffset != mLatePhaseOffset) { + if (mEventThread) { + mEventThread->setPhaseOffset(mLatePhaseOffset); + } + mPhaseOffset = mLatePhaseOffset; + } + } + } + + nsecs_t mLatePhaseOffset = 0; + nsecs_t mEarlyPhaseOffset = 0; + EventThread* mEventThread = nullptr; + std::atomic mPhaseOffset = 0; + std::atomic mTransactionStart = TransactionStart::NORMAL; + std::atomic mLastFrameUsedRenderEngine = false; +}; + +} // namespace android -- cgit v1.2.3-59-g8ed1b