From 953bbeb7de60bce65039962c421ce6f7b287adba Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Wed, 28 Mar 2018 00:31:57 +0200 Subject: [PATCH 1/4] Update idle loop to reduce calls to suspend Change tickless handling to use public RTX calls and to prevent unnecessary calls to suspend/resume by looping until the kernel needs to be resumed. --- rtos/TARGET_CORTEX/SysTimer.cpp | 73 +++++++++++++++++++++++----- rtos/TARGET_CORTEX/SysTimer.h | 45 ++++++++++++----- rtos/TARGET_CORTEX/mbed_rtx_idle.cpp | 31 ++++++------ 3 files changed, 109 insertions(+), 40 deletions(-) diff --git a/rtos/TARGET_CORTEX/SysTimer.cpp b/rtos/TARGET_CORTEX/SysTimer.cpp index fc2e06bc363..632a1314cf2 100644 --- a/rtos/TARGET_CORTEX/SysTimer.cpp +++ b/rtos/TARGET_CORTEX/SysTimer.cpp @@ -24,6 +24,7 @@ #if DEVICE_LOWPOWERTIMER #include "hal/lp_ticker_api.h" +#include "mbed_critical.h" #include "rtx_core_cm.h" extern "C" { #include "rtx_lib.h" @@ -45,6 +46,8 @@ SysTimer::SysTimer() : TimerEvent(get_lp_ticker_data()), _start_time(0), _tick(0) { _start_time = ticker_read_us(_ticker_data); + _suspend_time_passed = true; + _suspended = false; } void SysTimer::setup_irq() @@ -61,23 +64,30 @@ void SysTimer::setup_irq() #endif } -void SysTimer::schedule_tick(uint32_t delta) +void SysTimer::suspend(uint32_t ticks) { - insert_absolute(_start_time + (_tick + delta) * 1000000ULL / OS_TICK_FREQ); -} + core_util_critical_section_enter(); -void SysTimer::cancel_tick() -{ - remove(); + schedule_tick(ticks); + _suspend_time_passed = false; + _suspended = true; + + core_util_critical_section_exit(); } -uint32_t SysTimer::get_tick() +bool SysTimer::suspend_time_passed() { - return _tick & 0xFFFFFFFF; + return _suspend_time_passed; } -uint32_t SysTimer::update_tick() +uint32_t SysTimer::resume() { + core_util_critical_section_enter(); + + _suspended = false; + _suspend_time_passed = true; + remove(); + uint64_t new_tick = (ticker_read_us(_ticker_data) - _start_time) * OS_TICK_FREQ / 1000000; if (new_tick > _tick) { // Don't update to the current tick. Instead, update to the @@ -88,9 +98,34 @@ uint32_t SysTimer::update_tick() } uint32_t elapsed_ticks = new_tick - _tick; _tick = new_tick; + + core_util_critical_section_exit(); return elapsed_ticks; } +void SysTimer::schedule_tick(uint32_t delta) +{ + core_util_critical_section_enter(); + + insert_absolute(_start_time + (_tick + delta) * 1000000ULL / OS_TICK_FREQ); + + core_util_critical_section_exit(); +} + +void SysTimer::cancel_tick() +{ + core_util_critical_section_enter(); + + remove(); + + core_util_critical_section_exit(); +} + +uint32_t SysTimer::get_tick() +{ + return _tick & 0xFFFFFFFF; +} + us_timestamp_t SysTimer::get_time() { return ticker_read_us(_ticker_data); @@ -100,8 +135,10 @@ SysTimer::~SysTimer() { } -void SysTimer::set_irq_pending() +void SysTimer::_set_irq_pending() { + // Protected function synchronized externally + #if (defined(NO_SYSTICK)) NVIC_SetPendingIRQ(mbed_get_m0_tick_irqn()); #else @@ -109,15 +146,25 @@ void SysTimer::set_irq_pending() #endif } -void SysTimer::increment_tick() +void SysTimer::_increment_tick() { + // Protected function synchronized externally + _tick++; } void SysTimer::handler() { - set_irq_pending(); - increment_tick(); + core_util_critical_section_enter(); + + if (_suspended) { + _suspend_time_passed = true; + } else { + _set_irq_pending(); + _increment_tick(); + } + + core_util_critical_section_exit(); } } diff --git a/rtos/TARGET_CORTEX/SysTimer.h b/rtos/TARGET_CORTEX/SysTimer.h index 2af7c8ec33f..e43668405fb 100644 --- a/rtos/TARGET_CORTEX/SysTimer.h +++ b/rtos/TARGET_CORTEX/SysTimer.h @@ -56,6 +56,34 @@ class SysTimer: private mbed::TimerEvent, private mbed::NonCopyable { */ static void setup_irq(); + /** + * Set wakeup time and schedule a wakeup event after delta ticks + * + * After suspend has been called the function suspend_time_passed + * can be used to determine if the suspend time has passed. + * + * @param delta Ticks to remain suspended + */ + void suspend(uint32_t delta); + + /** + * Check if the suspend time has passed + * + * @return true if the specified number of ticks has passed otherwise false + */ + bool suspend_time_passed(); + + /** + * Exit suspend mode and return elapsed ticks + * + * Due to a scheduling issue, the number of ticks returned is decremented + * by 1 so that a handler can be called and update to the current value. + * This allows scheduling restart successfully after the OS is resumed. + * + * @return the number of elapsed ticks minus 1 + */ + uint32_t resume(); + /** * Schedule an os tick to fire * @@ -77,17 +105,6 @@ class SysTimer: private mbed::TimerEvent, private mbed::NonCopyable { */ uint32_t get_tick(); - /** - * Update the internal tick count - * - * @return The number of ticks incremented - * - * @note Due to a scheduling issue, the number of ticks returned is decremented - * by 1 so that a handler can be called and update to the current value. - * This allows scheduling restart successfully after the OS is resumed. - */ - uint32_t update_tick(); - /** * Get the time * @@ -97,10 +114,12 @@ class SysTimer: private mbed::TimerEvent, private mbed::NonCopyable { protected: virtual void handler(); - void increment_tick(); - static void set_irq_pending(); + void _increment_tick(); + static void _set_irq_pending(); us_timestamp_t _start_time; uint64_t _tick; + bool _suspend_time_passed; + bool _suspended; }; /** diff --git a/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp b/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp index 6e568e415c0..a4a003fa9bf 100644 --- a/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp +++ b/rtos/TARGET_CORTEX/mbed_rtx_idle.cpp @@ -95,21 +95,24 @@ uint32_t OS_Tick_GetInterval (void) { static void default_idle_hook(void) { - uint32_t elapsed_ticks = 0; - - core_util_critical_section_enter(); - uint32_t ticks_to_sleep = svcRtxKernelSuspend(); - if (ticks_to_sleep) { - os_timer->schedule_tick(ticks_to_sleep); - - sleep(); - - os_timer->cancel_tick(); - // calculate how long we slept - elapsed_ticks = os_timer->update_tick(); + uint32_t ticks_to_sleep = osKernelSuspend(); + os_timer->suspend(ticks_to_sleep); + + bool event_pending = false; + while (!os_timer->suspend_time_passed() && !event_pending) { + + core_util_critical_section_enter(); + if (osRtxInfo.kernel.pendSV) { + event_pending = true; + } else { + sleep(); + } + core_util_critical_section_exit(); + + // Ensure interrupts get a chance to fire + __ISB(); } - svcRtxKernelResume(elapsed_ticks); - core_util_critical_section_exit(); + osKernelResume(os_timer->resume()); } #elif defined(FEATURE_UVISOR) From 044eb1e8ce718eecf4eb5c5e1b61df3f77f73de9 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Tue, 10 Apr 2018 10:54:31 -0500 Subject: [PATCH 2/4] Update the SysTimer test Update the SysTimer test to match the updated API. Changes are: - increment_tick() renamed to _increment_tick() and explicitly synchronized -update_tick() replaced with resume() and a call to suspend() was added before this --- TESTS/mbedmicro-rtos-mbed/systimer/main.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp index 51a85cea963..4a732341960 100644 --- a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp +++ b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp @@ -44,7 +44,9 @@ class SysTimerTest: public rtos::internal::SysTimer { Semaphore _sem; virtual void handler() { - increment_tick(); + core_util_critical_section_enter(); + _increment_tick(); + core_util_critical_section_exit(); _sem.release(); } @@ -79,26 +81,29 @@ void test_created_with_zero_tick_count(void) /** Test tick count is updated correctly * * Given a SysTimer - * When @a update_tick method is called immediately after creation + * When the @a suspend and @a resume methods are called immediately after creation * Then the tick count is not updated - * When @a update_tick is called again after a delay + * When @a suspend and @a resume methods are called again after a delay * Then the tick count is updated * and the number of ticks incremented is equal TEST_TICKS - 1 - * When @a update_tick is called again without a delay + * When @a suspend and @a resume methods are called again without a delay * Then the tick count is not updated */ void test_update_tick(void) { SysTimerTest st; - TEST_ASSERT_EQUAL_UINT32(0, st.update_tick()); + st.suspend(TEST_TICKS * 2); + TEST_ASSERT_EQUAL_UINT32(0, st.resume()); TEST_ASSERT_EQUAL_UINT32(0, st.get_tick()); us_timestamp_t test_ticks_elapsed_ts = st.get_time() + DELAY_US; + st.suspend(TEST_TICKS * 2); while (st.get_time() <= test_ticks_elapsed_ts) {} - TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.update_tick()); + TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.resume()); TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.get_tick()); - TEST_ASSERT_EQUAL_UINT32(0, st.update_tick()); + st.suspend(TEST_TICKS * 2); + TEST_ASSERT_EQUAL_UINT32(0, st.resume()); TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.get_tick()); } From 20013d75d6b09fc2fb64b1d11dc1ec7bf3f4ed53 Mon Sep 17 00:00:00 2001 From: Filip Jagodzinski Date: Thu, 12 Apr 2018 15:22:24 +0200 Subject: [PATCH 3/4] RTOS: SysTimer: Update tests Use a mock ticker object instead of the lp_ticker for update_tick() and get_time() tests. --- TESTS/mbedmicro-rtos-mbed/systimer/main.cpp | 78 +++++++++++++++++++-- rtos/TARGET_CORTEX/SysTimer.cpp | 6 ++ rtos/TARGET_CORTEX/SysTimer.h | 1 + 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp index 4a732341960..edc71e2d41b 100644 --- a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp +++ b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp @@ -25,6 +25,7 @@ #include "greentea-client/test_env.h" #include "unity.h" #include "utest.h" +#include "ticker_api.h" extern "C" { #include "rtx_lib.h" @@ -56,6 +57,11 @@ class SysTimerTest: public rtos::internal::SysTimer { { } + SysTimerTest(const ticker_data_t *data) : + SysTimer(data), _sem(0, 1) + { + } + virtual ~SysTimerTest() { } @@ -66,6 +72,65 @@ class SysTimerTest: public rtos::internal::SysTimer { } }; +timestamp_t mock_ticker_timestamp; + +void mock_ticker_init() +{ +} + +uint32_t mock_ticker_read() +{ + return mock_ticker_timestamp; +} + +void mock_ticker_disable_interrupt() +{ +} + +void mock_ticker_clear_interrupt() +{ +} + +void mock_ticker_set_interrupt(timestamp_t timestamp) +{ +} + +void mock_ticker_fire_interrupt() +{ +} + +const ticker_info_t *mock_ticker_get_info() +{ + static const ticker_info_t mock_ticker_info = { + .frequency = 1000000, + .bits = 32 + }; + return &mock_ticker_info; +} + +ticker_interface_t mock_ticker_interface = { + .init = mock_ticker_init, + .read = mock_ticker_read, + .disable_interrupt = mock_ticker_disable_interrupt, + .clear_interrupt = mock_ticker_clear_interrupt, + .set_interrupt = mock_ticker_set_interrupt, + .fire_interrupt = mock_ticker_fire_interrupt, + .get_info = mock_ticker_get_info, +}; + +ticker_event_queue_t mock_ticker_event_queue; + +const ticker_data_t mock_ticker_data = { + .interface = &mock_ticker_interface, + .queue = &mock_ticker_event_queue +}; + +void mock_ticker_reset() +{ + mock_ticker_timestamp = 0; + mock_ticker_event_queue = {0}; +} + /** Test tick count is zero upon creation * * Given a SysTimer @@ -91,14 +156,14 @@ void test_created_with_zero_tick_count(void) */ void test_update_tick(void) { - SysTimerTest st; + mock_ticker_reset(); + SysTimerTest st(&mock_ticker_data); st.suspend(TEST_TICKS * 2); TEST_ASSERT_EQUAL_UINT32(0, st.resume()); TEST_ASSERT_EQUAL_UINT32(0, st.get_tick()); - us_timestamp_t test_ticks_elapsed_ts = st.get_time() + DELAY_US; st.suspend(TEST_TICKS * 2); - while (st.get_time() <= test_ticks_elapsed_ts) {} + mock_ticker_timestamp = DELAY_US; TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.resume()); TEST_ASSERT_EQUAL_UINT32(TEST_TICKS - 1, st.get_tick()); @@ -115,12 +180,13 @@ void test_update_tick(void) */ void test_get_time(void) { - SysTimerTest st; + mock_ticker_reset(); + SysTimerTest st(&mock_ticker_data); us_timestamp_t t1 = st.get_time(); - wait_us(DELAY_US); + mock_ticker_timestamp = DELAY_US; us_timestamp_t t2 = st.get_time(); - TEST_ASSERT_UINT64_WITHIN(DELAY_DELTA_US, DELAY_US, t2 - t1); + TEST_ASSERT_EQUAL_UINT64(DELAY_US, t2 - t1); } /** Test cancel_tick diff --git a/rtos/TARGET_CORTEX/SysTimer.cpp b/rtos/TARGET_CORTEX/SysTimer.cpp index 632a1314cf2..1eece6c501a 100644 --- a/rtos/TARGET_CORTEX/SysTimer.cpp +++ b/rtos/TARGET_CORTEX/SysTimer.cpp @@ -50,6 +50,12 @@ SysTimer::SysTimer() : _suspended = false; } +SysTimer::SysTimer(const ticker_data_t *data) : + TimerEvent(data), _start_time(0), _tick(0) +{ + _start_time = ticker_read_us(_ticker_data); +} + void SysTimer::setup_irq() { #if (defined(NO_SYSTICK)) diff --git a/rtos/TARGET_CORTEX/SysTimer.h b/rtos/TARGET_CORTEX/SysTimer.h index e43668405fb..267714c9077 100644 --- a/rtos/TARGET_CORTEX/SysTimer.h +++ b/rtos/TARGET_CORTEX/SysTimer.h @@ -49,6 +49,7 @@ class SysTimer: private mbed::TimerEvent, private mbed::NonCopyable { public: SysTimer(); + SysTimer(const ticker_data_t *data); virtual ~SysTimer(); /** From ec59cbb6f83c4349f33c1981760a62115d2d838b Mon Sep 17 00:00:00 2001 From: Filip Jagodzinski Date: Tue, 17 Apr 2018 10:54:22 +0200 Subject: [PATCH 4/4] RTOS: SysTimer: Fix test compilation error on ARM & IAR --- TESTS/mbedmicro-rtos-mbed/systimer/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp index edc71e2d41b..68d27977da3 100644 --- a/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp +++ b/TESTS/mbedmicro-rtos-mbed/systimer/main.cpp @@ -128,7 +128,7 @@ const ticker_data_t mock_ticker_data = { void mock_ticker_reset() { mock_ticker_timestamp = 0; - mock_ticker_event_queue = {0}; + memset(&mock_ticker_event_queue, 0, sizeof mock_ticker_event_queue); } /** Test tick count is zero upon creation