Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Unified Diff: ui/display/chromeos/display_configurator.cc

Issue 1861593002: chromeos: Turn off displays on suspend (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: chromeos: Turn off displays on suspend Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/display/chromeos/display_configurator.cc
diff --git a/ui/display/chromeos/display_configurator.cc b/ui/display/chromeos/display_configurator.cc
index e1def2037131a0edf7611ec5def3d6eca7f2aba3..a414c56aeefebb71bd123d142e3902c232c9e48c 100644
--- a/ui/display/chromeos/display_configurator.cc
+++ b/ui/display/chromeos/display_configurator.cc
@@ -35,12 +35,6 @@ typedef std::vector<const DisplayMode*> DisplayModeList;
// |configure_timer_|.
const int kConfigureDelayMs = 500;
-// The delay spent before reading the display configuration after coming out of
-// suspend. While coming out of suspend the display state may be updating. This
-// is used to wait until the hardware had a chance to update the display state
-// such that we read an up to date state.
-const int kResumeDelayMs = 500;
-
// The EDID specification marks the top bit of the manufacturer id as reserved.
const int16_t kReservedManufacturerID = static_cast<int16_t>(1 << 15);
@@ -476,8 +470,9 @@ DisplayConfigurator::DisplayConfigurator()
current_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
requested_display_state_(MULTIPLE_DISPLAY_STATE_INVALID),
requested_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
- requested_power_state_change_(false),
- requested_power_flags_(kSetDisplayPowerNoFlags),
+ pending_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
+ has_pending_power_state_(false),
+ pending_power_flags_(kSetDisplayPowerNoFlags),
force_configure_(false),
next_display_protection_client_id_(1),
display_externally_controlled_(false),
@@ -844,6 +839,24 @@ void DisplayConfigurator::PrepareForExit() {
configure_display_ = false;
}
+void DisplayConfigurator::SetDisplayPowerInternal(
+ chromeos::DisplayPowerState power_state,
+ int flags,
+ const ConfigurationCallback& callback) {
+ if (power_state == current_power_state_ &&
+ !(flags & kSetDisplayPowerForceProbe)) {
+ callback.Run(true);
+ return;
+ }
+
+ pending_power_state_ = power_state;
+ has_pending_power_state_ = true;
+ pending_power_flags_ = flags;
+ queued_configuration_callbacks_.push_back(callback);
+
+ RunPendingConfiguration();
+}
+
void DisplayConfigurator::SetDisplayPower(
chromeos::DisplayPowerState power_state,
int flags,
@@ -857,18 +870,9 @@ void DisplayConfigurator::SetDisplayPower(
<< DisplayPowerStateToString(power_state) << " flags=" << flags
<< ", configure timer="
<< (configure_timer_.IsRunning() ? "Running" : "Stopped");
- if (power_state == requested_power_state_ &&
- !(flags & kSetDisplayPowerForceProbe)) {
- callback.Run(true);
- return;
- }
requested_power_state_ = power_state;
- requested_power_state_change_ = true;
- requested_power_flags_ = flags;
- queued_configuration_callbacks_.push_back(callback);
-
- RunPendingConfiguration();
+ SetDisplayPowerInternal(requested_power_state_, flags, callback);
}
void DisplayConfigurator::SetDisplayMode(MultipleDisplayState new_state) {
@@ -931,38 +935,31 @@ void DisplayConfigurator::RemoveObserver(Observer* observer) {
void DisplayConfigurator::SuspendDisplays(
const ConfigurationCallback& callback) {
- // If the display is off due to user inactivity and there's only a single
- // internal display connected, switch to the all-on state before
- // suspending. This shouldn't be very noticeable to the user since the
- // backlight is off at this point, and doing this lets us resume directly
- // into the "on" state, which greatly reduces resume times.
- if (requested_power_state_ == chromeos::DISPLAY_POWER_ALL_OFF) {
- SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,
- kSetDisplayPowerOnlyIfSingleInternalDisplay, callback);
-
- // We need to make sure that the monitor configuration we just did actually
- // completes before we return, because otherwise the X message could be
- // racing with the HandleSuspendReadiness message.
- native_display_delegate_->SyncWithServer();
- } else {
- callback.Run(true);
- }
-
displays_suspended_ = true;
// Stop |configure_timer_| because we will force probe and configure all the
// displays at resume time anyway.
configure_timer_.Stop();
+
+ // Turn off the displays for suspend. This way, if we wake up for lucid sleep,
+ // the displays will not turn on (all displays should be off for lucid sleep
+ // unless explicitly requested by lucid sleep code). Use
+ // SetDisplayPowerInternal so requested_power_state_ is maintained.
+ SetDisplayPowerInternal(chromeos::DISPLAY_POWER_ALL_OFF,
+ kSetDisplayPowerNoFlags, callback);
+
+ // We need to make sure that the monitor configuration we just did actually
+ // completes before we return.
+ native_display_delegate_->SyncWithServer();
}
void DisplayConfigurator::ResumeDisplays() {
displays_suspended_ = false;
- configure_timer_.Start(
- FROM_HERE,
- base::TimeDelta::FromMilliseconds(kResumeDelayMs),
- base::Bind(&DisplayConfigurator::RestoreRequestedPowerStateAfterResume,
- base::Unretained(this)));
+ // If requested_power_state_ is ALL_OFF due to idle suspend, powerd will turn
+ // the display power on when it enables the backlight.
+ SetDisplayPower(requested_power_state_, kSetDisplayPowerNoFlags,
+ base::Bind(&DoNothing));
}
void DisplayConfigurator::ConfigureDisplays() {
@@ -988,7 +985,7 @@ void DisplayConfigurator::RunPendingConfiguration() {
configuration_task_.reset(new UpdateDisplayConfigurationTask(
native_display_delegate_.get(), layout_manager_.get(),
- requested_display_state_, requested_power_state_, requested_power_flags_,
+ requested_display_state_, pending_power_state_, pending_power_flags_,
0, force_configure_, base::Bind(&DisplayConfigurator::OnConfigured,
weak_ptr_factory_.GetWeakPtr())));
configuration_task_->set_virtual_display_snapshots(
@@ -997,8 +994,8 @@ void DisplayConfigurator::RunPendingConfiguration() {
// Reset the flags before running the task; otherwise it may end up scheduling
// another configuration.
force_configure_ = false;
- requested_power_flags_ = kSetDisplayPowerNoFlags;
- requested_power_state_change_ = false;
+ pending_power_flags_ = kSetDisplayPowerNoFlags;
+ has_pending_power_state_ = false;
requested_display_state_ = MULTIPLE_DISPLAY_STATE_INVALID;
DCHECK(in_progress_configuration_callbacks_.empty());
@@ -1034,12 +1031,12 @@ void DisplayConfigurator::OnConfigured(
if (!framebuffer_size.IsEmpty())
framebuffer_size_ = framebuffer_size;
- // If the requested power state hasn't changed then make sure that value
+ // If the pending power state hasn't changed then make sure that value
// gets updated as well since the last requested value may have been
// dependent on certain conditions (ie: if only the internal monitor was
// present).
- if (!requested_power_state_change_)
- requested_power_state_ = new_power_state;
+ if (!has_pending_power_state_)
+ pending_power_state_ = new_power_state;
if (old_power_state != current_power_state_)
NotifyPowerStateObservers();
@@ -1072,7 +1069,7 @@ bool DisplayConfigurator::ShouldRunConfigurationTask() const {
return true;
// Schedule if there is a request to change the power state.
- if (requested_power_state_change_)
+ if (has_pending_power_state_)
return true;
return false;
@@ -1092,13 +1089,6 @@ void DisplayConfigurator::CallAndClearQueuedCallbacks(bool success) {
queued_configuration_callbacks_.clear();
}
-void DisplayConfigurator::RestoreRequestedPowerStateAfterResume() {
- // Force probing to ensure that we pick up any changes that were made while
- // the system was suspended.
- SetDisplayPower(requested_power_state_, kSetDisplayPowerForceProbe,
- base::Bind(&DoNothing));
-}
-
void DisplayConfigurator::NotifyDisplayStateObservers(
bool success,
MultipleDisplayState attempted_state) {

Powered by Google App Engine
This is Rietveld 408576698