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
diff --git a/libs/gui/BitTube.cpp b/libs/gui/BitTube.cpp
index ef7a6f5..351af65 100644
--- a/libs/gui/BitTube.cpp
+++ b/libs/gui/BitTube.cpp
@@ -86,6 +86,10 @@
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 result = reply->writeDupFileDescriptor(mReceiveFd);
mReceiveFd.reset();
+ if (result != NO_ERROR) {
+ return result;
+ }
+ result = reply->writeDupFileDescriptor(mSendFd);
+ mSendFd.reset();
return result;
}
@@ -126,6 +135,13 @@
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 51fbb97..2cc7c34 100644
--- a/libs/gui/DisplayEventDispatcher.cpp
+++ b/libs/gui/DisplayEventDispatcher.cpp
@@ -89,12 +89,8 @@
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 {
@@ -156,6 +152,9 @@
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;
@@ -167,4 +166,5 @@
}
return gotVsync;
}
+
} // namespace android
diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp
index 1fed509..f2b0962 100644
--- a/libs/gui/DisplayEventReceiver.cpp
+++ b/libs/gui/DisplayEventReceiver.cpp
@@ -79,14 +79,6 @@
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 @@
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 aa74bfd..c0e246f 100644
--- a/libs/gui/IDisplayEventConnection.cpp
+++ b/libs/gui/IDisplayEventConnection.cpp
@@ -26,8 +26,7 @@
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 @@
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 @@
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 f210c34..eb5b004 100644
--- a/libs/gui/include/gui/DisplayEventDispatcher.h
+++ b/libs/gui/include/gui/DisplayEventDispatcher.h
@@ -31,7 +31,7 @@
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 @@
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 8d49184..0e10d1a 100644
--- a/libs/gui/include/gui/DisplayEventReceiver.h
+++ b/libs/gui/include/gui/DisplayEventReceiver.h
@@ -53,6 +53,7 @@
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 @@
* 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 @@
*/
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 674aafd..cff22a3 100644
--- a/libs/gui/include/gui/IDisplayEventConnection.h
+++ b/libs/gui/include/gui/IDisplayEventConnection.h
@@ -51,11 +51,6 @@
* 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 13c0162..8048518 100644
--- a/libs/gui/include/private/gui/BitTube.h
+++ b/libs/gui/include/private/gui/BitTube.h
@@ -58,6 +58,9 @@
// 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 @@
// 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 ebc8909..ff1b5e6 100644
--- a/libs/nativedisplay/AChoreographer.cpp
+++ b/libs/nativedisplay/AChoreographer.cpp
@@ -136,6 +136,7 @@
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(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 @@
} 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);
}
}
@@ -373,27 +380,14 @@
// 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=%s, configId=%d).",
this, to_string(displayId).c_str(), 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) {