8000 #1812 improvements to best_effort_wfe_or_timeout · raspberrypi/pico-sdk@f62f242 · GitHub
[go: up one dir, main page]
Skip to content

Commit f62f242

Browse files
kilograhamkilograham
committed
#1812 improvements to best_effort_wfe_or_timeout
1 parent f4a691a commit f62f242

File tree

5 files changed

+46
-18
lines changed

5 files changed

+46
-18
lines changed

‎src/common/pico_time/include/pico/time.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,8 @@ void sleep_ms(uint32_t ms);
284284
* return false; // timed out
285285
* }
286286
* ```
287+
* NOTE: This method should always be used in a loop associated with checking another "event" variable, since
288+
* processor events are a shared resource and can happen for a large number of reasons.
287289
*
288290
* @param timeout_timestamp the timeout time
289291
* @return true if the target time is reached, false otherwise

‎src/common/pico_time/time.c

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,12 @@ static void alarm_pool_irq_handler(void) {
265265
// need to wait
266266
alarm_pool_entry_t *earliest_entry = &pool->entries[earliest_index];
267267
earliest_target = earliest_entry->target;
268-
ta_set_timeout(timer, timer_alarm_num, earliest_target);
268+
// we are leaving a timeout every 2^32 microseconds anyway if there is no valid target, so we can choose any value.
269+
// best_effort_wfe_or_timeout now relies on it being the last value set, and arguably this is the
270+
// best value anyway, as it is the furthest away from the last fire.
271+
if (earliest_target != -1) {
272+
ta_set_timeout(timer, timer_alarm_num, earliest_target);
273+
}
269274
// check we haven't now past the target time; if not we don't want to loop again
270275
} while ((earliest_target - now) <= 0);
271276
}
@@ -439,26 +444,35 @@ bool best_effort_wfe_or_timeout(absolute_time_t timeout_timestamp) {
439444
return time_reached(timeout_timestamp);
440445
} else {
441446
alarm_id_t id;
442-
id = add_alarm_at(timeout_timestamp, sleep_until_callback, NULL, false);
443-
if (id <= 0) {
444-
tight_loop_contents();
447+
// note that as of SDK2.0.0 calling add_alarm_at always causes a SEV. Whwat we really
448+
// want to do is cause an IRQ at the specified time in the future if there is not
449+
// an IRQ already happening before then. The problem is that the IRQ may be happening on the
450+
// other core, so taking an IRQ is the only way to get the state protection.
451+
//
452+
// Therefore, we make a compromise; we will set the alarm, if we won't wake up before the right time
453+
// already. This means that repeated calls to this function with the same timeout will work correctly
454+
// after the first one! This is fine, because we ask callers to use a polling loop on another
455+
// event variable when using this function
456+
if (ta_wakes_up_on_or_before(alarm_pool_get_default()->timer, alarm_pool_get_default()->timer_alarm_num,
457+
(int64_t)to_us_since_boot(timeout_timestamp))) {
458+
__wfe();
445459
return time_reached(timeout_timestamp);
446460
} else {
447-
// the above alarm add now may force an IRQ which will wake us up,
448-
// so we want to consume one __wfe.. we do an explicit __sev
449-
// just to make sure there is one
450-
__sev(); // make sure there is an event sow ee don't block
451-
__wfe();
452-
if (!time_reached(timeout_timestamp))
453-
{
454-
// ^ at the point above the timer hadn't fired, so it is safe
455-
// to wait; the event will happen due to IRQ at some point between
456-
// then and the correct wakeup time
457-
__wfe();
461+
id = add_alarm_at(timeout_timestamp, sleep_until_callback, NULL, false);
462+
if (id <= 0) {
463+
tight_loop_contents();
464+
return time_reached(timeout_timestamp);
465+
} else {
466+
if (!time_reached(timeout_timestamp)) {
467+
// ^ at the point above the timer hadn't fired, so it is safe
468+
// to wait; the event will happen due to IRQ at some point between
469+
// then and the correct wakeup time
470+
__wfe();
471+
}
472+
// we need to clean up if it wasn't us that caused the wfe; if it was this will be a noop.
473+
cancel_alarm(id);
474+
return time_reached(timeout_timestamp);
458475
}
459-
// we need to clean up if it wasn't us that caused the wfe; if it was this will be a noop.
460-
cancel_alarm(id);
461-
return time_reached(timeout_timestamp);
462476
}
463477
}
464478
#else

‎src/host/pico_time_adapter/include/pico/time_adapter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ void ta_clear_force_irq(alarm_pool_timer_t *timer, uint hardware_alarm_num);
1919
void ta_clear_irq(alarm_pool_timer_t *timer, uint hardware_alarm_num);
2020
void ta_force_irq(alarm_pool_timer_t *timer, uint hardware_alarm_num);
2121
void ta_set_timeout(alarm_pool_timer_t *timer, uint hardware_alarm_num, int64_t target);
22+
void ta_wakes_up_on_or_before(alarm_pool_timer_t *timer, uint alarm_num, int64_t target);
2223
void ta_enable_irq_handler(alarm_pool_timer_t *timer, uint hardware_alarm_num, void (*irq_handler)(void));
2324
void ta_disable_irq_handler(alarm_pool_timer_t *timer, uint hardware_alarm_num, void (*irq_handler)(void));
2425
void ta_hardware_alarm_claim(alarm_pool_timer_t *timer, uint hardware_alarm_num);

‎src/host/pico_time_adapter/time_adapter.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ PICO_WEAK_FUNCTION_DEF(ta_set_timeout)
2727
void PICO_WEAK_FUNCTION_IMPL_NAME(ta_set_timeout)(alarm_pool_timer_t *timer, uint hardware_alarm_num, int64_t target) {
2828
panic_unsupported();
2929
}
30+
PICO_WEAK_FUNCTION_DEF(ta_wakes_up_on_or_before)
31+
void PICO_WEAK_FUNCTION_IMPL_NAME(ta_wakes_up_on_or_before)(alarm_pool_timer_t *timer, uint hardware_alarm_num, int64_t target) {
32+
panic_unsupported();
33+
}
3034
PICO_WEAK_FUNCTION_DEF(ta_enable_irq_handler)
3135
void PICO_WEAK_FUNCTION_IMPL_NAME(ta_enable_irq_handler)(alarm_pool_timer_t *timer, uint hardware_alarm_num, void (*irq_handler)(void)) {
3236
panic_unsupported();

‎src/rp2_common/pico_time_adapter/include/pico/time_adapter.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ static inline void ta_set_timeout(alarm_pool_timer_t *timer, uint alarm_num, int
3939
timer_hw_from_timer(timer)->alarm[alarm_num] = (uint32_t) target;
4040
}
4141

42+
static inline bool ta_wakes_up_on_or_before(alarm_pool_timer_t *timer, uint alarm_num, int64_t target) {
43+
uint32_t current = timer_time_us_32(timer_hw_from_timer(timer));
44+
uint32_t time_til_target = (uint32_t) target - current;
45+
uint32_t time_til_alarm = timer_hw_from_timer(timer)->alarm[alarm_num] - current;
46+
return time_til_alarm <= time_til_target;
47+
}
48+
4249
static inline uint64_t ta_time_us_64(alarm_pool_timer_t *timer) {
4350
return timer_time_us_64(timer_hw_from_timer(timer));
4451
}

0 commit comments

Comments
 (0)
0