Skip to content

Commit 4759f60

Browse files
author
Manasi Navare
committed
SF: Fix dispatch of DISPLAY_EVENT_MODE_REJECTION
The onModeRejected() callback was not getting dispatched correctly from SF to DM because of the missing case for this display event in EventThread.cpp, so add that. While at it, make the Display Event Types enum an enum class so that the compiler will complain for any missing cases. Do the necessary refactor in other files for this. Bug: 393133868 Test: m surfaceflinger, End to End testing forcing Display config failure in DRM HWC and checking that the correct failure and is propagated from DRM HWC to SF and received in DM Flag: com.android.graphics.surfaceflinger.flags.display_config_error_hal Change-Id: I63914a3555466bc6c382ab1bf9ed57eb5eef7cd0 Signed-off-by: Manasi Navare <navaremanasi@google.com>
1 parent 6cd42cb commit 4759f60

9 files changed

Lines changed: 74 additions & 66 deletions

File tree

libs/gui/Choreographer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ void Choreographer::scheduleLatestConfigRequest() {
238238
// socket should be atomic across processes.
239239
DisplayEventReceiver::Event event;
240240
event.header =
241-
DisplayEventReceiver::Event::Header{DisplayEventReceiver::DISPLAY_EVENT_NULL,
241+
DisplayEventReceiver::Event::Header{DisplayEventType::DISPLAY_EVENT_NULL,
242242
PhysicalDisplayId::fromPort(0), systemTime()};
243243
injectEvent(event);
244244
}

libs/gui/DisplayEventDispatcher.cpp

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
167167
for (ssize_t i = 0; i < n; i++) {
168168
const DisplayEventReceiver::Event& ev = buf[i];
169169
switch (ev.header.type) {
170-
case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
170+
case DisplayEventType::DISPLAY_EVENT_VSYNC:
171171
// Later vsync events will just overwrite the info from earlier
172172
// ones. That's fine, we only care about the most recent.
173173
gotVsync = true;
@@ -183,7 +183,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
183183
ATRACE_INT("RenderRate", fps);
184184
}
185185
break;
186-
case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
186+
case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
187187
if (ev.hotplug.connectionError == 0) {
188188
dispatchHotplug(ev.header.timestamp, ev.header.displayId,
189189
ev.hotplug.connected);
@@ -192,31 +192,28 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp,
192192
ev.hotplug.connectionError);
193193
}
194194
break;
195-
case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE:
195+
case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE:
196196
dispatchModeChanged(ev.header.timestamp, ev.header.displayId,
197197
ev.modeChange.modeId, ev.modeChange.vsyncPeriod);
198198
break;
199-
case DisplayEventReceiver::DISPLAY_EVENT_NULL:
199+
case DisplayEventType::DISPLAY_EVENT_NULL:
200200
dispatchNullEvent(ev.header.timestamp, ev.header.displayId);
201201
break;
202-
case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
202+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
203203
mFrameRateOverrides.emplace_back(ev.frameRateOverride);
204204
break;
205-
case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
205+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
206206
dispatchFrameRateOverrides(ev.header.timestamp, ev.header.displayId,
207207
std::move(mFrameRateOverrides));
208208
break;
209-
case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
209+
case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
210210
dispatchHdcpLevelsChanged(ev.header.displayId,
211211
ev.hdcpLevelsChange.connectedLevel,
212212
ev.hdcpLevelsChange.maxLevel);
213213
break;
214-
case DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION:
214+
case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
215215
dispatchModeRejected(ev.header.displayId, ev.modeRejection.modeId);
216216
break;
217-
default:
218-
ALOGW("dispatcher %p ~ ignoring unknown event type %#x", this, ev.header.type);
219-
break;
220217
}
221218
}
222219
}

libs/gui/include/gui/DisplayEventReceiver.h

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,20 @@ static inline constexpr uint32_t fourcc(char c1, char c2, char c3, char c4) {
5555
static_cast<uint32_t>(c4);
5656
}
5757

58+
enum class DisplayEventType : uint32_t {
59+
DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'),
60+
DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'),
61+
DISPLAY_EVENT_MODE_CHANGE = fourcc('m', 'o', 'd', 'e'),
62+
DISPLAY_EVENT_MODE_REJECTION = fourcc('r', 'e', 'j', 'e'),
63+
DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'),
64+
DISPLAY_EVENT_FRAME_RATE_OVERRIDE = fourcc('r', 'a', 't', 'e'),
65+
DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH = fourcc('f', 'l', 's', 'h'),
66+
DISPLAY_EVENT_HDCP_LEVELS_CHANGE = fourcc('h', 'd', 'c', 'p'),
67+
};
68+
5869
// ----------------------------------------------------------------------------
5970
class DisplayEventReceiver {
6071
public:
61-
enum {
62-
DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'),
63-
DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'),
64-
DISPLAY_EVENT_MODE_CHANGE = fourcc('m', 'o', 'd', 'e'),
65-
DISPLAY_EVENT_MODE_REJECTION = fourcc('r', 'e', 'j', 'e'),
66-
DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'),
67-
DISPLAY_EVENT_FRAME_RATE_OVERRIDE = fourcc('r', 'a', 't', 'e'),
68-
DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH = fourcc('f', 'l', 's', 'h'),
69-
DISPLAY_EVENT_HDCP_LEVELS_CHANGE = fourcc('h', 'd', 'c', 'p'),
70-
};
71-
7272
struct Event {
7373
// We add __attribute__((aligned(8))) for nsecs_t fields because
7474
// we need to make sure all fields are aligned the same with x86
@@ -77,7 +77,7 @@ class DisplayEventReceiver {
7777
// https://en.wikipedia.org/wiki/Data_structure_alignment
7878

7979
struct Header {
80-
uint32_t type;
80+
DisplayEventType type;
8181
PhysicalDisplayId displayId __attribute__((aligned(8)));
8282
nsecs_t timestamp __attribute__((aligned(8)));
8383
};

libs/gui/tests/RegionSampling_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ struct ChoreographerSync {
4040
std::unique_lock<decltype(mutex_)> lk(mutex_);
4141

4242
auto check_event = [](auto const& ev) -> bool {
43-
return ev.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
43+
return ev.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC;
4444
};
4545
DisplayEventReceiver::Event ev_;
4646
int evs = receiver_.getEvents(&ev_, 1);

services/displayservice/DisplayEventReceiver.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <android/frameworks/displayservice/1.0/BpHwEventCallback.h>
2323

2424
#include <thread>
25+
#include <ftl/enum.h>
2526

2627
namespace android {
2728
namespace frameworks {
@@ -97,19 +98,19 @@ int DisplayEventReceiver::AttachedEvent::handleEvent(int fd, int events, void* /
9798
for (size_t i = 0; i < static_cast<size_t>(n); ++i) {
9899
const FwkReceiver::Event &event = buf[i];
99100

100-
uint32_t type = event.header.type;
101+
android::DisplayEventType type = event.header.type;
101102
uint64_t timestamp = event.header.timestamp;
102103

103104
switch(buf[i].header.type) {
104-
case FwkReceiver::DISPLAY_EVENT_VSYNC: {
105+
case DisplayEventType::DISPLAY_EVENT_VSYNC: {
105106
auto ret = mCallback->onVsync(timestamp, event.vsync.count);
106107
if (!ret.isOk()) {
107108
LOG(ERROR) << "AttachedEvent handleEvent fails on onVsync callback"
108109
<< " because of " << ret.description();
109110
return 0; // remove the callback
110111
}
111112
} break;
112-
case FwkReceiver::DISPLAY_EVENT_HOTPLUG: {
113+
case DisplayEventType::DISPLAY_EVENT_HOTPLUG: {
113114
auto ret = mCallback->onHotplug(timestamp, event.hotplug.connected);
114115
if (!ret.isOk()) {
115116
LOG(ERROR) << "AttachedEvent handleEvent fails on onHotplug callback"
@@ -118,7 +119,8 @@ int DisplayEventReceiver::AttachedEvent::handleEvent(int fd, int events, void* /
118119
}
119120
} break;
120121
default: {
121-
LOG(ERROR) << "AttachedEvent handleEvent unknown type: " << type;
122+
LOG(ERROR) << "AttachedEvent handleEvent unknown type: "
123+
<< ftl::to_underlying(type);
122124
}
123125
}
124126
}

services/surfaceflinger/Scheduler/EventThread.cpp

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -86,44 +86,51 @@ std::string toString(const EventThreadConnection& connection) {
8686

8787
std::string toString(const DisplayEventReceiver::Event& event) {
8888
switch (event.header.type) {
89-
case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
89+
case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
9090
return StringPrintf("Hotplug{displayId=%s, %s}",
9191
to_string(event.header.displayId).c_str(),
9292
event.hotplug.connected ? "connected" : "disconnected");
93-
case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
93+
case DisplayEventType::DISPLAY_EVENT_VSYNC:
9494
return StringPrintf("VSync{displayId=%s, count=%u, expectedPresentationTime=%" PRId64
9595
"}",
9696
to_string(event.header.displayId).c_str(), event.vsync.count,
9797
event.vsync.vsyncData.preferredExpectedPresentationTime());
98-
case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE:
98+
case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE:
9999
return StringPrintf("ModeChanged{displayId=%s, modeId=%u}",
100100
to_string(event.header.displayId).c_str(), event.modeChange.modeId);
101-
case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
101+
case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
102102
return StringPrintf("HdcpLevelsChange{displayId=%s, connectedLevel=%d, maxLevel=%d}",
103103
to_string(event.header.displayId).c_str(),
104104
event.hdcpLevelsChange.connectedLevel,
105105
event.hdcpLevelsChange.maxLevel);
106-
case DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION:
106+
case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
107107
return StringPrintf("ModeRejected{displayId=%s, modeId=%u}",
108108
to_string(event.header.displayId).c_str(),
109109
event.modeRejection.modeId);
110-
default:
111-
return "Event{}";
110+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
111+
return StringPrintf("FrameRateOverride{displayId=%s, frameRateHz=%f}",
112+
to_string(event.header.displayId).c_str(),
113+
event.frameRateOverride.frameRateHz);
114+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
115+
return StringPrintf("FrameRateOverrideFlush{displayId=%s}",
116+
to_string(event.header.displayId).c_str());
117+
case DisplayEventType::DISPLAY_EVENT_NULL:
118+
return "NULL";
112119
}
113120
}
114121

115122
DisplayEventReceiver::Event makeHotplug(PhysicalDisplayId displayId, nsecs_t timestamp,
116123
bool connected) {
117124
DisplayEventReceiver::Event event;
118-
event.header = {DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, displayId, timestamp};
125+
event.header = {DisplayEventType::DISPLAY_EVENT_HOTPLUG, displayId, timestamp};
119126
event.hotplug.connected = connected;
120127
return event;
121128
}
122129

123130
DisplayEventReceiver::Event makeHotplugError(nsecs_t timestamp, int32_t connectionError) {
124131
DisplayEventReceiver::Event event;
125132
PhysicalDisplayId unusedDisplayId;
126-
event.header = {DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, unusedDisplayId, timestamp};
133+
event.header = {DisplayEventType::DISPLAY_EVENT_HOTPLUG, unusedDisplayId, timestamp};
127134
event.hotplug.connected = false;
128135
event.hotplug.connectionError = connectionError;
129136
return event;
@@ -133,7 +140,7 @@ DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t times
133140
uint32_t count, nsecs_t expectedPresentationTime,
134141
nsecs_t deadlineTimestamp) {
135142
DisplayEventReceiver::Event event;
136-
event.header = {DisplayEventReceiver::DISPLAY_EVENT_VSYNC, displayId, timestamp};
143+
event.header = {DisplayEventType::DISPLAY_EVENT_VSYNC, displayId, timestamp};
137144
event.vsync.count = count;
138145
event.vsync.vsyncData.preferredFrameTimelineIndex = 0;
139146
// Temporarily store the current vsync information in frameTimelines[0], marked as
@@ -148,7 +155,7 @@ DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t times
148155

149156
DisplayEventReceiver::Event makeModeChanged(const scheduler::FrameRateMode& mode) {
150157
DisplayEventReceiver::Event event;
151-
event.header = {DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE,
158+
event.header = {DisplayEventType::DISPLAY_EVENT_MODE_CHANGE,
152159
mode.modePtr->getPhysicalDisplayId(), systemTime()};
153160
event.modeChange.modeId = ftl::to_underlying(mode.modePtr->getId());
154161
event.modeChange.vsyncPeriod = mode.fps.getPeriodNsecs();
@@ -160,7 +167,7 @@ DisplayEventReceiver::Event makeFrameRateOverrideEvent(PhysicalDisplayId display
160167
return DisplayEventReceiver::Event{
161168
.header =
162169
DisplayEventReceiver::Event::Header{
163-
.type = DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE,
170+
.type = DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE,
164171
.displayId = displayId,
165172
.timestamp = systemTime(),
166173
},
@@ -171,7 +178,7 @@ DisplayEventReceiver::Event makeFrameRateOverrideEvent(PhysicalDisplayId display
171178
DisplayEventReceiver::Event makeFrameRateOverrideFlushEvent(PhysicalDisplayId displayId) {
172179
return DisplayEventReceiver::Event{
173180
.header = DisplayEventReceiver::Event::Header{
174-
.type = DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH,
181+
.type = DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH,
175182
.displayId = displayId,
176183
.timestamp = systemTime(),
177184
}};
@@ -182,7 +189,7 @@ DisplayEventReceiver::Event makeHdcpLevelsChange(PhysicalDisplayId displayId,
182189
return DisplayEventReceiver::Event{
183190
.header =
184191
DisplayEventReceiver::Event::Header{
185-
.type = DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE,
192+
.type = DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE,
186193
.displayId = displayId,
187194
.timestamp = systemTime(),
188195
},
@@ -195,7 +202,7 @@ DisplayEventReceiver::Event makeModeRejection(PhysicalDisplayId displayId, Displ
195202
return DisplayEventReceiver::Event{
196203
.header =
197204
DisplayEventReceiver::Event::Header{
198-
.type = DisplayEventReceiver::DISPLAY_EVENT_MODE_REJECTION,
205+
.type = DisplayEventType::DISPLAY_EVENT_MODE_REJECTION,
199206
.displayId = displayId,
200207
.timestamp = systemTime(),
201208
},
@@ -263,10 +270,10 @@ status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& eve
263270
return size < 0 ? status_t(size) : status_t(NO_ERROR);
264271
};
265272

266-
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE ||
267-
event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH) {
273+
if (event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE ||
274+
event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH) {
268275
mPendingEvents.emplace_back(event);
269-
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE) {
276+
if (event.header.type == DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE) {
270277
return status_t(NO_ERROR);
271278
}
272279

@@ -524,7 +531,7 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {
524531
event = mPendingEvents.front();
525532
mPendingEvents.pop_front();
526533

527-
if (event->header.type == DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG) {
534+
if (event->header.type == DisplayEventType::DISPLAY_EVENT_HOTPLUG) {
528535
if (event->hotplug.connectionError == 0) {
529536
if (event->hotplug.connected && !mVSyncState) {
530537
mVSyncState.emplace();
@@ -636,18 +643,21 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event,
636643
};
637644

638645
switch (event.header.type) {
639-
case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG:
646+
case DisplayEventType::DISPLAY_EVENT_HOTPLUG:
640647
return true;
641648

642-
case DisplayEventReceiver::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
649+
case DisplayEventType::DISPLAY_EVENT_HDCP_LEVELS_CHANGE:
643650
return true;
644651

645-
case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE: {
652+
case DisplayEventType::DISPLAY_EVENT_MODE_CHANGE: {
646653
return connection->mEventRegistration.test(
647654
gui::ISurfaceComposer::EventRegistration::modeChanged);
648655
}
649656

650-
case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
657+
case DisplayEventType::DISPLAY_EVENT_MODE_REJECTION:
658+
return true;
659+
660+
case DisplayEventType::DISPLAY_EVENT_VSYNC:
651661
switch (connection->vsyncRequest) {
652662
case VSyncRequest::None:
653663
return false;
@@ -673,13 +683,12 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event,
673683
return event.vsync.count % vsyncPeriod(connection->vsyncRequest) == 0;
674684
}
675685

676-
case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
686+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE:
677687
[[fallthrough]];
678-
case DisplayEventReceiver::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
688+
case DisplayEventType::DISPLAY_EVENT_FRAME_RATE_OVERRIDE_FLUSH:
679689
return connection->mEventRegistration.test(
680690
gui::ISurfaceComposer::EventRegistration::frameRateOverride);
681-
682-
default:
691+
case DisplayEventType::DISPLAY_EVENT_NULL:
683692
return false;
684693
}
685694
}
@@ -758,7 +767,7 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
758767
ftl::SmallVector<uid_t, 10> uidsPostedQueuedBuffers;
759768
for (const auto& consumer : consumers) {
760769
DisplayEventReceiver::Event copy = event;
761-
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
770+
if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC) {
762771
const Period frameInterval = mCallback.getVsyncPeriod(consumer->mOwnerUid);
763772
copy.vsync.vsyncData.frameInterval = frameInterval.ns();
764773
generateFrameTimeline(copy.vsync.vsyncData, frameInterval.ns(), copy.header.timestamp,
@@ -793,7 +802,7 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event,
793802
for (auto uid : uidsPostedQueuedBuffers) {
794803
mBufferStuffedUids.erase(uid);
795804
}
796-
if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC &&
805+
if (event.header.type == DisplayEventType::DISPLAY_EVENT_VSYNC &&
797806
FlagManager::getInstance().vrr_config()) {
798807
mLastCommittedVsyncTime =
799808
TimePoint::fromNs(event.vsync.vsyncData.preferredExpectedPresentationTime());

services/surfaceflinger/tests/LayerCallback_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class LayerCallbackTest : public LayerTransactionTest {
147147
<< "Timeout waiting for vsync event";
148148
DisplayEventReceiver::Event event;
149149
while (mDisplayEventReceiver.getEvents(&event, 1) > 0) {
150-
if (event.header.type != DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
150+
if (event.header.type != DisplayEventType::DISPLAY_EVENT_VSYNC) {
151151
continue;
152152
}
153153

0 commit comments

Comments
 (0)