diff --git a/components/esp_pm/linker.lf b/components/esp_pm/linker.lf index 0b4c6636e3..381faa4b3b 100644 --- a/components/esp_pm/linker.lf +++ b/components/esp_pm/linker.lf @@ -47,13 +47,11 @@ entries: archive: libesp_timer.a entries: if PM_SLP_IRAM_OPT = y: + esp_timer_impl_common:esp_timer_impl_lock (noflash) + esp_timer_impl_common:esp_timer_impl_unlock (noflash) if ESP_TIMER_IMPL_TG0_LAC = y: - esp_timer_impl_lac:esp_timer_impl_lock (noflash) - esp_timer_impl_lac:esp_timer_impl_unlock (noflash) esp_timer_impl_lac:esp_timer_impl_advance (noflash) elif ESP_TIMER_IMPL_SYSTIMER = y: - esp_timer_impl_systimer:esp_timer_impl_lock (noflash) - esp_timer_impl_systimer:esp_timer_impl_unlock (noflash) esp_timer_impl_systimer:esp_timer_impl_advance (noflash) [mapping:newlib_pm] diff --git a/components/esp_timer/CMakeLists.txt b/components/esp_timer/CMakeLists.txt index 02a93c13dc..f2a1816840 100644 --- a/components/esp_timer/CMakeLists.txt +++ b/components/esp_timer/CMakeLists.txt @@ -2,7 +2,8 @@ idf_build_get_property(target IDF_TARGET) set(srcs "src/esp_timer.c" "src/ets_timer_legacy.c" - "src/system_time.c") + "src/system_time.c" + "src/esp_timer_impl_common.c") if(CONFIG_ESP_TIMER_IMPL_TG0_LAC) list(APPEND srcs "src/esp_timer_impl_lac.c") diff --git a/components/esp_timer/private_include/esp_timer_impl.h b/components/esp_timer/private_include/esp_timer_impl.h index d59a13eb13..3977d6719f 100644 --- a/components/esp_timer/private_include/esp_timer_impl.h +++ b/components/esp_timer/private_include/esp_timer_impl.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -142,3 +142,12 @@ uint64_t esp_timer_impl_get_alarm_reg(void); */ void esp_timer_impl_init_system_time(void); #endif + +#if CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD +/** + * @brief Set the next alarm if there is such an alarm in the cached array. + * + * @note Available only when CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD is enabled. + */ +void esp_timer_impl_try_to_set_next_alarm(void); +#endif diff --git a/components/esp_timer/src/esp_timer.c b/components/esp_timer/src/esp_timer.c index 41d7fa34b2..29190f9f82 100644 --- a/components/esp_timer/src/esp_timer.c +++ b/components/esp_timer/src/esp_timer.c @@ -489,6 +489,7 @@ static void IRAM_ATTR timer_alarm_handler(void* arg) bool isr_timers_processed = false; #ifdef CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD + esp_timer_impl_try_to_set_next_alarm(); // process timers with ISR dispatch method isr_timers_processed = timer_process_alarm(ESP_TIMER_ISR); xHigherPriorityTaskWoken = s_isr_dispatch_need_yield; diff --git a/components/esp_timer/src/esp_timer_impl_common.c b/components/esp_timer/src/esp_timer_impl_common.c new file mode 100644 index 0000000000..d0c86f685d --- /dev/null +++ b/components/esp_timer/src/esp_timer_impl_common.c @@ -0,0 +1,70 @@ +/* + * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "esp_timer_impl.h" +#include "esp_timer.h" +#include "esp_err.h" +#include "esp_task.h" +#include "esp_attr.h" + +/* Spinlock used to protect access to the hardware registers. */ +portMUX_TYPE s_time_update_lock = portMUX_INITIALIZER_UNLOCKED; + +/* Alarm values to generate interrupt on match + * [0] - for ESP_TIMER_TASK alarms, + * [1] - for ESP_TIMER_ISR alarms. +*/ +uint64_t timestamp_id[2] = { UINT64_MAX, UINT64_MAX }; + +void esp_timer_impl_lock(void) +{ + portENTER_CRITICAL(&s_time_update_lock); +} + +void esp_timer_impl_unlock(void) +{ + portEXIT_CRITICAL(&s_time_update_lock); +} + +void esp_timer_private_lock(void) __attribute__((alias("esp_timer_impl_lock"))); +void esp_timer_private_unlock(void) __attribute__((alias("esp_timer_impl_unlock"))); + +void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) +{ + esp_timer_impl_set_alarm_id(timestamp, 0); +} + +#ifdef CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD +void IRAM_ATTR esp_timer_impl_try_to_set_next_alarm(void) { + portENTER_CRITICAL_ISR(&s_time_update_lock); + unsigned now_alarm_idx; // ISR is called due to this current alarm + unsigned next_alarm_idx; // The following alarm after now_alarm_idx + if (timestamp_id[0] < timestamp_id[1]) { + now_alarm_idx = 0; + next_alarm_idx = 1; + } else { + now_alarm_idx = 1; + next_alarm_idx = 0; + } + + if (timestamp_id[next_alarm_idx] != UINT64_MAX) { + // The following alarm is valid and can be used. + // Remove the current alarm from consideration. + esp_timer_impl_set_alarm_id(UINT64_MAX, now_alarm_idx); + } else { + // There is no the following alarm. + // Remove the current alarm from consideration as well. + timestamp_id[now_alarm_idx] = UINT64_MAX; + } + portEXIT_CRITICAL_ISR(&s_time_update_lock); +} +#endif + +/* FIXME: This value is safe for 80MHz APB frequency, should be modified to depend on clock frequency. */ +uint64_t IRAM_ATTR esp_timer_impl_get_min_period_us(void) +{ + return 50; +} diff --git a/components/esp_timer/src/esp_timer_impl_lac.c b/components/esp_timer/src/esp_timer_impl_lac.c index dc3ab65023..7a30833b22 100644 --- a/components/esp_timer/src/esp_timer_impl_lac.c +++ b/components/esp_timer/src/esp_timer_impl_lac.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -92,18 +92,10 @@ static intr_handle_t s_timer_interrupt_handle; static intr_handler_t s_alarm_handler = NULL; /* Spinlock used to protect access to the hardware registers. */ -portMUX_TYPE s_time_update_lock = portMUX_INITIALIZER_UNLOCKED; +extern portMUX_TYPE s_time_update_lock; - -void esp_timer_impl_lock(void) -{ - portENTER_CRITICAL(&s_time_update_lock); -} - -void esp_timer_impl_unlock(void) -{ - portEXIT_CRITICAL(&s_time_update_lock); -} +/* Alarm values to generate interrupt on match */ +extern uint64_t timestamp_id[2]; uint64_t IRAM_ATTR esp_timer_impl_get_counter_reg(void) { @@ -145,7 +137,7 @@ int64_t esp_timer_get_time(void) __attribute__((alias("esp_timer_impl_get_time") void IRAM_ATTR esp_timer_impl_set_alarm_id(uint64_t timestamp, unsigned alarm_id) { - static uint64_t timestamp_id[2] = { UINT64_MAX, UINT64_MAX }; + assert(alarm_id < sizeof(timestamp_id) / sizeof(timestamp_id[0])); portENTER_CRITICAL_SAFE(&s_time_update_lock); timestamp_id[alarm_id] = timestamp; timestamp = MIN(timestamp_id[0], timestamp_id[1]); @@ -173,16 +165,12 @@ void IRAM_ATTR esp_timer_impl_set_alarm_id(uint64_t timestamp, unsigned alarm_id portEXIT_CRITICAL_SAFE(&s_time_update_lock); } -void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) -{ - esp_timer_impl_set_alarm_id(timestamp, 0); -} - static void IRAM_ATTR timer_alarm_isr(void *arg) { /* Clear interrupt status */ REG_WRITE(INT_CLR_REG, TIMG_LACT_INT_CLR); - /* Call the upper layer handler */ + + /* Call the upper layer handler */ (*s_alarm_handler)(arg); } @@ -273,12 +261,6 @@ void esp_timer_impl_deinit(void) s_timer_interrupt_handle = NULL; } -/* FIXME: This value is safe for 80MHz APB frequency, should be modified to depend on clock frequency. */ -uint64_t IRAM_ATTR esp_timer_impl_get_min_period_us(void) -{ - return 50; -} - uint64_t esp_timer_impl_get_alarm_reg(void) { portENTER_CRITICAL_SAFE(&s_time_update_lock); @@ -293,5 +275,3 @@ uint64_t esp_timer_impl_get_alarm_reg(void) void esp_timer_private_update_apb_freq(uint32_t apb_ticks_per_us) __attribute__((alias("esp_timer_impl_update_apb_freq"))); void esp_timer_private_set(uint64_t new_us) __attribute__((alias("esp_timer_impl_set"))); void esp_timer_private_advance(int64_t time_diff_us) __attribute__((alias("esp_timer_impl_advance"))); -void esp_timer_private_lock(void) __attribute__((alias("esp_timer_impl_lock"))); -void esp_timer_private_unlock(void) __attribute__((alias("esp_timer_impl_unlock"))); diff --git a/components/esp_timer/src/esp_timer_impl_systimer.c b/components/esp_timer/src/esp_timer_impl_systimer.c index 30abe9bf75..07adbe4dee 100644 --- a/components/esp_timer/src/esp_timer_impl_systimer.c +++ b/components/esp_timer/src/esp_timer_impl_systimer.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -48,17 +48,10 @@ static intr_handler_t s_alarm_handler = NULL; static systimer_hal_context_t systimer_hal; /* Spinlock used to protect access to the hardware registers. */ -static portMUX_TYPE s_time_update_lock = portMUX_INITIALIZER_UNLOCKED; +extern portMUX_TYPE s_time_update_lock; -void esp_timer_impl_lock(void) -{ - portENTER_CRITICAL(&s_time_update_lock); -} - -void esp_timer_impl_unlock(void) -{ - portEXIT_CRITICAL(&s_time_update_lock); -} +/* Alarm values to generate interrupt on match */ +extern uint64_t timestamp_id[2]; uint64_t IRAM_ATTR esp_timer_impl_get_counter_reg(void) { @@ -76,7 +69,7 @@ int64_t esp_timer_get_time(void) __attribute__((alias("esp_timer_impl_get_time") void IRAM_ATTR esp_timer_impl_set_alarm_id(uint64_t timestamp, unsigned alarm_id) { - static uint64_t timestamp_id[2] = { UINT64_MAX, UINT64_MAX }; + assert(alarm_id < sizeof(timestamp_id) / sizeof(timestamp_id[0])); portENTER_CRITICAL_SAFE(&s_time_update_lock); timestamp_id[alarm_id] = timestamp; timestamp = MIN(timestamp_id[0], timestamp_id[1]); @@ -86,11 +79,6 @@ void IRAM_ATTR esp_timer_impl_set_alarm_id(uint64_t timestamp, unsigned alarm_id portEXIT_CRITICAL_SAFE(&s_time_update_lock); } -void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) -{ - esp_timer_impl_set_alarm_id(timestamp, 0); -} - static void IRAM_ATTR timer_alarm_isr(void *arg) { // clear the interrupt @@ -200,11 +188,6 @@ void esp_timer_impl_deinit(void) s_alarm_handler = NULL; } -uint64_t IRAM_ATTR esp_timer_impl_get_min_period_us(void) -{ - return 50; -} - uint64_t esp_timer_impl_get_alarm_reg(void) { portENTER_CRITICAL_SAFE(&s_time_update_lock); @@ -216,5 +199,3 @@ uint64_t esp_timer_impl_get_alarm_reg(void) void esp_timer_private_update_apb_freq(uint32_t apb_ticks_per_us) __attribute__((alias("esp_timer_impl_update_apb_freq"))); void esp_timer_private_set(uint64_t new_us) __attribute__((alias("esp_timer_impl_set"))); void esp_timer_private_advance(int64_t time_diff_us) __attribute__((alias("esp_timer_impl_advance"))); -void esp_timer_private_lock(void) __attribute__((alias("esp_timer_impl_lock"))); -void esp_timer_private_unlock(void) __attribute__((alias("esp_timer_impl_unlock"))); diff --git a/components/esp_timer/test/test_esp_timer.c b/components/esp_timer/test/test_esp_timer.c index 90446e4776..7fa9488544 100644 --- a/components/esp_timer/test/test_esp_timer.c +++ b/components/esp_timer/test/test_esp_timer.c @@ -1180,4 +1180,73 @@ TEST_CASE("Test ESP_TIMER_ISR, stop API cleans alarm reg if ISR timer list is em vSemaphoreDelete(done); printf("timer deleted\n"); } + +volatile uint64_t task_t1; +volatile uint64_t isr_t1; +const uint64_t period_task_ms = 200; +const uint64_t period_isr_ms = 20; + +void task_timer_cb(void *arg) { + uint64_t t2 = esp_timer_get_time(); + uint64_t dt_task_ms = (t2 - task_t1) / 1000; + task_t1 = t2; + printf("task callback, %d msec\n", (int)dt_task_ms); + vTaskDelay((period_task_ms / 2) / portTICK_PERIOD_MS); // very long callback in timer task + static bool first_run = true; + if (first_run) { + first_run = false; + } else { + TEST_ASSERT_INT_WITHIN(period_task_ms / 3, period_task_ms, dt_task_ms); + } +} + +void IRAM_ATTR isr_timer_cb(void *arg) { + uint64_t t2 = esp_timer_get_time(); + uint64_t dt_isr_ms = (t2 - isr_t1) / 1000; + isr_t1 = t2; + esp_rom_printf("isr callback, %d msec\n", (int)dt_isr_ms); + static bool first_run = true; + if (first_run) { + first_run = false; + } else { + TEST_ASSERT_INT_WITHIN(period_isr_ms / 3, period_isr_ms, dt_isr_ms); + } +} + +TEST_CASE("Test ISR dispatch callbacks are not blocked even if TASK callbacks take more time", "[esp_timer][isr_dispatch]") +{ + esp_timer_handle_t task_timer_handle; + esp_timer_handle_t isr_timer_handle; + + const esp_timer_create_args_t task_timer_args = { + .callback = &task_timer_cb, + .arg = NULL, + .dispatch_method = ESP_TIMER_TASK, + .name = "task_timer", + .skip_unhandled_events = true, + }; + + const esp_timer_create_args_t isr_timer_args = { + .callback = &isr_timer_cb, + .arg = NULL, + .dispatch_method = ESP_TIMER_ISR, + .name = "isr_timer", + .skip_unhandled_events = true, + }; + + ESP_ERROR_CHECK(esp_timer_create(&task_timer_args, &task_timer_handle)); + ESP_ERROR_CHECK(esp_timer_create(&isr_timer_args, &isr_timer_handle)); + ESP_ERROR_CHECK(esp_timer_start_periodic(task_timer_handle, period_task_ms * 1000)); + task_t1 = esp_timer_get_time(); + ESP_ERROR_CHECK(esp_timer_start_periodic(isr_timer_handle, period_isr_ms * 1000)); + isr_t1 = esp_timer_get_time(); + + vTaskDelay(period_task_ms * 5 / portTICK_PERIOD_MS); + + TEST_ESP_OK(esp_timer_stop(task_timer_handle)); + TEST_ESP_OK(esp_timer_stop(isr_timer_handle)); + TEST_ESP_OK(esp_timer_delete(task_timer_handle)); + TEST_ESP_OK(esp_timer_delete(isr_timer_handle)); +} + #endif // CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD