diff options
author | Alec Mouri <alecmouri@google.com> | 2020-08-05 12:50:03 -0700 |
---|---|---|
committer | Alec Mouri <alecmouri@google.com> | 2020-08-07 20:59:20 +0000 |
commit | c080030e715082f7a910066bbcd8cc7a3a8500eb (patch) | |
tree | d6579862669cfd5249a433405726603cf77412f4 | |
parent | 140f65189fd2aaa43bc991556a9678a4ff981731 (diff) | |
download | native-c080030e715082f7a910066bbcd8cc7a3a8500eb.tar.gz |
Fix refresh rate callback fan-out for choreographer
* AChoreographer receives refresh rates from DisplayManager
already, so there's no need to default-enable them - AChoreographer only
needs to pump an event with the latest refresh rate to wake up looper. This
also ensures that AChoreographer's callbacks are entirely in-sync with
DisplayManager since there's no raciness.
* Instead of re-requesting a config change from SF, instead inject it in
AChoreographer correctly to save on binder.
Bug: 154874011
Bug: 158680912
Bug: 161406626
Test: while [ true ]; do adb shell service call SurfaceFlinger 1035 i32
1; adb shell service call SurfaceFlinger 1035 i32 0; and repeatedly
rotate the home screen with auto-rotate off.
Change-Id: I66abc2e28e60f06987ce3a54be294c94b77524fc
Merged-In: I66abc2e28e60f06987ce3a54be294c94b77524fc
-rw-r--r-- | libs/gui/BitTube.cpp | 16 | ||||
-rw-r--r-- | libs/gui/DisplayEventDispatcher.cpp | 12 | ||||
-rw-r--r-- | libs/gui/DisplayEventReceiver.cpp | 12 | ||||
-rw-r--r-- | libs/gui/IDisplayEventConnection.cpp | 10 | ||||
-rw-r--r-- | libs/gui/include/gui/DisplayEventDispatcher.h | 5 | ||||
-rw-r--r-- | libs/gui/include/gui/DisplayEventReceiver.h | 8 | ||||
-rw-r--r-- | libs/gui/include/gui/IDisplayEventConnection.h | 5 | ||||
-rw-r--r-- | libs/gui/include/private/gui/BitTube.h | 5 | ||||
-rw-r--r-- | libs/nativedisplay/AChoreographer.cpp | 36 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/EventThread.cpp | 39 | ||||
-rw-r--r-- | services/surfaceflinger/Scheduler/EventThread.h | 14 |
11 files changed, 55 insertions, 107 deletions
diff --git a/libs/gui/BitTube.cpp b/libs/gui/BitTube.cpp index ef7a6f54d9..351af652e2 100644 --- a/libs/gui/BitTube.cpp +++ b/libs/gui/BitTube.cpp @@ -86,6 +86,10 @@ void BitTube::setReceiveFd(base::unique_fd&& receiveFd) { mReceiveFd = std::move(receiveFd); } +void BitTube::setSendFd(base::unique_fd&& sendFd) { + mSendFd = std::move(sendFd); +} + ssize_t BitTube::write(void const* vaddr, size_t size) { ssize_t err, len; do { @@ -115,6 +119,11 @@ status_t BitTube::writeToParcel(Parcel* reply) const { status_t result = reply->writeDupFileDescriptor(mReceiveFd); mReceiveFd.reset(); + if (result != NO_ERROR) { + return result; + } + result = reply->writeDupFileDescriptor(mSendFd); + mSendFd.reset(); return result; } @@ -126,6 +135,13 @@ status_t BitTube::readFromParcel(const Parcel* parcel) { ALOGE("BitTube::readFromParcel: can't dup file descriptor (%s)", strerror(error)); return -error; } + mSendFd.reset(dup(parcel->readFileDescriptor())); + if (mSendFd < 0) { + mSendFd.reset(); + int error = errno; + ALOGE("BitTube::readFromParcel: can't dup file descriptor (%s)", strerror(error)); + return -error; + } return NO_ERROR; } diff --git a/libs/gui/DisplayEventDispatcher.cpp b/libs/gui/DisplayEventDispatcher.cpp index b33bc9e556..682fe911de 100644 --- a/libs/gui/DisplayEventDispatcher.cpp +++ b/libs/gui/DisplayEventDispatcher.cpp @@ -89,12 +89,8 @@ status_t DisplayEventDispatcher::scheduleVsync() { return OK; } -void DisplayEventDispatcher::requestLatestConfig() { - status_t status = mReceiver.requestLatestConfig(); - if (status) { - ALOGW("Failed enable config events, status=%d", status); - return; - } +void DisplayEventDispatcher::injectEvent(const DisplayEventReceiver::Event& event) { + mReceiver.sendEvents(&event, 1); } int DisplayEventDispatcher::getFd() const { @@ -157,6 +153,9 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp, dispatchConfigChanged(ev.header.timestamp, ev.header.displayId, ev.config.configId, ev.config.vsyncPeriod); break; + case DisplayEventReceiver::DISPLAY_EVENT_NULL: + dispatchNullEvent(ev.header.timestamp, ev.header.displayId); + break; default: ALOGW("dispatcher %p ~ ignoring unknown event type %#x", this, ev.header.type); break; @@ -168,4 +167,5 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp, } return gotVsync; } + } // namespace android diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp index 1fed509003..f2b0962eb7 100644 --- a/libs/gui/DisplayEventReceiver.cpp +++ b/libs/gui/DisplayEventReceiver.cpp @@ -79,14 +79,6 @@ status_t DisplayEventReceiver::requestNextVsync() { return NO_INIT; } -status_t DisplayEventReceiver::requestLatestConfig() { - if (mEventConnection != nullptr) { - mEventConnection->requestLatestConfig(); - return NO_ERROR; - } - return NO_INIT; -} - ssize_t DisplayEventReceiver::getEvents(DisplayEventReceiver::Event* events, size_t count) { return DisplayEventReceiver::getEvents(mDataChannel.get(), events, count); @@ -98,6 +90,10 @@ ssize_t DisplayEventReceiver::getEvents(gui::BitTube* dataChannel, return gui::BitTube::recvObjects(dataChannel, events, count); } +ssize_t DisplayEventReceiver::sendEvents(Event const* events, size_t count) { + return DisplayEventReceiver::sendEvents(mDataChannel.get(), events, count); +} + ssize_t DisplayEventReceiver::sendEvents(gui::BitTube* dataChannel, Event const* events, size_t count) { diff --git a/libs/gui/IDisplayEventConnection.cpp b/libs/gui/IDisplayEventConnection.cpp index aa74bfd3f8..c0e246fa15 100644 --- a/libs/gui/IDisplayEventConnection.cpp +++ b/libs/gui/IDisplayEventConnection.cpp @@ -26,8 +26,7 @@ enum class Tag : uint32_t { STEAL_RECEIVE_CHANNEL = IBinder::FIRST_CALL_TRANSACTION, SET_VSYNC_RATE, REQUEST_NEXT_VSYNC, - REQUEST_LATEST_CONFIG, - LAST = REQUEST_LATEST_CONFIG, + LAST = REQUEST_NEXT_VSYNC, }; } // Anonymous namespace @@ -54,11 +53,6 @@ public: callRemoteAsync<decltype(&IDisplayEventConnection::requestNextVsync)>( Tag::REQUEST_NEXT_VSYNC); } - - void requestLatestConfig() override { - callRemoteAsync<decltype(&IDisplayEventConnection::requestLatestConfig)>( - Tag::REQUEST_LATEST_CONFIG); - } }; // Out-of-line virtual method definition to trigger vtable emission in this translation unit (see @@ -80,8 +74,6 @@ status_t BnDisplayEventConnection::onTransact(uint32_t code, const Parcel& data, return callLocal(data, reply, &IDisplayEventConnection::setVsyncRate); case Tag::REQUEST_NEXT_VSYNC: return callLocalAsync(data, reply, &IDisplayEventConnection::requestNextVsync); - case Tag::REQUEST_LATEST_CONFIG: - return callLocalAsync(data, reply, &IDisplayEventConnection::requestLatestConfig); } } diff --git a/libs/gui/include/gui/DisplayEventDispatcher.h b/libs/gui/include/gui/DisplayEventDispatcher.h index f210c34196..eb5b00418a 100644 --- a/libs/gui/include/gui/DisplayEventDispatcher.h +++ b/libs/gui/include/gui/DisplayEventDispatcher.h @@ -31,7 +31,7 @@ public: status_t initialize(); void dispose(); status_t scheduleVsync(); - void requestLatestConfig(); + void injectEvent(const DisplayEventReceiver::Event& event); int getFd() const; virtual int handleEvent(int receiveFd, int events, void* data); @@ -48,6 +48,9 @@ private: bool connected) = 0; virtual void dispatchConfigChanged(nsecs_t timestamp, PhysicalDisplayId displayId, int32_t configId, nsecs_t vsyncPeriod) = 0; + // AChoreographer-specific hook for processing null-events so that looper + // can be properly poked. + virtual void dispatchNullEvent(nsecs_t timestamp, PhysicalDisplayId displayId) = 0; bool processPendingEvents(nsecs_t* outTimestamp, PhysicalDisplayId* outDisplayId, uint32_t* outCount); diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index 8d49184caf..0e10d1ad8e 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -53,6 +53,7 @@ public: DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'), DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'), DISPLAY_EVENT_CONFIG_CHANGED = fourcc('c', 'o', 'n', 'f'), + DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'), }; struct Event { @@ -130,6 +131,7 @@ public: * sendEvents write events to the queue and returns how many events were * written. */ + ssize_t sendEvents(Event const* events, size_t count); static ssize_t sendEvents(gui::BitTube* dataChannel, Event const* events, size_t count); /* @@ -146,12 +148,6 @@ public: */ status_t requestNextVsync(); - /* - * requestLatestConfig() force-requests the current config for the primary - * display. - */ - status_t requestLatestConfig(); - private: sp<IDisplayEventConnection> mEventConnection; std::unique_ptr<gui::BitTube> mDataChannel; diff --git a/libs/gui/include/gui/IDisplayEventConnection.h b/libs/gui/include/gui/IDisplayEventConnection.h index 674aafd81c..cff22a368a 100644 --- a/libs/gui/include/gui/IDisplayEventConnection.h +++ b/libs/gui/include/gui/IDisplayEventConnection.h @@ -51,11 +51,6 @@ public: * requestNextVsync() schedules the next vsync event. It has no effect if the vsync rate is > 0. */ virtual void requestNextVsync() = 0; // Asynchronous - - /* - * requestLatestConfig() requests the config for the primary display. - */ - virtual void requestLatestConfig() = 0; // Asynchronous }; class BnDisplayEventConnection : public SafeBnInterface<IDisplayEventConnection> { diff --git a/libs/gui/include/private/gui/BitTube.h b/libs/gui/include/private/gui/BitTube.h index 13c01623b7..80485184bd 100644 --- a/libs/gui/include/private/gui/BitTube.h +++ b/libs/gui/include/private/gui/BitTube.h @@ -58,6 +58,9 @@ public: // resets this BitTube's receive file descriptor to receiveFd void setReceiveFd(base::unique_fd&& receiveFd); + // resets this BitTube's send file descriptor to sendFd + void setSendFd(base::unique_fd&& sendFd); + // send objects (sized blobs). All objects are guaranteed to be written or the call fails. template <typename T> static ssize_t sendObjects(BitTube* tube, T const* events, size_t count) { @@ -85,7 +88,7 @@ private: // the message, excess data is silently discarded. ssize_t read(void* vaddr, size_t size); - base::unique_fd mSendFd; + mutable base::unique_fd mSendFd; mutable base::unique_fd mReceiveFd; static ssize_t sendObjects(BitTube* tube, void const* events, size_t count, size_t objSize); diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp index e458b2ecfb..f6a95ce40c 100644 --- a/libs/nativedisplay/AChoreographer.cpp +++ b/libs/nativedisplay/AChoreographer.cpp @@ -136,6 +136,7 @@ private: void dispatchHotplug(nsecs_t timestamp, PhysicalDisplayId displayId, bool connected) override; void dispatchConfigChanged(nsecs_t timestamp, PhysicalDisplayId displayId, int32_t configId, nsecs_t vsyncPeriod) override; + void dispatchNullEvent(nsecs_t, PhysicalDisplayId) override; void scheduleCallbacks(); @@ -170,7 +171,7 @@ Choreographer* Choreographer::getForThread() { Choreographer::Choreographer(const sp<Looper>& looper) : DisplayEventDispatcher(looper, ISurfaceComposer::VsyncSource::eVsyncSourceApp, - ISurfaceComposer::ConfigChanged::eConfigChangedDispatch), + ISurfaceComposer::ConfigChanged::eConfigChangedSuppress), mLooper(looper), mThreadId(std::this_thread::get_id()) { std::lock_guard<std::mutex> _l(gChoreographers.lock); @@ -294,8 +295,14 @@ void Choreographer::scheduleLatestConfigRequest() { } else { // If the looper thread is detached from Choreographer, then refresh rate // changes will be handled in AChoreographer_handlePendingEvents, so we - // need to redispatch a config from SF - requestLatestConfig(); + // need to wake up the looper thread by writing to the write-end of the + // socket the looper is listening on. + // Fortunately, these events are small so sending packets across the + // socket should be atomic across processes. + DisplayEventReceiver::Event event; + event.header = DisplayEventReceiver::Event::Header{DisplayEventReceiver::DISPLAY_EVENT_NULL, + PhysicalDisplayId(0), systemTime()}; + injectEvent(event); } } @@ -374,28 +381,15 @@ void Choreographer::dispatchHotplug(nsecs_t, PhysicalDisplayId displayId, bool c // displays. When multi-display choreographer is properly supported, then // PhysicalDisplayId should no longer be ignored. void Choreographer::dispatchConfigChanged(nsecs_t, PhysicalDisplayId displayId, int32_t configId, - nsecs_t vsyncPeriod) { + nsecs_t) { ALOGV("choreographer %p ~ received config change event " "(displayId=%" ANDROID_PHYSICAL_DISPLAY_ID_FORMAT ", configId=%d).", this, displayId, configId); +} - const nsecs_t lastPeriod = mLatestVsyncPeriod; - std::vector<RefreshRateCallback> callbacks{}; - { - std::lock_guard<std::mutex> _l{mLock}; - for (auto& cb : mRefreshRateCallbacks) { - callbacks.push_back(cb); - cb.firstCallbackFired = true; - } - } - - for (auto& cb : callbacks) { - if (!cb.firstCallbackFired || (vsyncPeriod > 0 && vsyncPeriod != lastPeriod)) { - cb.callback(vsyncPeriod, cb.data); - } - } - - mLatestVsyncPeriod = vsyncPeriod; +void Choreographer::dispatchNullEvent(nsecs_t, PhysicalDisplayId) { + ALOGV("choreographer %p ~ received null event.", this); + handleRefreshRateUpdates(); } void Choreographer::handleMessage(const Message& message) { diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index cee36a121f..845bf50ad3 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -139,6 +139,7 @@ void EventThreadConnection::onFirstRef() { status_t EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) { outChannel->setReceiveFd(mChannel.moveReceiveFd()); + outChannel->setSendFd(base::unique_fd(dup(mChannel.getSendFd()))); return NO_ERROR; } @@ -152,11 +153,6 @@ void EventThreadConnection::requestNextVsync() { mEventThread->requestNextVsync(this); } -void EventThreadConnection::requestLatestConfig() { - ATRACE_NAME("requestLatestConfig"); - mEventThread->requestLatestConfig(this); -} - status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& event) { ssize_t size = DisplayEventReceiver::sendEvents(&mChannel, &event, 1); return size < 0 ? status_t(size) : status_t(NO_ERROR); @@ -269,28 +265,6 @@ void EventThread::requestNextVsync(const sp<EventThreadConnection>& connection) } } -void EventThread::requestLatestConfig(const sp<EventThreadConnection>& connection) { - std::lock_guard<std::mutex> lock(mMutex); - if (connection->mForcedConfigChangeDispatch) { - return; - } - connection->mForcedConfigChangeDispatch = true; - auto pendingConfigChange = - std::find_if(std::begin(mPendingEvents), std::end(mPendingEvents), - [&](const DisplayEventReceiver::Event& event) { - return event.header.type == - DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED; - }); - - // If we didn't find a pending config change event, then push out the - // latest one we've ever seen. - if (pendingConfigChange == std::end(mPendingEvents)) { - mPendingEvents.push_back(mLastConfigChangeEvent); - } - - mCondition.notify_all(); -} - void EventThread::onScreenReleased() { std::lock_guard<std::mutex> lock(mMutex); if (!mVSyncState || mVSyncState->synthetic) { @@ -366,9 +340,6 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { mInterceptVSyncsCallback(event->header.timestamp); } break; - case DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED: - mLastConfigChangeEvent = *event; - break; } } @@ -381,10 +352,6 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { vsyncRequested |= connection->vsyncRequest != VSyncRequest::None; if (event && shouldConsumeEvent(*event, connection)) { - if (event->header.type == DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED && - connection->mForcedConfigChangeDispatch) { - connection->mForcedConfigChangeDispatch = false; - } consumers.push_back(connection); } @@ -460,9 +427,7 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event, return true; case DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED: { - const bool oneTimeDispatch = connection->mForcedConfigChangeDispatch; - return oneTimeDispatch || - connection->mConfigChanged == ISurfaceComposer::eConfigChangedDispatch; + return connection->mConfigChanged == ISurfaceComposer::eConfigChangedDispatch; } case DisplayEventReceiver::DISPLAY_EVENT_VSYNC: diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index 64acbd72d0..49f624c35e 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -81,19 +81,13 @@ public: status_t stealReceiveChannel(gui::BitTube* outChannel) override; status_t setVsyncRate(uint32_t rate) override; void requestNextVsync() override; // asynchronous - void requestLatestConfig() override; // asynchronous // Called in response to requestNextVsync. const ResyncCallback resyncCallback; VSyncRequest vsyncRequest = VSyncRequest::None; - ISurfaceComposer::ConfigChanged mConfigChanged = + const ISurfaceComposer::ConfigChanged mConfigChanged = ISurfaceComposer::ConfigChanged::eConfigChangedSuppress; - // Store whether we need to force dispatching a config change separately - - // if mConfigChanged ever changes before the config change is dispatched - // then we still need to propagate an initial config to the app if we - // haven't already. - bool mForcedConfigChangeDispatch = false; private: virtual void onFirstRef(); @@ -129,10 +123,6 @@ public: virtual void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) = 0; // Requests the next vsync. If resetIdleTimer is set to true, it resets the idle timer. virtual void requestNextVsync(const sp<EventThreadConnection>& connection) = 0; - // Dispatches the most recent configuration - // Usage of this method assumes that only the primary internal display - // supports multiple display configurations. - virtual void requestLatestConfig(const sp<EventThreadConnection>& connection) = 0; // Retrieves the number of event connections tracked by this EventThread. virtual size_t getEventThreadConnectionCount() = 0; @@ -153,7 +143,6 @@ public: status_t registerDisplayEventConnection(const sp<EventThreadConnection>& connection) override; void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) override; void requestNextVsync(const sp<EventThreadConnection>& connection) override; - void requestLatestConfig(const sp<EventThreadConnection>& connection) override; // called before the screen is turned off from main thread void onScreenReleased() override; @@ -201,7 +190,6 @@ private: std::vector<wp<EventThreadConnection>> mDisplayEventConnections GUARDED_BY(mMutex); std::deque<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex); - DisplayEventReceiver::Event mLastConfigChangeEvent GUARDED_BY(mMutex); // VSYNC state of connected display. struct VSyncState { |