refactor(i2s): make the slot_bit_width checks unique

components/esp_driver_i2s/i2s_common.c

@@ -113,13 +113,11 @@ static esp_err_t s_i2s_extract_duplex_candidate(i2s_chan_handle_t handle, i2s_du
     i2s_std_clk_config_t *clk_cfg = NULL;
     i2s_std_gpio_config_t *gpio_cfg = NULL;
     uint32_t slot_bits = 0;
-    uint32_t data_bits = 0;
     if (handle->mode == I2S_COMM_MODE_STD) {
         i2s_std_config_t *cfg = (i2s_std_config_t *)handle->mode_info;
         clk_cfg = &cfg->clk_cfg;
         gpio_cfg = &cfg->gpio_cfg;
         slot_bits = cfg->slot_cfg.slot_bit_width;
-        data_bits = cfg->slot_cfg.data_bit_width;
     }
 #if SOC_I2S_SUPPORTS_TDM
     else {
@@ -127,14 +125,9 @@ static esp_err_t s_i2s_extract_duplex_candidate(i2s_chan_handle_t handle, i2s_du
         clk_cfg = (i2s_std_clk_config_t *)&cfg->clk_cfg;
         gpio_cfg = (i2s_std_gpio_config_t *)&cfg->gpio_cfg;
         slot_bits = cfg->slot_cfg.slot_bit_width;
-        data_bits = cfg->slot_cfg.data_bit_width;
     }
 #endif
 
-    if (slot_bits == I2S_SLOT_BIT_WIDTH_AUTO || (int)slot_bits < (int)data_bits) {
-        slot_bits = data_bits;
-    }
-
     out->ws_pin = gpio_cfg->ws;
     out->bclk_pin = gpio_cfg->bclk;
     out->total_frame_bits = handle->total_slot * slot_bits;
@@ -201,6 +194,8 @@ void i2s_channel_try_to_constitute_duplex(i2s_chan_handle_t handle, const i2s_du
             another_handle->role == I2S_ROLE_MASTER) {
         handle->role = I2S_ROLE_SLAVE;
         handle->full_duplex_slave = true;
+        ESP_LOGW(TAG, "the %s channel on I2S%d is switched from master to slave for full-duplex mode",
+                 handle->dir == I2S_DIR_TX ? "tx" : "rx", handle->controller->id);
     }
 }
 

components/esp_driver_i2s/i2s_pdm.c

@@ -320,11 +320,12 @@ esp_err_t i2s_channel_reconfig_pdm_tx_slot(i2s_chan_handle_t handle, const i2s_p
     i2s_pdm_tx_config_t *pdm_tx_cfg = (i2s_pdm_tx_config_t *)handle->mode_info;
     ESP_GOTO_ON_FALSE(pdm_tx_cfg, ESP_ERR_INVALID_STATE, err, TAG, "initialization not complete");
 
+    uint32_t old_slot_bit_width = pdm_tx_cfg->slot_cfg.slot_bit_width;
+
     ESP_GOTO_ON_ERROR(i2s_pdm_tx_set_slot(handle, slot_cfg), err, TAG, "set i2s standard slot failed");
 
     /* If the slot bit width changed, then need to update the clock */
-    uint32_t slot_bits = slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO ? slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
+    if (pdm_tx_cfg->slot_cfg.slot_bit_width != old_slot_bit_width) {
-    if (pdm_tx_cfg->slot_cfg.slot_bit_width != slot_bits) {
         ESP_GOTO_ON_ERROR(i2s_pdm_tx_set_clock(handle, &pdm_tx_cfg->clk_cfg), err, TAG, "update clock failed");
     }
 
@@ -652,11 +653,12 @@ esp_err_t i2s_channel_reconfig_pdm_rx_slot(i2s_chan_handle_t handle, const i2s_p
     i2s_pdm_rx_config_t *pdm_rx_cfg = (i2s_pdm_rx_config_t *)handle->mode_info;
     ESP_GOTO_ON_FALSE(pdm_rx_cfg, ESP_ERR_INVALID_STATE, err, TAG, "initialization not complete");
 
+    uint32_t old_slot_bit_width = pdm_rx_cfg->slot_cfg.slot_bit_width;
+
     ESP_GOTO_ON_ERROR(i2s_pdm_rx_set_slot(handle, slot_cfg), err, TAG, "set i2s standard slot failed");
 
     /* If the slot bit width changed, then need to update the clock */
-    uint32_t slot_bits = slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO ? slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
+    if (pdm_rx_cfg->slot_cfg.slot_bit_width != old_slot_bit_width) {
-    if (pdm_rx_cfg->slot_cfg.slot_bit_width != slot_bits) {
         ESP_GOTO_ON_ERROR(i2s_pdm_rx_set_clock(handle, &pdm_rx_cfg->clk_cfg), err, TAG, "update clock failed");
     }
 

components/esp_driver_i2s/i2s_std.c

@@ -30,10 +30,7 @@ static esp_err_t i2s_std_calculate_clock(i2s_chan_handle_t handle, const i2s_std
 {
     uint32_t rate = clk_cfg->sample_rate_hz;
     i2s_std_slot_config_t *slot_cfg = &((i2s_std_config_t *)(handle->mode_info))->slot_cfg;
-    uint32_t slot_bits = (slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO) ||
+    uint32_t slot_bits = slot_cfg->slot_bit_width;
-                         ((int)slot_cfg->slot_bit_width < (int)slot_cfg->data_bit_width) ?
-                         slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
-    slot_cfg->slot_bit_width = slot_bits;
     /* Calculate multiple
      * Fmclk = bck_div*fbck = fsclk/(mclk_div+b/a) */
     if (handle->role == I2S_ROLE_MASTER || handle->full_duplex_slave) {
@@ -85,8 +82,7 @@ static esp_err_t i2s_std_set_clock(i2s_chan_handle_t handle, const i2s_std_clk_c
 {
     esp_err_t ret = ESP_OK;
     i2s_std_config_t *std_cfg = (i2s_std_config_t *)(handle->mode_info);
-    i2s_data_bit_width_t real_slot_bit = (int)std_cfg->slot_cfg.slot_bit_width < (int)std_cfg->slot_cfg.data_bit_width ?
+    i2s_data_bit_width_t real_slot_bit = std_cfg->slot_cfg.slot_bit_width;
-                                         std_cfg->slot_cfg.data_bit_width : std_cfg->slot_cfg.slot_bit_width;
     ESP_RETURN_ON_FALSE(real_slot_bit != I2S_DATA_BIT_WIDTH_24BIT ||
                         (clk_cfg->mclk_multiple % 3 == 0), ESP_ERR_INVALID_ARG, TAG,
                         "The 'mclk_multiple' should be the multiple of 3 while using 24-bit data width");
@@ -167,7 +163,7 @@ static esp_err_t i2s_std_set_slot(i2s_chan_handle_t handle, const i2s_std_slot_c
 
     /* Update the mode info: slot configuration */
     i2s_std_config_t *std_cfg = (i2s_std_config_t *)(handle->mode_info);
-    memcpy(&(std_cfg->slot_cfg), slot_cfg, sizeof(i2s_std_slot_config_t));
+    memcpy(&(std_cfg->slot_cfg), &norm_slot_cfg, sizeof(i2s_std_slot_config_t));
 
     return ESP_OK;
 }
@@ -438,11 +434,12 @@ esp_err_t i2s_channel_reconfig_std_slot(i2s_chan_handle_t handle, const i2s_std_
     i2s_std_config_t *std_cfg = (i2s_std_config_t *)handle->mode_info;
     ESP_GOTO_ON_FALSE(std_cfg, ESP_ERR_INVALID_STATE, err, TAG, "initialization not complete");
 
+    uint32_t old_slot_bit_width = std_cfg->slot_cfg.slot_bit_width;
+
     ESP_GOTO_ON_ERROR(i2s_std_set_slot(handle, slot_cfg), err, TAG, "set i2s standard slot failed");
 
     /* If the slot bit width changed, then need to update the clock */
-    uint32_t slot_bits = slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO ? slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
+    if (std_cfg->slot_cfg.slot_bit_width != old_slot_bit_width) {
-    if (std_cfg->slot_cfg.slot_bit_width != slot_bits) {
         ESP_GOTO_ON_ERROR(i2s_std_set_clock(handle, &std_cfg->clk_cfg), err, TAG, "update clock failed");
     }
 

components/esp_driver_i2s/i2s_tdm.c

@@ -31,10 +31,7 @@ static esp_err_t i2s_tdm_calculate_clock(i2s_chan_handle_t handle, const i2s_tdm
 {
     uint32_t rate = clk_cfg->sample_rate_hz;
     i2s_tdm_slot_config_t *slot_cfg = &((i2s_tdm_config_t *)(handle->mode_info))->slot_cfg;
-    uint32_t slot_bits = (slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO) ||
+    uint32_t slot_bits = slot_cfg->slot_bit_width;
-                         ((int)slot_cfg->slot_bit_width < (int)slot_cfg->data_bit_width) ?
-                         slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
-    slot_cfg->slot_bit_width = slot_bits;
     /* Calculate multiple
      * Fmclk = bck_div*fbck = fsclk/(mclk_div+b/a) */
     if (handle->role == I2S_ROLE_MASTER || handle->full_duplex_slave) {
@@ -120,30 +117,32 @@ static esp_err_t i2s_tdm_set_clock(i2s_chan_handle_t handle, const i2s_tdm_clk_c
     return ret;
 }
 
-static i2s_tdm_slot_config_t s_i2s_tdm_normalize_slot_config(const i2s_tdm_slot_config_t *slot_cfg)
+static esp_err_t s_i2s_tdm_normalize_slot_config(const i2s_tdm_slot_config_t *slot_cfg, i2s_tdm_slot_config_t *normalized_slot_cfg)
 {
-    i2s_tdm_slot_config_t normalized_slot_cfg = *slot_cfg;
+    ESP_RETURN_ON_FALSE(slot_cfg->slot_mask, ESP_ERR_INVALID_ARG, TAG, "At least one channel should be enabled");
-    uint32_t max_slot_num = 32 - __builtin_clz(normalized_slot_cfg.slot_mask);
+
-    /* 1. Normalize the ws width */
+    *normalized_slot_cfg = *slot_cfg;
-    normalized_slot_cfg.ws_width = normalized_slot_cfg.ws_width == I2S_TDM_AUTO_WS_WIDTH ?
+    /* 1. Normalize the slot bit width */
-                                   normalized_slot_cfg.total_slot * normalized_slot_cfg.slot_bit_width / 2 : normalized_slot_cfg.ws_width;
+    normalized_slot_cfg->slot_bit_width = (int)normalized_slot_cfg->slot_bit_width < (int)normalized_slot_cfg->data_bit_width ?
+                                          normalized_slot_cfg->data_bit_width : normalized_slot_cfg->slot_bit_width;
 
     /* 2. Normalize the total slot number */
-    normalized_slot_cfg.total_slot = normalized_slot_cfg.total_slot < max_slot_num ? max_slot_num : normalized_slot_cfg.total_slot;
+    uint32_t max_slot_num = 32 - __builtin_clz(normalized_slot_cfg->slot_mask);
+    normalized_slot_cfg->total_slot = normalized_slot_cfg->total_slot < max_slot_num ? max_slot_num : normalized_slot_cfg->total_slot;
     // At least two slots in a frame if not using PCM short format
-    normalized_slot_cfg.total_slot = ((normalized_slot_cfg.total_slot < 2) && (normalized_slot_cfg.ws_width != 1)) ? 2 : normalized_slot_cfg.total_slot;
+    normalized_slot_cfg->total_slot = ((normalized_slot_cfg->total_slot < 2) && (normalized_slot_cfg->ws_width != 1)) ? 2 : normalized_slot_cfg->total_slot;
 
-    /* 3. Normalize the slot bit width */
+    /* 3. Normalize the ws width */
-    normalized_slot_cfg.slot_bit_width = normalized_slot_cfg.slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO ?
+    normalized_slot_cfg->ws_width = normalized_slot_cfg->ws_width == I2S_TDM_AUTO_WS_WIDTH ?
-                                         normalized_slot_cfg.data_bit_width : normalized_slot_cfg.slot_bit_width;
+                                    normalized_slot_cfg->total_slot * normalized_slot_cfg->slot_bit_width / 2 : normalized_slot_cfg->ws_width;
-    return normalized_slot_cfg;
+
+    return ESP_OK;
 }
 
 static esp_err_t i2s_tdm_set_slot(i2s_chan_handle_t handle, const i2s_tdm_slot_config_t *slot_cfg)
 {
-    ESP_RETURN_ON_FALSE(slot_cfg->slot_mask, ESP_ERR_INVALID_ARG, TAG, "At least one channel should be enabled");
+    i2s_tdm_slot_config_t norm_slot_cfg = {};
-
+    ESP_RETURN_ON_ERROR(s_i2s_tdm_normalize_slot_config(slot_cfg, &norm_slot_cfg), TAG, "invalid slot configuration");
-    i2s_tdm_slot_config_t norm_slot_cfg = s_i2s_tdm_normalize_slot_config(slot_cfg);
     /* Update the total slot num and active slot num */
     handle->active_slot = norm_slot_cfg.slot_mode == I2S_SLOT_MODE_MONO ? 1 : __builtin_popcount(norm_slot_cfg.slot_mask);
     handle->total_slot = norm_slot_cfg.total_slot;
@@ -171,9 +170,9 @@ static esp_err_t i2s_tdm_set_slot(i2s_chan_handle_t handle, const i2s_tdm_slot_c
     portENTER_CRITICAL(&g_i2s.spinlock);
     /* Configure the hardware to apply TDM format */
     if (handle->dir == I2S_DIR_TX) {
-        i2s_hal_tdm_set_tx_slot(&(handle->controller->hal), is_slave, (i2s_hal_slot_config_t *)slot_cfg);
+        i2s_hal_tdm_set_tx_slot(&(handle->controller->hal), is_slave, (i2s_hal_slot_config_t *)&norm_slot_cfg);
     } else {
-        i2s_hal_tdm_set_rx_slot(&(handle->controller->hal), is_slave, (i2s_hal_slot_config_t *)slot_cfg);
+        i2s_hal_tdm_set_rx_slot(&(handle->controller->hal), is_slave, (i2s_hal_slot_config_t *)&norm_slot_cfg);
     }
     portEXIT_CRITICAL(&g_i2s.spinlock);
 
@@ -265,7 +264,7 @@ static esp_err_t i2s_tdm_set_gpio(i2s_chan_handle_t handle, const i2s_tdm_gpio_c
     return ESP_OK;
 }
 
-static void s_i2s_channel_try_to_constitute_tdm_duplex(i2s_chan_handle_t handle, const i2s_tdm_config_t *tdm_cfg)
+static esp_err_t s_i2s_channel_try_to_constitute_tdm_duplex(i2s_chan_handle_t handle, const i2s_tdm_config_t *tdm_cfg)
 {
     /* Build the duplex candidate from the current channel's configuration */
     i2s_duplex_candidate_t candidate = {0};
@@ -276,10 +275,12 @@ static void s_i2s_channel_try_to_constitute_tdm_duplex(i2s_chan_handle_t handle,
     candidate.bclk_inv = tdm_cfg->gpio_cfg.invert_flags.bclk_inv;
     candidate.ws_inv = tdm_cfg->gpio_cfg.invert_flags.ws_inv;
     /* Compute total frame bits for TDM mode */
-    i2s_tdm_slot_config_t norm_slot = s_i2s_tdm_normalize_slot_config(&tdm_cfg->slot_cfg);
+    i2s_tdm_slot_config_t norm_slot = {};
+    ESP_RETURN_ON_ERROR(s_i2s_tdm_normalize_slot_config(&tdm_cfg->slot_cfg, &norm_slot), TAG, "invalid slot configuration");
     candidate.total_frame_bits = norm_slot.total_slot * norm_slot.slot_bit_width;
 
     i2s_channel_try_to_constitute_duplex(handle, &candidate);
+    return ESP_OK;
 }
 
 esp_err_t i2s_channel_init_tdm_mode(i2s_chan_handle_t handle, const i2s_tdm_config_t *tdm_cfg)
@@ -300,7 +301,7 @@ esp_err_t i2s_channel_init_tdm_mode(i2s_chan_handle_t handle, const i2s_tdm_conf
     handle->mode_info = calloc(1, sizeof(i2s_tdm_config_t));
     ESP_GOTO_ON_FALSE(handle->mode_info, ESP_ERR_NO_MEM, err, TAG, "no memory for storing the configurations");
     /* Try to constitute full-duplex mode if the TDM configuration is totally same as another channel */
-    s_i2s_channel_try_to_constitute_tdm_duplex(handle, tdm_cfg);
+    ESP_GOTO_ON_ERROR(s_i2s_channel_try_to_constitute_tdm_duplex(handle, tdm_cfg), err, TAG, "failed to constitute full-duplex mode");
     /* i2s_set_tdm_slot should be called before i2s_set_tdm_clock while initializing, because clock is relay on the slot */
     ESP_GOTO_ON_ERROR(i2s_tdm_set_slot(handle, &tdm_cfg->slot_cfg), err, TAG, "initialize channel failed while setting slot");
 #if SOC_I2S_SUPPORTS_APLL
@@ -413,11 +414,13 @@ esp_err_t i2s_channel_reconfig_tdm_slot(i2s_chan_handle_t handle, const i2s_tdm_
     i2s_tdm_config_t *tdm_cfg = (i2s_tdm_config_t *)handle->mode_info;
     ESP_GOTO_ON_FALSE(tdm_cfg, ESP_ERR_INVALID_STATE, err, TAG, "initialization not complete");
 
+    uint32_t old_total_slot = handle->total_slot;
+    uint32_t old_slot_bit_width = tdm_cfg->slot_cfg.slot_bit_width;
+
     ESP_GOTO_ON_ERROR(i2s_tdm_set_slot(handle, slot_cfg), err, TAG, "set i2s standard slot failed");
 
-    /* If the slot bit width changed, then need to update the clock */
+    /* If the total frame bits changed, then need to update the clock */
-    uint32_t slot_bits = slot_cfg->slot_bit_width == I2S_SLOT_BIT_WIDTH_AUTO ? slot_cfg->data_bit_width : slot_cfg->slot_bit_width;
+    if (old_total_slot * old_slot_bit_width != handle->total_slot * tdm_cfg->slot_cfg.slot_bit_width) {
-    if (tdm_cfg->slot_cfg.slot_bit_width != slot_bits) {
         ESP_GOTO_ON_ERROR(i2s_tdm_set_clock(handle, &tdm_cfg->clk_cfg), err, TAG, "update clock failed");
     }
 

components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c

@@ -270,8 +270,10 @@ TEST_CASE("I2S_lazy_duplex_constitution_boundary_test", "[i2s]")
     TEST_ESP_OK(i2s_channel_init_tdm_mode(tx_handle, &tdm_64bit));
     TEST_ESP_OK(i2s_channel_get_info(tx_handle, &chan_info));
     TEST_ASSERT(chan_info.pair_chan == rx_handle);
+    TEST_ASSERT_EQUAL(I2S_ROLE_SLAVE, chan_info.role);
     TEST_ESP_OK(i2s_channel_get_info(rx_handle, &chan_info));
     TEST_ASSERT(chan_info.pair_chan == tx_handle);
+    TEST_ASSERT_EQUAL(I2S_ROLE_MASTER, chan_info.role);
     TEST_ESP_OK(i2s_del_channel(tx_handle));
     TEST_ESP_OK(i2s_del_channel(rx_handle));
 

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

@@ -912,6 +912,10 @@ Full-duplex mode registers TX and RX channel in an I2S port at the same time, an
 
 Note that one handle can only stand for one channel. Therefore, it is still necessary to configure the slot and clock for both TX and RX channels one by one.
 
+.. note::
+
+    In full-duplex mode, only one channel can work as the master that generates BCLK and WS. If both paired handles are configured as ``I2S_ROLE_MASTER``, the handle initialized later is automatically switched to ``I2S_ROLE_SLAVE``.
+
 There are two methods to allocate a pair of full-duplex channels:
 
 1. Allocate both TX and RX handles in a single call of :cpp:func:`i2s_new_channel`.

docs/zh_CN/api-reference/peripherals/i2s.rst

@@ -912,6 +912,10 @@ STD RX 模式
 
 请注意，一个句柄只能代表一个通道，因此仍然需要对 TX 和 RX 通道逐个进行声道和时钟配置。
 
+.. note::
+
+    全双工模式下只能有一个通道作为 master 生成 BCLK 和 WS。如果配对的两个句柄都配置为 ``I2S_ROLE_MASTER``，后初始化的句柄会被自动切换为 ``I2S_ROLE_SLAVE``。
+
 驱动支持两种分配全双工通道的方法：
 
 1. 在调用 :cpp:func:`i2s_new_channel` 函数时，同时分配 TX 和 RX 通道两个通道。