Merge branch 'refactor/mipi_lcd_iram_safe' into 'master'

test(rmt): the IO level can keep at low level after channel delete

Closes IDFGH-14255

See merge request espressif/esp-idf!34186

components/esp_driver_rmt/Kconfig

@@ -1,22 +1,36 @@
 menu "ESP-Driver:RMT Configurations"
     depends on SOC_RMT_SUPPORTED
-    config RMT_ISR_IRAM_SAFE
-        bool "RMT ISR IRAM-Safe"
-        default n
-        select GDMA_ISR_IRAM_SAFE if SOC_RMT_SUPPORT_DMA     # RMT basic functionality relies on GDMA callback
-        select GDMA_CTRL_FUNC_IN_IRAM if SOC_RMT_SUPPORT_DMA # RMT needs to restart the GDMA in the interrupt
+    config RMT_ISR_HANDLER_IN_IRAM
+        bool "Place RMT ISR handler into IRAM"
+        select GDMA_CTRL_FUNC_IN_IRAM if SOC_RMT_SUPPORT_DMA
+        select RMT_OBJ_CACHE_SAFE
+        default y
         help
-            Ensure the RMT interrupt is IRAM-Safe by allowing the interrupt handler to be
-            executable when the cache is disabled (e.g. SPI Flash write).
+            Place RMT ISR handler into IRAM for better performance and fewer cache misses.
 
     config RMT_RECV_FUNC_IN_IRAM
         bool "Place RMT receive function into IRAM"
         default n
-        select GDMA_CTRL_FUNC_IN_IRAM if SOC_RMT_SUPPORT_DMA # RMT needs to start the GDMA in the receive function
+        select GDMA_CTRL_FUNC_IN_IRAM if SOC_RMT_SUPPORT_DMA
+        select RMT_OBJ_CACHE_SAFE
+        help
+            Place RMT receive function into IRAM for better performance and fewer cache misses.
+
+    config RMT_ISR_CACHE_SAFE
+        bool "RMT ISR Cache-Safe"
+        select GDMA_ISR_IRAM_SAFE if SOC_RMT_SUPPORT_DMA
+        select RMT_ISR_HANDLER_IN_IRAM
+        default n
+        help
+            Ensure the RMT interrupt is Cache-Safe by allowing the interrupt handler to be
+            executable when the cache is disabled (e.g. SPI Flash write).
+
+    config RMT_OBJ_CACHE_SAFE
+        bool
+        default n
         help
-            Place RMT receive function into IRAM,
-            so that the receive function can be IRAM-safe and able to be called when the flash cache is disabled.
-            Enabling this option can improve driver performance as well.
+            This will ensure the RMT object will not be allocated from a memory region
+            where its cache can be disabled.
 
     config RMT_ENABLE_DEBUG_LOG
         bool "Enable debug log"

components/esp_driver_rmt/include/driver/rmt_rx.h

@@ -18,7 +18,7 @@ extern "C" {
 /**
  * @brief Group of RMT RX callbacks
  * @note The callbacks are all running under ISR environment
- * @note When CONFIG_RMT_ISR_IRAM_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
+ * @note When CONFIG_RMT_ISR_CACHE_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
  *       The variables used in the function should be in the SRAM as well.
  */
 typedef struct {
@@ -100,7 +100,7 @@ esp_err_t rmt_receive(rmt_channel_handle_t rx_channel, void *buffer, size_t buff
  * @brief Set callbacks for RMT RX channel
  *
  * @note User can deregister a previously registered callback by calling this function and setting the callback member in the `cbs` structure to NULL.
- * @note When CONFIG_RMT_ISR_IRAM_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
+ * @note When CONFIG_RMT_ISR_CACHE_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
  *       The variables used in the function should be in the SRAM as well. The `user_data` should also reside in SRAM.
  *
  * @param[in] rx_channel RMT generic channel that created by `rmt_new_rx_channel()`

components/esp_driver_rmt/include/driver/rmt_tx.h

@@ -19,7 +19,7 @@ extern "C" {
 /**
  * @brief Group of RMT TX callbacks
  * @note The callbacks are all running under ISR environment
- * @note When CONFIG_RMT_ISR_IRAM_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
+ * @note When CONFIG_RMT_ISR_CACHE_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
  *       The variables used in the function should be in the SRAM as well.
  */
 typedef struct {
@@ -128,7 +128,7 @@ esp_err_t rmt_tx_wait_all_done(rmt_channel_handle_t tx_channel, int timeout_ms);
  * @brief Set event callbacks for RMT TX channel
  *
  * @note User can deregister a previously registered callback by calling this function and setting the callback member in the `cbs` structure to NULL.
- * @note When CONFIG_RMT_ISR_IRAM_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
+ * @note When CONFIG_RMT_ISR_CACHE_SAFE is enabled, the callback itself and functions called by it should be placed in IRAM.
  *       The variables used in the function should be in the SRAM as well. The `user_data` should also reside in SRAM.
  *
  * @param[in] tx_channel RMT generic channel that created by `rmt_new_tx_channel()`

components/esp_driver_rmt/linker.lf

@@ -1,6 +1,12 @@
 [mapping:rmt_driver]
 archive: libesp_driver_rmt.a
 entries:
+    if RMT_ISR_HANDLER_IN_IRAM = y:
+        rmt_tx: rmt_tx_default_isr (noflash)
+        rmt_rx: rmt_rx_default_isr (noflash)
+        if SOC_RMT_SUPPORT_DMA = y:
+            rmt_tx: rmt_dma_tx_eof_cb (noflash)
+            rmt_rx: rmt_dma_rx_one_block_cb (noflash)
     if RMT_RECV_FUNC_IN_IRAM = y:
         rmt_rx: rmt_receive (noflash)
         if SOC_RMT_SUPPORT_DMA = y:

components/esp_driver_rmt/sdkconfig.rename

@@ -0,0 +1,4 @@
+# sdkconfig replacement configurations for deprecated options formatted as
+# CONFIG_DEPRECATED_OPTION CONFIG_NEW_OPTION
+
+CONFIG_RMT_ISR_IRAM_SAFE       CONFIG_RMT_ISR_CACHE_SAFE

components/esp_driver_rmt/src/rmt_private.h

@@ -34,14 +34,14 @@
 extern "C" {
 #endif
 
-#if CONFIG_RMT_ISR_IRAM_SAFE || CONFIG_RMT_RECV_FUNC_IN_IRAM
+#if CONFIG_RMT_OBJ_CACHE_SAFE
 #define RMT_MEM_ALLOC_CAPS      (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)
 #else
 #define RMT_MEM_ALLOC_CAPS      MALLOC_CAP_DEFAULT
 #endif
 
 // RMT driver object is per-channel, the interrupt source is shared between channels
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
 #define RMT_INTR_ALLOC_FLAG     (ESP_INTR_FLAG_SHARED | ESP_INTR_FLAG_IRAM)
 #else
 #define RMT_INTR_ALLOC_FLAG     (ESP_INTR_FLAG_SHARED)

components/esp_driver_rmt/src/rmt_rx.c

@@ -364,7 +364,7 @@ esp_err_t rmt_rx_register_event_callbacks(rmt_channel_handle_t channel, const rm
     ESP_RETURN_ON_FALSE(channel->direction == RMT_CHANNEL_DIRECTION_RX, ESP_ERR_INVALID_ARG, TAG, "invalid channel direction");
     rmt_rx_channel_t *rx_chan = __containerof(channel, rmt_rx_channel_t, base);
 
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
     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 callback not in IRAM");
     }
@@ -742,7 +742,7 @@ static bool IRAM_ATTR rmt_isr_handle_rx_threshold(rmt_rx_channel_t *rx_chan)
 }
 #endif // SOC_RMT_SUPPORT_RX_PINGPONG
 
-static void IRAM_ATTR rmt_rx_default_isr(void *args)
+static void rmt_rx_default_isr(void *args)
 {
     rmt_rx_channel_t *rx_chan = (rmt_rx_channel_t *)args;
     rmt_channel_t *channel = &rx_chan->base;
@@ -797,7 +797,7 @@ static size_t IRAM_ATTR rmt_rx_count_symbols_for_single_block(rmt_rx_channel_t *
     return received_bytes / sizeof(rmt_symbol_word_t);
 }
 
-static bool IRAM_ATTR rmt_dma_rx_one_block_cb(gdma_channel_handle_t dma_chan, gdma_event_data_t *event_data, void *user_data)
+static bool rmt_dma_rx_one_block_cb(gdma_channel_handle_t dma_chan, gdma_event_data_t *event_data, void *user_data)
 {
     bool need_yield = false;
     rmt_rx_channel_t *rx_chan = (rmt_rx_channel_t *)user_data;

components/esp_driver_rmt/src/rmt_tx.c

@@ -528,7 +528,7 @@ esp_err_t rmt_tx_register_event_callbacks(rmt_channel_handle_t channel, const rm
     ESP_RETURN_ON_FALSE(channel->direction == RMT_CHANNEL_DIRECTION_TX, ESP_ERR_INVALID_ARG, TAG, "invalid channel direction");
     rmt_tx_channel_t *tx_chan = __containerof(channel, rmt_tx_channel_t, base);
 
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
     if (cbs->on_trans_done) {
         ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_trans_done), ESP_ERR_INVALID_ARG, TAG, "on_trans_done callback not in IRAM");
     }
@@ -549,7 +549,7 @@ esp_err_t rmt_transmit(rmt_channel_handle_t channel, rmt_encoder_t *encoder, con
 #if !SOC_RMT_SUPPORT_TX_LOOP_COUNT
     ESP_RETURN_ON_FALSE(config->loop_count <= 0, ESP_ERR_NOT_SUPPORTED, TAG, "loop count is not supported");
 #endif // !SOC_RMT_SUPPORT_TX_LOOP_COUNT
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
     // payload is retrieved by the encoder, we should make sure it's still accessible even when the cache is disabled
     ESP_RETURN_ON_FALSE(esp_ptr_internal(payload), ESP_ERR_INVALID_ARG, TAG, "payload not in internal RAM");
 #endif
@@ -1071,7 +1071,7 @@ static bool IRAM_ATTR rmt_isr_handle_tx_loop_end(rmt_tx_channel_t *tx_chan)
 }
 #endif // SOC_RMT_SUPPORT_TX_LOOP_COUNT
 
-static void IRAM_ATTR rmt_tx_default_isr(void *args)
+static void rmt_tx_default_isr(void *args)
 {
     rmt_tx_channel_t *tx_chan = (rmt_tx_channel_t *)args;
     rmt_channel_t *channel = &tx_chan->base;
@@ -1112,7 +1112,7 @@ static void IRAM_ATTR rmt_tx_default_isr(void *args)
 }
 
 #if SOC_RMT_SUPPORT_DMA
-static bool IRAM_ATTR rmt_dma_tx_eof_cb(gdma_channel_handle_t dma_chan, gdma_event_data_t *event_data, void *user_data)
+static bool rmt_dma_tx_eof_cb(gdma_channel_handle_t dma_chan, gdma_event_data_t *event_data, void *user_data)
 {
     rmt_tx_channel_t *tx_chan = (rmt_tx_channel_t *)user_data;
     // tx_eof_desc_addr must be non-zero, guaranteed by the hardware

components/esp_driver_rmt/test_apps/rmt/main/CMakeLists.txt

@@ -4,8 +4,8 @@ set(srcs "test_app_main.c"
          "test_rmt_rx.c"
          "test_util_rmt_encoders.c")
 
-if(CONFIG_RMT_ISR_IRAM_SAFE)
-    list(APPEND srcs "test_rmt_iram.c")
+if(CONFIG_RMT_ISR_CACHE_SAFE)
+    list(APPEND srcs "test_rmt_cache_safe.c")
 endif()
 
 if(CONFIG_SOC_LIGHT_SLEEP_SUPPORTED AND CONFIG_PM_ENABLE)

components/esp_driver_rmt/test_apps/rmt/main/test_rmt_iram.c → components/esp_driver_rmt/test_apps/rmt/main/test_rmt_cache_safe.c

@@ -79,7 +79,7 @@ static void test_rmt_tx_iram_safe(size_t mem_block_symbols, bool with_dma)
     TEST_ESP_OK(rmt_del_encoder(led_strip_encoder));
 }
 
-TEST_CASE("rmt tx iram safe", "[rmt]")
+TEST_CASE("rmt tx works with cache disabled", "[rmt]")
 {
     test_rmt_tx_iram_safe(SOC_RMT_MEM_WORDS_PER_CHANNEL, false);
 #if SOC_RMT_SUPPORT_DMA
@@ -190,7 +190,7 @@ static void test_rmt_rx_iram_safe(size_t mem_block_symbols, bool with_dma, rmt_c
     free(remote_codes);
 }
 
-TEST_CASE("rmt rx iram safe", "[rmt]")
+TEST_CASE("rmt rx works with cache disabled", "[rmt]")
 {
     test_rmt_rx_iram_safe(SOC_RMT_MEM_WORDS_PER_CHANNEL, false, RMT_CLK_SRC_DEFAULT);
 #if SOC_RMT_SUPPORT_DMA

components/esp_driver_rmt/test_apps/rmt/main/test_rmt_common.c

@@ -100,7 +100,7 @@ TEST_CASE("rmt channel install & uninstall", "[rmt]")
 #endif // SOC_RMT_SUPPORT_DMA
 }
 
-TEST_CASE("RMT interrupt priority", "[rmt]")
+TEST_CASE("rmt interrupt priority", "[rmt]")
 {
     rmt_tx_channel_config_t tx_channel_cfg = {
         .mem_block_symbols = SOC_RMT_MEM_WORDS_PER_CHANNEL,

components/esp_driver_rmt/test_apps/rmt/main/test_rmt_rx.c

@@ -16,7 +16,7 @@
 #include "test_util_rmt_encoders.h"
 #include "test_board.h"
 
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
 #define TEST_RMT_CALLBACK_ATTR IRAM_ATTR
 #else
 #define TEST_RMT_CALLBACK_ATTR

components/esp_driver_rmt/test_apps/rmt/main/test_rmt_tx.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -10,25 +10,38 @@
 #include "freertos/task.h"
 #include "unity.h"
 #include "driver/rmt_tx.h"
+#include "driver/gpio.h"
 #include "esp_timer.h"
 #include "soc/soc_caps.h"
 #include "test_util_rmt_encoders.h"
 #include "test_board.h"
 
-#if CONFIG_RMT_ISR_IRAM_SAFE
+#if CONFIG_RMT_ISR_CACHE_SAFE
 #define TEST_RMT_CALLBACK_ATTR IRAM_ATTR
 #else
 #define TEST_RMT_CALLBACK_ATTR
 #endif
 
 TEST_CASE("rmt bytes encoder", "[rmt]")
 {
+    // If you want to keep the IO level after unintall the RMT channel, a workaround is to set the pull up/down register here
+    // because when we uninstall the RMT channel, we will also disable the GPIO output
+    gpio_config_t io_conf = {
+        .pin_bit_mask = (1ULL << TEST_RMT_GPIO_NUM_B),
+        .mode = GPIO_MODE_INPUT,
+        .pull_up_en = GPIO_PULLUP_DISABLE,
+        // Note: make sure the test board doesn't have a pull-up resistor attached to the GPIO, otherwise this test case will fail in the check
+        .pull_down_en = GPIO_PULLDOWN_ENABLE,
+        .intr_type = GPIO_INTR_DISABLE,
+    };
+    TEST_ESP_OK(gpio_config(&io_conf));
+
     rmt_tx_channel_config_t tx_channel_cfg = {
         .mem_block_symbols = SOC_RMT_MEM_WORDS_PER_CHANNEL,
         .clk_src = RMT_CLK_SRC_DEFAULT,
         .resolution_hz = 1000000, // 1MHz, 1 tick = 1us
         .trans_queue_depth = 4,
-        .gpio_num = TEST_RMT_GPIO_NUM_A,
+        .gpio_num = TEST_RMT_GPIO_NUM_B,
         .intr_priority = 3
     };
     printf("install tx channel\r\n");
@@ -73,14 +86,31 @@ TEST_CASE("rmt bytes encoder", "[rmt]")
     vTaskDelay(pdMS_TO_TICKS(500));
 
     printf("disable tx channel\r\n");
+    TEST_ESP_OK(rmt_disable(tx_channel));
+
+    printf("remove tx channel\r\n");
+    TEST_ESP_OK(rmt_del_channel(tx_channel));
+    vTaskDelay(pdMS_TO_TICKS(500));
+
+    // check the IO level after uninstall the RMT channel
+    TEST_ASSERT_EQUAL(0, gpio_get_level(TEST_RMT_GPIO_NUM_B));
+
+    printf("install tx channel again\r\n");
+    TEST_ESP_OK(rmt_new_tx_channel(&tx_channel_cfg, &tx_channel));
+    printf("enable tx channel\r\n");
+    TEST_ESP_OK(rmt_enable(tx_channel));
+
+    printf("start transaction\r\n");
+    TEST_ESP_OK(rmt_transmit(tx_channel, bytes_encoder, (uint8_t[]) {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05
+    }, 6, &transmit_config));
+    // adding extra delay here for visualizing
+    vTaskDelay(pdMS_TO_TICKS(500));
+
     TEST_ESP_OK(rmt_disable(tx_channel));
     printf("remove tx channel and encoder\r\n");
     TEST_ESP_OK(rmt_del_channel(tx_channel));
     TEST_ESP_OK(rmt_del_encoder(bytes_encoder));
-
-    // Test if intr_priority check works
-    tx_channel_cfg.intr_priority = 4;  // 4 is an invalid interrupt priority
-    TEST_ESP_ERR(rmt_new_tx_channel(&tx_channel_cfg, &tx_channel), ESP_ERR_INVALID_ARG);
 }
 
 static void test_rmt_channel_single_trans(size_t mem_block_symbols, bool with_dma)

components/esp_driver_rmt/test_apps/rmt/pytest_rmt.py

@@ -1,4 +1,4 @@
-# SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: CC0-1.0
 import pytest
 from pytest_embedded import Dut
@@ -15,7 +15,7 @@
 @pytest.mark.parametrize(
     'config',
     [
-        'iram_safe',
+        'cache_safe',
         'release',
     ],
     indirect=True,
@@ -29,7 +29,7 @@ def test_rmt(dut: Dut) -> None:
 @pytest.mark.parametrize(
     'config',
     [
-        'iram_safe',
+        'cache_safe',
         'release',
     ],
     indirect=True,

components/esp_driver_rmt/test_apps/rmt/sdkconfig.ci.iram_safe → components/esp_driver_rmt/test_apps/rmt/sdkconfig.ci.cache_safe

@@ -1,5 +1,5 @@
 CONFIG_COMPILER_DUMP_RTL_FILES=y
-CONFIG_RMT_ISR_IRAM_SAFE=y
+CONFIG_RMT_ISR_CACHE_SAFE=y
 CONFIG_RMT_RECV_FUNC_IN_IRAM=y
 CONFIG_GPIO_CTRL_FUNC_IN_IRAM=y
 CONFIG_COMPILER_OPTIMIZATION_NONE=y

docs/en/api-reference/peripherals/rmt.rst

@@ -63,7 +63,7 @@ The description of the RMT functionality is divided into the following sections:
 - :ref:`rmt-multiple-channels-simultaneous-transmission` - describes how to collect multiple channels into a sync group so that their transmissions can be started simultaneously.
 - :ref:`rmt-rmt-encoder` - focuses on how to write a customized encoder by combining multiple primitive encoders that are provided by the driver.
 - :ref:`rmt-power-management` - describes how different clock sources affects power consumption.
-- :ref:`rmt-iram-safe` - describes how disabling the cache affects the RMT driver, and tips to mitigate it.
+- :ref:`rmt-cache-safe` - describes how disabling the cache affects the RMT driver, and tips to mitigate it.
 - :ref:`rmt-thread-safety` - lists which APIs are guaranteed to be thread-safe by the driver.
 - :ref:`rmt-kconfig-options` - describes the various Kconfig options supported by the RMT driver.
 
@@ -560,14 +560,14 @@ The driver can prevent the above issue by creating a power management lock. The
 
     Besides the potential changes to the clock source, when the power management is enabled, the system can also power down the RMT hardware before sleep. Set :cpp:member:`rmt_tx_channel_config_t::allow_pd` and :cpp:member:`rmt_rx_channel_config_t::allow_pd` to ``true`` to enable the power down feature. RMT registers will be backed up before sleep and restored after wake up. Please note, enabling this option will increase the memory consumption.
 
-.. _rmt-iram-safe:
+.. _rmt-cache-safe:
 
-IRAM Safe
-^^^^^^^^^
+Cache Safe
+^^^^^^^^^^
 
 By default, the RMT interrupt is deferred when the Cache is disabled for reasons like writing or erasing the main Flash. Thus the transaction-done interrupt does not get handled in time, which is not acceptable in a real-time application. What is worse, when the RMT transaction relies on **ping-pong** interrupt to successively encode or copy RMT symbols, a delayed interrupt can lead to an unpredictable result.
 
-There is a Kconfig option :ref:`CONFIG_RMT_ISR_IRAM_SAFE` that has the following features:
+There is a Kconfig option :ref:`CONFIG_RMT_ISR_CACHE_SAFE` that has the following features:
 
 1. Enable the interrupt being serviced even when the cache is disabled
 2. Place all functions used by the ISR into IRAM [2]_
@@ -594,9 +594,9 @@ The following functions are allowed to use under ISR context as well.
 Kconfig Options
 ^^^^^^^^^^^^^^^
 
-- :ref:`CONFIG_RMT_ISR_IRAM_SAFE` controls whether the default ISR handler can work when cache is disabled, see also :ref:`rmt-iram-safe` for more information.
+- :ref:`CONFIG_RMT_ISR_CACHE_SAFE` controls whether the default ISR handler can work when cache is disabled, see also :ref:`rmt-cache-safe` for more information.
 - :ref:`CONFIG_RMT_ENABLE_DEBUG_LOG` is used to enable the debug log at the cost of increased firmware binary size.
-- :ref:`CONFIG_RMT_RECV_FUNC_IN_IRAM` controls where to place the RMT receive function (IRAM or Flash), see :ref:`rmt-iram-safe` for more information.
+- :ref:`CONFIG_RMT_RECV_FUNC_IN_IRAM` controls where to place the RMT receive function (IRAM or Flash), see :ref:`rmt-cache-safe` for more information.
 
 Application Examples
 --------------------