espressif / esp-idf

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

ESP Timer's `CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD` requires user callbacks to be placed in IRAM (IDFGH-8821) #10250

Open ivmarkov opened 1 year ago

ivmarkov commented 1 year ago

Is your feature request related to a problem?

Unlike most other ESP IDF APIs, the ESP Timer service - when configured with CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD - unconditionally registers its ISR callback handler with the ESP_INTR_FLAG_IRAM flag enabled. What this means is that user code that would like to be notified by timers directly from an ISR must be placed in IRAM.

We have a use case (ESP Timer service exposed via Rust wrappers) where it is quite possible to have the user code be notified directly from an ISR - thus skipping the extra timer task that would be needed otherwise - however it is not always realistic to expect that users would place all of their timer callback code in IRAM, as controlling what is placed in IRAM from Rust is a bit more difficult (and might actually be undesired).

Describe the solution you'd like.

Introduce a way to control whether the ESP Timer service ISR callback is registered with or without the ESP_INTR_FLAG_IRAM attribute.

The simplest non-API approach would be to introduce another kconfig constant - say - CONFIG_ESP_TIMER_SUPPORTS_IRAM_ISR_DISPATCH_METHOD which is set to y by default. When set to n however, ESP Timer C code would not set the ESP_INTR_FLAG_IRAM flag.

Describe alternatives you've considered.

The simplest workaround would be to just use the task basedEspTimer service dispatch method

However, that brings the overhead of an extra FreeRtos task, plus some memory used for stack by that task. Specifically for Rust async code running on top of ESP IDF with e.g. ISR-safe async executor like this one all of that seems unnecessary, as the ESP Timer service - for the most part - just needs to awake the executor and reschedule the task whose timeout had expired. And one of the primary use cases of Rust async is to significantly cut the number of FreeRtos tasks used (as it introduces a M:N mapping between "task-like" user workloads, and actual OS threads on which the user tasks are scheduled).

Moreover, notifications coming from other ESP IDF device drivers can hit the executor directly from the ISR, as they all seem to provide the ISR-callback-might-not-be-completely-in-IRAM option. Namely:

Additional context.

No response

igrr commented 1 year ago

Introduce a way to control whether the ESP Timer service ISR callback is registered with or without the ESP_INTR_FLAG_IRAM attribute.

I'm afraid this solution would not be easily possible, as some of the components (bluetooth and Wi-Fi stacks, to be specific) expect that the delivery of timer callbacks is not blocked by the Flash operations. If the interrupt is registered without the IRAM flag, it will be suspended for the duration of time when cache is disabled during the flash operation.

ivmarkov commented 1 year ago

Introduce a way to control whether the ESP Timer service ISR callback is registered with or without the ESP_INTR_FLAG_IRAM attribute.

I'm afraid this solution would not be easily possible, as some of the components (bluetooth and Wi-Fi stacks, to be specific) expect that the delivery of timer callbacks is not blocked by the Flash operations. If the interrupt is registered without the IRAM flag, it will be suspended for the duration of time when cache is disabled during the flash operation.

Just to make sure I understand: wouldn't that imply, that if the wifi or BT component is enabled, CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD is automatically set to 'y'? Or are Wifi and BT somehow "hooked" deeper into the ESP Timer service, without going via its public API?

ivmarkov commented 1 year ago

@igrr if you could only confirm / deny my understanding from the last comment ^^^. I'm also a bit confused as the status of the ticket is now "selected for development" yet your original comment seemed to imply that Espressif would be unwilling to provide this flexibility due to wifi / ble constraints.

igrr commented 1 year ago

I'm sorry, my comment was wrong. Wi-Fi and BLE stacks don't use the ISR dispatch method at all. So adding a Kconfig option is viable. We just need to add a test to ensure the new option doesn't get broken later.

ivmarkov commented 1 year ago

Perfect, thanks a lot!