Merge "SF: Fix dispatch of DISPLAY_EVENT_MODE_REJECTION" into main

Author: Manasi Navare

libs/gui/Choreographer.cpp

@@ -238,7 +238,7 @@ void Choreographer::scheduleLatestConfigRequest() {
         // socket should be atomic across processes.
         DisplayEventReceiver::Event event;
         event.header =
-                DisplayEventReceiver::Event::Header{DisplayEventReceiver::DISPLAY_EVENT_NULL,
+                DisplayEventReceiver::Event::Header{DisplayEventType::DISPLAY_EVENT_NULL,
                                                     PhysicalDisplayId::fromPort(0), systemTime()};
         injectEvent(event);
     }

libs/gui/DisplayEventDispatcher.cpp

@@ -167,7 +167,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
         for (ssize_t i = 0; i < n; i++) {
             const DisplayEventReceiver::Event& ev = buf[i];
             switch (ev.header.type) {
-                case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+                case DisplayEventType::DISPLAY_EVENT_VSYNC:
                     // Later vsync events will just overwrite the info from earlier
                     // ones. That's fine, we only care about the most recent.
                     gotVsync = true;
@@ -183,7 +183,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
                         ATRACE_INT("RenderRate", fps);
                     }
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+                case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
                     if (ev.hotplug.connectionError == 0) {
                         dispatchHotplug(ev.header.timestamp, ev.header.displayId,
                                         ev.hotplug.connected);
@@ -192,31 +192,28 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
                                                        ev.hotplug.connectionError);
                     }
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE:
+                case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE:
                     dispatchModeChanged(ev.header.timestamp, ev.header.displayId,
                                         ev.modeChange.modeId, ev.modeChange.vsyncPeriod);
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_NULL:
+                case DisplayEventType::DISPLAY_EVENT_NULL:
                     dispatchNullEvent(ev.header.timestamp, ev.header.displayId);
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
+                case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
                     mFrameRateOverrides.emplace_back(ev.frameRateOverride);
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
+                case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
                     dispatchFrameRateOverrides(ev.header.timestamp, ev.header.displayId,
                                                std::move(mFrameRateOverrides));
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
+                case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
                     dispatchHdcpLevelsChanged(ev.header.displayId,
                                               ev.hdcpLevelsChange.connectedLevel,
                                               ev.hdcpLevelsChange.maxLevel);
                     break;
-                case DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION:
+                case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
                     dispatchModeRejected(ev.header.displayId, ev.modeRejection.modeId);
                     break;
-                default:
-                    ALOGW("dispatcher %p ~ ignoring unknown event type %#x", this, ev.header.type);
-                    break;
             }
         }
     }

libs/gui/include/gui/DisplayEventReceiver.h

@@ -55,20 +55,20 @@ static inline constexpr uint32_t fourcc(char c1, char c2, char c3, char c4) {
         static_cast<uint32_t>(c4);
 }
 
+enum class DisplayEventType : uint32_t {
+    DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'),
+    DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'),
+    DISPLAY_EVENT_MODE_CHANGE = fourcc('m', 'o', 'd', 'e'),
+    DISPLAY_EVENT_MODE_REJECTION = fourcc('r', 'e', 'j', 'e'),
+    DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'),
+    DISPLAY_EVENT_FRAME_RATE_OVERRIDE = fourcc('r', 'a', 't', 'e'),
+    DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH = fourcc('f', 'l', 's', 'h'),
+    DISPLAY_EVENT_HDCP_LEVELS_CHANGE = fourcc('h', 'd', 'c', 'p'),
+};
+
 // ----------------------------------------------------------------------------
 class DisplayEventReceiver {
 public:
-    enum {
-        DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'),
-        DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'),
-        DISPLAY_EVENT_MODE_CHANGE = fourcc('m', 'o', 'd', 'e'),
-        DISPLAY_EVENT_MODE_REJECTION = fourcc('r', 'e', 'j', 'e'),
-        DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'),
-        DISPLAY_EVENT_FRAME_RATE_OVERRIDE = fourcc('r', 'a', 't', 'e'),
-        DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH = fourcc('f', 'l', 's', 'h'),
-        DISPLAY_EVENT_HDCP_LEVELS_CHANGE = fourcc('h', 'd', 'c', 'p'),
-    };
-
     struct Event {
         // We add __attribute__((aligned(8))) for nsecs_t fields because
         // we need to make sure all fields are aligned the same with x86
@@ -77,7 +77,7 @@ class DisplayEventReceiver {
         // https://en.wikipedia.org/wiki/Data_structure_alignment
 
         struct Header {
-            uint32_t type;
+            DisplayEventType type;
             PhysicalDisplayId displayId __attribute__((aligned(8)));
             nsecs_t timestamp __attribute__((aligned(8)));
         };

libs/gui/tests/RegionSampling_test.cpp

@@ -40,7 +40,7 @@ struct ChoreographerSync {
         std::unique_lock<decltype(mutex_)> lk(mutex_);
 
         auto check_event = [](auto const& ev) -> bool {
-            return ev.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+            return ev.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC;
         };
         DisplayEventReceiver::Event ev_;
         int evs = receiver_.getEvents(&ev_, 1);

services/displayservice/DisplayEventReceiver.cpp

@@ -22,6 +22,7 @@
 #include <android/frameworks/displayservice/1.0/BpHwEventCallback.h>
 
 #include <thread>
+#include <ftl/enum.h>
 
 namespace android {
 namespace frameworks {
@@ -97,19 +98,19 @@ int DisplayEventReceiver::AttachedEvent::handleEvent(int fd, int events, void* /
         for (size_t i = 0; i < static_cast<size_t>(n); ++i) {
             const FwkReceiver::Event &event = buf[i];
 
-            uint32_t type = event.header.type;
+            android::DisplayEventType type = event.header.type;
             uint64_t timestamp = event.header.timestamp;
 
             switch(buf[i].header.type) {
-                case FwkReceiver::DISPLAY_EVENT_VSYNC: {
+                case DisplayEventType::DISPLAY_EVENT_VSYNC: {
                     auto ret = mCallback->onVsync(timestamp, event.vsync.count);
                     if (!ret.isOk()) {
                         LOG(ERROR) << "AttachedEvent handleEvent fails on onVsync callback"
                                    << " because of " << ret.description();
                         return 0;  // remove the callback
                     }
                 } break;
-                case FwkReceiver::DISPLAY_EVENT_HOTPLUG: {
+                case DisplayEventType::DISPLAY_EVENT_HOTPLUG: {
                     auto ret = mCallback->onHotplug(timestamp, event.hotplug.connected);
                     if (!ret.isOk()) {
                         LOG(ERROR) << "AttachedEvent handleEvent fails on onHotplug callback"
@@ -118,7 +119,8 @@ int DisplayEventReceiver::AttachedEvent::handleEvent(int fd, int events, void* /
                     }
                 } break;
                 default: {
-                    LOG(ERROR) << "AttachedEvent handleEvent unknown type: " << type;
+                    LOG(ERROR) << "AttachedEvent handleEvent unknown type: "
+                                << ftl::to_underlying(type);
                 }
             }
         }

services/surfaceflinger/Scheduler/EventThread.cpp

@@ -86,44 +86,51 @@ std::string toString(const EventThreadConnection& connection) {
 
 std::string toString(const DisplayEventReceiver::Event& event) {
     switch (event.header.type) {
-        case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+        case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
             return StringPrintf("Hotplug{displayId=%s, %s}",
                                 to_string(event.header.displayId).c_str(),
                                 event.hotplug.connected ? "connected" : "disconnected");
-        case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+        case DisplayEventType::DISPLAY_EVENT_VSYNC:
             return StringPrintf("VSync{displayId=%s, count=%u, expectedPresentationTime=%" PRId64
                                 "}",
                                 to_string(event.header.displayId).c_str(), event.vsync.count,
                                 event.vsync.vsyncData.preferredExpectedPresentationTime());
-        case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE:
+        case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE:
             return StringPrintf("ModeChanged{displayId=%s, modeId=%u}",
                                 to_string(event.header.displayId).c_str(), event.modeChange.modeId);
-        case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
+        case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
             return StringPrintf("HdcpLevelsChange{displayId=%s, connectedLevel=%d, maxLevel=%d}",
                                 to_string(event.header.displayId).c_str(),
                                 event.hdcpLevelsChange.connectedLevel,
                                 event.hdcpLevelsChange.maxLevel);
-        case DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION:
+        case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
             return StringPrintf("ModeRejected{displayId=%s, modeId=%u}",
                                 to_string(event.header.displayId).c_str(),
                                 event.modeRejection.modeId);
-        default:
-            return "Event{}";
+        case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
+            return StringPrintf("FrameRateOverride{displayId=%s, frameRateHz=%f}",
+                                to_string(event.header.displayId).c_str(),
+                                event.frameRateOverride.frameRateHz);
+        case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
+            return StringPrintf("FrameRateOverrideFlush{displayId=%s}",
+                                to_string(event.header.displayId).c_str());
+        case DisplayEventType::DISPLAY_EVENT_NULL:
+            return "NULL";
     }
 }
 
 DisplayEventReceiver::Event makeHotplug(PhysicalDisplayId displayId, nsecs_t timestamp,
                                         bool connected) {
     DisplayEventReceiver::Event event;
-    event.header = {DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, displayId, timestamp};
+    event.header = {DisplayEventType::DISPLAY_EVENT_HOTPLUG, displayId, timestamp};
     event.hotplug.connected = connected;
     return event;
 }
 
 DisplayEventReceiver::Event makeHotplugError(nsecs_t timestamp, int32_t connectionError) {
     DisplayEventReceiver::Event event;
     PhysicalDisplayId unusedDisplayId;
-    event.header = {DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, unusedDisplayId, timestamp};
+    event.header = {DisplayEventType::DISPLAY_EVENT_HOTPLUG, unusedDisplayId, timestamp};
     event.hotplug.connected = false;
     event.hotplug.connectionError = connectionError;
     return event;
@@ -133,7 +140,7 @@ DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t times
                                       uint32_t count, nsecs_t expectedPresentationTime,
                                       nsecs_t deadlineTimestamp) {
     DisplayEventReceiver::Event event;
-    event.header = {DisplayEventReceiver::DISPLAY_EVENT_VSYNC, displayId, timestamp};
+    event.header = {DisplayEventType::DISPLAY_EVENT_VSYNC, displayId, timestamp};
     event.vsync.count = count;
     event.vsync.vsyncData.preferredFrameTimelineIndex = 0;
     // Temporarily store the current vsync information in frameTimelines[0], marked as
@@ -148,7 +155,7 @@ DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t times
 
 DisplayEventReceiver::Event makeModeChanged(const scheduler::FrameRateMode& mode) {
     DisplayEventReceiver::Event event;
-    event.header = {DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE,
+    event.header = {DisplayEventType::DISPLAY_EVENT_MODE_CHANGE,
                     mode.modePtr->getPhysicalDisplayId(), systemTime()};
     event.modeChange.modeId = ftl::to_underlying(mode.modePtr->getId());
     event.modeChange.vsyncPeriod = mode.fps.getPeriodNsecs();
@@ -160,7 +167,7 @@ DisplayEventReceiver::Event makeFrameRateOverrideEvent(PhysicalDisplayId display
     return DisplayEventReceiver::Event{
             .header =
                     DisplayEventReceiver::Event::Header{
-                            .type = DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE,
+                            .type = DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE,
                             .displayId = displayId,
                             .timestamp = systemTime(),
                     },
@@ -171,7 +178,7 @@ DisplayEventReceiver::Event makeFrameRateOverrideEvent(PhysicalDisplayId display
 DisplayEventReceiver::Event makeFrameRateOverrideFlushEvent(PhysicalDisplayId displayId) {
     return DisplayEventReceiver::Event{
             .header = DisplayEventReceiver::Event::Header{
-                    .type = DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH,
+                    .type = DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH,
                     .displayId = displayId,
                     .timestamp = systemTime(),
             }};
@@ -182,7 +189,7 @@ DisplayEventReceiver::Event makeHdcpLevelsChange(PhysicalDisplayId displayId,
     return DisplayEventReceiver::Event{
             .header =
                     DisplayEventReceiver::Event::Header{
-                            .type = DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE,
+                            .type = DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE,
                             .displayId = displayId,
                             .timestamp = systemTime(),
                     },
@@ -195,7 +202,7 @@ DisplayEventReceiver::Event makeModeRejection(PhysicalDisplayId displayId, Displ
     return DisplayEventReceiver::Event{
             .header =
                     DisplayEventReceiver::Event::Header{
-                            .type = DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION,
+                            .type = DisplayEventType::DISPLAY_EVENT_MODE_REJECTION,
                             .displayId = displayId,
                             .timestamp = systemTime(),
                     },
@@ -263,10 +270,10 @@ status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& eve
         return size < 0 ? status_t(size) : status_t(NO_ERROR);
     };
 
-    if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE ||
-        event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH) {
+    if (event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE ||
+        event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH) {
         mPendingEvents.emplace_back(event);
-        if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE) {
+        if (event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE) {
             return status_t(NO_ERROR);
         }
 
@@ -524,7 +531,7 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {
             event = mPendingEvents.front();
             mPendingEvents.pop_front();
 
-            if (event->header.type == DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG) {
+            if (event->header.type == DisplayEventType::DISPLAY_EVENT_HOTPLUG) {
                 if (event->hotplug.connectionError == 0) {
                     if (event->hotplug.connected && !mVSyncState) {
                         mVSyncState.emplace();
@@ -636,18 +643,21 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event,
     };
 
     switch (event.header.type) {
-        case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
+        case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
             return true;
 
-        case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
+        case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
             return true;
 
-        case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE: {
+        case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE: {
             return connection->mEventRegistration.test(
                     gui::ISurfaceComposer::EventRegistration::modeChanged);
         }
 
-        case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
+        case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
+            return true;
+
+        case DisplayEventType::DISPLAY_EVENT_VSYNC:
             switch (connection->vsyncRequest) {
                 case VSyncRequest::None:
                     return false;
@@ -673,13 +683,12 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event,
                     return event.vsync.count % vsyncPeriod(connection->vsyncRequest) == 0;
             }
 
-        case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
+        case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
             [[fallthrough]];
-        case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
+        case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
             return connection->mEventRegistration.test(
                     gui::ISurfaceComposer::EventRegistration::frameRateOverride);
-
-        default:
+        case DisplayEventType::DISPLAY_EVENT_NULL:
             return false;
     }
 }
@@ -758,7 +767,7 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
     ftl::SmallVector<uid_t, 10> uidsPostedQueuedBuffers;
     for (const auto& consumer : consumers) {
         DisplayEventReceiver::Event copy = event;
-        if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
+        if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC) {
             const Period frameInterval = mCallback.getVsyncPeriod(consumer->mOwnerUid);
             copy.vsync.vsyncData.frameInterval = frameInterval.ns();
             generateFrameTimeline(copy.vsync.vsyncData, frameInterval.ns(), copy.header.timestamp,
@@ -793,7 +802,7 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
     for (auto uid : uidsPostedQueuedBuffers) {
         mBufferStuffedUids.erase(uid);
     }
-    if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC &&
+    if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC &&
         FlagManager::getInstance().vrr_config()) {
         mLastCommittedVsyncTime =
                 TimePoint::fromNs(event.vsync.vsyncData.preferredExpectedPresentationTime());

services/surfaceflinger/tests/LayerCallback_test.cpp

@@ -147,7 +147,7 @@ class LayerCallbackTest : public LayerTransactionTest {
                 << "Timeout waiting for vsync event";
         DisplayEventReceiver::Event event;
         while (mDisplayEventReceiver.getEvents(&event, 1) > 0) {
-            if (event.header.type != DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
+            if (event.header.type != DisplayEventType::DISPLAY_EVENT_VSYNC) {
                 continue;
             }