Skip to content

Commit 296cc77

Browse files
Drew Davenportphreer
authored andcommitted
drm_hwcomposer: Fix deadlock with flattening
FlatteningController acquires its internal lock first, and then acquires the global lock in DrmHwcThree::SendRefreshEventToClient. During normal validate/present, the thread acquires the global lock first, and will acquire the FlatteningController internal lock in NewFrame. Move the "must_validate" flag out of Hwc3Display to remove the need to acquire the global lock in the SendRefreshEventToClient callback. Add a std::set to track which displays must be validated, and protect access to this set with a different mutex to avoid deadlocks. Tracked-On: OAM-133858 Change-Id: I62d59252107f9f16a4b8bb43f850115bf327a127 Signed-off-by: Drew Davenport <ddavenport@google.com> Signed-off-by: ZhuChenyanX <zhucx@intel.com>
1 parent 7026d8c commit 296cc77

File tree

3 files changed

+27
-12
lines changed

3 files changed

+27
-12
lines changed

hwc3/ComposerClient.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ void ComposerClient::ExecuteDisplayCommand(const DisplayCommand& command) {
730730
}
731731
cmd_result_writer_->AddChanges(changes);
732732
auto hwc3_display = DrmHwcThree::GetHwc3Display(*display);
733-
hwc3_display->must_validate = false;
733+
hwc_->ClearMustValidateDisplay(display_id);
734734
display->setExpectedPresentTime(command.expectedPresentTime);
735735
// TODO: DisplayRequests are not implemented.
736736
}
@@ -751,7 +751,7 @@ void ComposerClient::ExecuteDisplayCommand(const DisplayCommand& command) {
751751

752752
if (command.presentDisplay || shall_present_now) {
753753
auto hwc3_display = DrmHwcThree::GetHwc3Display(*display);
754-
if (hwc3_display->must_validate) {
754+
if (hwc_->GetMustValidateDisplay(display_id)) {
755755
cmd_result_writer_->AddError(hwc3::Error::kNotValidated);
756756
return;
757757
}
@@ -1505,7 +1505,7 @@ ndk::ScopedAStatus ComposerClient::getDisplayConfigurations(
15051505
const auto bounds = display->GetDisplayBoundsMm();
15061506
for (const auto& [id, config] : configs.hwc_configs) {
15071507
configurations->push_back(
1508-
HwcDisplayConfigToAidlConfiguration(/*width =*/ bounds.first,
1508+
HwcDisplayConfigToAidlConfiguration(/*width =*/ bounds.first,
15091509
/*height =*/ bounds.second,
15101510
config));
15111511
}

hwc3/DrmHwcThree.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,8 @@ void DrmHwcThree::SendVsyncPeriodTimingChangedEventToClient(
6262

6363
void DrmHwcThree::SendRefreshEventToClient(uint64_t display_id) {
6464
{
65-
const std::unique_lock lock(GetResMan().GetMainLock());
66-
auto* idisplay = GetDisplay(display_id);
67-
if (idisplay == nullptr) {
68-
ALOGE("Failed to get display %" PRIu64, display_id);
69-
return;
70-
}
71-
auto hwc3_display = GetHwc3Display(*idisplay);
72-
hwc3_display->must_validate = true;
65+
const std::scoped_lock lock(must_validate_lock_);
66+
must_validate_.insert(display_id);
7367
}
7468
composer_callback_->onRefresh(static_cast<int64_t>(display_id));
7569
}
@@ -96,6 +90,9 @@ void DrmHwcThree::SendHotplugEventToClient(
9690
event = common::DisplayHotplugEvent::ERROR_INCOMPATIBLE_CABLE;
9791
break;
9892
}
93+
if (event == common::DisplayHotplugEvent::DISCONNECTED) {
94+
ClearMustValidateDisplay(display_id);
95+
}
9996
composer_callback_->onHotplugEvent(static_cast<int64_t>(display_id), event);
10097
}
10198

@@ -104,9 +101,22 @@ void DrmHwcThree::SendHotplugEventToClient(
104101
void DrmHwcThree::SendHotplugEventToClient(
105102
hwc2_display_t display_id, DrmHwc::DisplayStatus display_status) {
106103
bool connected = display_status != DrmHwc::kDisconnected;
104+
if (!connected) {
105+
ClearMustValidateDisplay(display_id);
106+
}
107107
composer_callback_->onHotplug(static_cast<int64_t>(display_id), connected);
108108
}
109109

110110
#endif
111111

112+
auto DrmHwcThree::GetMustValidateDisplay(uint64_t display_id) -> bool {
113+
std::scoped_lock lock(must_validate_lock_);
114+
return must_validate_.find(display_id) != must_validate_.end();
115+
}
116+
117+
void DrmHwcThree::ClearMustValidateDisplay(uint64_t display_id) {
118+
std::scoped_lock lock(must_validate_lock_);
119+
must_validate_.erase(display_id);
120+
}
121+
112122
} // namespace aidl::android::hardware::graphics::composer3::impl

hwc3/DrmHwcThree.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ namespace aidl::android::hardware::graphics::composer3::impl {
2525

2626
class Hwc3Display : public ::android::FrontendDisplayBase {
2727
public:
28-
bool must_validate = false;
2928

3029
int64_t next_layer_id = 1;
3130
};
@@ -49,7 +48,13 @@ class DrmHwcThree : public ::android::DrmHwc {
4948
static auto GetHwc3Display(::android::HwcDisplay& display)
5049
-> std::shared_ptr<Hwc3Display>;
5150

51+
auto GetMustValidateDisplay(uint64_t display_id) -> bool;
52+
void ClearMustValidateDisplay(uint64_t display_id);
53+
5254
private:
5355
std::shared_ptr<IComposerCallback> composer_callback_;
56+
57+
std::mutex must_validate_lock_;
58+
std::set<uint64_t> must_validate_ GUARDED_BY(must_validate_lock_);
5459
};
5560
} // namespace aidl::android::hardware::graphics::composer3::impl

0 commit comments

Comments
 (0)