From a662ec0a8d802afc957341f2e3e216357040f248 Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 7 Aug 2023 11:46:28 +0800 Subject: [PATCH] fix(gptimer): hal function placement under wrong condition timer_hal_capture_and_get_counter_value should be placed in the IRAM for speed optimization because the default ISR handler is placed in the IRAM. Closes https://github.com/espressif/esp-idf/issues/12021 --- components/driver/Kconfig | 32 +-------------- components/driver/gptimer/Kconfig.gptimer | 39 +++++++++++++++++++ components/driver/gptimer/gptimer.c | 3 +- components/driver/gptimer/linker.lf | 4 ++ docs/en/api-reference/peripherals/gptimer.rst | 5 ++- .../api-reference/peripherals/gptimer.rst | 5 ++- 6 files changed, 51 insertions(+), 37 deletions(-) create mode 100644 components/driver/gptimer/Kconfig.gptimer diff --git a/components/driver/Kconfig b/components/driver/Kconfig index da23ea059f..2f8839940a 100644 --- a/components/driver/Kconfig +++ b/components/driver/Kconfig @@ -306,37 +306,7 @@ menu "Driver Configurations" Note that, this option only controls the Analog Comparator driver log, won't affect other drivers. endmenu # Analog Comparator Configuration - menu "GPTimer Configuration" - config GPTIMER_CTRL_FUNC_IN_IRAM - bool "Place GPTimer control functions into IRAM" - default n - help - Place GPTimer control functions (like start/stop) into IRAM, - so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context. - Enabling this option can improve driver performance as well. - - config GPTIMER_ISR_IRAM_SAFE - bool "GPTimer ISR IRAM-Safe" - default n - help - Ensure the GPTimer interrupt is IRAM-Safe by allowing the interrupt handler to be - executable when the cache is disabled (e.g. SPI Flash write). - - config GPTIMER_SUPPRESS_DEPRECATE_WARN - bool "Suppress legacy driver deprecated warning" - default n - help - Wether to suppress the deprecation warnings when using legacy timer group driver (driver/timer.h). - If you want to continue using the legacy driver, and don't want to see related deprecation warnings, - you can enable this option. - - config GPTIMER_ENABLE_DEBUG_LOG - bool "Enable debug log" - default n - help - Wether to enable the debug log message for GPTimer driver. - Note that, this option only controls the GPTimer driver log, won't affect other drivers. - endmenu # GPTimer Configuration + orsource "./gptimer/Kconfig.gptimer" menu "PCNT Configuration" depends on SOC_PCNT_SUPPORTED diff --git a/components/driver/gptimer/Kconfig.gptimer b/components/driver/gptimer/Kconfig.gptimer new file mode 100644 index 0000000000..1283241204 --- /dev/null +++ b/components/driver/gptimer/Kconfig.gptimer @@ -0,0 +1,39 @@ +menu "GPTimer Configuration" + config GPTIMER_ISR_HANDLER_IN_IRAM + bool "Place GPTimer ISR handler into IRAM" + default y + help + Place GPTimer ISR handler into IRAM for better performance and fewer cache misses. + + config GPTIMER_CTRL_FUNC_IN_IRAM + bool "Place GPTimer control functions into IRAM" + default n + help + Place GPTimer control functions (like start/stop) into IRAM, + so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context. + Enabling this option can improve driver performance as well. + + config GPTIMER_ISR_IRAM_SAFE + bool "GPTimer ISR IRAM-Safe" + select GPTIMER_ISR_HANDLER_IN_IRAM + select GPTIMER_ISR_NON_MASKABLE + default n + help + Ensure the GPTimer interrupt is IRAM-Safe by allowing the interrupt handler to be + executable when the cache is disabled (e.g. SPI Flash write). + + config GPTIMER_SUPPRESS_DEPRECATE_WARN + bool "Suppress legacy driver deprecated warning" + default n + help + Wether to suppress the deprecation warnings when using legacy timer group driver (driver/timer.h). + If you want to continue using the legacy driver, and don't want to see related deprecation warnings, + you can enable this option. + + config GPTIMER_ENABLE_DEBUG_LOG + bool "Enable debug log" + default n + help + Wether to enable the debug log message for GPTimer driver. + Note that, this option only controls the GPTimer driver log, won't affect other drivers. +endmenu # GPTimer Configuration diff --git a/components/driver/gptimer/gptimer.c b/components/driver/gptimer/gptimer.c index f26e2a3dcf..fc7322759b 100644 --- a/components/driver/gptimer/gptimer.c +++ b/components/driver/gptimer/gptimer.c @@ -487,8 +487,7 @@ static esp_err_t gptimer_select_periph_clock(gptimer_t *timer, gptimer_clock_sou return ESP_OK; } -// Put the default ISR handler in the IRAM for better performance -IRAM_ATTR static void gptimer_default_isr(void *args) +static void gptimer_default_isr(void *args) { bool need_yield = false; gptimer_t *timer = (gptimer_t *)args; diff --git a/components/driver/gptimer/linker.lf b/components/driver/gptimer/linker.lf index 1e80c4b0bd..11d7eeb26c 100644 --- a/components/driver/gptimer/linker.lf +++ b/components/driver/gptimer/linker.lf @@ -1,6 +1,8 @@ [mapping:gptimer_driver] archive: libdriver.a entries: + if GPTIMER_ISR_HANDLER_IN_IRAM = y: + gptimer: gptimer_default_isr (noflash) if GPTIMER_CTRL_FUNC_IN_IRAM = y: gptimer: gptimer_set_raw_count (noflash) gptimer: gptimer_get_raw_count (noflash) @@ -12,6 +14,8 @@ entries: [mapping:gptimer_hal] archive: libhal.a entries: + if GPTIMER_ISR_HANDLER_IN_IRAM = y: + timer_hal: timer_hal_capture_and_get_counter_value (noflash) if GPTIMER_CTRL_FUNC_IN_IRAM = y: timer_hal: timer_hal_set_counter_value (noflash) timer_hal: timer_hal_capture_and_get_counter_value (noflash) diff --git a/docs/en/api-reference/peripherals/gptimer.rst b/docs/en/api-reference/peripherals/gptimer.rst index 68ed5a8a37..fa53831d28 100644 --- a/docs/en/api-reference/peripherals/gptimer.rst +++ b/docs/en/api-reference/peripherals/gptimer.rst @@ -329,8 +329,9 @@ All the APIs provided by the driver are guaranteed to be thread safe, which mean Kconfig Options ^^^^^^^^^^^^^^^ -- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` controls where to place the GPTimer control functions (IRAM or flash), see Section :ref:`gptimer-iram-safe` for more information. -- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` controls whether the default ISR handler can work when the cache is disabled, see Section :ref:`gptimer-iram-safe` for more information. +- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` controls where to place the GPTimer control functions (IRAM or flash). +- :ref:`CONFIG_GPTIMER_ISR_HANDLER_IN_IRAM` controls where to place the GPTimer ISR handler (IRAM or flash). +- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` controls whether the default ISR handler should be masked when the cache is disabled, see Section :ref:`gptimer-iram-safe` for more information. - :ref:`CONFIG_GPTIMER_ENABLE_DEBUG_LOG` is used to enabled the debug log output. Enable this option will increase the firmware binary size. Application Examples diff --git a/docs/zh_CN/api-reference/peripherals/gptimer.rst b/docs/zh_CN/api-reference/peripherals/gptimer.rst index 0b833782a1..ae5b6af949 100644 --- a/docs/zh_CN/api-reference/peripherals/gptimer.rst +++ b/docs/zh_CN/api-reference/peripherals/gptimer.rst @@ -329,8 +329,9 @@ IRAM 安全 Kconfig 选项 ^^^^^^^^^^^^^^^^^^^^^^ -- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` 控制放置通用定时器控制函数（IRAM 或 flash）的位置。了解更多信息，请参考章节 :ref:`gptimer-iram-safe`。 -- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` 控制默认 ISR 程序在 cache 禁用时是否可以运行。了解更多信息，请参考章节 :ref:`gptimer-iram-safe`。 +- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` 控制着定时器控制函数的存放位置（IRAM 或 flash）。 +- :ref:`CONFIG_GPTIMER_ISR_HANDLER_IN_IRAM` 控制着定时器中断处理函数的存放位置（IRAM 或 flash）。 +- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` 控制着中断处理函数是否需要在 cache 关闭的时候被屏蔽掉。更多信息，请参阅 :ref:`gptimer-iram-safe`。 - :ref:`CONFIG_GPTIMER_ENABLE_DEBUG_LOG` 用于启用调试日志输出。启用这一选项将增加固件二进制文件大小。 应用示例