New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614624 link

Starred by 9 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ChromeOS is too quick to disable external monitor, when resuming from sleep

Reported by is.rafi...@gmail.com, May 25 2016

Issue description

Chrome Version       : 50.0.2661.104 (Official Build) (64-bit)
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL, along with the version, after other browsers where you
have tested this issue:
     Safari:
    Firefox:
         IE:

What steps will reproduce the problem?
(1) connect an external monitor via HDMI. Make an 'extended desktop'
. Everything is as expected
(2) put system to sleep (eg: power_dbus_suspend)
(3) chromebook goes to sleep. Monitor goes to power saving mode. So far, so good.
(4) resume the chromebook

What is the expected result?
I want everything to be as before. Extended display, with windows where I left them.

What happens instead?
- chromeos checks the external monitor, which I guess hasn't sufficiently woken up yet
- extended display is turned off, and all windows that were on the external monitor are now on the internal display
- within a second, chromeos now realises that the external monitor is connected, and re-establishes the extended display
- BUT, now all the windows are crammed onto the internal display, and I have to move them back over, manually.

Is this some kind of silly race condition between the displays waking up, and chromeos looking for it? It happens on 2 different chromebooks (HP 14, Acer 15) connected to 2 Hanns G 24" screens (with HDMI to DVI), but it doesn't happen when connected to a Hanns G 19" (with HDMI to VGA).

Please provide any additional information below. Attach a screenshot if
possible.

 
My mistake. I've posted this in the web browser section. Re-entered it for chrome os (https://bugs.chromium.org/p/chromium/issues/detail?id=614630).

This bug should be closed. Sorry for any confusion.
Cc: ashej...@chromium.org
 Issue 614630  has been merged into this issue.
Labels: OS-Chrome
I can confirm the same issue on my i3 Asus Chromebox(Panther) with 2 monitors attached via HDMI and DP. Here are the Chrome specifics:
Version 51.0.2704.79 beta (64-bit)
Platform 8172.47.0 (Official Build) beta-channel panther
Firmware Google_Panther.4920.24.26

Monitors I am using are:
Acer T232HL (Primary Monitor)
HP w1707 via DP to DVI adapter.

Comment 5 by dchan@google.com, Jun 20 2016

Components: OS>Kernel>Graphics UI>Shell>ExtendedDesktop
Components: OS>Kernel>Display
Components: -OS>Kernel>Graphics
Owner: afakhry@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: osh...@chromium.org
I couldn't repro this with an external Mimo Magic Touch monitor on samus at 56.0.2889.0. I'll try another HDMI display on stumpy.
Chromebook will switch to docked mode (which means single display) when you close the lid with external display.
I think the issue here is suspend, rather than closing the lid. I test that by pressing Search+Shift+L, wait for a few seconds until the device is fully suspended and then wake it up by pressing any key.
Cc: marc...@chromium.org
Ah, I see. Thank you for clarification.

My guess is that we do some shortcut to wake up internal displays, while it can take longer to probe & wake up external display.

+marcheu@ who should know better.
Status: Started (was: Assigned)
marcheu@ could you clarify how display probing works (especially around resume)? We used to have a delayed probe, but my understanding is that it was a X11 specific workaround (not needed for Freon), and even then we used 500ms (though that was arbitrarily chosen).
That is definitely not an X11-specific problem, for this X11 is just a passthrough.

When a display is added or removed, the DRM kernel module sends an event to user space. Either Chrome or X11 picks up the event and acts upon it. During suspend, some drivers will disconnect monitors, others don't; that behaviour is platform-specific. So some platforms will make the displays disappear and reappear on suspend, and the timing for this is platform-dependent as well. To hide this, Chrome was delaying probe work on resume until after things have settled down; that's  what the 500ms is for.
Turns out that we don't wait when waking up. It can also take a lot longer for UDL display because it can be disconnected first then reconnect upon wakeup.
afakhry@ is working on it right now.
Cc: dnicoara@chromium.org derat@chromium.org
I tried several values for the delay: 500ms, 700ms, 1.5s, and 2s. 1.5s worked well in the case of samus. However on stumpy, even 2s may or may not work (especially after I moved the delay from "setting the power" to "resume displays" based on suggestions on the CL).

It's hard to arbitrarily select a delay value that is guaranteed to work in all cases. 

One of the things that I want to ask about is in DisplayConfigurator::ResumeDisplays(). Why do we set the power to the |requested_power_state_| value instead of DISPLAY_POWER_ALL_ON [1]? Oshima mentioned that this could be for lucid sleep (which might have been abandoned). Looking at where that was added [2], it seems we used to have a delay in ResumeDisplays() before, and was removed.


[1]: https://cs.chromium.org/chromium/src/ui/display/chromeos/display_configurator.cc?q=DisplayConfigurator::ResumeDisplays&sq=package:chromium&l=977

[2]: https://codereview.chromium.org/1955103002

Comment 18 by derat@chromium.org, Oct 20 2016

Is there any way to avoid choosing an arbitrary delay? :-P Like, maybe we could cache the multi-monitor values of window positions and sizes so they can be restored when the second display shows up again. Is that the path to madness?

Re your question, presumably we don't just use DISPLAY_POWER_ALL_ON since the user could have previously requested DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON by reducing the backlight brightness to zero, and we don't want to ignore the state they previously requested every time the system suspends and resumes.
We shouldn't be relying on the delay; there is no guarantee that we will ever get the monitor to reconnect in time, in particular with solutions like evdi that involve waking the USB stack + a user space daemon, there can be quite a bit of latency.

IMO we should:
- on resume, restore everything as it was at suspend, without probing
- on resume, arm a timer to reprobe after 1 or 2 seconds
- when we get a hotplug event, (re-)arm the same timer
- when the timer fires, do probing + reconfiguration

Separately from this, I think we probably need the ability to save window positions per-monitor so that we can restore them. The timer above makes the experience a little better, but will never handle all cases...
derat@, it turns out that chromeos::ChromeDisplayPowerServiceProviderDelegate::SetDisplayPower() [1] is what calls DisplayConfigurator::SetDisplayPower() to set it to ALL_ON when resuming. That's why it was working fine before I moved the delay from SetDisplayPowerInternal() to ResumeDisplays().

The delayed SetDisplayPower() called from ResumeDisplays() becomes effectively useless if ChromeDisplayPowerServiceProviderDelegate::SetDisplayPower() calls it too early.

I suggest moving the delay back to SetDisplayPowerInternal() and some clever logic to determine when it makes sense to introduce this delay and keep the delay value at minimum taking into account comment #19.

[1]: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/dbus/chrome_display_power_service_provider_delegate.cc?q=ChromeDisplayPowerServiceProviderDelegate::SetDisplayPower&sq=package:chromium&l=32

Comment 21 by derat@chromium.org, Oct 21 2016

When Chrome hasn't yet resumed the displays, should DisplayConfigurator just update the requested state on the ChromeDisplayPowerServiceProviderDelegate::SetDisplayPower path (called via powerd) instead of applying the change immediately? That seems like it should be sufficient to prevent powerd's call from making the delayed call from ResumeDisplays a no-op.
Going one step further: do we even need powerd's ALL_ON message now? We are already using the resume observer to trigger display setup.

Comment 23 by derat@chromium.org, Oct 21 2016

#22: The tricky part here is that there are multiple ways we can suspend. Hopefully I'm getting this right:

1. From idle:
a) powerd dims the backlight
b) later, powerd turns the backlight off and sends ALL_OFF to Chrome
c) later, powerd emits SuspendImminent and Chrome calls SuspendDisplays
d) system suspends
e) system resumes
f) powerd emits SuspendDone (asynchronously!) and Chrome calls ResumeDisplays
g) powerd turns the backlight on and sends ALL_ON or INT_OFF_EXT_ON (if we were in docked mode) to Chrome

2. Lid closed:
a) powerd turns the backlight off but doesn't send anything to Chrome
b) powerd emits SuspendImminent and Chrome calls SuspendDisplays
[same as 1d-g]

So I think we need the ALL_ON or INT_OFF_EXT_ON call in 1g, because otherwise Chrome may just restore the last-requested ALL_OFF state from 1b in ResumeDisplays.

Making big changes to this is definitely a (somewhat scary) option, but it may be easier to just make Chrome cache the state that's passed in 1g if it actually happens before the displays are resumed by 1f.
> Is there any way to avoid choosing an arbitrary delay? :-P
> Like, maybe we could cache the multi-monitor values of window positions and
> sizes so they can be restored when the second display shows up again. Is that
> the path to madness?

Remembering layout is actually tricky and not so easy because windows have more state than just positions (size, docked, snapped, apps has its own control, etc). It's not impossible, but it'll have a lot of edge cases we have to handle. It probably won't be worth the effort if we can address most of scenarios by using delay.
Issue 558409 has been merged into this issue.
Project Member

Comment 26 by bugdroid1@chromium.org, Nov 1 2016

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

commit 343040c3e082e53591a4d7d822cefc70ba900c82
Author: afakhry <afakhry@chromium.org>
Date: Tue Nov 01 19:25:19 2016

Delay display configuration after waking up from suspend with multi displays

Perfrom display configuration after a delay when waking up from suspend in multi
display mode, for all displays to have a chance to be detected and added, before
the first configuration.

BUG= 614624 
TEST=display_unittests --gtest_filter=DisplayConfiguratorTest.*
TEST=connect an external monitor, have several windows open in both displays,
press Search+Shift+L to suspend the device, wait for a few seconds until the
device is fully suspeneded, wake up by pressing any key. The windows in both
displays should be exactly where you left them before suspend.

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

[modify] https://crrev.com/343040c3e082e53591a4d7d822cefc70ba900c82/ui/display/chromeos/display_configurator.cc
[modify] https://crrev.com/343040c3e082e53591a4d7d822cefc70ba900c82/ui/display/chromeos/display_configurator.h
[modify] https://crrev.com/343040c3e082e53591a4d7d822cefc70ba900c82/ui/display/chromeos/display_configurator_unittest.cc

Status: Fixed (was: Started)

Comment 28 by son...@google.com, Nov 10 2016

Status: Verified (was: Fixed)
Verified on build 8977.0.0
Cc: durga.behera@chromium.org drott@chromium.org ajha@chromium.org rponnada@chromium.org
 Issue 468683  has been merged into this issue.
this issue exists today, pixelbook, using a displayport connection and the Samsung S34E790C. the pixelbook wakes, removes the monitor, repositions all my windows and then re-attaches the monitor with everything being in the wrong place. 

i assume because the arbitrary timer value for reconfiguring is too low 

Sign in to add a comment