chromeos: Turn off displays on suspend

ui/display/chromeos/display_configurator.cc

Index: ui/display/chromeos/display_configurator.cc
diff --git a/ui/display/chromeos/display_configurator.cc b/ui/display/chromeos/display_configurator.cc
index 93afee24d063b52d409922d4fd0cd5d65454e5eb..2803ca24bdb69a9ab2a79e4f825dbb3f31cf5649 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,6 +470,7 @@ 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),
+      saved_power_state_(chromeos::DISPLAY_POWER_ALL_OFF),
       requested_power_state_change_(false),
       requested_power_flags_(kSetDisplayPowerNoFlags),
       force_configure_(false),
@@ -928,38 +923,36 @@ 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();
+
+  saved_power_state_ = requested_power_state_;
+  // 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).
+  SetDisplayPower(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;
+  chromeos::DisplayPowerState power_state = chromeos::DISPLAY_POWER_ALL_ON;
 
-  configure_timer_.Start(
-      FROM_HERE,
-      base::TimeDelta::FromMilliseconds(kResumeDelayMs),
-      base::Bind(&DisplayConfigurator::RestoreRequestedPowerStateAfterResume,
-                 base::Unretained(this)));

[marcheu1, 2016/04/06 20:21:50]

I think this is still needed. The kernel makes no guarantee (and as a matter of
fact it doesn't always work in practice) that you will get a hotplug event if
the monitor configuration changes while you were suspended (for example if you
added or removed a monitor).

[dbasehore, 2016/04/06 20:48:41]

We're just doing this in the ResumeDisplays function. Do you mean we need to
start the timer to run ConfigureDisplays?

+  if (requested_power_state_ != chromeos::DISPLAY_POWER_ALL_OFF) {
+    power_state = requested_power_state_;
+  } else if (saved_power_state_ != chromeos::DISPLAY_POWER_ALL_OFF) {
+    power_state = saved_power_state_;
+  }
+
+  displays_suspended_ = false;
+  SetDisplayPower(power_state, kSetDisplayPowerForceProbe,
+                  base::Bind(&DoNothing));
 }
 
 void DisplayConfigurator::ConfigureDisplays() {
@@ -1089,13 +1082,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) {