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

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

Issue 1955103002: Reland of chromeos: Turn off displays on suspend (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 7 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
« no previous file with comments | « ui/display/chromeos/display_configurator.h ('k') | ui/display/chromeos/display_configurator_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/display/chromeos/display_configurator.cc
diff --git a/ui/display/chromeos/display_configurator.cc b/ui/display/chromeos/display_configurator.cc
index f00f9ea1683b664081ea32138145772bfa2eb34c..91bb8863b99d5acf44738469b8eb76b6130983ca 100644
--- a/ui/display/chromeos/display_configurator.cc
+++ b/ui/display/chromeos/display_configurator.cc
@@ -35,12 +35,6 @@
// |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 @@
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 @@
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 @@
<< 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,21 +935,9 @@
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);
+ if (!configure_display_ || display_externally_controlled_) {
+ callback.Run(false);
+ return;
}
displays_suspended_ = true;
@@ -953,16 +945,29 @@
// 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() {
+ if (!configure_display_ || display_externally_controlled_)
+ return;
+
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 +993,7 @@
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 +1002,8 @@
// 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 +1039,12 @@
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 +1077,7 @@
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;
@@ -1090,13 +1095,6 @@
callback.Run(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(
« no previous file with comments | « ui/display/chromeos/display_configurator.h ('k') | ui/display/chromeos/display_configurator_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698