From 7ef286fc30235c93342626b9dd854e2178f55ad7 Mon Sep 17 00:00:00 2001 From: Youngjae Cho Date: Wed, 8 Feb 2023 18:17:17 +0900 Subject: [PATCH] power: separate action_for_state() callback Separated the callback action_for_state() into two type - pre_action_state() - post_action_state() Literally, it only differs when the time it is invoked based on completing transition, one is before and the other is after finishing transition. It is necessary for echo mem. Previously, the action_for_state() always invoked 'right before' transition completion. As it is echo mem, system is forced to go suspend within the action_for_state() callback. After resuming from the suspend, the processing thread still be in the action_for_state(), which is still not having completed sleep transition. As a result, a system is waking up but the internal power state is about to be changed to sleep state. This is contradiction and causes various undesirable behavior. To address it, move echo mem to post_action_state(). Unlike the previous action_for_state(), the post_action_state() is invoked 'after' the completion of transition. That is, it is natural that waking up within the post_action_state() as it already has been finished sleep transition. Thus it won't change the internal state to sleep anymore. Change-Id: I49e12a19538bdb4241cba34fd536c942257ccaec Signed-off-by: Youngjae Cho --- src/power/power-suspend.c | 2 +- src/power/power.c | 97 +++++++++++++++++++++++++++++------------------ 2 files changed, 61 insertions(+), 38 deletions(-) diff --git a/src/power/power-suspend.c b/src/power/power-suspend.c index d475305..ff4b606 100644 --- a/src/power/power-suspend.c +++ b/src/power/power-suspend.c @@ -381,7 +381,6 @@ static void suspend_echo_mem(void) update_wakeup_reason(); device_notify(DEVICE_NOTIFIER_POWER_RESUME_FROM_ECHO_MEM, NULL); - power_request_change_state(DEVICED_POWER_STATE_NORMAL, (int) wakeup_reason); /* * FIXME: If the wakeup reason lingers after wakeup, a transition triggered from other * than the above line would take the lingering wakeup reason as a transition reason, @@ -391,6 +390,7 @@ static void suspend_echo_mem(void) * the transition is requested, not the time when it is executed. If then, the below resetting * line can be removed. */ + power_request_change_state_strict(DEVICED_POWER_STATE_SLEEP, DEVICED_POWER_STATE_NORMAL, (int) wakeup_reason, NULL); wakeup_reason = HAL_DEVICE_POWER_TRANSITION_REASON_UNKNOWN; } diff --git a/src/power/power.c b/src/power/power.c index 906ad61..8bd0b28 100644 --- a/src/power/power.c +++ b/src/power/power.c @@ -105,17 +105,25 @@ static void prepare_transition(const struct trans_info *ti); static void action_transition(void); static uint64_t get_next_state(void); -static void action_for_normal(void *data); -static void action_for_sleep(void *data); -static void action_for_poweroff(void *data); -static void (*const action_for_state[DEVICED_POWER_STATE_MAX_INDEX]) (void *) = { - [DEVICED_POWER_STATE_NORMAL_INDEX] = action_for_normal, - [DEVICED_POWER_STATE_SLEEP_INDEX] = action_for_sleep, - [DEVICED_POWER_STATE_POWEROFF_INDEX] = action_for_poweroff, - [DEVICED_POWER_STATE_REBOOT_INDEX] = action_for_poweroff, - [DEVICED_POWER_STATE_EXIT_INDEX] = action_for_poweroff, +static void pre_action_normal(void *data); +static void pre_action_poweroff(void *data); +static void (*const pre_action_state[DEVICED_POWER_STATE_MAX_INDEX]) (void *) = { + [DEVICED_POWER_STATE_NORMAL_INDEX] = pre_action_normal, + [DEVICED_POWER_STATE_POWEROFF_INDEX] = pre_action_poweroff, + [DEVICED_POWER_STATE_REBOOT_INDEX] = pre_action_poweroff, + [DEVICED_POWER_STATE_EXIT_INDEX] = pre_action_poweroff, }; +static void post_action_sleep(void *data); +static void post_action_poweroff(void *data); +static void (*const post_action_state[DEVICED_POWER_STATE_MAX_INDEX]) (void *) = { + [DEVICED_POWER_STATE_SLEEP_INDEX] = post_action_sleep, + [DEVICED_POWER_STATE_POWEROFF_INDEX] = post_action_poweroff, + [DEVICED_POWER_STATE_REBOOT_INDEX] = post_action_poweroff, + [DEVICED_POWER_STATE_EXIT_INDEX] = post_action_poweroff, +}; + + uint64_t power_get_state(void) { return current; @@ -281,22 +289,25 @@ static uint64_t alloc_unique_id(void) return ++id; } -static void action_for_normal(void *udata) +static void pre_action_normal(void *udata) { power_resume(transition_context.ti.reason); } -static void action_for_sleep(void *udata) +static void post_action_sleep(void *udata) { power_suspend(transition_context.ti.reason); } -static void action_for_poweroff(void *data) +static void pre_action_poweroff(void *data) { // do not transition anymore after poweroff unregister_notifier(DEVICE_NOTIFIER_REQUEST_TRANSITION_STATE, transition_request_callback); - poweroff_prepare(current); +} + +static void post_action_poweroff(void *data) +{ poweroff_main((void *)(intptr_t) current); } @@ -621,36 +632,48 @@ static void cancel_transition(void) static void action_transition(void) { uint64_t state = get_next_state(); + struct trans_info ti = { 0 , }; + int retval; + int index; transition_description(0); - if (state == transition_context.ti.next) { - // it has reached the destination state - struct trans_info ti = { 0 , }; - int retval; - int index; - - index = DEVICED_POWER_STATE_INDEX(transition_context.ti.next); - if (transition_context.cancel) - cancel_transition(); - else if (action_for_state[index]) - action_for_state[index](transition_context.ti.data); - - // transition end - transition_context.ongoing = 0; - current = transition_context.ti.next; - event_wake_unlock(EL_POWER_TRANSITION_STATE); - - // trigger next transition if exist - retval = dequeue_transition(&ti); - if (retval == 0) { - prepare_transition(&ti); - trigger_transition(); - } - } else { + if (state != transition_context.ti.next) { // step into the next transient state ++transition_context.transient_step; trigger_transition(); + return; + } + + /* it reached the destination state */ + index = DEVICED_POWER_STATE_INDEX(transition_context.ti.next); + if (transition_context.cancel) { + cancel_transition(); + goto trigger_next; + } + + if (pre_action_state[index]) + pre_action_state[index](transition_context.ti.data); + + current = transition_context.ti.next; + + if (post_action_state[index]) + post_action_state[index](transition_context.ti.data); + + /** + * transition_context.ongiong must be reset after the post_action_state(). + * This prevents the post_action_state() from triggering another transition + * that might have been requested during its execution. Transition will be + * enqueued, not executed, if transition_context.ongoing is 1. + */ + transition_context.ongoing = 0; + event_wake_unlock(EL_POWER_TRANSITION_STATE); + +trigger_next: + retval = dequeue_transition(&ti); + if (retval == 0) { + prepare_transition(&ti); + trigger_transition(); } } -- 2.7.4