New issue
Advanced search Search tips

Issue 692193 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

DisplayController needs to expose power-related methods

Project Member Reported by derat@chromium.org, Feb 14 2017

Issue description

The DisplayController interface in services/ui/public/interfaces/display/display_controller.mojom needs to expose several power-related methods from display::DisplayConfigurator:

  (this enum comes from dbus/service_constants.h in the shared system_api repo)
  enum DisplayPowerState {
    DISPLAY_POWER_ALL_ON = 0,
    DISPLAY_POWER_ALL_OFF = 1,
    DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON = 2,
    DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF = 3,
  };

  // Flags that can be passed to SetDisplayPower().
  static const int kSetDisplayPowerNoFlags;
  // Configure displays even if the passed-in state matches |power_state_|.
  static const int kSetDisplayPowerForceProbe;
  // Do not change the state if multiple displays are connected or if the
  // only connected display is external.
  static const int kSetDisplayPowerOnlyIfSingleInternalDisplay;

  // Called when powerd notifies us that some set of displays should be turned
  // on or off.  This requires enabling or disabling the CRTC associated with
  // the display(s) in question so that the low power state is engaged.
  // |flags| contains bitwise-or-ed kSetDisplayPower* values. After the
  // configuration finishes |callback| is called with the status of the
  // operation.
  void SetDisplayPower(chromeos::DisplayPowerState power_state,
                       int flags,
                       const ConfigurationCallback& callback);

  // Sets all the displays into pre-suspend mode; usually this means
  // configure them for their resume state. This allows faster resume on
  // machines where display configuration is slow. On completion of the display
  // configuration |callback| is executed synchronously or asynchronously.
  void SuspendDisplays(const ConfigurationCallback& callback);
  
  // Reprobes displays to handle changes made while the system was
  // suspended.
  void ResumeDisplays();

SetDisplayPower is used to service D-Bus method calls from powerd, while SuspendDisplays and ResumeDisplays are called within ash when the system is about to suspend or has just resumed. The callbacks may make these tricky.

I'm commenting out ash's SuspendDisplays and ResumeDisplays calls for now to avoid the crash in  issue 686938 , but it should be updated to call into mus when possible.
 
Labels: display-mustash
I don't suspend/resume displays is required for our 30 minutes browsing OKR, so this seems like the right thing to do for Q1.
Labels: -display-mustash mustash-display
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b892c8633d4e17894340683093748d229dcd3e7

commit 2b892c8633d4e17894340683093748d229dcd3e7
Author: derat <derat@chromium.org>
Date: Tue Feb 14 21:25:21 2017

mash: Disable calls to suspend and resume displays.

To avoid crashes when suspending or resuming, make
ash::PowerEventObserver skip calling
DisplayConfigurator::SuspendDisplays or ResumeDisplays when
running in mash. Once mus exposes this functionality, these
calls should go through it instead.

BUG= 686938 , 692193 

Review-Url: https://codereview.chromium.org/2690263005
Cr-Commit-Position: refs/heads/master@{#450480}

[modify] https://crrev.com/2b892c8633d4e17894340683093748d229dcd3e7/ash/system/chromeos/power/power_event_observer.cc

Project Member

Comment 4 by sheriffbot@chromium.org, Feb 15 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: -Internals>MUS Internals>Services>WindowService
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Labels: -Proj-Mustash-Mus
Migrating Proj-Mustash-Mus to components Internals>Services>WindowService and Internals>Services>Ash

Status: WontFix (was: Untriaged)
This is no longer an issue with mash.
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/694d97c069a1412cf1e17978f8469b0ba37ecc62

commit 694d97c069a1412cf1e17978f8469b0ba37ecc62
Author: Scott Violet <sky@chromium.org>
Date: Wed Aug 15 17:46:17 2018

chromeos: removes some dead code

MASH_DEPRECATED is never set anymore.

BUG= 692193 
TEST=none

Change-Id: I49a99e9e3c6bbc7f3c77238e450e18c4ffd6b992
Reviewed-on: https://chromium-review.googlesource.com/1176102
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583307}
[modify] https://crrev.com/694d97c069a1412cf1e17978f8469b0ba37ecc62/ash/system/power/power_event_observer.cc

Sign in to add a comment