SF: Fix VSYNC injection
Disabling VSYNC injection causes the main thread to destroy the injected
EventThreadConnection, while it might be accessed by a Binder thread via
MessageQueue::invalidate.
Also, fix injection itself:
- The EventThread did not dispatch injected events, falling back to
fake VSYNC after timeout.
- The MessageQueue would try listening to invalid FDs when enabling
injection more than once.
- Injection was not disabled after each test.
Finally, rename MessageQueue members only used for injection since S.
Bug: 150226265
Test: Toggle VSYNC injection in a tight loop while using phone.
Test: sffakehwc_test --gtest_repeat=-1 --gtest_break_on_failure
--gtest_filter='*VsyncInjection'
Change-Id: Ia58859cd8a36749bf56bb94fa724efe7c1b27b46
diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp
index 47a4f42..7ff0ddf 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.cpp
+++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp
@@ -69,17 +69,33 @@
// TODO(b/169865816): refactor VSyncInjections to use MessageQueue directly
// and remove the EventThread from MessageQueue
-void MessageQueue::setEventConnection(const sp<EventThreadConnection>& connection) {
- if (mEventTube.getFd() >= 0) {
- mLooper->removeFd(mEventTube.getFd());
+void MessageQueue::setInjector(sp<EventThreadConnection> connection) {
+ auto& tube = mInjector.tube;
+
+ if (const int fd = tube.getFd(); fd >= 0) {
+ mLooper->removeFd(fd);
}
- mEvents = connection;
- if (mEvents) {
- mEvents->stealReceiveChannel(&mEventTube);
- mLooper->addFd(mEventTube.getFd(), 0, Looper::EVENT_INPUT, MessageQueue::cb_eventReceiver,
- this);
+ if (connection) {
+ // The EventThreadConnection is retained when disabling injection, so avoid subsequently
+ // stealing invalid FDs. Note that the stolen FDs are kept open.
+ if (tube.getFd() < 0) {
+ connection->stealReceiveChannel(&tube);
+ } else {
+ ALOGW("Recycling channel for VSYNC injection.");
+ }
+
+ mLooper->addFd(
+ tube.getFd(), 0, Looper::EVENT_INPUT,
+ [](int, int, void* data) {
+ reinterpret_cast<MessageQueue*>(data)->injectorCallback();
+ return 1; // Keep registration.
+ },
+ this);
}
+
+ std::lock_guard lock(mInjector.mutex);
+ mInjector.connection = std::move(connection);
}
void MessageQueue::vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, nsecs_t readyTime) {
@@ -149,29 +165,31 @@
void MessageQueue::invalidate() {
ATRACE_CALL();
- if (mEvents) {
- mEvents->requestNextVsync();
- } else {
- std::lock_guard lock(mVsync.mutex);
- mVsync.mScheduled = true;
- mVsync.registration->schedule({mVsync.workDuration.get().count(), /*readyDuration=*/0,
- mVsync.lastCallbackTime.count()});
+
+ {
+ std::lock_guard lock(mInjector.mutex);
+ if (CC_UNLIKELY(mInjector.connection)) {
+ ALOGD("%s while injecting VSYNC", __FUNCTION__);
+ mInjector.connection->requestNextVsync();
+ return;
+ }
}
+
+ std::lock_guard lock(mVsync.mutex);
+ mVsync.mScheduled = true;
+ mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(),
+ .readyDuration = 0,
+ .earliestVsync = mVsync.lastCallbackTime.count()});
}
void MessageQueue::refresh() {
mHandler->dispatchRefresh();
}
-int MessageQueue::cb_eventReceiver(int fd, int events, void* data) {
- MessageQueue* queue = reinterpret_cast<MessageQueue*>(data);
- return queue->eventReceiver(fd, events);
-}
-
-int MessageQueue::eventReceiver(int /*fd*/, int /*events*/) {
+void MessageQueue::injectorCallback() {
ssize_t n;
DisplayEventReceiver::Event buffer[8];
- while ((n = DisplayEventReceiver::getEvents(&mEventTube, buffer, 8)) > 0) {
+ while ((n = DisplayEventReceiver::getEvents(&mInjector.tube, buffer, 8)) > 0) {
for (int i = 0; i < n; i++) {
if (buffer[i].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
mHandler->dispatchInvalidate(buffer[i].vsync.vsyncId,
@@ -180,8 +198,6 @@
}
}
}
- return 1;
}
} // namespace android::impl
-
diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h
index 99ce3a6..2934af0 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.h
+++ b/services/surfaceflinger/Scheduler/MessageQueue.h
@@ -21,12 +21,11 @@
#include <type_traits>
#include <utility>
-#include <utils/Looper.h>
-#include <utils/Timers.h>
-#include <utils/threads.h>
-
+#include <android-base/thread_annotations.h>
#include <gui/IDisplayEventConnection.h>
#include <private/gui/BitTube.h>
+#include <utils/Looper.h>
+#include <utils/Timers.h>
#include "EventThread.h"
#include "TracedOrdinal.h"
@@ -68,7 +67,7 @@
virtual void initVsync(scheduler::VSyncDispatch&, frametimeline::TokenManager&,
std::chrono::nanoseconds workDuration) = 0;
virtual void setDuration(std::chrono::nanoseconds workDuration) = 0;
- virtual void setEventConnection(const sp<EventThreadConnection>& connection) = 0;
+ virtual void setInjector(sp<EventThreadConnection>) = 0;
virtual void waitMessage() = 0;
virtual void postMessage(sp<MessageHandler>&&) = 0;
virtual void invalidate() = 0;
@@ -99,7 +98,6 @@
sp<SurfaceFlinger> mFlinger;
sp<Looper> mLooper;
- sp<EventThreadConnection> mEvents;
struct Vsync {
frametimeline::TokenManager* tokenManager = nullptr;
@@ -113,14 +111,19 @@
TracedOrdinal<int> value = {"VSYNC-sf", 0};
};
- Vsync mVsync;
+ struct Injector {
+ gui::BitTube tube;
+ std::mutex mutex;
+ sp<EventThreadConnection> connection GUARDED_BY(mutex);
+ };
- gui::BitTube mEventTube;
+ Vsync mVsync;
+ Injector mInjector;
+
sp<Handler> mHandler;
- static int cb_eventReceiver(int fd, int events, void* data);
- int eventReceiver(int fd, int events);
void vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, nsecs_t readyTime);
+ void injectorCallback();
public:
~MessageQueue() override = default;
@@ -128,7 +131,7 @@
void initVsync(scheduler::VSyncDispatch&, frametimeline::TokenManager&,
std::chrono::nanoseconds workDuration) override;
void setDuration(std::chrono::nanoseconds workDuration) override;
- void setEventConnection(const sp<EventThreadConnection>& connection) override;
+ void setInjector(sp<EventThreadConnection>) override;
void waitMessage() override;
void postMessage(sp<MessageHandler>&&) override;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 52bf483..0092c7a 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -399,6 +399,10 @@
impl::EventThread::InterceptVSyncsCallback(),
impl::EventThread::ThrottleVsyncCallback());
+ // EventThread does not dispatch VSYNC unless the display is connected and powered on.
+ eventThread->onHotplugReceived(PhysicalDisplayId::fromPort(0), true);
+ eventThread->onScreenAcquired();
+
mInjectorConnectionHandle = createConnection(std::move(eventThread));
}