Merge branch 'bugfix/gdma_uhci_id_4.4' into 'release/v4.4'

gdma: correct the dma trigger of UHCI  && fix async memcpy conflict with peripheral DMA (v4.4)

See merge request espressif/esp-idf!22007

components/driver/gdma.c

@@ -71,6 +71,8 @@ struct gdma_group_t {
     int group_id;           // Group ID, index from 0
     gdma_hal_context_t hal; // HAL instance is at group level
     portMUX_TYPE spinlock;  // group level spinlock
+    uint32_t tx_periph_in_use_mask; // each bit indicates which peripheral (TX direction) has been occupied
+    uint32_t rx_periph_in_use_mask; // each bit indicates which peripheral (RX direction) has been occupied
     gdma_pair_t *pairs[SOC_GDMA_PAIRS_PER_GROUP];  // handles of GDMA pairs
     int pair_ref_counts[SOC_GDMA_PAIRS_PER_GROUP]; // reference count used to protect pair install/uninstall
 };
@@ -246,53 +248,97 @@ esp_err_t gdma_get_channel_id(gdma_channel_handle_t dma_chan, int *channel_id)
 
 esp_err_t gdma_connect(gdma_channel_handle_t dma_chan, gdma_trigger_t trig_periph)
 {
-    esp_err_t ret = ESP_OK;
     gdma_pair_t *pair = NULL;
     gdma_group_t *group = NULL;
-    ESP_GOTO_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument");
-    ESP_GOTO_ON_FALSE(dma_chan->periph_id == GDMA_INVALID_PERIPH_TRIG, ESP_ERR_INVALID_STATE, err, TAG, "channel is using by peripheral: %d", dma_chan->periph_id);
+    ESP_RETURN_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
+    ESP_RETURN_ON_FALSE(dma_chan->periph_id == GDMA_INVALID_PERIPH_TRIG, ESP_ERR_INVALID_STATE, TAG, "channel is using by peripheral: %d", dma_chan->periph_id);
     pair = dma_chan->pair;
     group = pair->group;
-
-    dma_chan->periph_id = trig_periph.instance_id;
-    // enable/disable m2m mode
-    gdma_ll_enable_m2m_mode(group->hal.dev, pair->pair_id, trig_periph.periph == GDMA_TRIG_PERIPH_M2M);
+    bool periph_conflict = false;
 
     if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_TX) {
-        gdma_ll_tx_reset_channel(group->hal.dev, pair->pair_id); // reset channel
-        if (trig_periph.periph != GDMA_TRIG_PERIPH_M2M) {
-            gdma_ll_tx_connect_to_periph(group->hal.dev, pair->pair_id, trig_periph.instance_id);
+        if (trig_periph.instance_id >= 0) {
+            portENTER_CRITICAL(&group->spinlock);
+            if (group->tx_periph_in_use_mask & (1 << trig_periph.instance_id)) {
+                periph_conflict = true;
+            } else {
+                group->tx_periph_in_use_mask |= (1 << trig_periph.instance_id);
+            }
+            portEXIT_CRITICAL(&group->spinlock);
+        }
+        if (!periph_conflict) {
+            gdma_ll_tx_reset_channel(group->hal.dev, pair->pair_id); // reset channel
+            gdma_ll_tx_connect_to_periph(group->hal.dev, pair->pair_id, trig_periph.periph, trig_periph.instance_id);
         }
     } else {
-        gdma_ll_rx_reset_channel(group->hal.dev, pair->pair_id); // reset channel
-        if (trig_periph.periph != GDMA_TRIG_PERIPH_M2M) {
-            gdma_ll_rx_connect_to_periph(group->hal.dev, pair->pair_id, trig_periph.instance_id);
+        if (trig_periph.instance_id >= 0) {
+            portENTER_CRITICAL(&group->spinlock);
+            if (group->rx_periph_in_use_mask & (1 << trig_periph.instance_id)) {
+                periph_conflict = true;
+            } else {
+                group->rx_periph_in_use_mask |= (1 << trig_periph.instance_id);
+            }
+            portEXIT_CRITICAL(&group->spinlock);
+        }
+        if (!periph_conflict) {
+            gdma_ll_rx_reset_channel(group->hal.dev, pair->pair_id); // reset channel
+            gdma_ll_rx_connect_to_periph(group->hal.dev, pair->pair_id, trig_periph.periph, trig_periph.instance_id);
         }
     }
 
-err:
-    return ret;
+    ESP_RETURN_ON_FALSE(!periph_conflict, ESP_ERR_INVALID_STATE, TAG, "peripheral %d is already used by another channel", trig_periph.instance_id);
+    dma_chan->periph_id = trig_periph.instance_id;
+    return ESP_OK;
 }
 
 esp_err_t gdma_disconnect(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(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument");
-    ESP_GOTO_ON_FALSE(dma_chan->periph_id != GDMA_INVALID_PERIPH_TRIG, ESP_ERR_INVALID_STATE, err, TAG, "no peripheral is connected to the channel");
+    ESP_RETURN_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
+    ESP_RETURN_ON_FALSE(dma_chan->periph_id != GDMA_INVALID_PERIPH_TRIG, ESP_ERR_INVALID_STATE, TAG, "no peripheral is connected to the channel");
+
     pair = dma_chan->pair;
     group = pair->group;
+    int save_periph_id = dma_chan->periph_id;
 
-    dma_chan->periph_id = GDMA_INVALID_PERIPH_TRIG;
     if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_TX) {
-        gdma_ll_tx_connect_to_periph(group->hal.dev, pair->pair_id, GDMA_INVALID_PERIPH_TRIG);
+        if (save_periph_id >= 0) {
+            portENTER_CRITICAL(&group->spinlock);
+            group->tx_periph_in_use_mask &= ~(1 << save_periph_id);
+            portEXIT_CRITICAL(&group->spinlock);
+        }
+        gdma_ll_tx_disconnect_from_periph(group->hal.dev, pair->pair_id);
     } else {
-        gdma_ll_rx_connect_to_periph(group->hal.dev, pair->pair_id, GDMA_INVALID_PERIPH_TRIG);
+        if (save_periph_id >= 0) {
+            portENTER_CRITICAL(&group->spinlock);
+            group->rx_periph_in_use_mask &= ~(1 << save_periph_id);
+            portEXIT_CRITICAL(&group->spinlock);
+        }
+        gdma_ll_rx_disconnect_from_periph(group->hal.dev, pair->pair_id);
     }
 
-err:
-    return ret;
+    dma_chan->periph_id = GDMA_INVALID_PERIPH_TRIG;
+    return ESP_OK;
+}
+
+esp_err_t gdma_get_free_m2m_trig_id_mask(gdma_channel_handle_t dma_chan, uint32_t *mask)
+{
+    gdma_pair_t *pair = NULL;
+    gdma_group_t *group = NULL;
+    ESP_RETURN_ON_FALSE(dma_chan && mask, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
+
+    uint32_t free_mask = GDMA_LL_M2M_FREE_PERIPH_ID_MASK;
+    pair = dma_chan->pair;
+    group = pair->group;
+
+    portENTER_CRITICAL(&group->spinlock);
+    free_mask &= ~(group->tx_periph_in_use_mask);
+    free_mask &= ~(group->rx_periph_in_use_mask);
+    portEXIT_CRITICAL(&group->spinlock);
+
+    *mask = free_mask;
+    return ESP_OK;
 }
 
 esp_err_t gdma_set_transfer_ability(gdma_channel_handle_t dma_chan, const gdma_transfer_ability_t *ability)

components/driver/include/esp_private/gdma.h

@@ -11,6 +11,7 @@
 
 #include <stdbool.h>
 #include "soc/gdma_channel.h"
+#include "hal/gdma_types.h"
 #include "esp_err.h"
 
 #ifdef __cplusplus
@@ -23,34 +24,6 @@ extern "C" {
  */
 typedef struct gdma_channel_t *gdma_channel_handle_t;
 
-/**
- * @brief Enumeration of peripherals which have the DMA capability
- * @note Some peripheral might not be available on certain chip, please refer to `soc_caps.h` for detail.
- *
- */
-typedef enum {
-    GDMA_TRIG_PERIPH_M2M,  /*!< GDMA trigger peripheral: M2M */
-    GDMA_TRIG_PERIPH_UART, /*!< GDMA trigger peripheral: UART */
-    GDMA_TRIG_PERIPH_SPI,  /*!< GDMA trigger peripheral: SPI */
-    GDMA_TRIG_PERIPH_I2S,  /*!< GDMA trigger peripheral: I2S */
-    GDMA_TRIG_PERIPH_AES,  /*!< GDMA trigger peripheral: AES */
-    GDMA_TRIG_PERIPH_SHA,  /*!< GDMA trigger peripheral: SHA */
-    GDMA_TRIG_PERIPH_ADC,  /*!< GDMA trigger peripheral: ADC */
-    GDMA_TRIG_PERIPH_DAC,  /*!< GDMA trigger peripheral: DAC */
-    GDMA_TRIG_PERIPH_LCD,  /*!< GDMA trigger peripheral: LCD */
-    GDMA_TRIG_PERIPH_CAM,  /*!< GDMA trigger peripheral: CAM */
-    GDMA_TRIG_PERIPH_RMT,  /*!< GDMA trigger peripheral: RMT */
-} gdma_trigger_peripheral_t;
-
-/**
- * @brief Enumeration of GDMA channel direction
- *
- */
-typedef enum {
-    GDMA_CHANNEL_DIRECTION_TX, /*!< GDMA channel direction: TX */
-    GDMA_CHANNEL_DIRECTION_RX, /*!< GDMA channel direction: RX */
-} gdma_channel_direction_t;
-
 /**
  * @brief Collection of configuration items that used for allocating GDMA channel
  *
@@ -124,13 +97,13 @@ typedef struct {
  */
 typedef struct {
     gdma_trigger_peripheral_t periph; /*!< Target peripheral which will trigger DMA operations */
-    int instance_id;                  /*!< Peripheral instance ID. Supported IDs are listed in `soc/gdma_channel.h`, e.g. SOC_GDMA_TRIG_PERIPH_UART0 */
+    int instance_id;                  /*!< Peripheral instance ID. Supported IDs are listed in `soc/gdma_channel.h`, e.g. SOC_GDMA_TRIG_PERIPH_UHCI0 */
 } gdma_trigger_t;
 
 /**
  * @brief Helper macro to initialize GDMA trigger
  * @note value of `peri` must be selected from `gdma_trigger_peripheral_t` enum.
- *       e.g. GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_UART,0)
+ *       e.g. GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_I2S,0)
  *
  */
 #define GDMA_MAKE_TRIGGER(peri, id) \
@@ -325,6 +298,22 @@ esp_err_t gdma_append(gdma_channel_handle_t dma_chan);
  */
 esp_err_t gdma_reset(gdma_channel_handle_t dma_chan);
 
+/**
+ * @brief Get the mask of free M2M trigger IDs
+ *
+ * @note On some ESP targets (e.g. ESP32C3/S3), DMA trigger used for memory copy can be any of valid peripheral's trigger ID,
+ *       which can bring conflict if the peripheral is also using the same trigger ID. This function can return the free IDs
+ *       for memory copy, at the runtime.
+ *
+ * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel`
+ * @param[out] mask Returned mask of free M2M trigger IDs
+ * @return
+ *      - ESP_OK: Get free M2M trigger IDs successfully
+ *      - ESP_ERR_INVALID_ARG: Get free M2M trigger IDs failed because of invalid argument
+ *      - ESP_FAIL: Get free M2M trigger IDs failed because of other error
+ */
+esp_err_t gdma_get_free_m2m_trig_id_mask(gdma_channel_handle_t dma_chan, uint32_t *mask);
+
 #ifdef __cplusplus
 }
 #endif

components/driver/test/test_gdma.c

@@ -18,31 +18,27 @@ TEST_CASE("GDMA channel allocation", "[gdma]")
     gdma_tx_event_callbacks_t tx_cbs = {};
     gdma_rx_event_callbacks_t rx_cbs = {};
 
-    // install TX channels for different peripherals
+    // install TX channels
     for (int i = 0; i < SOC_GDMA_PAIRS_PER_GROUP; i++) {
         TEST_ESP_OK(gdma_new_channel(&channel_config, &tx_channels[i]));
-        TEST_ESP_OK(gdma_connect(tx_channels[i], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0)));
         TEST_ESP_OK(gdma_register_tx_event_callbacks(tx_channels[i], &tx_cbs, NULL));
     };
     TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND, gdma_new_channel(&channel_config, &tx_channels[0]));
 
     // Free interrupts before installing RX interrupts to ensure enough free interrupts
     for (int i = 0; i < SOC_GDMA_PAIRS_PER_GROUP; i++) {
-        TEST_ESP_OK(gdma_disconnect(tx_channels[i]));
         TEST_ESP_OK(gdma_del_channel(tx_channels[i]));
     }
 
-    // install RX channels for different peripherals
+    // install RX channels
     channel_config.direction = GDMA_CHANNEL_DIRECTION_RX;
     for (int i = 0; i < SOC_GDMA_PAIRS_PER_GROUP; i++) {
         TEST_ESP_OK(gdma_new_channel(&channel_config, &rx_channels[i]));
-        TEST_ESP_OK(gdma_connect(rx_channels[i], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0)));
         TEST_ESP_OK(gdma_register_rx_event_callbacks(rx_channels[i], &rx_cbs, NULL));
     }
     TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND, gdma_new_channel(&channel_config, &rx_channels[0]));
 
     for (int i = 0; i < SOC_GDMA_PAIRS_PER_GROUP; i++) {
-        TEST_ESP_OK(gdma_disconnect(rx_channels[i]));
         TEST_ESP_OK(gdma_del_channel(rx_channels[i]));
     }
 
@@ -63,7 +59,18 @@ TEST_CASE("GDMA channel allocation", "[gdma]")
     TEST_ESP_OK(gdma_new_channel(&channel_config, &rx_channels[1]));
     channel_config.sibling_chan = NULL;
     TEST_ESP_OK(gdma_new_channel(&channel_config, &rx_channels[0]));
+
+    TEST_ESP_OK(gdma_connect(tx_channels[0], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_SPI, 2)));
+    // can't connect multiple channels to the same peripheral
+    TEST_ESP_ERR(ESP_ERR_INVALID_STATE, gdma_connect(tx_channels[1], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_SPI, 2)));
+    TEST_ESP_OK(gdma_connect(tx_channels[1], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0)));
+
+    TEST_ESP_OK(gdma_connect(rx_channels[0], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_SPI, 2)));
+    // but rx and tx can connect to the same peripheral
+    TEST_ESP_OK(gdma_connect(rx_channels[1], GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0)));
     for (int i = 0; i < 2; i++) {
+        TEST_ESP_OK(gdma_disconnect(tx_channels[i]));
+        TEST_ESP_OK(gdma_disconnect(rx_channels[i]));
         TEST_ESP_OK(gdma_del_channel(tx_channels[i]));
         TEST_ESP_OK(gdma_del_channel(rx_channels[i]));
     }

components/esp_hw_support/port/async_memcpy_impl_gdma.c

@@ -48,8 +48,13 @@ esp_err_t async_memcpy_impl_init(async_memcpy_impl_t *impl)
         goto err;
     }
 
-    gdma_connect(impl->rx_channel, GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0));
-    gdma_connect(impl->tx_channel, GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0));
+    gdma_trigger_t m2m_trigger = GDMA_MAKE_TRIGGER(GDMA_TRIG_PERIPH_M2M, 0);
+    // get a free DMA trigger ID for memory copy
+    uint32_t free_m2m_id_mask = 0;
+    gdma_get_free_m2m_trig_id_mask(impl->tx_channel, &free_m2m_id_mask);
+    m2m_trigger.instance_id = __builtin_ctz(free_m2m_id_mask);
+    gdma_connect(impl->rx_channel, m2m_trigger);
+    gdma_connect(impl->tx_channel, m2m_trigger);
 
     gdma_strategy_config_t strategy_config = {
         .auto_update_desc = true,

components/hal/esp32c3/include/hal/gdma_ll.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 #include <stdbool.h>
+#include "hal/gdma_types.h"
 #include "soc/gdma_struct.h"
 #include "soc/gdma_reg.h"
 
@@ -19,6 +20,10 @@ extern "C" {
 #define GDMA_LL_RX_EVENT_MASK       (0x06A7)
 #define GDMA_LL_TX_EVENT_MASK       (0x1958)
 
+// any "valid" peripheral ID can be used for M2M mode
+#define GDMA_LL_M2M_FREE_PERIPH_ID_MASK (0x1CD)
+#define GDMA_LL_INVALID_PERIPH_ID       (0x3F)
+
 #define GDMA_LL_EVENT_TX_FIFO_UDF   (1<<12)
 #define GDMA_LL_EVENT_TX_FIFO_OVF   (1<<11)
 #define GDMA_LL_EVENT_RX_FIFO_UDF   (1<<10)
@@ -34,19 +39,6 @@ extern "C" {
 #define GDMA_LL_EVENT_RX_DONE       (1<<0)
 
 ///////////////////////////////////// Common /////////////////////////////////////////
-/**
- * @brief Enable DMA channel M2M mode (TX channel n forward data to RX channel n), disabled by default
- */
-static inline void gdma_ll_enable_m2m_mode(gdma_dev_t *dev, uint32_t channel, bool enable)
-{
-    dev->channel[channel].in.in_conf0.mem_trans_en = enable;
-    if (enable) {
-        // to enable m2m mode, the tx chan has to be the same to rx chan, and set to a valid value
-        dev->channel[channel].in.in_peri_sel.sel = 0;
-        dev->channel[channel].out.out_peri_sel.sel = 0;
-    }
-}
-
 /**
  * @brief Enable DMA clock gating
  */
@@ -254,9 +246,19 @@ static inline void gdma_ll_rx_set_priority(gdma_dev_t *dev, uint32_t channel, ui
 /**
  * @brief Connect DMA RX channel to a given peripheral
  */
-static inline void gdma_ll_rx_connect_to_periph(gdma_dev_t *dev, uint32_t channel, int periph_id)
+static inline void gdma_ll_rx_connect_to_periph(gdma_dev_t *dev, uint32_t channel, gdma_trigger_peripheral_t periph, int periph_id)
 {
     dev->channel[channel].in.in_peri_sel.sel = periph_id;
+    dev->channel[channel].in.in_conf0.mem_trans_en = (periph == GDMA_TRIG_PERIPH_M2M);
+}
+
+/**
+ * @brief Disconnect DMA RX channel from peripheral
+ */
+static inline void gdma_ll_rx_disconnect_from_periph(gdma_dev_t *dev, uint32_t channel)
+{
+    dev->channel[channel].in.in_peri_sel.sel = GDMA_LL_INVALID_PERIPH_ID;
+    dev->channel[channel].in.in_conf0.mem_trans_en = false;
 }
 
 ///////////////////////////////////// TX /////////////////////////////////////////
@@ -457,11 +459,20 @@ static inline void gdma_ll_tx_set_priority(gdma_dev_t *dev, uint32_t channel, ui
 /**
  * @brief Connect DMA TX channel to a given peripheral
  */
-static inline void gdma_ll_tx_connect_to_periph(gdma_dev_t *dev, uint32_t channel, int periph_id)
+static inline void gdma_ll_tx_connect_to_periph(gdma_dev_t *dev, uint32_t channel, gdma_trigger_peripheral_t periph, int periph_id)
 {
+    (void)periph;
     dev->channel[channel].out.out_peri_sel.sel = periph_id;
 }
 
+/**
+ * @brief Disconnect DMA TX channel from peripheral
+ */
+static inline void gdma_ll_tx_disconnect_from_periph(gdma_dev_t *dev, uint32_t channel)
+{
+    dev->channel[channel].out.out_peri_sel.sel = GDMA_LL_INVALID_PERIPH_ID;
+}
+
 #ifdef __cplusplus
 }
 #endif