Skip to content

Commit

Permalink
fix(esp_timer): Fix delay in ISR dispatch callbacks
Browse files Browse the repository at this point in the history 
Set the following alarm before calling the alarm handler.

Closes #11637
Closes #11636
  • Loading branch information
@acf-bwl @KonstantinKondrashov
acf-bwl authored and KonstantinKondrashov committed Aug 23, 2023
1 parent 8b692dd commit 41dc26f
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 5 deletions.
39 changes: 36 additions & 3 deletions components/esp_timer/src/esp_timer_impl_lac.c
View file
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -94,6 +94,8 @@ 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;

/* Alarm values to generate interrupt on match */
static uint64_t timestamp_id[2] = { UINT64_MAX, UINT64_MAX };

void esp_timer_impl_lock(void)
{
Expand Down Expand Up @@ -145,7 +147,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]);
Expand Down Expand Up @@ -178,11 +180,42 @@ 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
static void IRAM_ATTR 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);
}
#else
#define try_to_set_next_alarm()
#endif

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 */

try_to_set_next_alarm();

/* Call the upper layer handler */
(*s_alarm_handler)(arg);
}

Expand Down
38 changes: 36 additions & 2 deletions components/esp_timer/src/esp_timer_impl_systimer.c
View file
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -50,6 +50,9 @@ 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;

/* Alarm values to generate interrupt on match */
static uint64_t timestamp_id[2] = { UINT64_MAX, UINT64_MAX };

void esp_timer_impl_lock(void)
{
portENTER_CRITICAL(&s_time_update_lock);
Expand All @@ -76,7 +79,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]);
Expand All @@ -91,10 +94,41 @@ 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
static void IRAM_ATTR 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);
}
#else
#define try_to_set_next_alarm()
#endif

static void IRAM_ATTR timer_alarm_isr(void *arg)
{
// clear the interrupt
systimer_ll_clear_alarm_int(systimer_hal.dev, SYSTIMER_LL_ALARM_CLOCK);

try_to_set_next_alarm();

/* Call the upper layer handler */
(*s_alarm_handler)(arg);
}
Expand Down
69 changes: 69 additions & 0 deletions components/esp_timer/test/test_esp_timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 41dc26f

Please sign in to comment.