From 4084c88a2e83b86c2ba7f43c211c7fb443b027f5 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 26 Mar 2026 16:09:15 +0800 Subject: [PATCH] refactor(rmt): switch group interrupts to private shared binding Bind RMT channel interrupts by module name and allocate them as private shared so channels in the same RMT instance reuse one CPU interrupt line without exposing it to other peripherals. Drop RMT-side interrupt priority conflict state and let the interrupt allocator enforce compatibility. Use soc_rmt_signals[group_id].module_name when creating RMT PM locks and drop the per-channel formatted lock name buffer. This keeps naming consistent with interrupt binding and removes unnecessary string formatting state. --- components/esp_driver_rmt/src/rmt_common.c | 51 +------------------ components/esp_driver_rmt/src/rmt_private.h | 29 ++--------- components/esp_driver_rmt/src/rmt_rx.c | 20 ++++---- components/esp_driver_rmt/src/rmt_tx.c | 22 ++++---- components/esp_hal_rmt/esp32/rmt_periph.c | 1 + components/esp_hal_rmt/esp32c3/rmt_periph.c | 1 + components/esp_hal_rmt/esp32c5/rmt_periph.c | 1 + components/esp_hal_rmt/esp32c6/rmt_periph.c | 1 + components/esp_hal_rmt/esp32h2/rmt_periph.c | 1 + components/esp_hal_rmt/esp32h21/rmt_periph.c | 1 + components/esp_hal_rmt/esp32h4/rmt_periph.c | 1 + components/esp_hal_rmt/esp32p4/rmt_periph.c | 1 + components/esp_hal_rmt/esp32s2/rmt_periph.c | 1 + components/esp_hal_rmt/esp32s3/rmt_periph.c | 1 + .../esp_hal_rmt/include/hal/rmt_periph.h | 1 + 15 files changed, 38 insertions(+), 95 deletions(-) diff --git a/components/esp_driver_rmt/src/rmt_common.c b/components/esp_driver_rmt/src/rmt_common.c index 05670cbc505..9a913910641 100644 --- a/components/esp_driver_rmt/src/rmt_common.c +++ b/components/esp_driver_rmt/src/rmt_common.c @@ -39,8 +39,6 @@ rmt_group_t *rmt_acquire_group_handle(int group_id) group->occupy_mask = UINT32_MAX & ~((1 << RMT_LL_GET(CHANS_PER_INST)) - 1); // group clock won't be configured at this stage, it will be set when allocate the first channel group->clk_src = 0; - // group interrupt priority is shared between all channels, it will be set when allocate the first channel - group->intr_priority = RMT_GROUP_INTR_PRIORITY_UNINITIALIZED; // enable the bus clock for the RMT peripheral PERIPH_RCC_ATOMIC() { rmt_ll_enable_bus_clock(group_id, true); @@ -217,8 +215,7 @@ esp_err_t rmt_select_periph_clock(rmt_channel_handle_t chan, rmt_clock_source_t // note, even if the clock source is APB, we still use CPU_FREQ_MAX lock to ensure the stability of the RMT operation. esp_pm_lock_type_t pm_lock_type = chan->dma_chan ? ESP_PM_NO_LIGHT_SLEEP : ESP_PM_CPU_FREQ_MAX; - sprintf(chan->pm_lock_name, "rmt_%d_%d", group->group_id, chan->channel_id); // e.g. rmt_0_0 - ret = esp_pm_lock_create(pm_lock_type, 0, chan->pm_lock_name, &chan->pm_lock); + ret = esp_pm_lock_create(pm_lock_type, 0, soc_rmt_signals[group->group_id].module_name, &chan->pm_lock); ESP_RETURN_ON_ERROR(ret, TAG, "create pm lock failed"); #endif // CONFIG_PM_ENABLE @@ -293,52 +290,6 @@ esp_err_t rmt_disable(rmt_channel_handle_t channel) return channel->disable(channel); } -bool rmt_set_intr_priority_to_group(rmt_group_t *group, int intr_priority) -{ - bool priority_conflict = false; - portENTER_CRITICAL(&group->spinlock); - if (group->intr_priority == RMT_GROUP_INTR_PRIORITY_UNINITIALIZED) { - // intr_priority never allocated, accept user's value unconditionally - // intr_priority could only be set once here - group->intr_priority = intr_priority; - } else { - // group intr_priority already specified - // If interrupt priority specified before, it CANNOT BE CHANGED until `rmt_release_group_handle()` called - // So we have to check if the new priority specified conflicts with the old one - if (intr_priority) { - // User specified intr_priority, check if conflict or not - // Even though the `group->intr_priority` is 0, an intr_priority must have been specified automatically too, - // although we do not know it exactly now, so specifying the intr_priority again might also cause conflict. - // So no matter if `group->intr_priority` is 0 or not, we have to check. - // Value `0` of `group->intr_priority` means "unknown", NOT "unspecified"! - if (intr_priority != (group->intr_priority)) { - // intr_priority conflicts! - priority_conflict = true; - } - } - // else do nothing - // user did not specify intr_priority, then keep the old priority - // We'll use the `RMT_INTR_ALLOC_FLAG | RMT_ALLOW_INTR_PRIORITY_MASK`, which should always success - } - // The `group->intr_priority` will not change any longer, even though another task tries to modify it. - // So we could exit critical here safely. - portEXIT_CRITICAL(&group->spinlock); - return priority_conflict; -} - -int rmt_isr_priority_to_flags(rmt_group_t *group) -{ - int isr_flags = 0; - if (group->intr_priority) { - // Use user-specified priority bit - isr_flags |= (1 << (group->intr_priority)); - } else { - // Allow all LOWMED priority bits - isr_flags |= RMT_ALLOW_INTR_PRIORITY_MASK; - } - return isr_flags; -} - #if RMT_USE_RETENTION_LINK static esp_err_t rmt_create_sleep_retention_link_cb(void *arg) { diff --git a/components/esp_driver_rmt/src/rmt_private.h b/components/esp_driver_rmt/src/rmt_private.h index 13434a0016c..ffd6a6865c5 100644 --- a/components/esp_driver_rmt/src/rmt_private.h +++ b/components/esp_driver_rmt/src/rmt_private.h @@ -61,15 +61,15 @@ extern "C" { // RMT driver object is per-channel, the interrupt source is shared between channels #if CONFIG_RMT_TX_ISR_CACHE_SAFE -#define RMT_TX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED | ESP_INTR_FLAG_IRAM) +#define RMT_TX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED_PRIVATE | ESP_INTR_FLAG_IRAM) #else -#define RMT_TX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED) +#define RMT_TX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED_PRIVATE) #endif #if CONFIG_RMT_RX_ISR_CACHE_SAFE -#define RMT_RX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED | ESP_INTR_FLAG_IRAM) +#define RMT_RX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED_PRIVATE | ESP_INTR_FLAG_IRAM) #else -#define RMT_RX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED) +#define RMT_RX_INTR_ALLOC_FLAG (ESP_INTR_FLAG_SHARED_PRIVATE) #endif // Hopefully the channel offset won't change in other targets @@ -79,8 +79,6 @@ extern "C" { #define RMT_ALLOW_INTR_PRIORITY_MASK ESP_INTR_FLAG_LOWMED #define RMT_DMA_NODES_PING_PONG 2 // two nodes ping-pong -#define RMT_PM_LOCK_NAME_LEN_MAX 16 -#define RMT_GROUP_INTR_PRIORITY_UNINITIALIZED (-1) // RMT is a slow peripheral, it only supports AHB-GDMA #define RMT_DMA_DESC_ALIGN 4 @@ -143,7 +141,6 @@ struct rmt_group_t { rmt_tx_channel_t *tx_channels[RMT_LL_GET(TX_CANDIDATES_PER_INST)]; // array of RMT TX channels rmt_rx_channel_t *rx_channels[RMT_LL_GET(RX_CANDIDATES_PER_INST)]; // array of RMT RX channels rmt_sync_manager_t *sync_manager; // sync manager, this can be extended into an array if there're more sync controllers in one RMT group - int intr_priority; // RMT interrupt priority }; struct rmt_channel_t { @@ -161,7 +158,6 @@ struct rmt_channel_t { gdma_channel_handle_t dma_chan; // DMA channel #if CONFIG_PM_ENABLE esp_pm_lock_handle_t pm_lock; // power management lock - char pm_lock_name[RMT_PM_LOCK_NAME_LEN_MAX]; // pm lock name #endif // RMT channel common interface // The following IO functions will have per-implementation for TX and RX channel @@ -257,23 +253,6 @@ void rmt_release_group_handle(rmt_group_t *group); */ esp_err_t rmt_select_periph_clock(rmt_channel_handle_t chan, rmt_clock_source_t clk_src, uint32_t expect_channel_resolution); -/** - * @brief Set interrupt priority to RMT group - * @param group RMT group to set interrupt priority to - * @param intr_priority User-specified interrupt priority (in num, not bitmask) - * @return If the priority conflicts - * - true: Interrupt priority conflict with previous specified - * - false: Interrupt priority set successfully - */ -bool rmt_set_intr_priority_to_group(rmt_group_t *group, int intr_priority); - -/** - * @brief Convert the interrupt priority to flags - * @param group RMT group - * @return isr_flags which is compatible to `ESP_INTR_FLAG_*` - */ -int rmt_isr_priority_to_flags(rmt_group_t *group); - /** * @brief Create sleep retention link * diff --git a/components/esp_driver_rmt/src/rmt_rx.c b/components/esp_driver_rmt/src/rmt_rx.c index 8bc8142f89d..dbf250b4f27 100644 --- a/components/esp_driver_rmt/src/rmt_rx.c +++ b/components/esp_driver_rmt/src/rmt_rx.c @@ -242,15 +242,17 @@ esp_err_t rmt_new_rx_channel(const rmt_rx_channel_config_t *config, rmt_channel_ // RMT interrupt is mandatory if the channel doesn't use DMA // --- install interrupt service // interrupt is mandatory to run basic RMT transactions, so it's not lazy installed in `rmt_tx_register_event_callbacks()` - // 1-- Set user specified priority to `group->intr_priority` - bool priority_conflict = rmt_set_intr_priority_to_group(group, config->intr_priority); - ESP_GOTO_ON_FALSE(!priority_conflict, ESP_ERR_INVALID_ARG, err, TAG, "intr_priority conflict"); - // 2-- Get interrupt allocation flag - int isr_flags = rmt_isr_priority_to_flags(group) | RMT_RX_INTR_ALLOC_FLAG; - // 3-- Allocate interrupt using isr_flag - ret = esp_intr_alloc_intrstatus(soc_rmt_signals[group_id].irq, isr_flags, - (uint32_t)rmt_ll_get_interrupt_status_reg(hal->regs), - RMT_LL_EVENT_RX_MASK(channel_id), rmt_rx_default_isr, rx_channel, &rx_channel->base.intr); + int isr_flags = (config->intr_priority ? (1 << config->intr_priority) : RMT_ALLOW_INTR_PRIORITY_MASK) | RMT_RX_INTR_ALLOC_FLAG; + esp_intr_alloc_info_t intr_info = { + .source = soc_rmt_signals[group_id].irq, + .flags = isr_flags, + .intrstatusreg = (uint32_t)rmt_ll_get_interrupt_status_reg(hal->regs), + .intrstatusmask = RMT_LL_EVENT_RX_MASK(channel_id), + .handler = rmt_rx_default_isr, + .arg = rx_channel, + .bind_by.name = soc_rmt_signals[group_id].module_name, + }; + ret = esp_intr_alloc_info(&intr_info, &rx_channel->base.intr); ESP_GOTO_ON_ERROR(ret, err, TAG, "install rx interrupt failed"); } diff --git a/components/esp_driver_rmt/src/rmt_tx.c b/components/esp_driver_rmt/src/rmt_tx.c index 660bb126552..fbe3b48037a 100644 --- a/components/esp_driver_rmt/src/rmt_tx.c +++ b/components/esp_driver_rmt/src/rmt_tx.c @@ -300,18 +300,18 @@ esp_err_t rmt_new_tx_channel(const rmt_tx_channel_config_t *config, rmt_channel_ rmt_hal_tx_channel_reset(&group->hal, channel_id); portEXIT_CRITICAL(&group->spinlock); // install tx interrupt - // --- install interrupt service // interrupt is mandatory to run basic RMT transactions, so it's not lazy installed in `rmt_tx_register_event_callbacks()` - // 1-- Set user specified priority to `group->intr_priority` - bool priority_conflict = rmt_set_intr_priority_to_group(group, config->intr_priority); - ESP_GOTO_ON_FALSE(!priority_conflict, ESP_ERR_INVALID_ARG, err, TAG, "intr_priority conflict"); - // 2-- Get interrupt allocation flag - int isr_flags = rmt_isr_priority_to_flags(group) | RMT_TX_INTR_ALLOC_FLAG; - // 3-- Allocate interrupt using isr_flag - ret = esp_intr_alloc_intrstatus(soc_rmt_signals[group_id].irq, isr_flags, - (uint32_t) rmt_ll_get_interrupt_status_reg(hal->regs), - RMT_LL_EVENT_TX_MASK(channel_id), rmt_tx_default_isr, tx_channel, - &tx_channel->base.intr); + int isr_flags = (config->intr_priority ? (1 << config->intr_priority) : RMT_ALLOW_INTR_PRIORITY_MASK) | RMT_TX_INTR_ALLOC_FLAG; + esp_intr_alloc_info_t intr_info = { + .source = soc_rmt_signals[group_id].irq, + .flags = isr_flags, + .intrstatusreg = (uint32_t)rmt_ll_get_interrupt_status_reg(hal->regs), + .intrstatusmask = RMT_LL_EVENT_TX_MASK(channel_id), + .handler = rmt_tx_default_isr, + .arg = tx_channel, + .bind_by.name = soc_rmt_signals[group_id].module_name, + }; + ret = esp_intr_alloc_info(&intr_info, &tx_channel->base.intr); ESP_GOTO_ON_ERROR(ret, err, TAG, "install tx interrupt failed"); // install DMA service #if SOC_RMT_SUPPORT_DMA diff --git a/components/esp_hal_rmt/esp32/rmt_periph.c b/components/esp_hal_rmt/esp32/rmt_periph.c index 84669b64e52..f27af906959 100644 --- a/components/esp_hal_rmt/esp32/rmt_periph.c +++ b/components/esp_hal_rmt/esp32/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32c3/rmt_periph.c b/components/esp_hal_rmt/esp32c3/rmt_periph.c index 1f11259e394..e938ee07b10 100644 --- a/components/esp_hal_rmt/esp32c3/rmt_periph.c +++ b/components/esp_hal_rmt/esp32c3/rmt_periph.c @@ -10,6 +10,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32c5/rmt_periph.c b/components/esp_hal_rmt/esp32c5/rmt_periph.c index 4bc8ffb5b0d..69458a85948 100644 --- a/components/esp_hal_rmt/esp32c5/rmt_periph.c +++ b/components/esp_hal_rmt/esp32c5/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32c6/rmt_periph.c b/components/esp_hal_rmt/esp32c6/rmt_periph.c index e09b7171c38..51ee06886fc 100644 --- a/components/esp_hal_rmt/esp32c6/rmt_periph.c +++ b/components/esp_hal_rmt/esp32c6/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32h2/rmt_periph.c b/components/esp_hal_rmt/esp32h2/rmt_periph.c index e09b7171c38..51ee06886fc 100644 --- a/components/esp_hal_rmt/esp32h2/rmt_periph.c +++ b/components/esp_hal_rmt/esp32h2/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32h21/rmt_periph.c b/components/esp_hal_rmt/esp32h21/rmt_periph.c index 9935ab948d7..8aa6b814472 100644 --- a/components/esp_hal_rmt/esp32h21/rmt_periph.c +++ b/components/esp_hal_rmt/esp32h21/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32h4/rmt_periph.c b/components/esp_hal_rmt/esp32h4/rmt_periph.c index 9935ab948d7..8aa6b814472 100644 --- a/components/esp_hal_rmt/esp32h4/rmt_periph.c +++ b/components/esp_hal_rmt/esp32h4/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32p4/rmt_periph.c b/components/esp_hal_rmt/esp32p4/rmt_periph.c index 13d1181b3f9..a30d52d0ca7 100644 --- a/components/esp_hal_rmt/esp32p4/rmt_periph.c +++ b/components/esp_hal_rmt/esp32p4/rmt_periph.c @@ -11,6 +11,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_PAD_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32s2/rmt_periph.c b/components/esp_hal_rmt/esp32s2/rmt_periph.c index a0f0b17ebac..7f346f58440 100644 --- a/components/esp_hal_rmt/esp32s2/rmt_periph.c +++ b/components/esp_hal_rmt/esp32s2/rmt_periph.c @@ -10,6 +10,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/esp32s3/rmt_periph.c b/components/esp_hal_rmt/esp32s3/rmt_periph.c index 117b933af49..309a4482591 100644 --- a/components/esp_hal_rmt/esp32s3/rmt_periph.c +++ b/components/esp_hal_rmt/esp32s3/rmt_periph.c @@ -10,6 +10,7 @@ const soc_rmt_signal_desc_t soc_rmt_signals[1] = { [0] = { .irq = ETS_RMT_INTR_SOURCE, + .module_name = "rmt0", .channels = { [0] = { .tx_sig = RMT_SIG_OUT0_IDX, diff --git a/components/esp_hal_rmt/include/hal/rmt_periph.h b/components/esp_hal_rmt/include/hal/rmt_periph.h index b51e9c187e2..4effffa3f8a 100644 --- a/components/esp_hal_rmt/include/hal/rmt_periph.h +++ b/components/esp_hal_rmt/include/hal/rmt_periph.h @@ -25,6 +25,7 @@ extern "C" { typedef struct { const int irq; + const char *module_name; struct { struct { const int tx_sig;