power/intel_x86: Fix S0ix suspend/resume hook notifications

There is a fundamental difference in host behavior w.r.t. S3 and
S0ix. When the host enters S3, it asserts the SLP_S3# signal until it
is woken back up. Thus, EC depends on the SLP_S3# signal state to
decide when to notify listeners about CHIPSET_SUSPEND and
CHIPSET_RESUME state.

With S0ix, SLP_S0# signal is asserted whenever host enters
S0ix. However, periodically (every 8 seconds), the host wakes up for
some bookkeeping activities, but does not come out of the low power
mode completely. This bookkeeping activity takes ~2-5 ms and the host
goes back into S0ix state. Because of this periodic activity, SLP_S0#
signal is de-asserted and asserted back every 8 seconds.

Thus, if the power state machine depends solely on the SLP_S0# signal
to notify CHIPSET_SUSPEND and CHIPSET_RESUME states, then all the
listeners would be performing unnecessary actions every 8
seconds. This leads to a number of side-effects including:
1. Dual-role toggle being enabled and disabled every 8 seconds.
2. Power spikes in EC power consumption during S0ix every 8 seconds.

In order to avoid the side-effects of periodic host activity in S0ix,
this change adds a new flag s0ix_notify, which is set based on the
notifications that are pending based on host sleep event.

On receiving host sleep event for S0ix suspend, s0ix_notify will be
set to S0IX_NOTIFY_SUSPEND. Next, whenever SLP_S0# is asserted,
power_state machine notifies listeners of CHIPSET_SUSPEND
and resets s0ix_notify flag to S0IX_NOTIFY_NONE. Thus, all future
assertions of SLP_S0# do not result in the suspend notification.

Similarly, on resume, power_state machine will not notify
CHIPSET_RESUME on SLP_S0# deassertion. Instead the host sleep event
for S0ix resume will set s0ix_notify flag to S0IX_NOTIFY_RESUME and
wake chipset task. The power state machine in turn will notify
listeners of the resume event and reset s0ix_notify flag.

BUG=b:65356050,b:67750352
BRANCH=None
TEST=Verified that the CHIPSET_SUSPEND/CHIPSET_RESUME notification
happens only once during a system suspend/resume cycle. Periodic host
wakes for book-keeping activities do not result in
CHIPSET_SUSPEND/CHIPSET_RESUME notifications.

Change-Id: Idf253b9393a0c25ff2eac63c60ddbcd3af954818
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729478
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

1 files changed, 42 insertions, 7 deletions

diff --git a/power/intel_x86.c b/power/intel_x86.c
index e26d9a1920..6e9904ba43 100644
--- a/power/intel_x86.c
+++ b/power/intel_x86.c
@@ -101,6 +101,24 @@ static enum power_state power_wait_s5_rtc_reset(void)
 #endif
 
 #ifdef CONFIG_POWER_S0IX
+
+enum s0ix_notify_type {
+	S0IX_NOTIFY_NONE,
+	S0IX_NOTIFY_SUSPEND,
+	S0IX_NOTIFY_RESUME,
+};
+
+/* Flag to notify listeners about S0ix suspend/resume events. */
+enum s0ix_notify_type s0ix_notify = S0IX_NOTIFY_NONE;
+
+static void s0ix_transition(int check_state, int hook_id)
+{
+	if (s0ix_notify != check_state)
+		return;
+	hook_notify(hook_id);
+	s0ix_notify = S0IX_NOTIFY_NONE;
+}
+
 /*
  * In AP S0 -> S3 & S0ix transitions,
  * the chipset_suspend is called.
@@ -151,6 +169,7 @@ static void handle_chipset_reset(void)
 	}
 }
 DECLARE_HOOK(HOOK_CHIPSET_RESET, handle_chipset_reset, HOOK_PRIO_FIRST);
+
 #endif
 
 void chipset_throttle_cpu(int throttle)
@@ -231,6 +250,9 @@ enum power_state common_intel_x86_power_handle_state(enum power_state state)
 					== HOST_SLEEP_EVENT_S0IX_SUSPEND &&
 				chipset_get_sleep_signal(SYS_SLEEP_S0IX) == 0) {
 			return POWER_S0S0ix;
+		} else {
+			s0ix_transition(S0IX_NOTIFY_RESUME,
+					HOOK_CHIPSET_RESUME);
 #endif
 		}
 
@@ -360,12 +382,16 @@ enum power_state common_intel_x86_power_handle_state(enum power_state state)
 
 #ifdef CONFIG_POWER_S0IX
 	case POWER_S0S0ix:
-		/* call hooks before standby */
-		hook_notify(HOOK_CHIPSET_SUSPEND);
 
 		s0ix_lpc_enable_wake_mask();
 
 		/*
+		 * Call hooks only if we haven't notified listeners of S0ix
+		 * suspend.
+		 */
+		s0ix_transition(S0IX_NOTIFY_SUSPEND, HOOK_CHIPSET_SUSPEND);
+
+		/*
 		 * Enable idle task deep sleep. Allow the low power idle task
 		 * to go into deep sleep in S0ix.
 		 */
@@ -377,9 +403,6 @@ enum power_state common_intel_x86_power_handle_state(enum power_state state)
 	case POWER_S0ixS0:
 		s0ix_lpc_disable_wake_mask();
 
-		/* Call hooks now that rails are up */
-		hook_notify(HOOK_CHIPSET_RESUME);
-
 		/*
 		 * Disable idle task deep sleep. This means that the low
 		 * power idle task will not go into deep sleep while in S0.
@@ -461,9 +484,21 @@ void power_chipset_handle_host_sleep_event(enum host_sleep_event state)
 	power_board_handle_host_sleep_event(state);
 
 #ifdef CONFIG_POWER_S0IX
-	if (state == HOST_SLEEP_EVENT_S0IX_SUSPEND)
+	if (state == HOST_SLEEP_EVENT_S0IX_SUSPEND) {
+		/*
+		 * Indicate to power state machine that a new host event for
+		 * s0ix suspend has been received and so chipset suspend
+		 * notification needs to be sent to listeners.
+		 */
+		s0ix_notify = S0IX_NOTIFY_SUSPEND;
 		power_signal_enable_interrupt(sleep_sig[SYS_SLEEP_S0IX]);
-	else if (state == HOST_SLEEP_EVENT_S0IX_RESUME) {
+	} else if (state == HOST_SLEEP_EVENT_S0IX_RESUME) {
+		/*
+		 * Wake up chipset task and indicate to power state machine that
+		 * listeners need to be notified of chipset resume.
+		 */
+		s0ix_notify = S0IX_NOTIFY_RESUME;
+		task_wake(TASK_ID_CHIPSET);
 		/* clear host events */
 		while (lpc_get_next_host_event() != 0)
 			;