From 14299130423a3903fa231aa039440bf49bbb5b04 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 9 Mar 2023 11:42:06 +0800 Subject: [PATCH 1/2] gdma: support override default channel priority --- components/esp_hw_support/dma/gdma.c | 22 +++++++++++++++++++ .../esp_hw_support/include/esp_private/gdma.h | 18 +++++++++++++-- components/hal/esp32c2/include/hal/gdma_ll.h | 2 ++ components/hal/esp32c3/include/hal/gdma_ll.h | 2 ++ components/hal/esp32c6/include/hal/gdma_ll.h | 2 ++ components/hal/esp32h2/include/hal/gdma_ll.h | 2 ++ components/hal/esp32h4/include/hal/gdma_ll.h | 2 ++ components/hal/esp32s3/include/hal/gdma_ll.h | 2 ++ 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/components/esp_hw_support/dma/gdma.c b/components/esp_hw_support/dma/gdma.c index 1d55e466d5..0cbd5ae455 100644 --- a/components/esp_hw_support/dma/gdma.c +++ b/components/esp_hw_support/dma/gdma.c @@ -359,6 +359,24 @@ err: return ret; } +esp_err_t gdma_set_priority(gdma_channel_handle_t dma_chan, uint32_t priority) +{ + gdma_pair_t *pair = NULL; + gdma_group_t *group = NULL; + ESP_RETURN_ON_FALSE(dma_chan && priority <= GDMA_LL_CHANNEL_MAX_PRIORITY, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + pair = dma_chan->pair; + group = pair->group; + + if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_TX) { + gdma_ll_tx_set_priority(group->hal.dev, pair->pair_id, priority); + } else { + gdma_ll_rx_set_priority(group->hal.dev, pair->pair_id, priority); + } + + return ESP_OK; + +} + esp_err_t gdma_register_tx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_tx_event_callbacks_t *cbs, void *user_data) { esp_err_t ret = ESP_OK; @@ -651,6 +669,8 @@ static esp_err_t gdma_del_tx_channel(gdma_channel_t *dma_channel) ESP_LOGD(TAG, "uninstall interrupt service for tx channel (%d,%d)", group_id, pair_id); } + gdma_ll_tx_set_priority(group->hal.dev, pair_id, 0); // reset the priority to 0 (lowest) + free(tx_chan); ESP_LOGD(TAG, "del tx channel (%d,%d)", group_id, pair_id); // channel has a reference on pair, release it now @@ -679,6 +699,8 @@ static esp_err_t gdma_del_rx_channel(gdma_channel_t *dma_channel) ESP_LOGD(TAG, "uninstall interrupt service for rx channel (%d,%d)", group_id, pair_id); } + gdma_ll_rx_set_priority(group->hal.dev, pair_id, 0); // reset the priority to 0 (lowest) + free(rx_chan); ESP_LOGD(TAG, "del rx channel (%d,%d)", group_id, pair_id); gdma_release_pair_handle(pair); diff --git a/components/esp_hw_support/include/esp_private/gdma.h b/components/esp_hw_support/include/esp_private/gdma.h index d71f3c6fc8..08c3f8f688 100644 --- a/components/esp_hw_support/include/esp_private/gdma.h +++ b/components/esp_hw_support/include/esp_private/gdma.h @@ -177,14 +177,28 @@ esp_err_t gdma_set_transfer_ability(gdma_channel_handle_t dma_chan, const gdma_t /** * @brief Apply channel strategy for GDMA channel * - * @param dma_chan GDMA channel handle, allocated by `gdma_new_channel` - * @param config Configuration of GDMA channel strategy + * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` + * @param[in] config Configuration of GDMA channel strategy * - ESP_OK: Apply channel strategy successfully * - ESP_ERR_INVALID_ARG: Apply channel strategy failed because of invalid argument * - ESP_FAIL: Apply channel strategy failed because of other error */ esp_err_t gdma_apply_strategy(gdma_channel_handle_t dma_chan, const gdma_strategy_config_t *config); +/** + * @brief Set GDMA channel priority + * + * @note By default, all GDMA channels are with the same priority: 0. Channels with the same priority are served in round-robin manner. + * + * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` + * @param[in] priority Priority of GDMA channel, higher value means higher priority + * @return + * - ESP_OK: Set GDMA channel priority successfully + * - ESP_ERR_INVALID_ARG: Set GDMA channel priority failed because of invalid argument, e.g. priority out of range [0,GDMA_LL_CHANNEL_MAX_PRIORITY] + * - ESP_FAIL: Set GDMA channel priority failed because of other error + */ +esp_err_t gdma_set_priority(gdma_channel_handle_t dma_chan, uint32_t priority); + /** * @brief Delete GDMA channel * @note If you call `gdma_new_channel` several times for a same peripheral, make sure you call this API the same times. diff --git a/components/hal/esp32c2/include/hal/gdma_ll.h b/components/hal/esp32c2/include/hal/gdma_ll.h index 639309f972..966e7836f0 100644 --- a/components/hal/esp32c2/include/hal/gdma_ll.h +++ b/components/hal/esp32c2/include/hal/gdma_ll.h @@ -18,6 +18,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x06A7) #define GDMA_LL_TX_EVENT_MASK (0x1958) diff --git a/components/hal/esp32c3/include/hal/gdma_ll.h b/components/hal/esp32c3/include/hal/gdma_ll.h index 617b118f21..3ccb5a0fac 100644 --- a/components/hal/esp32c3/include/hal/gdma_ll.h +++ b/components/hal/esp32c3/include/hal/gdma_ll.h @@ -18,6 +18,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x06A7) #define GDMA_LL_TX_EVENT_MASK (0x1958) diff --git a/components/hal/esp32c6/include/hal/gdma_ll.h b/components/hal/esp32c6/include/hal/gdma_ll.h index 21c55bfdad..57969ca513 100644 --- a/components/hal/esp32c6/include/hal/gdma_ll.h +++ b/components/hal/esp32c6/include/hal/gdma_ll.h @@ -19,6 +19,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x7F) #define GDMA_LL_TX_EVENT_MASK (0x3F) diff --git a/components/hal/esp32h2/include/hal/gdma_ll.h b/components/hal/esp32h2/include/hal/gdma_ll.h index 21c55bfdad..57969ca513 100644 --- a/components/hal/esp32h2/include/hal/gdma_ll.h +++ b/components/hal/esp32h2/include/hal/gdma_ll.h @@ -19,6 +19,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x7F) #define GDMA_LL_TX_EVENT_MASK (0x3F) diff --git a/components/hal/esp32h4/include/hal/gdma_ll.h b/components/hal/esp32h4/include/hal/gdma_ll.h index 617b118f21..3ccb5a0fac 100644 --- a/components/hal/esp32h4/include/hal/gdma_ll.h +++ b/components/hal/esp32h4/include/hal/gdma_ll.h @@ -18,6 +18,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x06A7) #define GDMA_LL_TX_EVENT_MASK (0x1958) diff --git a/components/hal/esp32s3/include/hal/gdma_ll.h b/components/hal/esp32s3/include/hal/gdma_ll.h index 2fc7029303..88ca2b91c4 100644 --- a/components/hal/esp32s3/include/hal/gdma_ll.h +++ b/components/hal/esp32s3/include/hal/gdma_ll.h @@ -18,6 +18,8 @@ extern "C" { #define GDMA_LL_GET_HW(id) (((id) == 0) ? (&GDMA) : NULL) +#define GDMA_LL_CHANNEL_MAX_PRIORITY 5 // supported priority levels: [0,5] + #define GDMA_LL_RX_EVENT_MASK (0x3FF) #define GDMA_LL_TX_EVENT_MASK (0xFF) From 6c19e7b8a782e3440bd4eec2e9515deccb6c3ace Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 3 Apr 2023 15:55:00 +0800 Subject: [PATCH 2/2] gdma: avoid manually start/stop when channel is controled by ETM --- .../bootloader_support/main/test_app_main.c | 5 +++-- components/esp_hw_support/dma/gdma.c | 14 ++++++-------- components/esp_hw_support/dma/gdma_etm.c | 3 +++ components/esp_hw_support/dma/gdma_priv.h | 3 +++ .../esp_hw_support/include/esp_private/gdma.h | 3 +++ 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/components/bootloader_support/test_apps/bootloader_support/main/test_app_main.c b/components/bootloader_support/test_apps/bootloader_support/main/test_app_main.c index b920de7e30..1027ff129f 100644 --- a/components/bootloader_support/test_apps/bootloader_support/main/test_app_main.c +++ b/components/bootloader_support/test_apps/bootloader_support/main/test_app_main.c @@ -9,8 +9,9 @@ #include "esp_heap_caps.h" -// Some resources are lazy allocated (newlib locks) in the bootloader support code, the threshold is left for that case -#define TEST_MEMORY_LEAK_THRESHOLD (-650) +// Some resources are lazy allocated, e.g. newlib locks, GDMA channel lazy installed by crypto driver +// the threshold is left for those cases +#define TEST_MEMORY_LEAK_THRESHOLD (-700) static size_t before_free_8bit; static size_t before_free_32bit; diff --git a/components/esp_hw_support/dma/gdma.c b/components/esp_hw_support/dma/gdma.c index 0cbd5ae455..1ecaa5e9b8 100644 --- a/components/esp_hw_support/dma/gdma.c +++ b/components/esp_hw_support/dma/gdma.c @@ -451,10 +451,10 @@ err: esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr) { - esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE_ISR(dma_chan->flags.start_stop_by_etm == false, ESP_ERR_INVALID_STATE, TAG, "channel is controlled by ETM"); pair = dma_chan->pair; group = pair->group; @@ -468,16 +468,15 @@ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr) } portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); -err: - return ret; + return ESP_OK; } esp_err_t gdma_stop(gdma_channel_handle_t dma_chan) { - esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE_ISR(dma_chan->flags.start_stop_by_etm == false, ESP_ERR_INVALID_STATE, TAG, "channel is controlled by ETM"); pair = dma_chan->pair; group = pair->group; @@ -489,8 +488,7 @@ esp_err_t gdma_stop(gdma_channel_handle_t dma_chan) } portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); -err: - return ret; + return ESP_OK; } esp_err_t gdma_append(gdma_channel_handle_t dma_chan) diff --git a/components/esp_hw_support/dma/gdma_etm.c b/components/esp_hw_support/dma/gdma_etm.c index 84c87c88b8..f849a40e3c 100644 --- a/components/esp_hw_support/dma/gdma_etm.c +++ b/components/esp_hw_support/dma/gdma_etm.c @@ -46,6 +46,7 @@ static esp_err_t gdma_del_etm_task(esp_etm_task_t *task) gdma_ll_tx_enable_etm_task(group->hal.dev, pair->pair_id, false); } free(gdma_task); + dma_chan->flags.start_stop_by_etm = false; return ESP_OK; } @@ -105,6 +106,8 @@ esp_err_t gdma_new_etm_task(gdma_channel_handle_t dma_chan, const gdma_etm_task_ } ESP_GOTO_ON_FALSE(task_id != 0, ESP_ERR_NOT_SUPPORTED, err, TAG, "not supported task type"); + // set a flag, now the GDMA channel is start/stop by ETM subsystem + dma_chan->flags.start_stop_by_etm = true; // fill the ETM task object task->chan = dma_chan; task->base.task_id = task_id; diff --git a/components/esp_hw_support/dma/gdma_priv.h b/components/esp_hw_support/dma/gdma_priv.h index 8dd70aa649..572a3502d1 100644 --- a/components/esp_hw_support/dma/gdma_priv.h +++ b/components/esp_hw_support/dma/gdma_priv.h @@ -67,6 +67,9 @@ struct gdma_channel_t { size_t sram_alignment; // alignment for memory in SRAM size_t psram_alignment; // alignment for memory in PSRAM 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 + } flags; }; struct gdma_tx_channel_t { diff --git a/components/esp_hw_support/include/esp_private/gdma.h b/components/esp_hw_support/include/esp_private/gdma.h index 08c3f8f688..bf5d97dd32 100644 --- a/components/esp_hw_support/include/esp_private/gdma.h +++ b/components/esp_hw_support/include/esp_private/gdma.h @@ -265,6 +265,7 @@ esp_err_t gdma_register_rx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ * @return * - ESP_OK: Start DMA engine successfully * - ESP_ERR_INVALID_ARG: Start DMA engine failed because of invalid argument + * - ESP_ERR_INVALID_STATE: Start DMA engine failed because of invalid state, e.g. the channel is controlled by ETM, so can't start it manually * - ESP_FAIL: Start DMA engine failed because of other error */ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr); @@ -279,6 +280,7 @@ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr); * @return * - ESP_OK: Stop DMA engine successfully * - ESP_ERR_INVALID_ARG: Stop DMA engine failed because of invalid argument + * - ESP_ERR_INVALID_STATE: Stop DMA engine failed because of invalid state, e.g. the channel is controlled by ETM, so can't stop it manually * - ESP_FAIL: Stop DMA engine failed because of other error */ esp_err_t gdma_stop(gdma_channel_handle_t dma_chan); @@ -347,6 +349,7 @@ typedef struct { * @brief Get the ETM task for GDMA channel * * @note The created ETM task object can be deleted later by calling `esp_etm_del_task` + * @note If the GDMA task (e.g. start/stop) is controlled by ETM, then you can't use `gdma_start`/`gdma_stop` to control it. * * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` * @param[in] config GDMA ETM task configuration