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

Issue 763002 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ec: Allow deep sleep in S3 when in SRC_DISCOVERY

Project Member Reported by sha...@chromium.org, Sep 7 2017

Issue description

(Forked from b/35582718)

Currently we allow deep sleep in S3 only if all ports are in one of these states:

PD_STATE_DISABLED
PD_STATE_DRP_AUTO_TOGGLE
PD_STATE_SNK_DISCONNECTED
PD_STATE_SNK_DISCONNECTED_DEBOUNCE
PD_STATE_SRC_DISCONNECTED
PD_STATE_SRC_DISCONNECTED_DEBOUNCE

The original motivation for inhibiting deep sleep was to eliminate EC wake latency so that we can respond to PD packets in time.

A common use case is to go to S3 with a non-PD USB-C peripheral attached. In that case, the port will stay in SRC_DISCOVERY forever (until the peripheral is unplugged), and we'll inhibit deep sleep. That seems like a waste of power, since the peripheral is not likely to suddenly start speaking PD.

Let's experiment with allowing deep sleep in S3 when ports are in SRC_DISCOVERY.
 
Components: OS>Firmware>EC
Labels: OS-Chrome
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/9abb9f762efb3f386befc74e90275fe8789e3bd2

commit 9abb9f762efb3f386befc74e90275fe8789e3bd2
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Sep 11 18:14:28 2017

pd: Allow deep sleep in SRC_DISCOVERY

If the PD state machine remains in SRC_DISCOVERY for an extended period
of time, it's likely that a non-PD USB peripheral is attached. In this
case, we don't need to inhibit deep sleep, since we're not likely to
receive PD packets.

This change will cause us to enter deep sleep slightly more
aggressively, not inhibiting deep sleep until source caps are received
or replied with GoodCRC by the port partner. We can accommodate
additional task latency up to this point, since the spec calls for
source caps to be sent up to 50 times before failure.

BUG=b:35582718, chromium:763002 
TEST=Test with `sleepmask 1` on kevin.
- Go to S3 with USB-C flash drive plugged, verify `sleepmask` shows 0.
- Go to S3 with zinger + USB C flash drive plugged
   - Unplug zinger, verify `sleepmask` shows 0.
   - Plug zinger, verify PD negotiates to 20V @ 2A.
   - Plug OEM kevin charger, verify same.
BRANCH=gru

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Ib8e1bc94bdbcfddea004d572edf1ccadc8c8c1ce
Reviewed-on: https://chromium-review.googlesource.com/655919
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/9abb9f762efb3f386befc74e90275fe8789e3bd2/common/usb_pd_protocol.c

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11 2017

Labels: merge-merged-firmware-gru-8785.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/193e4ff4e3a358fbe29762ea1bcc733b97cfc9f9

commit 193e4ff4e3a358fbe29762ea1bcc733b97cfc9f9
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Sep 11 20:26:06 2017

pd: Allow deep sleep in SRC_DISCOVERY

If the PD state machine remains in SRC_DISCOVERY for an extended period
of time, it's likely that a non-PD USB peripheral is attached. In this
case, we don't need to inhibit deep sleep, since we're not likely to
receive PD packets.

This change will cause us to enter deep sleep slightly more
aggressively, not inhibiting deep sleep until source caps are received
or replied with GoodCRC by the port partner. We can accommodate
additional task latency up to this point, since the spec calls for
source caps to be sent up to 50 times before failure.

BUG=b:35582718, chromium:763002 
TEST=Test with `sleepmask 1` on kevin.
- Go to S3 with USB-C flash drive plugged, verify `sleepmask` shows 0.
- Go to S3 with zinger + USB C flash drive plugged
   - Unplug zinger, verify `sleepmask` shows 0.
   - Plug zinger, verify PD negotiates to 20V @ 2A.
   - Plug OEM kevin charger, verify same.
BRANCH=gru

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Ib8e1bc94bdbcfddea004d572edf1ccadc8c8c1ce
Reviewed-on: https://chromium-review.googlesource.com/655919
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit 9abb9f762efb3f386befc74e90275fe8789e3bd2)
Reviewed-on: https://chromium-review.googlesource.com/660459

[modify] https://crrev.com/193e4ff4e3a358fbe29762ea1bcc733b97cfc9f9/common/usb_pd_protocol.c

Comment 4 by sha...@chromium.org, Sep 11 2017

Status: Fixed (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 2017

Labels: merge-merged-firmware-eve-9584.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/74955dd69bf78a88dbbd476f2d81a27974313672

commit 74955dd69bf78a88dbbd476f2d81a27974313672
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Thu Sep 28 11:10:13 2017

pd: Allow deep sleep in SRC_DISCOVERY

If the PD state machine remains in SRC_DISCOVERY for an extended period
of time, it's likely that a non-PD USB peripheral is attached. In this
case, we don't need to inhibit deep sleep, since we're not likely to
receive PD packets.

This change will cause us to enter deep sleep slightly more
aggressively, not inhibiting deep sleep until source caps are received
or replied with GoodCRC by the port partner. We can accommodate
additional task latency up to this point, since the spec calls for
source caps to be sent up to 50 times before failure.

BUG=b:35582718, chromium:763002 
TEST=Test with `sleepmask 1` on kevin.
- Go to S3 with USB-C flash drive plugged, verify `sleepmask` shows 0.
- Go to S3 with zinger + USB C flash drive plugged
   - Unplug zinger, verify `sleepmask` shows 0.
   - Plug zinger, verify PD negotiates to 20V @ 2A.
   - Plug OEM kevin charger, verify same.
BRANCH=gru

Change-Id: Iaedb8ed124a8aa614fabb58b7aed151ae7003dad
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Original-Commit-Id: 9abb9f762efb3f386befc74e90275fe8789e3bd2
Original-Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Original-Change-Id: Ib8e1bc94bdbcfddea004d572edf1ccadc8c8c1ce
Original-Reviewed-on: https://chromium-review.googlesource.com/655919
Original-Commit-Ready: Shawn N <shawnn@chromium.org>
Original-Tested-by: Shawn N <shawnn@chromium.org>
Original-Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Original-Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/690177

[modify] https://crrev.com/74955dd69bf78a88dbbd476f2d81a27974313672/common/usb_pd_protocol.c

Comment 6 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 7 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment