From 7bcb7248bb8d69f39906d4e71da83cb669a3b9b7 Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 14 Apr 2025 18:19:33 +0800 Subject: [PATCH] feat(gdma): support allocate cache safe ISR for channels separately --- .../esp_hw_support/deprecated/gdma_legacy.c | 12 +- components/esp_hw_support/dma/Kconfig.dma | 40 ++++-- components/esp_hw_support/dma/gdma.c | 120 +++++++++--------- components/esp_hw_support/dma/gdma_crc.c | 20 +-- components/esp_hw_support/dma/gdma_etm.c | 18 +-- components/esp_hw_support/dma/gdma_link.c | 2 +- components/esp_hw_support/dma/gdma_priv.h | 30 +++-- .../esp_hw_support/dma/gdma_sleep_retention.c | 19 +-- .../dma/include/esp_private/gdma.h | 1 + components/esp_hw_support/dma/linker.lf | 3 +- 10 files changed, 123 insertions(+), 142 deletions(-) diff --git a/components/esp_hw_support/deprecated/gdma_legacy.c b/components/esp_hw_support/deprecated/gdma_legacy.c index f25dd2c8ab..313f99dce2 100644 --- a/components/esp_hw_support/deprecated/gdma_legacy.c +++ b/components/esp_hw_support/deprecated/gdma_legacy.c @@ -1,23 +1,13 @@ /* - * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -#include -#include -#include -#include "sdkconfig.h" -#include "freertos/FreeRTOS.h" -#include "freertos/task.h" -#include "esp_log.h" -#include "esp_check.h" #include "../dma/gdma_priv.h" #include "hal/cache_hal.h" #include "hal/cache_ll.h" -static const char *TAG = "gdma"; - esp_err_t gdma_set_transfer_ability(gdma_channel_handle_t dma_chan, const gdma_transfer_ability_t *ability) { ESP_RETURN_ON_FALSE(dma_chan && ability, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); diff --git a/components/esp_hw_support/dma/Kconfig.dma b/components/esp_hw_support/dma/Kconfig.dma index 5620926e3e..d95212df71 100644 --- a/components/esp_hw_support/dma/Kconfig.dma +++ b/components/esp_hw_support/dma/Kconfig.dma @@ -3,24 +3,46 @@ menu "GDMA Configurations" config GDMA_CTRL_FUNC_IN_IRAM bool "Place GDMA control functions in IRAM" default n + select GDMA_OBJ_DRAM_SAFE help Place GDMA control functions (like start/stop/append/reset) into IRAM, so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context. - config GDMA_ISR_IRAM_SAFE - bool "GDMA ISR IRAM-Safe" + config GDMA_ISR_HANDLER_IN_IRAM + bool "Place GDMA ISR handler in IRAM to reduce latency" + default y + select GDMA_OBJ_DRAM_SAFE + help + Place GDMA ISR handler functions in IRAM to reduce latency caused by cache miss. + + config GDMA_OBJ_DRAM_SAFE + bool default n + help + This will ensure the GDMA object is DRAM-Safe, allow to avoid external memory + cache misses, and also be accessible whilst the cache is disabled. + + config GDMA_ENABLE_DEBUG_LOG + bool "Force enable debug log" + default n + help + If enabled, GDMA driver component will: + 1. ignore the global logging settings + 2. compile all log messages into the binary + 3. set the runtime log level to VERBOSE + Please enable this option by caution, as it will increase the binary size. + + config GDMA_ISR_IRAM_SAFE + bool "GDMA ISR IRAM-Safe (Deprecated. Read Help!)" + default n + select GDMA_ISR_HANDLER_IN_IRAM help This will ensure the GDMA interrupt handler is IRAM-Safe, allow to avoid flash cache misses, and also be able to run whilst the cache is disabled. (e.g. SPI Flash write). - - config GDMA_ENABLE_DEBUG_LOG - bool "Enable debug log" - default n - help - Whether to enable the debug log message for GDMA driver. - Note that, this option only controls the GDMA driver log, won't affect other drivers. + If this option is enabled, ALL GDMA channel's ISR handlers should be placed in IRAM, which is a overkill. + It's recommend to set the "isr_cache_safe" in the "gdma_channel_alloc_config_t". + Then other GDMA channels won't be influenced. endmenu # GDMA Configurations menu "DW_GDMA Configurations" diff --git a/components/esp_hw_support/dma/gdma.c b/components/esp_hw_support/dma/gdma.c index 9dd760792a..f222e9817e 100644 --- a/components/esp_hw_support/dma/gdma.c +++ b/components/esp_hw_support/dma/gdma.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -26,33 +26,14 @@ * - We're not using a global spin lock, instead, we created different spin locks at different level (group, pair). */ -#include -#include -#include -#include -#include "sdkconfig.h" -#if CONFIG_GDMA_ENABLE_DEBUG_LOG -// The local log level must be defined before including esp_log.h -// Set the maximum log level for this source file -#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG -#endif -#include "freertos/FreeRTOS.h" -#include "freertos/task.h" -#include "soc/soc_caps.h" -#include "soc/periph_defs.h" -#include "esp_log.h" -#include "esp_check.h" +#include "gdma_priv.h" #include "esp_memory_utils.h" #include "esp_flash_encrypt.h" -#include "esp_private/periph_ctrl.h" -#include "gdma_priv.h" #if CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP #include "esp_private/gdma_sleep_retention.h" #endif -static const char *TAG = "gdma"; - #if !SOC_RCC_IS_INDEPENDENT // Reset and Clock Control registers are mixing with other peripherals, so we need to use a critical section #define GDMA_RCC_ATOMIC() PERIPH_RCC_ATOMIC() @@ -94,9 +75,6 @@ typedef struct { static esp_err_t do_allocate_gdma_channel(const gdma_channel_search_info_t *search_info, const gdma_channel_alloc_config_t *config, gdma_channel_handle_t *ret_chan) { -#if CONFIG_GDMA_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif esp_err_t ret = ESP_OK; gdma_tx_channel_t *alloc_tx_channel = NULL; gdma_rx_channel_t *alloc_rx_channel = NULL; @@ -170,6 +148,12 @@ search_done: alloc_tx_channel->base.pair = pair; alloc_tx_channel->base.direction = GDMA_CHANNEL_DIRECTION_TX; alloc_tx_channel->base.periph_id = GDMA_INVALID_PERIPH_TRIG; + // for backward compatibility, `CONFIG_GDMA_ISR_IRAM_SAFE` can still force ALL GDMA ISRs to be cache safe +#if CONFIG_GDMA_ISR_IRAM_SAFE + alloc_tx_channel->base.flags.isr_cache_safe = true; +#else + alloc_tx_channel->base.flags.isr_cache_safe = config->flags.isr_cache_safe; +#endif alloc_tx_channel->base.del = gdma_del_tx_channel; // set channel deletion function *ret_chan = &alloc_tx_channel->base; // return the installed channel } @@ -180,6 +164,12 @@ search_done: alloc_rx_channel->base.pair = pair; alloc_rx_channel->base.direction = GDMA_CHANNEL_DIRECTION_RX; alloc_rx_channel->base.periph_id = GDMA_INVALID_PERIPH_TRIG; + // for backward compatibility, `CONFIG_GDMA_ISR_IRAM_SAFE` can still force ALL GDMA ISRs to be cache safe +#if CONFIG_GDMA_ISR_IRAM_SAFE + alloc_rx_channel->base.flags.isr_cache_safe = true; +#else + alloc_rx_channel->base.flags.isr_cache_safe = config->flags.isr_cache_safe; +#endif alloc_rx_channel->base.del = gdma_del_rx_channel; // set channel deletion function *ret_chan = &alloc_rx_channel->base; // return the installed channel } @@ -455,20 +445,20 @@ esp_err_t gdma_register_tx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ gdma_hal_context_t *hal = &group->hal; gdma_tx_channel_t *tx_chan = __containerof(dma_chan, gdma_tx_channel_t, base); -#if CONFIG_GDMA_ISR_IRAM_SAFE - if (cbs->on_trans_eof) { - ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_trans_eof), ESP_ERR_INVALID_ARG, - TAG, "on_trans_eof not in IRAM"); + if (dma_chan->flags.isr_cache_safe) { + if (cbs->on_trans_eof) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_trans_eof), ESP_ERR_INVALID_ARG, + TAG, "on_trans_eof not in IRAM"); + } + if (cbs->on_descr_err) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_descr_err), ESP_ERR_INVALID_ARG, + TAG, "on_descr_err not in IRAM"); + } + if (user_data) { + ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, + TAG, "user context not in internal RAM"); + } } - if (cbs->on_descr_err) { - ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_descr_err), ESP_ERR_INVALID_ARG, - TAG, "on_descr_err not in IRAM"); - } - if (user_data) { - ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, - TAG, "user context not in internal RAM"); - } -#endif // CONFIG_GDMA_ISR_IRAM_SAFE // lazy install interrupt service ESP_RETURN_ON_ERROR(gdma_install_tx_interrupt(tx_chan), TAG, "install interrupt service failed"); @@ -495,24 +485,24 @@ esp_err_t gdma_register_rx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ gdma_hal_context_t *hal = &group->hal; gdma_rx_channel_t *rx_chan = __containerof(dma_chan, gdma_rx_channel_t, base); -#if CONFIG_GDMA_ISR_IRAM_SAFE - if (cbs->on_recv_eof) { - ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_recv_eof), ESP_ERR_INVALID_ARG, - TAG, "on_recv_eof not in IRAM"); + if (dma_chan->flags.isr_cache_safe) { + if (cbs->on_recv_eof) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_recv_eof), ESP_ERR_INVALID_ARG, + TAG, "on_recv_eof not in IRAM"); + } + if (cbs->on_descr_err) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_descr_err), ESP_ERR_INVALID_ARG, + TAG, "on_descr_err not in IRAM"); + } + if (cbs->on_recv_done) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_recv_done), ESP_ERR_INVALID_ARG, + TAG, "on_recv_done not in IRAM"); + } + if (user_data) { + ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, + TAG, "user context not in internal RAM"); + } } - if (cbs->on_descr_err) { - ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_descr_err), ESP_ERR_INVALID_ARG, - TAG, "on_descr_err not in IRAM"); - } - if (cbs->on_recv_done) { - ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_recv_done), ESP_ERR_INVALID_ARG, - TAG, "on_recv_done not in IRAM"); - } - if (user_data) { - ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, - TAG, "user context not in internal RAM"); - } -#endif // CONFIG_GDMA_ISR_IRAM_SAFE // lazy install interrupt service ESP_RETURN_ON_ERROR(gdma_install_rx_interrupt(rx_chan), TAG, "install interrupt service failed"); @@ -870,9 +860,12 @@ static esp_err_t gdma_install_rx_interrupt(gdma_rx_channel_t *rx_chan) gdma_hal_context_t *hal = &group->hal; int pair_id = pair->pair_id; // pre-alloc a interrupt handle, with handler disabled - int isr_flags = GDMA_INTR_ALLOC_FLAGS; + int isr_flags = ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_LOWMED; + if (rx_chan->base.flags.isr_cache_safe) { + isr_flags |= ESP_INTR_FLAG_IRAM; + } #if GDMA_LL_AHB_TX_RX_SHARE_INTERRUPT - isr_flags |= ESP_INTR_FLAG_SHARED | ESP_INTR_FLAG_LOWMED; + isr_flags |= ESP_INTR_FLAG_SHARED; #endif intr_handle_t intr = NULL; ret = esp_intr_alloc_intrstatus(gdma_periph_signals.groups[group->group_id].pairs[pair_id].rx_irq_id, isr_flags, @@ -899,9 +892,12 @@ static esp_err_t gdma_install_tx_interrupt(gdma_tx_channel_t *tx_chan) gdma_hal_context_t *hal = &group->hal; int pair_id = pair->pair_id; // pre-alloc a interrupt handle, with handler disabled - int isr_flags = GDMA_INTR_ALLOC_FLAGS; + int isr_flags = ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_LOWMED; + if (tx_chan->base.flags.isr_cache_safe) { + isr_flags |= ESP_INTR_FLAG_IRAM; + } #if GDMA_LL_AHB_TX_RX_SHARE_INTERRUPT - isr_flags |= ESP_INTR_FLAG_SHARED | ESP_INTR_FLAG_LOWMED; + isr_flags |= ESP_INTR_FLAG_SHARED; #endif intr_handle_t intr = NULL; ret = esp_intr_alloc_intrstatus(gdma_periph_signals.groups[group->group_id].pairs[pair_id].tx_irq_id, isr_flags, @@ -919,3 +915,11 @@ static esp_err_t gdma_install_tx_interrupt(gdma_tx_channel_t *tx_chan) err: return ret; } + +#if CONFIG_GDMA_ENABLE_DEBUG_LOG +__attribute__((constructor)) +static void gdma_override_default_log_level(void) +{ + esp_log_level_set(TAG, ESP_LOG_VERBOSE); +} +#endif diff --git a/components/esp_hw_support/dma/gdma_crc.c b/components/esp_hw_support/dma/gdma_crc.c index da7425d39d..0979f32703 100644 --- a/components/esp_hw_support/dma/gdma_crc.c +++ b/components/esp_hw_support/dma/gdma_crc.c @@ -1,29 +1,11 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -#include -#include -#include -#include -#include "sdkconfig.h" -#if CONFIG_GDMA_ENABLE_DEBUG_LOG -// The local log level must be defined before including esp_log.h -// Set the maximum log level for this source file -#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG -#endif -#include "freertos/FreeRTOS.h" -#include "freertos/task.h" -#include "soc/soc_caps.h" -#include "soc/periph_defs.h" -#include "esp_log.h" -#include "esp_check.h" #include "gdma_priv.h" -static const char *TAG = "gdma"; - esp_err_t gdma_config_crc_calculator(gdma_channel_handle_t dma_chan, const gdma_crc_calculator_config_t *config) { ESP_RETURN_ON_FALSE(dma_chan && config, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); diff --git a/components/esp_hw_support/dma/gdma_etm.c b/components/esp_hw_support/dma/gdma_etm.c index 5749c1cc78..c0e9be8191 100644 --- a/components/esp_hw_support/dma/gdma_etm.c +++ b/components/esp_hw_support/dma/gdma_etm.c @@ -1,28 +1,14 @@ /* - * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - -#include -#include -#include "sdkconfig.h" -#include "esp_log.h" -#include "esp_check.h" -#include "esp_heap_caps.h" -#include "hal/gdma_hal.h" -#include "hal/gdma_ll.h" -#include "soc/gdma_periph.h" -#include "esp_private/gdma.h" -#include "esp_private/etm_interface.h" #include "gdma_priv.h" +#include "esp_private/etm_interface.h" #define ETM_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT -static const char *TAG = "gdma-etm"; - typedef struct gdma_etm_task_t { esp_etm_task_t base; gdma_channel_t *chan; diff --git a/components/esp_hw_support/dma/gdma_link.c b/components/esp_hw_support/dma/gdma_link.c index 29288decf1..6a3199ea91 100644 --- a/components/esp_hw_support/dma/gdma_link.c +++ b/components/esp_hw_support/dma/gdma_link.c @@ -17,7 +17,7 @@ #include "hal/cache_ll.h" #include "esp_cache.h" -static const char *TAG = "gdma"; +static const char *TAG = "gdma-link"; #if SOC_NON_CACHEABLE_OFFSET #define GDMA_CACHE_ADDR_TO_NON_CACHE_ADDR(addr) ((addr) + SOC_NON_CACHEABLE_OFFSET) diff --git a/components/esp_hw_support/dma/gdma_priv.h b/components/esp_hw_support/dma/gdma_priv.h index 600397be02..4c0d921a59 100644 --- a/components/esp_hw_support/dma/gdma_priv.h +++ b/components/esp_hw_support/dma/gdma_priv.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,33 +7,44 @@ #pragma once #include +#include +#include +#include +#include #include "sdkconfig.h" +#if CONFIG_GDMA_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for gdma driver +#define LOG_LOCAL_LEVEL ESP_LOG_VERBOSE +#endif +#include "soc/soc_caps.h" #include "freertos/FreeRTOS.h" +#include "freertos/task.h" #include "esp_err.h" +#include "esp_log.h" +#include "esp_check.h" #include "esp_intr_alloc.h" #include "esp_heap_caps.h" -#include "soc/soc_caps.h" #include "hal/gdma_hal.h" #include "hal/gdma_ll.h" #include "hal/gdma_hal_ahb.h" #include "hal/gdma_hal_axi.h" #include "soc/gdma_periph.h" +#include "soc/periph_defs.h" #include "esp_private/gdma.h" +#include "esp_private/periph_ctrl.h" -#if CONFIG_GDMA_ISR_IRAM_SAFE || CONFIG_GDMA_CTRL_FUNC_IN_IRAM +#if CONFIG_GDMA_OBJ_DRAM_SAFE #define GDMA_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else #define GDMA_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT #endif -#if CONFIG_GDMA_ISR_IRAM_SAFE -#define GDMA_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) -#else -#define GDMA_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED -#endif - #define GDMA_ACCESS_ENCRYPTION_MEM_ALIGNMENT 16 /*!< The alignment of the memory and size when DMA accesses the encryption memory */ +///!< Logging settings +#define TAG "gdma" + #ifdef __cplusplus extern "C" { #endif @@ -74,6 +85,7 @@ struct gdma_channel_t { esp_err_t (*del)(gdma_channel_t *channel); // channel deletion function, it's polymorphic, see `gdma_del_tx_channel` or `gdma_del_rx_channel` struct { uint32_t start_stop_by_etm: 1; // whether the channel is started/stopped by ETM + uint32_t isr_cache_safe: 1; // whether the interrupt of this channel need to be cache safe } flags; }; diff --git a/components/esp_hw_support/dma/gdma_sleep_retention.c b/components/esp_hw_support/dma/gdma_sleep_retention.c index 98d53e46c9..7deb476dac 100644 --- a/components/esp_hw_support/dma/gdma_sleep_retention.c +++ b/components/esp_hw_support/dma/gdma_sleep_retention.c @@ -1,28 +1,13 @@ /* - * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -#include -#include -#include "sdkconfig.h" -#include "soc/gdma_periph.h" -#include "soc/soc_caps.h" - -#include "esp_err.h" -#if CONFIG_GDMA_ENABLE_DEBUG_LOG -// The local log level must be defined before including esp_log.h -// Set the maximum log level for this source file -#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG -#endif -#include "esp_log.h" -#include "esp_check.h" +#include "gdma_priv.h" #include "esp_private/sleep_retention.h" #include "esp_private/esp_regdma.h" -static const char *TAG = "gdma"; - typedef struct { int group_id; int pair_id; diff --git a/components/esp_hw_support/dma/include/esp_private/gdma.h b/components/esp_hw_support/dma/include/esp_private/gdma.h index 22868fa3bd..af595f7c62 100644 --- a/components/esp_hw_support/dma/include/esp_private/gdma.h +++ b/components/esp_hw_support/dma/include/esp_private/gdma.h @@ -31,6 +31,7 @@ typedef struct { gdma_channel_direction_t direction; /*!< DMA channel direction */ struct { int reserve_sibling: 1; /*!< If set, DMA channel allocator would prefer to allocate new channel in a new pair, and reserve sibling channel for future use */ + int isr_cache_safe: 1; /*!< If set, DMA channel allocator would allocate interrupt in cache-safe region, and ISR is serviceable when cache is disabled */ } flags; } gdma_channel_alloc_config_t; diff --git a/components/esp_hw_support/dma/linker.lf b/components/esp_hw_support/dma/linker.lf index db6159eed1..0f3faf6cb3 100644 --- a/components/esp_hw_support/dma/linker.lf +++ b/components/esp_hw_support/dma/linker.lf @@ -1,8 +1,7 @@ [mapping:gdma_driver] archive: libesp_hw_support.a entries: - # performance optimization, always put the DMA default interrupt handler in IRAM - if SOC_GDMA_SUPPORTED = y: + if GDMA_ISR_HANDLER_IN_IRAM = y: gdma: gdma_default_tx_isr (noflash) gdma: gdma_default_rx_isr (noflash)