chromeos: Turn off displays on suspend

ui/display/chromeos/display_configurator.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/display/chromeos/display_configurator.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 17 matching lines @@
 namespace ui {
 
 namespace {
 
 typedef std::vector<const DisplayMode*> DisplayModeList;
 
 // The delay to perform configuration after RRNotify. See the comment for
 // |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);
 
 struct DisplayState {
   DisplaySnapshot* display = nullptr;  // Not owned.
 
   // User-selected mode for the display.
   const DisplayMode* selected_mode = nullptr;
 
   // Mode used when displaying the same desktop on multiple displays.
@@ skipping 415 matching lines @@
 
 DisplayConfigurator::DisplayConfigurator()
     : state_controller_(NULL),
       mirroring_controller_(NULL),
       is_panel_fitting_enabled_(false),
       configure_display_(base::SysInfo::IsRunningOnChromeOS()),
       current_display_state_(MULTIPLE_DISPLAY_STATE_INVALID),
       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),
       next_display_protection_client_id_(1),
       display_externally_controlled_(false),
       display_control_changing_(false),
       displays_suspended_(false),
       layout_manager_(new DisplayLayoutManagerImpl(this)),
       weak_ptr_factory_(this) {
 }
@@ skipping 432 matching lines @@
 void DisplayConfigurator::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
 
 void DisplayConfigurator::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(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, because otherwise the X message could be

[Eric Caruso, 2016/04/05 17:33:36]

Aren't we not using X anymore?

[dbasehore, 2016/04/06 04:40:39]

Done.

+  // racing with the HandleSuspendReadiness message.
+  native_display_delegate_->SyncWithServer();
 }
 
 void DisplayConfigurator::ResumeDisplays() {
+  chromeos::DisplayPowerState power_state = requested_power_state_;
+
+  if (power_state == chromeos::DISPLAY_POWER_ALL_OFF)
+    power_state = saved_power_state_;
+
   displays_suspended_ = false;
-
-  configure_timer_.Start(
-      FROM_HERE,
-      base::TimeDelta::FromMilliseconds(kResumeDelayMs),
-      base::Bind(&DisplayConfigurator::RestoreRequestedPowerStateAfterResume,
-                 base::Unretained(this)));
+  saved_power_state_ = chromeos::DISPLAY_POWER_ALL_OFF;

[Daniel Erat, 2016/04/05 14:40:03]

what's the reason for resetting this here? doesn't it always get updated before
suspending in SuspendDisplays?

[dbasehore, 2016/04/06 04:40:39]

Done.

+  if (power_state == chromeos::DISPLAY_POWER_ALL_OFF)

[Daniel Erat, 2016/04/05 14:40:04]

add curly brackets for this if/else

[dbasehore, 2016/04/06 04:40:39]

Done.

+    SetDisplayPower(chromeos::DISPLAY_POWER_ALL_ON,

[Daniel Erat, 2016/04/05 14:40:03]

what happens if the user sets the brightness to zero and then the system
suspends and resumes? doesn't this undo what the user asked for?

[dbasehore, 2016/04/06 04:40:40]

Don't know how, but it doesn't undo what the user set.

We do something similar right now by always turning on the display during
suspend if it's just a single display.

[Daniel Erat, 2016/04/06 15:04:19]

i'm uneasy not knowing how/why this works. so you're saying that with this
change, if you use the brightness-down key to reduce to 0% (and also turn the
display off), and then suspend/resume, the display is still off instead of
getting turned back on? does powerd send another message post-resume requesting
that the displays be turned off?

[dbasehore, 2016/04/06 20:44:45]

This seems to be how it works:
-Backlight set to 0 by user
-resume brightness set the 0 (since GetUndimmedBrightnessPercent in powerd
returns the current brightness of 0)
-System suspends (display turned off)
-System resumes (brightness set to 0 by resume brightness)
-Chrome "turns on" display, but brightness is still 0

This is also what happens *right now* since we turn on the display for a single
display on suspend. The reason that it turns on the display if it was turned off
for idle suspend is that GetUndimmedBrightnessPercent does not return 0 in that
case.

[dbasehore, 2016/04/06 20:53:26]

When we update the backlight state in powerd, we set the display power.  It
seems this happens no matter what on resume, but I can double check.

+                    kSetDisplayPowerOnlyIfSingleInternalDisplay,

[dnicoara, 2016/04/05 16:56:57]

I think you still want kSetDisplayPowerForceProbe regardless of the power state.
On resume we need to force a probe since there may have been display events
(unplugged/plugged displays).

[dbasehore, 2016/04/06 04:40:40]

I'm changing how this works for the next version.

+                    base::Bind(&DoNothing));
+  else
+    SetDisplayPower(power_state, kSetDisplayPowerForceProbe,
+                    base::Bind(&DoNothing));
 }
 
 void DisplayConfigurator::ConfigureDisplays() {
   if (!configure_display_ || display_externally_controlled_)
     return;
 
   force_configure_ = true;
   RunPendingConfiguration();
 }
 
@@ skipping 109 matching lines @@
   in_progress_configuration_callbacks_.clear();
 }
 
 void DisplayConfigurator::CallAndClearQueuedCallbacks(bool success) {
   for (const auto& callback : queued_configuration_callbacks_)
     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(
     bool success,
     MultipleDisplayState attempted_state) {
   if (success) {
     FOR_EACH_OBSERVER(
         Observer, observers_, OnDisplayModeChanged(cached_displays_));
   } else {
     FOR_EACH_OBSERVER(
         Observer, observers_, OnDisplayModeChangeFailed(cached_displays_,
                                                         attempted_state));
@@ skipping 42 matching lines @@
   last_virtual_display_id_ = max_display_id & 0xff;
 
   return true;
 }
 
 bool DisplayConfigurator::IsDisplayOn() const {
   return current_power_state_ != chromeos::DISPLAY_POWER_ALL_OFF;
 }
 
 }  // namespace ui