diff options
author | Evan Green <evgreen@chromium.org> | 2019-03-01 13:09:08 -0800 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-03-28 19:04:16 -0700 |
commit | 49b8070623ee1cecac387488ce668a4a11403653 (patch) | |
tree | bf4cc3078a496e7a9c8aa14c902be79cd3897c69 | |
parent | a029c7a27f3bd1a1066db9c167c6166688fe4ef3 (diff) | |
download | chrome-ec-49b8070623ee1cecac387488ce668a4a11403653.tar.gz |
power/intel_x86: Introduce s0ix failure detection
This change introduces logic in the EC that can detect if the host
attempted to go into S0ix, but never made it. The host already sends
commands indicating its intent to enter S0ix, and the EC has a SLP_S0
line that gets asserted by the AP when it actually enters S0ix.
All that's needed to monitor failures is to arm a timer when receiving
the S0ix suspend message. If the SLP_S0 pin goes low, then the suspend
occurred and the timer is canceled. If the timer expires before SLP_S0
goes low, then the EC wakes the AP up, since it has entered a shallower
idle state than intended, and should be alerted to avoid short battery
life.
The timer is also started when SLP_S0 is deasserted on resume. The
SoC comes out of S0ix to perform housekeeping activities unbeknownst
to Linux. In cases where housekeeping fails to suspend all the way back
down, this timer will wake the AP. Additionally, the number of S0ix
transitions is reported on resume. This enabled the AP to analyze the
amount of "sleepwalking" that is done, and can complain if it seems to
be waking up too often.
Design doc at:
https://docs.google.com/document/d/1mY-v02KAuOyET3td9s5GnxeUgMiAcD058oLEo57DZpY/edit
BUG=b:123716513
BRANCH=None
TEST=Test S0ix on hatch with modified code that forces a timeout,
use ectool to send messages manually before and after timeout,
Hack Linux to fail suspend very late to verify no regressions.
Signed-off-by: Evan Green <evgreen@chromium.org>
Change-Id: Ia64b496675a13dbed4ef74637f51e39eee68aa1a
Reviewed-on: https://chromium-review.googlesource.com/1501512
Commit-Ready: Evan Green <evgreen@chromium.org>
Tested-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org>
-rw-r--r-- | include/config.h | 12 | ||||
-rw-r--r-- | include/ec_commands.h | 57 | ||||
-rw-r--r-- | include/power.h | 18 | ||||
-rw-r--r-- | power/common.c | 52 | ||||
-rw-r--r-- | power/intel_x86.c | 117 | ||||
-rw-r--r-- | util/ectool.c | 113 |
6 files changed, 335 insertions, 34 deletions
diff --git a/include/config.h b/include/config.h index ea68efd2d0..cdc291cdda 100644 --- a/include/config.h +++ b/include/config.h @@ -2652,6 +2652,9 @@ /* Support S0ix */ #undef CONFIG_POWER_S0IX +/* Support detecting failure to enter S0ix */ +#undef CONFIG_POWER_S0IX_FAILURE_DETECTION + /* * Allow the host to self-report its sleep state, in case there is some delay * between the host beginning to enter the sleep state and power signals @@ -4371,4 +4374,13 @@ #error "CONFIG_DPTF_MULTI_PROFILE can be set only when CONFIG_DPTF is set." #endif /* CONFIG_DPTF_MULTI_PROFILE && !CONFIG_DPTF */ +/* + * Define the timeout in milliseconds between when the EC receives a suspend + * command and when the EC times out and asserts wake because the sleep signal + * SLP_S0 did not assert. + */ +#ifndef CONFIG_SLEEP_TIMEOUT_MS +#define CONFIG_SLEEP_TIMEOUT_MS 10000 +#endif + #endif /* __CROS_EC_CONFIG_H */ diff --git a/include/ec_commands.h b/include/ec_commands.h index 474ee428f4..590ee8202a 100644 --- a/include/ec_commands.h +++ b/include/ec_commands.h @@ -4208,6 +4208,63 @@ struct ec_params_host_sleep_event { uint8_t sleep_event; } __ec_align1; +/* + * Use a default timeout value (CONFIG_SLEEP_TIMEOUT_MS) for detecting sleep + * transition failures + */ +#define EC_HOST_SLEEP_TIMEOUT_DEFAULT 0 + +/* Disable timeout detection for this sleep transition */ +#define EC_HOST_SLEEP_TIMEOUT_INFINITE 0xFFFF + +struct ec_params_host_sleep_event_v1 { + /* The type of sleep being entered or exited. */ + uint8_t sleep_event; + + /* Padding */ + uint8_t reserved; + union { + /* Parameters that apply for suspend messages. */ + struct { + /* + * The timeout in milliseconds between when this message + * is received and when the EC will declare sleep + * transition failure if the sleep signal is not + * asserted. + */ + uint16_t sleep_timeout_ms; + } suspend_params; + + /* No parameters for non-suspend messages. */ + }; +} __ec_align2; + +/* A timeout occurred when this bit is set */ +#define EC_HOST_RESUME_SLEEP_TIMEOUT 0x80000000 + +/* + * The mask defining which bits correspond to the number of sleep transitions, + * as well as the maximum number of suspend line transitions that will be + * reported back to the host. + */ +#define EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK 0x7FFFFFFF + +struct ec_response_host_sleep_event_v1 { + union { + /* Response fields that apply for resume messages. */ + struct { + /* + * The number of sleep power signal transitions that + * occurred since the suspend message. The high bit + * indicates a timeout occurred. + */ + uint32_t sleep_transitions; + } resume_response; + + /* No response fields for non-resume messages. */ + }; +} __ec_align4; + /*****************************************************************************/ /* Device events */ #define EC_CMD_DEVICE_EVENT 0x00AA diff --git a/include/power.h b/include/power.h index 3bf598b182..2c57209e84 100644 --- a/include/power.h +++ b/include/power.h @@ -192,12 +192,28 @@ void power_set_pause_in_s5(int pause); enum host_sleep_event power_get_host_sleep_state(void); /** + * Set sleep state of host. + * + * @param state The new state to set. + */ +void power_set_host_sleep_state(enum host_sleep_event state); + +/* Context to pass to a host sleep command handler. */ +struct host_sleep_event_context { + uint32_t sleep_transitions; /* Number of sleep transitions observed */ + uint16_t sleep_timeout_ms; /* Timeout in milliseconds */ +}; + +/** * Provide callback to allow chipset to take any action on host sleep event * command. * * @param state Current host sleep state updated by the host. + * @param ctx Possible sleep parameters and return values, depending on state. */ -void power_chipset_handle_host_sleep_event(enum host_sleep_event state); +void +power_chipset_handle_host_sleep_event(enum host_sleep_event state, + struct host_sleep_event_context *ctx); /** * Provide callback to allow board to take any action on host sleep event diff --git a/power/common.c b/power/common.c index cccb34911a..09501a16ec 100644 --- a/power/common.c +++ b/power/common.c @@ -873,37 +873,69 @@ DECLARE_CONSOLE_COMMAND(pause_in_s5, command_pause_in_s5, static enum host_sleep_event host_sleep_state; void __attribute__((weak)) -power_chipset_handle_host_sleep_event(enum host_sleep_event state) +power_chipset_handle_host_sleep_event(enum host_sleep_event state, + struct host_sleep_event_context *ctx) { /* Default weak implementation -- no action required. */ } static int host_command_host_sleep_event(struct host_cmd_handler_args *args) { - const struct ec_params_host_sleep_event *p = args->params; + const struct ec_params_host_sleep_event_v1 *p = args->params; + struct ec_response_host_sleep_event_v1 *r = args->response; + struct host_sleep_event_context ctx; + enum host_sleep_event state = p->sleep_event; - host_sleep_state = p->sleep_event; + host_sleep_state = state; + switch (state) { + case HOST_SLEEP_EVENT_S3_SUSPEND: + case HOST_SLEEP_EVENT_S0IX_SUSPEND: + case HOST_SLEEP_EVENT_S3_WAKEABLE_SUSPEND: + ctx.sleep_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT; + + /* The original version contained only state. */ + if (args->version >= 1) + ctx.sleep_timeout_ms = + p->suspend_params.sleep_timeout_ms; + + break; + + default: + break; + } - power_chipset_handle_host_sleep_event(host_sleep_state); + power_chipset_handle_host_sleep_event(host_sleep_state, &ctx); + switch (state) { + case HOST_SLEEP_EVENT_S3_RESUME: + case HOST_SLEEP_EVENT_S0IX_RESUME: + if (args->version >= 1) { + r->resume_response.sleep_transitions = + ctx.sleep_transitions; + + args->response_size = sizeof(*r); + } + + break; + + default: + break; + } return EC_RES_SUCCESS; } DECLARE_HOST_COMMAND(EC_CMD_HOST_SLEEP_EVENT, host_command_host_sleep_event, - EC_VER_MASK(0)); + EC_VER_MASK(0) | EC_VER_MASK(1)); enum host_sleep_event power_get_host_sleep_state(void) { return host_sleep_state; } -#ifdef CONFIG_POWER_S0IX -void power_reset_host_sleep_state(void) +void power_set_host_sleep_state(enum host_sleep_event state) { - host_sleep_state = HOST_SLEEP_EVENT_DEFAULT_RESET; - power_chipset_handle_host_sleep_event(host_sleep_state); + host_sleep_state = state; } -#endif /* CONFIG_POWER_S0IX */ #endif /* CONFIG_POWER_TRACK_HOST_SLEEP_STATE */ diff --git a/power/intel_x86.c b/power/intel_x86.c index 758f8c2ed5..4e7230d395 100644 --- a/power/intel_x86.c +++ b/power/intel_x86.c @@ -39,6 +39,7 @@ /* Console output macros */ #define CPRINTS(format, args...) cprints(CC_CHIPSET, format, ## args) +#define CPRINTF(format, args...) cprintf(CC_CHIPSET, format, ## args) enum sys_sleep_state { SYS_SLEEP_S3, @@ -224,7 +225,109 @@ static void handle_chipset_reset(void) } DECLARE_HOOK(HOOK_CHIPSET_RESET, handle_chipset_reset, HOOK_PRIO_FIRST); -#endif +#ifdef CONFIG_POWER_TRACK_HOST_SLEEP_STATE +#ifdef CONFIG_POWER_S0IX_FAILURE_DETECTION + +static uint16_t slp_s0ix_timeout; +static uint32_t slp_s0ix_transitions; + +static void s0ix_transition_timeout(void); +DECLARE_DEFERRED(s0ix_transition_timeout); + +static void s0ix_increment_transition(void) +{ + if ((slp_s0ix_transitions & EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK) < + EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK) + slp_s0ix_transitions += 1; +} + +static void s0ix_suspend_transition(void) +{ + s0ix_increment_transition(); + hook_call_deferred(&s0ix_transition_timeout_data, -1); +} + +static void s0ix_resume_transition(void) +{ + s0ix_increment_transition(); + + /* + * Start the timer again to ensure the AP doesn't get itself stuck in + * a state where it's no longer in S0ix, but from the Linux perspective + * is still suspended. Perhaps a bug in the SoC-internal periodic + * housekeeping code might result in a situation like this. + */ + if (slp_s0ix_timeout) + hook_call_deferred(&s0ix_transition_timeout_data, + (uint32_t)slp_s0ix_timeout * 1000); +} + +static void s0ix_transition_timeout(void) +{ + /* Mark the timeout. */ + slp_s0ix_transitions |= EC_HOST_RESUME_SLEEP_TIMEOUT; + hook_call_deferred(&s0ix_transition_timeout_data, -1); + + /* + * Wake up the AP so they don't just chill in a non-suspended state and + * burn power. Overload a vaguely related event bit since event bits are + * at a premium. + */ + host_set_single_event(EC_HOST_EVENT_HANG_DETECT); +} + +static void s0ix_start_suspend(struct host_sleep_event_context *ctx) +{ + uint16_t timeout = ctx->sleep_timeout_ms; + + slp_s0ix_transitions = 0; + + /* Use zero internally to indicate no timeout. */ + if (timeout == EC_HOST_SLEEP_TIMEOUT_DEFAULT) { + timeout = CONFIG_SLEEP_TIMEOUT_MS; + + } else if (timeout == EC_HOST_SLEEP_TIMEOUT_INFINITE) { + slp_s0ix_timeout = 0; + return; + } + + slp_s0ix_timeout = timeout; + hook_call_deferred(&s0ix_transition_timeout_data, + (uint32_t)timeout * 1000); +} + +static void s0ix_complete_resume(struct host_sleep_event_context *ctx) +{ + hook_call_deferred(&s0ix_transition_timeout_data, -1); + ctx->sleep_transitions = slp_s0ix_transitions; +} + +static void s0ix_reset_tracking(void) +{ + slp_s0ix_transitions = 0; + slp_s0ix_timeout = 0; +} + +#else /* !CONFIG_POWER_S0IX_FAILURE_DETECTION */ + +#define s0ix_suspend_transition() +#define s0ix_resume_transition() +#define s0ix_start_suspend(_ctx) +#define s0ix_complete_resume(_ctx) +#define s0ix_reset_tracking() + +#endif /* CONFIG_POWER_S0IX_FAILURE_DETECTION */ + +void power_reset_host_sleep_state(void) +{ + power_set_host_sleep_state(HOST_SLEEP_EVENT_DEFAULT_RESET); + s0ix_reset_tracking(); + power_chipset_handle_host_sleep_event(HOST_SLEEP_EVENT_DEFAULT_RESET, + NULL); +} + +#endif /* CONFIG_POWER_TRACK_HOST_SLEEP_STATE */ +#endif /* CONFIG_POWER_S0IX */ void chipset_throttle_cpu(int throttle) { @@ -459,11 +562,13 @@ enum power_state common_intel_x86_power_handle_state(enum power_state state) #ifdef CONFIG_POWER_S0IX case POWER_S0S0ix: + /* * Call hooks only if we haven't notified listeners of S0ix * suspend. */ s0ix_transition(S0IX_NOTIFY_SUSPEND, HOOK_CHIPSET_SUSPEND); + s0ix_suspend_transition(); /* * Enable idle task deep sleep. Allow the low power idle task @@ -479,6 +584,7 @@ enum power_state common_intel_x86_power_handle_state(enum power_state state) */ disable_sleep(SLEEP_MASK_AP_RUN); + s0ix_resume_transition(); return POWER_S0; #endif @@ -549,7 +655,9 @@ power_board_handle_host_sleep_event(enum host_sleep_event state) /* Default weak implementation -- no action required. */ } -void power_chipset_handle_host_sleep_event(enum host_sleep_event state) +void +power_chipset_handle_host_sleep_event(enum host_sleep_event state, + struct host_sleep_event_context *ctx) { power_board_handle_host_sleep_event(state); @@ -561,6 +669,8 @@ void power_chipset_handle_host_sleep_event(enum host_sleep_event state) * notification needs to be sent to listeners. */ s0ix_notify = S0IX_NOTIFY_SUSPEND; + + s0ix_start_suspend(ctx); power_signal_enable_interrupt(sleep_sig[SYS_SLEEP_S0IX]); } else if (state == HOST_SLEEP_EVENT_S0IX_RESUME) { /* @@ -574,10 +684,13 @@ void power_chipset_handle_host_sleep_event(enum host_sleep_event state) ; lpc_s0ix_resume_restore_masks(); power_signal_disable_interrupt(sleep_sig[SYS_SLEEP_S0IX]); + s0ix_complete_resume(ctx); + } else if (state == HOST_SLEEP_EVENT_DEFAULT_RESET) { power_signal_disable_interrupt(sleep_sig[SYS_SLEEP_S0IX]); } #endif + } #endif diff --git a/util/ectool.c b/util/ectool.c index 707eab71ac..42646923c5 100644 --- a/util/ectool.c +++ b/util/ectool.c @@ -509,33 +509,116 @@ int cmd_hibdelay(int argc, char *argv[]) return 0; } +static int get_latest_cmd_version(uint8_t cmd, int *version) +{ + struct ec_params_get_cmd_versions p; + struct ec_response_get_cmd_versions r; + int rv; + + *version = 0; + /* Figure out the latest version of the given command the EC supports */ + p.cmd = cmd; + rv = ec_command(EC_CMD_GET_CMD_VERSIONS, 0, &p, sizeof(p), + &r, sizeof(r)); + if (rv < 0) { + if (rv == -EC_RES_INVALID_PARAM) + printf("Command 0x%02x not supported by EC.\n", + EC_CMD_GET_CMD_VERSIONS); + return rv; + } + + if (r.version_mask) + *version = __fls(r.version_mask); + + return rv; +} + int cmd_hostsleepstate(int argc, char *argv[]) { struct ec_params_host_sleep_event p; + struct ec_params_host_sleep_event_v1 p1; + struct ec_response_host_sleep_event_v1 r; + void *pp = &p; + size_t psize = sizeof(p), rsize = 0; + char *afterscan; + int rv; + int version = 0, max_version = 0; + uint32_t timeout, transitions; if (argc < 2) { fprintf(stderr, "Usage: %s " - "[suspend|wsuspend|resume|freeze|thaw]\n", + "[suspend|wsuspend|resume|freeze|thaw] [timeout]\n", argv[0]); return -1; } + rv = get_latest_cmd_version(EC_CMD_HOST_SLEEP_EVENT, &max_version); + if (rv < 0) + return rv; + if (!strcmp(argv[1], "suspend")) p.sleep_event = HOST_SLEEP_EVENT_S3_SUSPEND; else if (!strcmp(argv[1], "wsuspend")) p.sleep_event = HOST_SLEEP_EVENT_S3_WAKEABLE_SUSPEND; else if (!strcmp(argv[1], "resume")) p.sleep_event = HOST_SLEEP_EVENT_S3_RESUME; - else if (!strcmp(argv[1], "freeze")) + else if (!strcmp(argv[1], "freeze")) { p.sleep_event = HOST_SLEEP_EVENT_S0IX_SUSPEND; - else if (!strcmp(argv[1], "thaw")) + if (max_version >= 1) { + p1.sleep_event = p.sleep_event; + p1.reserved = 0; + p1.suspend_params.sleep_timeout_ms = + EC_HOST_SLEEP_TIMEOUT_DEFAULT; + + if (argc > 2) { + p1.suspend_params.sleep_timeout_ms = + strtoul(argv[2], &afterscan, 0); + + if ((*afterscan != '\0') || + (afterscan == argv[2])) { + fprintf(stderr, + "Invalid value: %s\n", + argv[2]); + + return -1; + } + } + + pp = &p1; + psize = sizeof(p1); + version = 1; + } + + } else if (!strcmp(argv[1], "thaw")) { p.sleep_event = HOST_SLEEP_EVENT_S0IX_RESUME; - else { + if (max_version >= 1) { + version = 1; + rsize = sizeof(r); + } + } else { fprintf(stderr, "Unknown command: %s\n", argv[1]); return -1; } - return ec_command(EC_CMD_HOST_SLEEP_EVENT, 0, &p, sizeof(p), NULL, 0); + rv = ec_command(EC_CMD_HOST_SLEEP_EVENT, version, pp, psize, &r, rsize); + if (rv < 0) { + fprintf(stderr, "EC host sleep command failed: %d\n", rv); + return rv; + } + + if (rsize) { + timeout = r.resume_response.sleep_transitions & + EC_HOST_RESUME_SLEEP_TIMEOUT; + + transitions = r.resume_response.sleep_transitions & + EC_HOST_RESUME_SLEEP_TRANSITIONS_MASK; + + printf("%s%d sleep line transitions.\n", + timeout ? "Timeout: " : "", + transitions); + } + + return 0; } int cmd_test(int argc, char *argv[]) @@ -4218,10 +4301,12 @@ static int cmd_motionsense(int argc, char **argv) } if (argc == 3 && !strcasecmp(argv[1], "info")) { - struct ec_params_get_cmd_versions p; - struct ec_response_get_cmd_versions r; int version = 0; + rv = get_latest_cmd_version(EC_CMD_MOTION_SENSE_CMD, &version); + if (rv < 0) + return rv; + param.cmd = MOTIONSENSE_CMD_INFO; param.sensor_odr.sensor_num = strtol(argv[2], &e, 0); if (e && *e) { @@ -4229,20 +4314,6 @@ static int cmd_motionsense(int argc, char **argv) return -1; } - /* tool defaults to using latest version of info command */ - p.cmd = EC_CMD_MOTION_SENSE_CMD; - rv = ec_command(EC_CMD_GET_CMD_VERSIONS, 0, &p, sizeof(p), - &r, sizeof(r)); - if (rv < 0) { - if (rv == -EC_RES_INVALID_PARAM) - printf("Command 0x%02x not supported by EC.\n", - EC_CMD_GET_CMD_VERSIONS); - return rv; - } - - if (r.version_mask) - version = __fls(r.version_mask); - rv = ec_command(EC_CMD_MOTION_SENSE_CMD, version, ¶m, ms_command_sizes[param.cmd].outsize, resp, ms_command_sizes[param.cmd].insize); |