i2s: fixed incorrect channel format on ESP32

Closes: #9635
espressif · Apr 4, 2023 · 73ca054 · 73ca054
1 parent ea51ee1
commit 73ca054
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 29 deletions.
diff --git a/components/hal/esp32/include/hal/i2s_ll.h b/components/hal/esp32/include/hal/i2s_ll.h
@@ -24,6 +24,7 @@
 
 #include <stdbool.h>
 #include "hal/misc.h"
+#include "hal/assert.h"
 #include "soc/i2s_periph.h"
 #include "soc/i2s_struct.h"
 #include "hal/i2s_types.h"
@@ -688,22 +689,57 @@ static inline void i2s_ll_rx_enable_msb_shift(i2s_dev_t *hw, bool msb_shift_enab
  * @brief Set I2S tx chan mode
  *
  * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx chan mode
- */
-static inline void i2s_ll_tx_set_chan_mod(i2s_dev_t *hw, uint32_t val)
-{
-    hw->conf_chan.tx_chan_mod = val;
+ * @param chan_fmt The channel format of the TX channel
+ */
+static inline void i2s_ll_tx_set_chan_mod(i2s_dev_t *hw, i2s_channel_fmt_t chan_fmt)
+{
+    switch (chan_fmt) {
+        case I2S_CHANNEL_FMT_ALL_RIGHT:
+            hw->conf_chan.tx_chan_mod = 1;
+            break;
+        case I2S_CHANNEL_FMT_ONLY_RIGHT:
+            hw->conf_chan.tx_chan_mod = 3;
+            break;
+        case I2S_CHANNEL_FMT_ALL_LEFT:
+            hw->conf_chan.tx_chan_mod = 2;
+            break;
+        case I2S_CHANNEL_FMT_ONLY_LEFT:
+            hw->conf_chan.tx_chan_mod = 4;
+            break;
+        case I2S_CHANNEL_FMT_RIGHT_LEFT:
+            hw->conf_chan.tx_chan_mod = 0;
+            break;
+        default:
+            HAL_ASSERT(false);
+    }
 }
 
 /**
  * @brief Set I2S rx chan mode
  *
  * @param hw Peripheral I2S hardware instance address.
- * @param val value to set rx chan mode
- */
-static inline void i2s_ll_rx_set_chan_mod(i2s_dev_t *hw, uint32_t val)
-{
-    hw->conf_chan.rx_chan_mod = val;
+ * @param chan_fmt The channel format of the RX channel
+ * @param is_msb_right Is msb_right enabled, if it does, we need to flip the channel
+ */
+static inline void i2s_ll_rx_set_chan_mod(i2s_dev_t *hw, i2s_channel_fmt_t chan_fmt, bool is_msb_right)
+{
+    switch (chan_fmt) {
+        case I2S_CHANNEL_FMT_ALL_RIGHT:
+            /* fall through */
+        case I2S_CHANNEL_FMT_ONLY_RIGHT:
+            hw->conf_chan.rx_chan_mod = is_msb_right ? 1 : 2;
+            break;
+        case I2S_CHANNEL_FMT_ALL_LEFT:
+            /* fall through */
+        case I2S_CHANNEL_FMT_ONLY_LEFT:
+            hw->conf_chan.rx_chan_mod = is_msb_right ? 2 : 1;
+            break;
+        case I2S_CHANNEL_FMT_RIGHT_LEFT:
+            hw->conf_chan.rx_chan_mod = 0;
+            break;
+        default:
+            HAL_ASSERT(false);
+    }
 }
 
 /**

diff --git a/components/hal/esp32s2/include/hal/i2s_ll.h b/components/hal/esp32s2/include/hal/i2s_ll.h
@@ -24,6 +24,7 @@
 
 #include <stdbool.h>
 #include "hal/misc.h"
+#include "hal/assert.h"
 #include "soc/i2s_periph.h"
 #include "soc/i2s_struct.h"
 #include "hal/i2s_types.h"
@@ -764,22 +765,57 @@ static inline void i2s_ll_rx_enable_msb_shift(i2s_dev_t *hw, bool msb_shift_enab
  * @brief Set I2S tx chan mode
  *
  * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx chan mode
- */
-static inline void i2s_ll_tx_set_chan_mod(i2s_dev_t *hw, uint32_t val)
-{
-    hw->conf_chan.tx_chan_mod = val;
+ * @param chan_fmt The channel format of the TX channel
+ */
+static inline void i2s_ll_tx_set_chan_mod(i2s_dev_t *hw, i2s_channel_fmt_t chan_fmt)
+{
+    switch (chan_fmt) {
+        case I2S_CHANNEL_FMT_ALL_RIGHT:
+            hw->conf_chan.tx_chan_mod = 1;
+            break;
+        case I2S_CHANNEL_FMT_ONLY_RIGHT:
+            hw->conf_chan.tx_chan_mod = 3;
+            break;
+        case I2S_CHANNEL_FMT_ALL_LEFT:
+            hw->conf_chan.tx_chan_mod = 2;
+            break;
+        case I2S_CHANNEL_FMT_ONLY_LEFT:
+            hw->conf_chan.tx_chan_mod = 4;
+            break;
+        case I2S_CHANNEL_FMT_RIGHT_LEFT:
+            hw->conf_chan.tx_chan_mod = 0;
+            break;
+        default:
+            HAL_ASSERT(false);
+    }
 }
 
 /**
  * @brief Set I2S rx chan mode
  *
  * @param hw Peripheral I2S hardware instance address.
- * @param val value to set rx chan mode
- */
-static inline void i2s_ll_rx_set_chan_mod(i2s_dev_t *hw, uint32_t val)
-{
-    hw->conf_chan.rx_chan_mod = val;
+ * @param chan_fmt The channel format of the RX channel
+ * @param is_msb_right Is msb_right enabled, if it does, we need to flip the channel
+ */
+static inline void i2s_ll_rx_set_chan_mod(i2s_dev_t *hw, i2s_channel_fmt_t chan_fmt, bool is_msb_right)
+{
+    switch (chan_fmt) {
+        case I2S_CHANNEL_FMT_ALL_RIGHT:
+            /* fall through */
+        case I2S_CHANNEL_FMT_ONLY_RIGHT:
+            hw->conf_chan.rx_chan_mod = is_msb_right ? 1 : 2;
+            break;
+        case I2S_CHANNEL_FMT_ALL_LEFT:
+            /* fall through */
+        case I2S_CHANNEL_FMT_ONLY_LEFT:
+            hw->conf_chan.rx_chan_mod = is_msb_right ? 2 : 1;
+            break;
+        case I2S_CHANNEL_FMT_RIGHT_LEFT:
+            hw->conf_chan.rx_chan_mod = 0;
+            break;
+        default:
+            HAL_ASSERT(false);
+    }
 }
 
 /**

diff --git a/components/hal/i2s_hal.c b/components/hal/i2s_hal.c
@@ -12,6 +12,11 @@
 #include "hal/i2s_hal.h"
 #include "sdkconfig.h"
 
+#ifndef SOC_I2S_SUPPORTS_TDM
+#define I2S_HAL_DEFAULT_MSB_RIGHT       (false) // Default msb_right bit to false
+#define I2S_HAL_DEFAULT_RIGHT_FIRST     (I2S_HAL_DEFAULT_MSB_RIGHT) // Normally right_first bit keeps same as msb_right
+#endif  // SOC_I2S_SUPPORTS_TDM
+
 /**
  * @brief Calculate the closest sample rate clock configuration.
  *        clock relationship:
@@ -185,9 +190,9 @@ void i2s_hal_tx_set_common_mode(i2s_hal_context_t *hal, const i2s_hal_config_t *
 #if CONFIG_IDF_TARGET_ESP32
     i2s_ll_tx_enable_msb_right(hal->dev, hal_cfg->sample_bits <= I2S_BITS_PER_SAMPLE_16BIT);
 #else
-    i2s_ll_tx_enable_msb_right(hal->dev, false);
+    i2s_ll_tx_enable_msb_right(hal->dev, I2S_HAL_DEFAULT_MSB_RIGHT);
 #endif
-    i2s_ll_tx_enable_right_first(hal->dev, false);
+    i2s_ll_tx_enable_right_first(hal->dev, I2S_HAL_DEFAULT_RIGHT_FIRST);
     i2s_ll_tx_force_enable_fifo_mod(hal->dev, true);
 #endif
 }
@@ -213,9 +218,9 @@ void i2s_hal_rx_set_common_mode(i2s_hal_context_t *hal, const i2s_hal_config_t *
 #if CONFIG_IDF_TARGET_ESP32
     i2s_ll_rx_enable_msb_right(hal->dev, hal_cfg->sample_bits <= I2S_BITS_PER_SAMPLE_16BIT);
 #else
-    i2s_ll_rx_enable_msb_right(hal->dev, false);
+    i2s_ll_rx_enable_msb_right(hal->dev, I2S_HAL_DEFAULT_MSB_RIGHT);
 #endif
-    i2s_ll_rx_enable_right_first(hal->dev, false);
+    i2s_ll_rx_enable_right_first(hal->dev, I2S_HAL_DEFAULT_RIGHT_FIRST);
     i2s_ll_rx_force_enable_fifo_mod(hal->dev, true);
 #endif
 }
@@ -248,8 +253,8 @@ void i2s_hal_tx_set_channel_style(i2s_hal_context_t *hal, const i2s_hal_config_t
     i2s_ll_tx_set_active_chan_mask(hal->dev, hal_cfg->chan_mask >> 16);
     i2s_ll_tx_set_chan_num(hal->dev, chan_num);
 #else
-    i2s_ll_tx_set_chan_mod(hal->dev, hal_cfg->chan_fmt < I2S_CHANNEL_FMT_ONLY_RIGHT ? hal_cfg->chan_fmt : (hal_cfg->chan_fmt >> 1)); // 0-two channel;1-right;2-left;3-righ;4-left
-#endif
+    i2s_ll_tx_set_chan_mod(hal->dev, hal_cfg->chan_fmt);
+#endif  // SOC_I2S_SUPPORTS_TDM
 #if SOC_I2S_SUPPORTS_PDM_CODEC
     if (hal_cfg->mode & I2S_MODE_PDM) {
         // Fixed to 16 while using mono mode and 32 while using stereo mode
@@ -290,10 +295,18 @@ void i2s_hal_rx_set_channel_style(i2s_hal_context_t *hal, const i2s_hal_config_t
     i2s_ll_rx_set_chan_num(hal->dev, chan_num);
 #if SOC_I2S_SUPPORTS_PDM_RX
     is_mono = (hal_cfg->mode & I2S_MODE_PDM) ? false : true;
-#endif
+#endif  // SOC_I2S_SUPPORTS_PDM_RX
 #else
-    i2s_ll_rx_set_chan_mod(hal->dev, hal_cfg->chan_fmt < I2S_CHANNEL_FMT_ONLY_RIGHT ? hal_cfg->chan_fmt : (hal_cfg->chan_fmt >> 1)); // 0-two channel;1-right;2-left;3-righ;4-left
-#endif
+    /* rx_chan_mod is related to msb_right, we take it into consideration here.
+     * It is calculated again here instead of reading the value from the register,
+     * so that we can avoid introducing the calling sequence dependency */
+    bool is_msb_right = I2S_HAL_DEFAULT_MSB_RIGHT;  // Set default to false for ESP32-S2
+#if CONFIG_IDF_TARGET_ESP32
+    /* Specially, `msb_right` on esp32 is related to sample bits and PDM mode */
+    is_msb_right |= (hal_cfg->sample_bits <= I2S_BITS_PER_SAMPLE_16BIT) || (hal_cfg->mode & I2S_MODE_PDM);
+#endif  // CONFIG_IDF_TARGET_ESP32
+    i2s_ll_rx_set_chan_mod(hal->dev, hal_cfg->chan_fmt, is_msb_right);
+#endif  // SOC_I2S_SUPPORTS_TDM
     i2s_ll_rx_set_sample_bit(hal->dev, chan_bits, data_bits);
     i2s_ll_rx_enable_mono_mode(hal->dev, is_mono);