espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.34k stars 7.2k forks source link

[BUG] ISR dispatch and GPTIMER problematic enable (IDFGH-13527) #14418

Open filzek opened 3 weeks ago

filzek commented 3 weeks ago

Answers checklist.

General issue report

### Issue Description: We are encountering an issue with the ESP32 GPTimer system when attempting to execute two timers in rapid succession, both being set from within an ISR. Specifically, the first GPTimer (gptimer_triacs) always executes perfectly, but the second GPTimer (gptimer_triacs_all_off) sometimes fails to execute entirely. No errors are reported during the arming or setting of the timers, but the callback for gptimer_triacs_all_off occasionally does not get dispatched.

### Code Overview: The following GPTimers are created and configured:

// Global configuration for both timers
gptimer_config_t IRAM_ATTR timer_config_us_up = {
    .clk_src = GPTIMER_CLK_SRC_DEFAULT,
    .direction = GPTIMER_COUNT_UP,
    .resolution_hz = 1 * 1000 * 1000, // 1MHz, 1 tick = 1us
    .intr_priority = 3,
    .flags.intr_shared = false,
};

// Creating and configuring the first timer (gptimer_triacs)
gptimer_new_timer(&timer_config_us_up, &gptimer_triacs);
gptimer_event_callbacks_t callback_source_for_triacs_arm = {
    .on_alarm = execute_alarm_for_triacs_arm,
};
gptimer_register_event_callbacks(gptimer_triacs, &callback_source_for_triacs_arm, NULL);
gptimer_enable(gptimer_triacs);

// Creating and configuring the second timer (gptimer_triacs_all_off)
gptimer_new_timer(&timer_config_us_up, &gptimer_triacs_all_off);
gptimer_event_callbacks_t callback_source_for_triacs_disarm_all = {
    .on_alarm = timer_disabletriacs,
};
gptimer_register_event_callbacks(gptimer_triacs_all_off, &callback_source_for_triacs_disarm_all, NULL);
gptimer_enable(gptimer_triacs_all_off);

### Timer Arming in ISR: Inside an ISR, both timers are re-armed sequentially:

// Re-arm and start gptimer_triacs
gptimer_stop(gptimer_triacs);
gptimer_set_raw_count(gptimer_triacs, 0);
gptimer_alarm_config_t us_triac_dispatch = {
    .alarm_count = diff,  // TRIAC timing
};
gptimer_set_alarm_action(gptimer_triacs, &us_triac_dispatch);
gptimer_start(gptimer_triacs);

// Re-arm and start gptimer_triacs_all_off
gptimer_stop(gptimer_triacs_all_off);
gptimer_alarm_config_t us_dispatch_all_triacs_off = {
    .alarm_count = diff_off,  // Delay for TRIACs off
};
esp_err_t ret = gptimer_set_raw_count(gptimer_triacs_all_off, 0);
if (ret != ESP_OK) {
    printf("Failed to set raw count for gptimer_triacs_all_off: %s\n", esp_err_to_name(ret));
}
ret = gptimer_set_alarm_action(gptimer_triacs_all_off, &us_dispatch_all_triacs_off);
if (ret != ESP_OK) {
    printf("Failed to set alarm action for gptimer_triacs_all_off: %s\n", esp_err_to_name(ret));
}
ret = gptimer_start(gptimer_triacs_all_off);
if (ret != ESP_OK) {
    printf("Failed to start gptimer_triacs_all_off: %s\n", esp_err_to_name(ret));
}

Problem Summary:

This issue persists even though:

Test Conditions:

Observations:

Request for Guidance or any Changes to Try to Solve Addressed Issue

We are seeking clarification on whether there are known issues with GPTimer under the following conditions:

Is there any potential for a race condition or other underlying issue that could cause the second timer not to dispatch its callback? Any advice or suggested debugging steps to identify why this is happening would be greatly appreciated.

filzek commented 3 weeks ago

Startup as: D (10410) gptimer: new group (0) @0x3ffcf090 D (10410) gptimer: new gptimer (0,0) at 0x3ffe3de0, resolution=1000000Hz D (10410) gptimer: new gptimer (0,1) at 0x3ffe3ea4, resolution=1000000Hz

Kainarx commented 3 weeks ago

I think there is no problem with sharing resources between two separate timers. Can you provide a simple reproduction project? Also, which version of the idf and which chip are you using? I will try to reproduce it.

filzek commented 1 week ago

Hi, the problem is among the GPTIMER dispatch, it has a serious problem.

gptimer_config_t IRAM_ATTR timer_config_us_up = { .clk_src = GPTIMER_CLK_SRC_DEFAULT, .direction = GPTIMER_COUNT_UP, .resolution_hz = 1 1000 1000, // 1MHz, 1 tick = 1us .intr_priority = 3, //3 tons of error, 1 still with some errors .flags.intr_shared = false, };

timer_creation_result = gptimer_new_timer(&timer_config_us_up, &gptimer_timelapse_action); gptimer_event_callbacks_t callback_source_timelapse = { .on_alarm = time_lapse_action, }; gptimer_register_event_callbacks(gptimer_timelapse_action, &callback_source_timelapse, NULL); gptimer_alarm_config_t us_triac_dispatch2 = { .reload_count = 0, .alarm_count = 75, .flags.auto_reload_on_alarm = true, }; gptimer_set_alarm_action(gptimer_timelapse_action, &us_triac_dispatch2); gptimer_enable(gptimer_timelapse_action); gptimer_start(gptimer_timelapse_action);

int IRAM_ATTR onoff = 1; static bool IRAM_ATTR time_lapse_action(gptimer_handle_t timer, const gptimer_alarm_event_data_t edata, void user_data) { int64_t testing = esp_timer_get_time(); // Record the current time xQueueSendFromISR(gpio_evt_queue_zc, &testing, NULL);

if (onoff){ GPIO.out_w1ts = (1 << 26); // Enable GPIO output high } else { GPIO.out_w1tc = (1 << 26); // Clear GPIO output low } onoff = !onoff; return true; }

Ocisloscope DS1104z with 100MHz 1GSa/s With this example running we can capture a lot of dispatch failure, timer trigger and actions with odd behavior with a 75us continous dispatch as in above code:

1) GPTIMER double act in dispatch, so its dispatch double at 5us even in a miss act. 1-wave-

2) Miss the dispatch twice concurrent 2-wave-

To have a minimum ammount of failures the highest level 1 is needed to have small ammount of error.

There is no free error GPTImer at all.

florentbr commented 1 week ago

@filzek Your variables are tagged as IRAM_ATTR. Is it not supposed to be DRAM_ATTR since they are data and not instruction ?

filzek commented 1 week ago

@filzek Your variables are tagged as IRAM_ATTR. Is it not supposed to be DRAM_ATTR since they are data and not instruction ?

@florentbr in this case makes no difference as the size part of the memory read and write is always exacly the same, it could have a problem with a uint/string larger type, but for int family there is no such as problem.

In the timer handle and others strucs the compiler moves it to the correct aligned part as well, it does it since sdk v3.

Kainarx commented 3 days ago

Hi @filzek I tested it with your configuration and didn't find this problem. Did you configure the options CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM=y and CONFIG_GPTIMER_ISR_IRAM_SAFE=y? Beacues gptimer_timelapse_action is user's callback function which we call it in the gptimer_default_isr. We need to ensure both of them are in the IRAM.