Merge branch 'bugfix/fix_incorrect_i2s_pdm_clk_issue' into 'master'

Bugfix(I2S):  Fix I2S PDM clock incorrect issue when using APLL.

Closes IDFGH-3423 and IDFGH-3106

See merge request espressif/esp-idf!9101

components/driver/i2s.c

@@ -413,17 +413,19 @@ esp_err_t i2s_set_clk(i2s_port_t i2s_num, uint32_t rate, i2s_bits_per_sample_t b
     } else if (p_i2s_obj[i2s_num]->mode & I2S_MODE_PDM) {
         uint32_t b_clk = 0;
         if (p_i2s_obj[i2s_num]->mode & I2S_MODE_TX) {
-            int fp;
-            int fs;
+            uint32_t fp, fs;
             i2s_hal_get_tx_pdm(&(p_i2s_obj[i2s_num]->hal), &fp, &fs);
-            b_clk = rate * I2S_PDM_BCK_FACTOR * (fp / fs);
-            fi2s_clk /= (I2S_PDM_BCK_FACTOR * (fp / fs));
+            // Recommended set `fp = 960, fs = sample_rate / 100`
+            fs = rate / 100;
+            i2s_hal_tx_pdm_cfg(&(p_i2s_obj[i2s_num]->hal), fp, fs);
+            b_clk = rate * I2S_PDM_BCK_FACTOR * fp / fs;
+            
         } else if (p_i2s_obj[i2s_num]->mode & I2S_MODE_RX) {
-            bool en;
-            i2s_hal_get_rx_sinc_dsr_16_en(&(p_i2s_obj[i2s_num]->hal), &en);
-            b_clk = rate * I2S_PDM_BCK_FACTOR * (en ? 2 : 1);
-            fi2s_clk /= (I2S_PDM_BCK_FACTOR * (en ? 2 : 1));
+            uint32_t dsr;
+            i2s_hal_get_rx_pdm(&(p_i2s_obj[i2s_num]->hal), &dsr);
+            b_clk = rate * I2S_PDM_BCK_FACTOR * (dsr ? 2 : 1);
         }
+        fi2s_clk = b_clk * m_scale;
         int factor2 = 5 ;
         mclk = b_clk * factor2;
         clkmdiv = ((double) I2S_BASE_CLK) / mclk;
@@ -818,7 +820,7 @@ esp_err_t i2s_set_sample_rates(i2s_port_t i2s_num, uint32_t rate)
 esp_err_t i2s_set_pdm_rx_down_sample(i2s_port_t i2s_num, i2s_pdm_dsr_t dsr)
 {
     I2S_CHECK((i2s_num < I2S_NUM_MAX), "i2s_num error", ESP_ERR_INVALID_ARG);
-    i2s_hal_set_pdm_rx_down_sample(&(p_i2s_obj[i2s_num]->hal), dsr);
+    i2s_hal_rx_pdm_cfg(&(p_i2s_obj[i2s_num]->hal), dsr);
     return i2s_set_clk(i2s_num, p_i2s_obj[i2s_num]->sample_rate, p_i2s_obj[i2s_num]->bits_per_sample, p_i2s_obj[i2s_num]->channel_num);
 }
 #endif

components/hal/esp32/include/hal/i2s_ll.h

@@ -390,61 +390,6 @@ static inline void i2s_ll_set_rx_eof_num(i2s_dev_t *hw, uint32_t val)
     hw->rx_eof_num = val / 4;
 }
 
-/**
- * @brief Get I2S tx pdm fp
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to get tx pdm fp
- */
-static inline void i2s_ll_get_tx_pdm_fp(i2s_dev_t *hw, uint32_t *val)
-{
-    *val = hw->pdm_freq_conf.tx_pdm_fp;
-}
-
-/**
- * @brief Get I2S tx pdm fs
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to get tx pdm fs
- */
-static inline void i2s_ll_get_tx_pdm_fs(i2s_dev_t *hw, uint32_t *val)
-{
-    *val = hw->pdm_freq_conf.tx_pdm_fs;
-}
-
-/**
- * @brief Set I2S tx pdm fp
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx pdm fp
- */
-static inline void i2s_ll_set_tx_pdm_fp(i2s_dev_t *hw, uint32_t val)
-{
-    hw->pdm_freq_conf.tx_pdm_fp = val;
-}
-
-/**
- * @brief Set I2S tx pdm fs
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx pdm fs
- */
-static inline void i2s_ll_set_tx_pdm_fs(i2s_dev_t *hw, uint32_t val)
-{
-    hw->pdm_freq_conf.tx_pdm_fs = val;
-}
-
-/**
- * @brief Get I2S rx sinc dsr 16 en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to get rx sinc dsr 16 en
- */
-static inline void i2s_ll_get_rx_sinc_dsr_16_en(i2s_dev_t *hw, bool *val)
-{
-    *val = hw->pdm_conf.rx_sinc_dsr_16_en;
-}
-
 /**
  * @brief Set I2S clkm div num
  *
@@ -533,17 +478,6 @@ static inline void i2s_ll_set_rx_bits_mod(i2s_dev_t *hw, uint32_t val)
     hw->sample_rate_conf.rx_bits_mod = val;
 }
 
-/**
- * @brief Set I2S rx sinc dsr 16 en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set rx sinc dsr 16 en
- */
-static inline void i2s_ll_set_rx_sinc_dsr_16_en(i2s_dev_t *hw, bool val)
-{
-    hw->pdm_conf.rx_sinc_dsr_16_en = val;
-}
-
 /**
  * @brief Set I2S dscr en
  *
@@ -577,50 +511,6 @@ static inline void i2s_ll_set_camera_en(i2s_dev_t *hw, bool val)
     hw->conf2.camera_en = val;
 }
 
-/**
- * @brief Set I2S pcm2pdm conv en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set pcm2pdm conv en
- */
-static inline void i2s_ll_set_pcm2pdm_conv_en(i2s_dev_t *hw, bool val)
-{
-    hw->pdm_conf.pcm2pdm_conv_en = val;
-}
-
-/**
- * @brief Set I2S pdm2pcm conv en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set pdm2pcm conv en
- */
-static inline void i2s_ll_set_pdm2pcm_conv_en(i2s_dev_t *hw, bool val)
-{
-    hw->pdm_conf.pdm2pcm_conv_en = val;
-}
-
-/**
- * @brief Set I2S rx pdm en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set rx pdm en
- */
-static inline void i2s_ll_set_rx_pdm_en(i2s_dev_t *hw, bool val)
-{
-    hw->pdm_conf.rx_pdm_en = val;
-}
-
-/**
- * @brief Set I2S tx pdm en
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx pdm en
- */
-static inline void i2s_ll_set_tx_pdm_en(i2s_dev_t *hw, bool val)
-{
-    hw->pdm_conf.tx_pdm_en = val;
-}
-
 /**
  * @brief Set I2S tx fifo mod force en
  *
@@ -753,17 +643,6 @@ static inline void i2s_ll_set_rx_mono(i2s_dev_t *hw, uint32_t val)
     hw->conf.rx_mono = val;
 }
 
-/**
- * @brief Set I2S tx sinc osr2
- *
- * @param hw Peripheral I2S hardware instance address.
- * @param val value to set tx sinc osr2
- */
-static inline void i2s_ll_set_tx_sinc_osr2(i2s_dev_t *hw, uint32_t val)
-{
-    hw->pdm_conf.tx_sinc_osr2 = val;
-}
-
 /**
  * @brief Set I2S sig loopback
  *
@@ -864,15 +743,64 @@ static inline void i2s_ll_set_rx_pcm_long(i2s_dev_t *hw)
 }
 
 /**
- * @brief Configure I2S TX pdm
+ * @brief Enable I2S build in ADC mode
+ *
+ * @param hw Peripheral I2S hardware instance address.
+ */
+static inline void i2s_ll_build_in_adc_ena(i2s_dev_t *hw)
+{
+    hw->conf2.lcd_en = 1;
+    hw->conf2.camera_en = 0;
+    hw->conf.rx_msb_shift = 0;
+    hw->conf.rx_short_sync = 0;
+}
+
+/**
+ * @brief Enable I2S build in DAC mode
+ *
+ * @param hw Peripheral I2S hardware instance address.
+ */
+static inline void i2s_ll_build_in_dac_ena(i2s_dev_t *hw)
+{
+    hw->conf2.lcd_en = 1;
+    hw->conf2.camera_en = 0;
+    hw->conf.tx_right_first = 1;
+    hw->conf.tx_msb_shift = 0;
+    hw->conf.tx_short_sync = 0;
+}
+
+
+/**
+ * @brief Enable I2S RX PDM mode
  *
- * @param sample_rate The sample rate to be set.
  * @param hw Peripheral I2S hardware instance address.
+ * @param pdm_en Set true to enable rx PDM mode
  */
-static inline void i2s_ll_tx_pdm_cfg(i2s_dev_t *hw, uint32_t sample_rate)
+static inline void i2s_ll_set_rx_pdm_en(i2s_dev_t *hw, bool pdm_en)
+{
+    hw->pdm_conf.rx_pdm_en = pdm_en;
+}
+
+/**
+ * @brief Enable I2S tx pdm mode
+ *
+ * @param hw Peripheral I2S hardware instance address.
+ * @param pdm_en Set true to enable tx PDM mode
+ */
+static inline void i2s_ll_set_tx_pdm_en(i2s_dev_t *hw, bool pdm_en)
+{
+    hw->pdm_conf.tx_pdm_en = pdm_en;
+}
+
+/**
+ * @brief Configure I2S tx PDM filter module group0
+ *
+ * @param hw Peripheral I2S hardware instance address.
+ * @param fp The fp value of TX PDM filter module group0.
+ * @param fs The fs value of TX PDM filter module group0. 
+ */
+static inline void i2s_ll_tx_pdm_cfg(i2s_dev_t *hw, uint32_t fp, uint32_t fs)
 {
-    uint32_t fp = 96;
-    uint32_t fs = sample_rate / 1000 * 10;
     hw->pdm_freq_conf.tx_pdm_fp = fp;
     hw->pdm_freq_conf.tx_pdm_fs = fs;
     hw->pdm_conf.tx_sinc_osr2 = fp/fs;
@@ -881,42 +809,40 @@ static inline void i2s_ll_tx_pdm_cfg(i2s_dev_t *hw, uint32_t sample_rate)
 }
 
 /**
- * @brief Configure I2S TX pdm
+ * @brief Configure I2S rx PDM
  *
  * @param hw Peripheral I2S hardware instance address.
+ * @param dsr Down-sampling rate value of rx PDM
  */
-static inline void i2s_ll_rx_pdm_cfg(i2s_dev_t *hw)
+static inline void i2s_ll_rx_pdm_cfg(i2s_dev_t *hw, uint32_t dsr)
 {
-    hw->pdm_conf.rx_sinc_dsr_16_en = 0;
+    hw->pdm_conf.rx_sinc_dsr_16_en = dsr;
     hw->pdm_conf.pdm2pcm_conv_en = 1;
     hw->pdm_conf.rx_pdm_en = 1;
 }
 
 /**
- * @brief Enable I2S build in ADC mode
+ * @brief Get I2S tx PDM  configuration
  *
  * @param hw Peripheral I2S hardware instance address.
+ * @param fp Pointer to store tx PDM fp configuration
+ * @param fs Pointer to store tx PDM fs configuration
  */
-static inline void i2s_ll_build_in_adc_ena(i2s_dev_t *hw)
+static inline void i2s_ll_get_tx_pdm(i2s_dev_t *hw, uint32_t *fp, uint32_t *fs)
 {
-    hw->conf2.lcd_en = 1;
-    hw->conf2.camera_en = 0;
-    hw->conf.rx_msb_shift = 0;
-    hw->conf.rx_short_sync = 0;
+    *fp = hw->pdm_freq_conf.tx_pdm_fp;
+    *fs = hw->pdm_freq_conf.tx_pdm_fs;
 }
 
 /**
- * @brief Enable I2S build in DAC mode
+ * @brief Get I2S rx PDM configuration
  *
  * @param hw Peripheral I2S hardware instance address.
+ * @param dsr Pointer to stoe the rx PDM down-sample rate configuration
  */
-static inline void i2s_ll_build_in_dac_ena(i2s_dev_t *hw)
+static inline void i2s_ll_get_rx_pdm(i2s_dev_t *hw, uint32_t *dsr)
 {
-    hw->conf2.lcd_en = 1;
-    hw->conf2.camera_en = 0;
-    hw->conf.tx_right_first = 1;
-    hw->conf.tx_msb_shift = 0;
-    hw->conf.tx_short_sync = 0;
+    *dsr = hw->pdm_conf.rx_sinc_dsr_16_en;
 }
 
 #ifdef __cplusplus

components/hal/i2s_hal.c

@@ -17,6 +17,9 @@
 #include "soc/soc.h"
 #include "hal/i2s_hal.h"
 
+#define I2S_TX_PDM_FP_DEF  960   // Set to the recommended value(960) in TRM
+#define I2S_RX_PDM_DSR_DEF 0
+
 void i2s_hal_set_tx_mode(i2s_hal_context_t *hal, i2s_channel_t ch, i2s_bits_per_sample_t bits)
 {
     if (bits <= I2S_BITS_PER_SAMPLE_16BIT) {
@@ -50,10 +53,24 @@ void i2s_hal_set_in_link(i2s_hal_context_t *hal, uint32_t bytes_num, uint32_t ad
 }
 
 #if SOC_I2S_SUPPORTS_PDM
-void i2s_hal_get_tx_pdm(i2s_hal_context_t *hal, int *fp, int *fs)
+void i2s_hal_tx_pdm_cfg(i2s_hal_context_t *hal, uint32_t fp, uint32_t fs)
+{
+    i2s_ll_tx_pdm_cfg(hal->dev, fp, fs);
+}
+
+void i2s_hal_get_tx_pdm(i2s_hal_context_t *hal, uint32_t *fp, uint32_t *fs)
+{
+    i2s_ll_get_tx_pdm(hal->dev, fp, fs);
+}
+
+void i2s_hal_rx_pdm_cfg(i2s_hal_context_t *hal, uint32_t dsr)
+{
+    i2s_ll_rx_pdm_cfg(hal->dev, dsr);
+}
+
+void i2s_hal_get_rx_pdm(i2s_hal_context_t *hal, uint32_t *dsr)
 {
-    i2s_ll_get_tx_pdm_fp(hal->dev, (uint32_t *)fp);
-    i2s_ll_get_tx_pdm_fs(hal->dev, (uint32_t *)fs);
+    i2s_ll_get_rx_pdm(hal->dev, dsr);
 }
 #endif
 
@@ -208,10 +225,10 @@ void i2s_hal_config_param(i2s_hal_context_t *hal, const i2s_config_t *i2s_config
         i2s_ll_set_tx_pdm_en(hal->dev, 0);
     } else {
         if (i2s_config->mode & I2S_MODE_TX) {
-            i2s_ll_tx_pdm_cfg(hal->dev, i2s_config->sample_rate);
+            i2s_ll_tx_pdm_cfg(hal->dev, I2S_TX_PDM_FP_DEF, i2s_config->sample_rate/100);
         }
         if(i2s_config->mode & I2S_MODE_RX) {
-            i2s_ll_rx_pdm_cfg(hal->dev);
+            i2s_ll_rx_pdm_cfg(hal->dev, I2S_RX_PDM_DSR_DEF);
         }
         // PDM mode have nothing to do with communication format configuration.
         return;

components/hal/include/hal/i2s_hal.h

@@ -151,25 +151,6 @@ void i2s_hal_set_rx_mode(i2s_hal_context_t *hal, i2s_channel_t ch, i2s_bits_per_
  */
 void i2s_hal_set_in_link(i2s_hal_context_t *hal, uint32_t rx_eof_num, uint32_t addr);
 
-#if SOC_I2S_SUPPORTS_PDM
-/**
- * @brief Get I2S tx pdm
- *
- * @param hal Context of the HAL layer
- * @param fp tx pdm fp
- * @param fs tx pdm fs
- */
-void i2s_hal_get_tx_pdm(i2s_hal_context_t *hal, int *fp, int *fs);
-#endif
-
-/**
- * @brief Get I2S rx sinc dsr 16 en
- *
- * @param hal Context of the HAL layer
- * @param en 0: disable, 1: enable
- */
-#define i2s_hal_get_rx_sinc_dsr_16_en(hal, en) i2s_ll_get_rx_sinc_dsr_16_en((hal)->dev, en)
-
 /**
  * @brief Set I2S clk div
  *
@@ -241,16 +222,6 @@ void i2s_hal_stop_tx(i2s_hal_context_t *hal);
  */
 void i2s_hal_stop_rx(i2s_hal_context_t *hal);
 
-#if SOC_I2S_SUPPORTS_PDM
-/**
- * @brief Set I2S pdm rx down sample
- *
- * @param hal Context of the HAL layer
- * @param dsr 0:disable, 1: enable
- */
-#define i2s_hal_set_pdm_rx_down_sample(hal, dsr) i2s_ll_set_rx_sinc_dsr_16_en((hal)->dev, dsr)
-#endif
-
 /**
  * @brief Config I2S param
  *
@@ -288,6 +259,42 @@ void i2s_hal_enable_slave_mode(i2s_hal_context_t *hal);
  */
 void i2s_hal_init(i2s_hal_context_t *hal, int i2s_num);
 
+#if SOC_I2S_SUPPORTS_PDM
+/**
+ * @brief Set I2S tx pdm
+ *
+ * @param hal Context of the HAL layer
+ * @param fp tx pdm fp
+ * @param fs tx pdm fs
+ */
+void i2s_hal_tx_pdm_cfg(i2s_hal_context_t *hal, uint32_t fp, uint32_t fs);
+
+/**
+ * @brief Get I2S tx pdm
+ *
+ * @param hal Context of the HAL layer
+ * @param dsr rx pdm dsr
+ */
+void i2s_hal_rx_pdm_cfg(i2s_hal_context_t *hal, uint32_t dsr);
+
+/**
+ * @brief Get I2S tx pdm configuration
+ *
+ * @param hal Context of the HAL layer
+ * @param fp Pointer to receive tx PDM fp configuration
+ * @param fs Pointer to receive tx PDM fs configuration
+ */
+void i2s_hal_get_tx_pdm(i2s_hal_context_t *hal, uint32_t *fp, uint32_t *fs);
+
+/**
+ * @brief Get I2S rx pdm configuration
+ *
+ * @param hal Context of the HAL layer
+ * @param dsr rx pdm dsr
+ */
+void i2s_hal_get_rx_pdm(i2s_hal_context_t *hal, uint32_t *dsr);
+#endif
+
 #ifdef __cplusplus
 }
 #endif