From b02eb0161925f25d0d7f08d3228a3c78125f964e Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Thu, 12 Dec 2024 21:28:59 +0800 Subject: [PATCH 1/4] fix(i2s): fixed incorrect buf size calculation --- components/esp_driver_i2s/i2s_common.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/esp_driver_i2s/i2s_common.c b/components/esp_driver_i2s/i2s_common.c index 3e0ba8fa4a..5d23e1ef12 100644 --- a/components/esp_driver_i2s/i2s_common.c +++ b/components/esp_driver_i2s/i2s_common.c @@ -417,7 +417,11 @@ err: uint32_t i2s_get_buf_size(i2s_chan_handle_t handle, uint32_t data_bit_width, uint32_t dma_frame_num) { uint32_t active_chan = handle->active_slot; +#if CONFIG_IDF_TARGET_ESP32 uint32_t bytes_per_sample = ((data_bit_width + 15) / 16) * 2; +#else + uint32_t bytes_per_sample = (data_bit_width + 7) / 8; +#endif // CONFIG_IDF_TARGET_ESP32 uint32_t bytes_per_frame = bytes_per_sample * active_chan; uint32_t bufsize = dma_frame_num * bytes_per_frame; #if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE From 6cc2c717a9043835d6d986c3d3f4b06198d3a9b2 Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Tue, 17 Dec 2024 14:41:02 +0800 Subject: [PATCH 2/4] fix(i2s): return error when mclk_div is smaller than 2 --- components/esp_driver_i2s/i2s_pdm.c | 8 ++++---- components/esp_driver_i2s/i2s_std.c | 6 ++++-- components/esp_driver_i2s/i2s_tdm.c | 5 +++-- components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c | 4 ++++ .../test_apps/i2s_multi_dev/main/test_i2s_multi_dev.c | 7 +++++++ 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/components/esp_driver_i2s/i2s_pdm.c b/components/esp_driver_i2s/i2s_pdm.c index f23e540f85..82cb7f29e9 100644 --- a/components/esp_driver_i2s/i2s_pdm.c +++ b/components/esp_driver_i2s/i2s_pdm.c @@ -55,8 +55,8 @@ static esp_err_t i2s_pdm_tx_calculate_clock(i2s_chan_handle_t handle, const i2s_ clk_info->sclk = i2s_get_source_clk_freq(clk_cfg->clk_src, clk_info->mclk); clk_info->mclk_div = clk_info->sclk / clk_info->mclk; - /* Check if the configuration is correct */ - ESP_RETURN_ON_FALSE(clk_info->mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); + /* Check if the configuration is correct. Use float for check in case the mclk division might be carried up in the fine division calculation */ + ESP_RETURN_ON_FALSE(clk_info->sclk / (float)clk_info->mclk > 1.99, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); ESP_RETURN_ON_FALSE(clk_info->mclk_div < 256, ESP_ERR_INVALID_ARG, TAG, "sample rate is too small"); #if SOC_I2S_SUPPORTS_PCM2PDM if (!handle->is_raw_pdm) { @@ -367,8 +367,8 @@ static esp_err_t i2s_pdm_rx_calculate_clock(i2s_chan_handle_t handle, const i2s_ clk_info->sclk = i2s_get_source_clk_freq(clk_cfg->clk_src, clk_info->mclk); clk_info->mclk_div = clk_info->sclk / clk_info->mclk; - /* Check if the configuration is correct */ - ESP_RETURN_ON_FALSE(clk_info->mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); + /* Check if the configuration is correct. Use float for check in case the mclk division might be carried up in the fine division calculation */ + ESP_RETURN_ON_FALSE(clk_info->sclk / (float)clk_info->mclk >= 0.99, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); #if SOC_I2S_SUPPORTS_PDM2PCM if (!handle->is_raw_pdm) { /* Set down-sampling configuration */ diff --git a/components/esp_driver_i2s/i2s_std.c b/components/esp_driver_i2s/i2s_std.c index 564f2cd4f7..8c6ddc72bf 100644 --- a/components/esp_driver_i2s/i2s_std.c +++ b/components/esp_driver_i2s/i2s_std.c @@ -51,13 +51,15 @@ static esp_err_t i2s_std_calculate_clock(i2s_chan_handle_t handle, const i2s_std #if SOC_I2S_HW_VERSION_2 clk_info->sclk = clk_cfg->clk_src == I2S_CLK_SRC_EXTERNAL ? clk_cfg->ext_clk_freq_hz : i2s_get_source_clk_freq(clk_cfg->clk_src, clk_info->mclk); + float min_mclk_div = clk_cfg->clk_src == I2S_CLK_SRC_EXTERNAL ? 0.99 : 1.99; #else clk_info->sclk = i2s_get_source_clk_freq(clk_cfg->clk_src, clk_info->mclk); + float min_mclk_div = 1.99; #endif clk_info->mclk_div = clk_info->sclk / clk_info->mclk; - /* Check if the configuration is correct */ - ESP_RETURN_ON_FALSE(clk_info->mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large for the current clock source"); + /* Check if the configuration is correct. Use float for check in case the mclk division might be carried up in the fine division calculation */ + ESP_RETURN_ON_FALSE(clk_info->sclk / (float)clk_info->mclk > min_mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate or mclk_multiple is too large for the current clock source"); return ESP_OK; } diff --git a/components/esp_driver_i2s/i2s_tdm.c b/components/esp_driver_i2s/i2s_tdm.c index 01236591f4..8f48f91ea4 100644 --- a/components/esp_driver_i2s/i2s_tdm.c +++ b/components/esp_driver_i2s/i2s_tdm.c @@ -61,10 +61,11 @@ static esp_err_t i2s_tdm_calculate_clock(i2s_chan_handle_t handle, const i2s_tdm } clk_info->sclk = clk_cfg->clk_src == I2S_CLK_SRC_EXTERNAL ? clk_cfg->ext_clk_freq_hz : i2s_get_source_clk_freq(clk_cfg->clk_src, clk_info->mclk); + float min_mclk_div = clk_cfg->clk_src == I2S_CLK_SRC_EXTERNAL ? 0.99 : 1.99; clk_info->mclk_div = clk_info->sclk / clk_info->mclk; - /* Check if the configuration is correct */ - ESP_RETURN_ON_FALSE(clk_info->mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large for the current clock source"); + /* Check if the configuration is correct. Use float for check in case the mclk division might be carried up in the fine division calculation */ + ESP_RETURN_ON_FALSE(clk_info->sclk / (float)clk_info->mclk > min_mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate or mclk_multiple is too large for the current clock source"); return ESP_OK; } diff --git a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c index 43b2c70526..98908519fe 100644 --- a/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c +++ b/components/esp_driver_i2s/test_apps/i2s/main/test_i2s.c @@ -879,7 +879,11 @@ TEST_CASE("I2S_package_lost_test", "[i2s]") TEST_ESP_OK(i2s_channel_register_event_callback(rx_handle, &cbs, &count)); uint32_t test_freq[] = {16000, 32000, 48000, 64000, 96000, 128000, 144000}; +#if CONFIG_IDF_TARGET_ESP32P4 + uint32_t test_num = 4; +#else uint32_t test_num = sizeof(test_freq) / sizeof(uint32_t); +#endif // CONFIG_IDF_TARGET_ESP32P4 uint8_t *data = (uint8_t *)calloc(TEST_RECV_BUF_LEN, sizeof(uint8_t)); size_t bytes_read = 0; int i; diff --git a/components/esp_driver_i2s/test_apps/i2s_multi_dev/main/test_i2s_multi_dev.c b/components/esp_driver_i2s/test_apps/i2s_multi_dev/main/test_i2s_multi_dev.c index b2d26c6e4b..f39c446351 100644 --- a/components/esp_driver_i2s/test_apps/i2s_multi_dev/main/test_i2s_multi_dev.c +++ b/components/esp_driver_i2s/test_apps/i2s_multi_dev/main/test_i2s_multi_dev.c @@ -339,6 +339,13 @@ static void test_i2s_external_clk_src(bool is_master, bool is_external) std_cfg.clk_cfg.clk_src = I2S_CLK_SRC_EXTERNAL; std_cfg.clk_cfg.ext_clk_freq_hz = 22579200; } +#if CONFIG_IDF_TARGET_ESP32P4 + else { + // Use APLL instead. + // Because the default clock source is not sufficient for 22.58M MCLK + std_cfg.clk_cfg.clk_src = I2S_CLK_SRC_APLL; + } +#endif TEST_ESP_OK(i2s_channel_init_std_mode(tx_handle, &std_cfg)); TEST_ESP_OK(i2s_channel_init_std_mode(rx_handle, &std_cfg)); From 74427172e18b49c70dc0770c73620b77ee6d8562 Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Tue, 17 Dec 2024 19:05:50 +0800 Subject: [PATCH 3/4] fix(i2s): fixed i2s coverity issue --- components/esp_driver_i2s/i2s_common.c | 55 +++++++++++++++++--------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/components/esp_driver_i2s/i2s_common.c b/components/esp_driver_i2s/i2s_common.c index 5d23e1ef12..11487a80a2 100644 --- a/components/esp_driver_i2s/i2s_common.c +++ b/components/esp_driver_i2s/i2s_common.c @@ -749,8 +749,9 @@ static void IRAM_ATTR i2s_dma_tx_callback(void *arg) #pragma GCC diagnostic pop +#if SOC_GDMA_SUPPORTED /** - * @brief I2S DMA interrupt initialization + * @brief I2S DMA interrupt initialization (implemented by I2S dedicated DMA) * @note I2S will use GDMA if chip supports, and the interrupt is triggered by GDMA. * * @param handle I2S channel handle @@ -766,7 +767,6 @@ esp_err_t i2s_init_dma_intr(i2s_chan_handle_t handle, int intr_flag) esp_err_t ret = ESP_OK; i2s_port_t port_id = handle->controller->id; ESP_RETURN_ON_FALSE((port_id >= 0) && (port_id < SOC_I2S_NUM), ESP_ERR_INVALID_ARG, TAG, "invalid handle"); -#if SOC_GDMA_SUPPORTED /* Set GDMA trigger module */ gdma_trigger_t trig = {.periph = GDMA_TRIG_PERIPH_I2S}; @@ -808,31 +808,48 @@ esp_err_t i2s_init_dma_intr(i2s_chan_handle_t handle, int intr_flag) /* Set callback function for GDMA, the interrupt is triggered by GDMA, then the GDMA ISR will call the callback function */ ESP_GOTO_ON_ERROR(gdma_register_rx_event_callbacks(handle->dma.dma_chan, &cb, handle), err2, TAG, "Register rx callback failed"); } -#else - intr_flag |= handle->intr_prio_flags; - /* Initialize I2S module interrupt */ - if (handle->dir == I2S_DIR_TX) { - esp_intr_alloc_intrstatus(i2s_periph_signal[port_id].irq, intr_flag, - (uint32_t)i2s_ll_get_interrupt_status_reg(handle->controller->hal.dev), I2S_LL_TX_EVENT_MASK, - i2s_dma_tx_callback, handle, &handle->dma.dma_chan); - } else { - esp_intr_alloc_intrstatus(i2s_periph_signal[port_id].irq, intr_flag, - (uint32_t)i2s_ll_get_interrupt_status_reg(handle->controller->hal.dev), I2S_LL_RX_EVENT_MASK, - i2s_dma_rx_callback, handle, &handle->dma.dma_chan); - } - /* Start DMA */ - i2s_ll_enable_dma(handle->controller->hal.dev, true); -#endif // SOC_GDMA_SUPPORTED return ret; -#if SOC_GDMA_SUPPORTED err2: gdma_disconnect(handle->dma.dma_chan); err1: gdma_del_channel(handle->dma.dma_chan); handle->dma.dma_chan = NULL; return ret; -#endif } +#else +/** + * @brief I2S DMA interrupt initialization (implemented by I2S dedicated DMA) + * @note I2S will use GDMA if chip supports, and the interrupt is triggered by GDMA. + * + * @param handle I2S channel handle + * @param intr_flag Interrupt allocation flag + * @return + * - ESP_OK I2S DMA interrupt initialize success + * - ESP_ERR_NOT_FOUND GDMA channel not found + * - ESP_ERR_INVALID_ARG Invalid arguments + * - ESP_ERR_INVALID_STATE GDMA state error + */ +esp_err_t i2s_init_dma_intr(i2s_chan_handle_t handle, int intr_flag) +{ + esp_err_t ret = ESP_OK; + i2s_port_t port_id = handle->controller->id; + ESP_RETURN_ON_FALSE((port_id >= 0) && (port_id < SOC_I2S_NUM), ESP_ERR_INVALID_ARG, TAG, "invalid handle"); + intr_flag |= handle->intr_prio_flags; + /* Initialize I2S module interrupt */ + if (handle->dir == I2S_DIR_TX) { + ESP_RETURN_ON_ERROR(esp_intr_alloc_intrstatus(i2s_periph_signal[port_id].irq, intr_flag, + (uint32_t)i2s_ll_get_interrupt_status_reg(handle->controller->hal.dev), I2S_LL_TX_EVENT_MASK, + i2s_dma_tx_callback, handle, &handle->dma.dma_chan), TAG, "Allocate tx dma channel failed"); + } else { + ESP_RETURN_ON_ERROR(esp_intr_alloc_intrstatus(i2s_periph_signal[port_id].irq, intr_flag, + (uint32_t)i2s_ll_get_interrupt_status_reg(handle->controller->hal.dev), I2S_LL_RX_EVENT_MASK, + i2s_dma_rx_callback, handle, &handle->dma.dma_chan), TAG, "Allocate rx dma channel failed"); + } + /* Start DMA */ + i2s_ll_enable_dma(handle->controller->hal.dev, true); + return ret; +} +#endif // SOC_GDMA_SUPPORTED static uint64_t s_i2s_get_pair_chan_gpio_mask(i2s_chan_handle_t handle) { From 0b809a1bc515cf6b784e0cd1ed4dda5e07d8caaf Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Wed, 18 Dec 2024 11:27:11 +0800 Subject: [PATCH 4/4] docs(example): added troubleshooting for i2s_es8311 example Closes https://github.com/espressif/esp-idf/issues/15047 --- .../peripherals/i2s/i2s_codec/i2s_es8311/README.md | 10 ++++++++++ .../i2s_codec/i2s_es8311/main/Kconfig.projbuild | 7 +++++++ .../i2s/i2s_codec/i2s_es8311/main/example_config.h | 1 + .../i2s_codec/i2s_es8311/main/i2s_es8311_example.c | 14 +++++++++++++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/examples/peripherals/i2s/i2s_codec/i2s_es8311/README.md b/examples/peripherals/i2s/i2s_codec/i2s_es8311/README.md index 79aee879b6..936964f98f 100644 --- a/examples/peripherals/i2s/i2s_codec/i2s_es8311/README.md +++ b/examples/peripherals/i2s/i2s_codec/i2s_es8311/README.md @@ -129,6 +129,11 @@ If you have a logic analyzer, you can use a logic analyzer to grab GPIO signal d | SDOUT |serial data out| GPIO_NUM_18/2 | | SDIN |serial data in | GPIO_NUM_19/3 | +Other pins like I2C please refer to `example_config.h`. + +Please note that the power amplifier on some development boards (like P4 EV board) are disabled by default, you might need to set the PA_CTRL pin to high to play the music via a speaker. +The PA_CTRL pin can be configured by `idf.py menuconfig`, please check if the PA_CTRL pin is correct on your board if the audio can only be played from the earphones but not the speaker. + ### Customize your own music The example have contained a piece of music in canon.pcm, if you want to play your own music, you can follow these steps: @@ -150,4 +155,9 @@ The example have contained a piece of music in canon.pcm, if you want to play yo * Hardware connection is not correct: run `idf.py -p PORT monitor`, and reboot your board to see if there are any output logs. * The baud rate for downloading is too high: lower your baud rate in the `menuconfig` menu, and try again. +* Failed to get audio from specker + + * The PA (Power Amplifier) on some dev-kits might be disabled by default, please check the schematic to see if PA_CTRL is connected to any GPIO or something. + * Pull-up the PA_CTRL pin either by setting that GPIO to high or by connecting it to 3.3V with a jump wire should help. + For any technical queries, please open an [issue](https://github.com/espressif/esp-idf/issues) on GitHub. We will get back to you soon. diff --git a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/Kconfig.projbuild b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/Kconfig.projbuild index 4595d6e1d8..72ce831be6 100644 --- a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/Kconfig.projbuild +++ b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/Kconfig.projbuild @@ -56,6 +56,13 @@ menu "Example Configuration" help Set voice volume + config EXAMPLE_PA_CTRL_IO + int "Power Amplifier control IO" + default 53 if IDF_TARGET_ESP32P4 + default -1 + help + Set GPIO number for PA control. Set -1 to disable PA control. + config EXAMPLE_BSP bool "Enable Board Support Package (BSP) support" default n diff --git a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/example_config.h b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/example_config.h index 87f1cbfecd..5c3181c4e2 100644 --- a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/example_config.h +++ b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/example_config.h @@ -14,6 +14,7 @@ #define EXAMPLE_MCLK_MULTIPLE (384) // If not using 24-bit data width, 256 should be enough #define EXAMPLE_MCLK_FREQ_HZ (EXAMPLE_SAMPLE_RATE * EXAMPLE_MCLK_MULTIPLE) #define EXAMPLE_VOICE_VOLUME CONFIG_EXAMPLE_VOICE_VOLUME +#define EXAMPLE_PA_CTRL_IO CONFIG_EXAMPLE_PA_CTRL_IO #if CONFIG_EXAMPLE_MODE_ECHO #define EXAMPLE_MIC_GAIN CONFIG_EXAMPLE_MIC_GAIN #endif diff --git a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/i2s_es8311_example.c b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/i2s_es8311_example.c index 3abbb23efd..d0fe16226a 100644 --- a/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/i2s_es8311_example.c +++ b/examples/peripherals/i2s/i2s_codec/i2s_es8311/main/i2s_es8311_example.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: CC0-1.0 */ @@ -10,6 +10,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "driver/i2s_std.h" +#include "driver/gpio.h" #include "esp_system.h" #include "esp_check.h" #include "es8311.h" @@ -198,6 +199,17 @@ void app_main(void) } else { ESP_LOGI(TAG, "es8311 codec init success"); } + +#if EXAMPLE_PA_CTRL_IO >= 0 + /* Enable PA by setting the PA_CTRL_IO to high, because the power amplifier on some dev-kits are disabled by default */ + gpio_config_t gpio_cfg = { + .pin_bit_mask = 1ULL << EXAMPLE_PA_CTRL_IO, + .mode = GPIO_MODE_OUTPUT, + }; + ESP_ERROR_CHECK(gpio_config(&gpio_cfg)); + ESP_ERROR_CHECK(gpio_set_level(EXAMPLE_PA_CTRL_IO, 1)); +#endif + #if CONFIG_EXAMPLE_MODE_MUSIC /* Play a piece of music in music mode */ xTaskCreate(i2s_music, "i2s_music", 4096, NULL, 5, NULL);