Skip to content

Commit

Permalink
ui: Add more logging around Take/RelinquishDisplayControl
Browse files Browse the repository at this point in the history
Take and RelinquishDisplayControls are called when ash takes or
releases DRM master for all DRM devices. When these calls fail, there's
often not a lot of information on why they failed, which makes
debugging/root causing difficult. This CL adds more verbose error
logging around failure cases to make the cause of failure more apparent
in logs.

Bug: b:330543865
Test: ozone_unittests && display_unittests
Change-Id: Iead5110d54099546e09247981446ed2b1ac34a3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5813185
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Su Hong Koo <sukoo@chromium.org>
Reviewed-by: Gil Dekel <gildekel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1347527}
  • Loading branch information
Su Hong Koo authored and pull[bot] committed Aug 29, 2024
1 parent ac58b67 commit 1893254
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
15 changes: 15 additions & 0 deletions ui/display/manager/display_configurator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -658,11 +658,16 @@ void DisplayConfigurator::SetConfigureDisplays(bool configure_displays) {

void DisplayConfigurator::TakeControl(DisplayControlCallback callback) {
if (display_control_changing_) {
LOG(ERROR) << __func__
<< " failed. There is another RelinquishControl() or "
"TakeControl() call in progress.";
std::move(callback).Run(false);
return;
}

if (!display_externally_controlled_) {
LOG(ERROR) << __func__
<< " failed. Displays are not controlled externally.";
std::move(callback).Run(true);
return;
}
Expand Down Expand Up @@ -692,17 +697,24 @@ void DisplayConfigurator::OnDisplayControlTaken(DisplayControlCallback callback,

void DisplayConfigurator::RelinquishControl(DisplayControlCallback callback) {
if (display_control_changing_) {
LOG(ERROR) << __func__
<< " failed. There is another RelinquishControl() or "
"TakeControl() call in progress.";
std::move(callback).Run(false);
return;
}

if (display_externally_controlled_) {
LOG(ERROR) << __func__
<< " failed. Displays are already controlled externally.";
std::move(callback).Run(true);
return;
}

// For simplicity, just fail if in the middle of a display configuration.
if (configuration_task_) {
LOG(ERROR) << __func__
<< " failed. There is a display configuration in progress.";
std::move(callback).Run(false);
return;
}
Expand Down Expand Up @@ -735,6 +747,9 @@ void DisplayConfigurator::SendRelinquishDisplayControl(
base::BindOnce(&DisplayConfigurator::OnDisplayControlRelinquished,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
} else {
LOG(ERROR) << __func__
<< "failed. Failed to turn off all displays before "
"relinquishing control.";
display_control_changing_ = false;
std::move(callback).Run(false);
}
Expand Down
19 changes: 15 additions & 4 deletions ui/ozone/platform/drm/gpu/drm_gpu_display_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,15 @@ MovableDisplaySnapshots DrmGpuDisplayManager::GetDisplays() {
bool DrmGpuDisplayManager::TakeDisplayControl() {
const DrmDeviceVector& devices = drm_device_manager_->GetDrmDevices();
bool status = true;
for (const auto& drm : devices)
status &= drm->SetMaster();
for (const auto& drm : devices) {
const bool set_master_status = drm->SetMaster();
if (!set_master_status) {
LOG(ERROR) << __func__ << "Drm set master failed for: " // nocheck
<< drm->device_path().value();
}

status &= set_master_status;
}

// Roll-back any successful operation.
if (!status) {
Expand All @@ -377,8 +384,12 @@ bool DrmGpuDisplayManager::TakeDisplayControl() {

void DrmGpuDisplayManager::RelinquishDisplayControl() {
const DrmDeviceVector& devices = drm_device_manager_->GetDrmDevices();
for (const auto& drm : devices)
drm->DropMaster();
for (const auto& drm : devices) {
if (!drm->DropMaster()) {
LOG(ERROR) << __func__ << "Drm drop master failed for: " // nocheck
<< drm->device_path().value();
}
}
}

bool DrmGpuDisplayManager::ShouldDisplayEventTriggerConfiguration(
Expand Down
8 changes: 6 additions & 2 deletions ui/ozone/platform/drm/host/host_drm_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,10 @@ void HostDrmDevice::GpuConfigureNativeDisplays(

bool HostDrmDevice::GpuTakeDisplayControl() {
DCHECK_CALLED_ON_VALID_THREAD(on_ui_thread_);
if (!IsConnected())
if (!IsConnected()) {
LOG(WARNING) << __func__ << " GPU service not connected.";
return false;
}
auto callback =
base::BindOnce(&HostDrmDevice::GpuTakeDisplayControlCallback, this);

Expand All @@ -158,8 +160,10 @@ bool HostDrmDevice::GpuTakeDisplayControl() {

bool HostDrmDevice::GpuRelinquishDisplayControl() {
DCHECK_CALLED_ON_VALID_THREAD(on_ui_thread_);
if (!IsConnected())
if (!IsConnected()) {
LOG(WARNING) << __func__ << " GPU service not connected.";
return false;
}
auto callback =
base::BindOnce(&HostDrmDevice::GpuRelinquishDisplayControlCallback, this);

Expand Down

0 comments on commit 1893254

Please sign in to comment.