From 1ffdccc46ea67d3e4c39e3fa44f21b36dd46f3c5 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 14 Jun 2012 23:39:35 -0700 Subject: [PATCH] SF could get stuck waiting for vsync when turning the screen off When turning the screen off we could have 2 waiters on the vsync condition: The main vsync waiter as well as one in onScreenReleased(). We were only signaling the condition though, so it it would be possible to wake onScreenReleased() without waking the main vsync thread which would then be stuck in .wait(). We fix this by just using broadcast() when receiving a vsync event. We also add a broadcast() to signal when the state of mUseSoftwareVSync changes. This is important particularly for the transition from hardware to software vsync because the main vsync waiter might have observed mUseSoftwareVSync == false and decided to block indefinitely pending a hardware vsync signal that will never arrive. Removed a potentially deadlocking wait for a signal in onScreenReleased(). The function was trying to wait for the last vsync event from the hardware to be delivered to clients but there was no guarantee that another thread would signal it to wake up again afterwards. (As far as I can tell, the only other other thread that might wake it up at this point would be a client application issuing a vsync request.) We don't really need to wait here anyhow. It's enough to set the mUseSoftwareVSync flag, wake up the thread loop and go. If there was a pending vsync timestamp from the hardware, then the thread loop will grab it and use it then start software vsync on the next iteration. Bug: 6672102 Change-Id: I7c6abc23bb021d1dfc94f101bd3ce18e3a81a73e --- services/surfaceflinger/EventThread.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 47fa2f3df..7c1aebe0b 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -60,7 +60,7 @@ status_t EventThread::registerDisplayEventConnection( const sp& connection) { Mutex::Autolock _l(mLock); mDisplayEventConnections.add(connection); - mCondition.signal(); + mCondition.broadcast(); return NO_ERROR; } @@ -68,7 +68,7 @@ status_t EventThread::unregisterDisplayEventConnection( const wp& connection) { Mutex::Autolock _l(mLock); mDisplayEventConnections.remove(connection); - mCondition.signal(); + mCondition.broadcast(); return NO_ERROR; } @@ -85,7 +85,7 @@ void EventThread::setVsyncRate(uint32_t count, const int32_t new_count = (count == 0) ? -1 : count; if (connection->count != new_count) { connection->count = new_count; - mCondition.signal(); + mCondition.broadcast(); } } } @@ -95,32 +95,33 @@ void EventThread::requestNextVsync( Mutex::Autolock _l(mLock); if (connection->count < 0) { connection->count = 0; - mCondition.signal(); + mCondition.broadcast(); } } void EventThread::onScreenReleased() { Mutex::Autolock _l(mLock); - // wait for an eventual pending vsync to be serviced if (!mUseSoftwareVSync) { - while (mVSyncTimestamp) { - mCondition.wait(mLock); - } + // disable reliance on h/w vsync + mUseSoftwareVSync = true; + mCondition.broadcast(); } - // disable reliance on h/w vsync - mUseSoftwareVSync = true; } void EventThread::onScreenAcquired() { Mutex::Autolock _l(mLock); - mUseSoftwareVSync = false; + if (mUseSoftwareVSync) { + // resume use of h/w vsync + mUseSoftwareVSync = false; + mCondition.broadcast(); + } } void EventThread::onVSyncReceived(int, nsecs_t timestamp) { Mutex::Autolock _l(mLock); mVSyncTimestamp = timestamp; - mCondition.signal(); + mCondition.broadcast(); } bool EventThread::threadLoop() {