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

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

Issue 1949753004: Revert of chromeos: Turn off displays on suspend (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Revert PS that actually landed: PS#11, not PS#12 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 91bb8863b99d5acf44738469b8eb76b6130983ca..f00f9ea1683b664081ea32138145772bfa2eb34c 100644
--- a/ui/display/chromeos/display_configurator.cc
+++ b/ui/display/chromeos/display_configurator.cc
@@ -35,6 +35,12 @@ 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);
@@ -470,9 +476,8 @@ 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),
- pending_power_state_(chromeos::DISPLAY_POWER_ALL_ON),
- has_pending_power_state_(false),
- pending_power_flags_(kSetDisplayPowerNoFlags),
+ requested_power_state_change_(false),
+ requested_power_flags_(kSetDisplayPowerNoFlags),
force_configure_(false),
next_display_protection_client_id_(1),
display_externally_controlled_(false),
@@ -839,24 +844,6 @@ 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,
@@ -870,9 +857,18 @@ 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;
- SetDisplayPowerInternal(requested_power_state_, flags, callback);
+ requested_power_state_change_ = true;
+ requested_power_flags_ = flags;
+ queued_configuration_callbacks_.push_back(callback);
+
+ RunPendingConfiguration();
}
void DisplayConfigurator::SetDisplayMode(MultipleDisplayState new_state) {
@@ -935,9 +931,21 @@ void DisplayConfigurator::RemoveObserver(Observer* observer) {
void DisplayConfigurator::SuspendDisplays(
const ConfigurationCallback& callback) {
- if (!configure_display_ || display_externally_controlled_) {
- callback.Run(false);
- return;
+ // 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;
@@ -945,29 +953,16 @@ void DisplayConfigurator::SuspendDisplays(
// 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;
- // 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));
+ configure_timer_.Start(
+ FROM_HERE,
+ base::TimeDelta::FromMilliseconds(kResumeDelayMs),
+ base::Bind(&DisplayConfigurator::RestoreRequestedPowerStateAfterResume,
+ base::Unretained(this)));
}
void DisplayConfigurator::ConfigureDisplays() {
@@ -993,7 +988,7 @@ void DisplayConfigurator::RunPendingConfiguration() {
configuration_task_.reset(new UpdateDisplayConfigurationTask(
native_display_delegate_.get(), layout_manager_.get(),
- requested_display_state_, pending_power_state_, pending_power_flags_,
+ requested_display_state_, requested_power_state_, requested_power_flags_,
0, force_configure_, base::Bind(&DisplayConfigurator::OnConfigured,
weak_ptr_factory_.GetWeakPtr())));
configuration_task_->set_virtual_display_snapshots(
@@ -1002,8 +997,8 @@ void DisplayConfigurator::RunPendingConfiguration() {
// Reset the flags before running the task; otherwise it may end up scheduling
// another configuration.
force_configure_ = false;
- pending_power_flags_ = kSetDisplayPowerNoFlags;
- has_pending_power_state_ = false;
+ requested_power_flags_ = kSetDisplayPowerNoFlags;
+ requested_power_state_change_ = false;
requested_display_state_ = MULTIPLE_DISPLAY_STATE_INVALID;
DCHECK(in_progress_configuration_callbacks_.empty());
@@ -1039,12 +1034,12 @@ void DisplayConfigurator::OnConfigured(
if (!framebuffer_size.IsEmpty())
framebuffer_size_ = framebuffer_size;
- // If the pending power state hasn't changed then make sure that value
+ // If the requested 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 (!has_pending_power_state_)
- pending_power_state_ = new_power_state;
+ if (!requested_power_state_change_)
+ requested_power_state_ = new_power_state;
if (old_power_state != current_power_state_)
NotifyPowerStateObservers();
@@ -1077,7 +1072,7 @@ bool DisplayConfigurator::ShouldRunConfigurationTask() const {
return true;
// Schedule if there is a request to change the power state.
- if (has_pending_power_state_)
+ if (requested_power_state_change_)
return true;
return false;
@@ -1097,6 +1092,13 @@ 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) {
« 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