From f6bbd44a23c2791277db7814a894633de04cd460 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 21 Aug 2012 15:47:28 -0700 Subject: [PATCH] simplify further vsync handling - we now clean-up "dead" connection in the main loop, this entirely avoid the problem with the side effects of releasing strong references. We now only hold on to strong reference for the connection we will signal. - also simplify how we build the list of "ready" connections, by only adding them to the list when we did receive a vsync event Change-Id: I2a84da431320a2af8e8a93e07622a1d258236f43 --- services/surfaceflinger/EventThread.cpp | 185 +++++++++++------------- services/surfaceflinger/EventThread.h | 4 +- 2 files changed, 91 insertions(+), 98 deletions(-) diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 59390c0ce..a39b7d8c5 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -60,14 +60,6 @@ status_t EventThread::registerDisplayEventConnection( return NO_ERROR; } -status_t EventThread::unregisterDisplayEventConnection( - const wp& connection) { - Mutex::Autolock _l(mLock); - mDisplayEventConnections.remove(connection); - mCondition.broadcast(); - return NO_ERROR; -} - void EventThread::removeDisplayEventConnection( const wp& connection) { Mutex::Autolock _l(mLock); @@ -122,97 +114,11 @@ void EventThread::onVSyncReceived(int, nsecs_t timestamp) { } bool EventThread::threadLoop() { - - nsecs_t timestamp; - size_t vsyncCount; DisplayEventReceiver::Event vsync; - Vector< sp > activeConnections; Vector< sp > signalConnections; - - do { - // release our references - signalConnections.clear(); - activeConnections.clear(); - - Mutex::Autolock _l(mLock); - - // latch VSYNC event if any - bool waitForVSync = false; - vsyncCount = mVSyncCount; - timestamp = mVSyncTimestamp; - mVSyncTimestamp = 0; - - // find out connections waiting for VSYNC events - size_t count = mDisplayEventConnections.size(); - for (size_t i=0 ; i connection(mDisplayEventConnections[i].promote()); - if (connection != NULL) { - activeConnections.add(connection); - if (connection->count >= 0) { - // we need vsync events because at least - // one connection is waiting for it - waitForVSync = true; - if (connection->count == 0) { - // fired this time around - if (timestamp) { - // only "consume" this event if we're going to - // report it - connection->count = -1; - } - signalConnections.add(connection); - } else if (connection->count == 1 || - (vsyncCount % connection->count) == 0) { - // continuous event, and time to report it - signalConnections.add(connection); - } - } - } - } - - if (timestamp) { - // we have a vsync event we can dispatch - if (!waitForVSync) { - // we received a VSYNC but we have no clients - // don't report it, and disable VSYNC events - disableVSyncLocked(); - } else { - // report VSYNC event - break; - } - } else { - // never disable VSYNC events immediately, instead - // we'll wait to receive the event and we'll - // reevaluate whether we need to dispatch it and/or - // disable VSYNC events then. - if (waitForVSync) { - // enable - enableVSyncLocked(); - } - } - - // wait for something to happen - if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) { - // h/w vsync cannot be used (screen is off), so we use - // a timeout instead. it doesn't matter how imprecise this - // is, we just need to make sure to serve the clients - if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) { - mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC); - mVSyncCount++; - } - } else { - if (!timestamp || signalConnections.isEmpty()) { - // This is where we spend most of our time, waiting - // for a vsync events and registered clients - mCondition.wait(mLock); - } - } - } while (!timestamp || signalConnections.isEmpty()); + signalConnections = waitForEvent(&vsync); // dispatch vsync events to listeners... - vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; - vsync.header.timestamp = timestamp; - vsync.vsync.count = vsyncCount; - const size_t count = signalConnections.size(); for (size_t i=0 ; i& conn(signalConnections[i]); @@ -231,10 +137,94 @@ bool EventThread::threadLoop() { removeDisplayEventConnection(signalConnections[i]); } } - return true; } +Vector< sp > EventThread::waitForEvent( + DisplayEventReceiver::Event* event) +{ + Mutex::Autolock _l(mLock); + + size_t vsyncCount; + nsecs_t timestamp; + Vector< sp > signalConnections; + + do { + // latch VSYNC event if any + bool waitForVSync = false; + vsyncCount = mVSyncCount; + timestamp = mVSyncTimestamp; + mVSyncTimestamp = 0; + + // find out connections waiting for events + size_t count = mDisplayEventConnections.size(); + for (size_t i=0 ; i connection(mDisplayEventConnections[i].promote()); + if (connection != NULL) { + if (connection->count >= 0) { + // we need vsync events because at least + // one connection is waiting for it + waitForVSync = true; + if (timestamp) { + // we consume the event only if it's time + // (ie: we received a vsync event) + if (connection->count == 0) { + // fired this time around + connection->count = -1; + signalConnections.add(connection); + } else if (connection->count == 1 || + (vsyncCount % connection->count) == 0) { + // continuous event, and time to report it + signalConnections.add(connection); + } + } + } + } else { + // we couldn't promote this reference, the connection has + // died, so clean-up! + mDisplayEventConnections.removeAt(i); + --i; --count; + } + } + + // Here we figure out if we need to enable or disable vsyncs + if (timestamp && !waitForVSync) { + // we received a VSYNC but we have no clients + // don't report it, and disable VSYNC events + disableVSyncLocked(); + } else if (!timestamp && waitForVSync) { + enableVSyncLocked(); + } + + // note: !timestamp implies signalConnections.isEmpty() + if (!timestamp) { + // wait for something to happen + if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) { + // h/w vsync cannot be used (screen is off), so we use + // a timeout instead. it doesn't matter how imprecise this + // is, we just need to make sure to serve the clients + if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) { + mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC); + mVSyncCount++; + } + } else { + // This is where we spend most of our time, waiting + // for a vsync events and registered clients + mCondition.wait(mLock); + } + } + } while (signalConnections.isEmpty()); + + // here we're guaranteed to have a timestamp and some connections to signal + + // dispatch vsync events to listeners... + event->header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; + event->header.timestamp = timestamp; + event->vsync.count = vsyncCount; + + return signalConnections; +} + void EventThread::enableVSyncLocked() { if (!mUseSoftwareVSync) { // never enable h/w VSYNC when screen is off @@ -280,7 +270,8 @@ EventThread::Connection::Connection( } EventThread::Connection::~Connection() { - mEventThread->unregisterDisplayEventConnection(this); + // do nothing here -- clean-up will happen automatically + // when the main thread wakes up } void EventThread::Connection::onFirstRef() { diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index aa0ea7f3c..20ea34d36 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -65,7 +65,6 @@ public: sp createEventConnection() const; status_t registerDisplayEventConnection(const sp& connection); - status_t unregisterDisplayEventConnection(const wp& connection); void setVsyncRate(uint32_t count, const sp& connection); void requestNextVsync(const sp& connection); @@ -79,6 +78,9 @@ public: // called when receiving a vsync event void onVSyncReceived(int display, nsecs_t timestamp); + Vector< sp > waitForEvent( + DisplayEventReceiver::Event* event); + void dump(String8& result, char* buffer, size_t SIZE) const; private: