From 6a5b8efdc57c4bb52ffb352d2a0e5d095349b2a7 Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Wed, 26 Mar 2025 15:24:29 +0800 Subject: [PATCH 1/2] fix(i2s): reset the dma buf_size while allocation failed Closes https://github.com/espressif/esp-idf/issues/15648 --- components/driver/i2s/i2s_common.c | 40 +++++++++++++++-------------- components/driver/i2s/i2s_pdm.c | 6 ++--- components/driver/i2s/i2s_private.h | 3 +-- components/driver/i2s/i2s_std.c | 3 +-- components/driver/i2s/i2s_tdm.c | 3 +-- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/components/driver/i2s/i2s_common.c b/components/driver/i2s/i2s_common.c index dcd06bd314..560db919f2 100644 --- a/components/driver/i2s/i2s_common.c +++ b/components/driver/i2s/i2s_common.c @@ -386,37 +386,39 @@ uint32_t i2s_get_buf_size(i2s_chan_handle_t handle, uint32_t data_bit_width, uin esp_err_t i2s_free_dma_desc(i2s_chan_handle_t handle) { I2S_NULL_POINTER_CHECK(TAG, handle); - if (!handle->dma.desc) { - return ESP_OK; - } - for (int i = 0; i < handle->dma.desc_num; i++) { - if (handle->dma.bufs[i]) { - free(handle->dma.bufs[i]); - handle->dma.bufs[i] = NULL; - } - if (handle->dma.desc[i]) { - free(handle->dma.desc[i]); - handle->dma.desc[i] = NULL; - } - } - if (handle->dma.bufs) { - free(handle->dma.bufs); - handle->dma.bufs = NULL; - } + handle->dma.buf_size = 0; + if (handle->dma.desc) { + for (int i = 0; i < handle->dma.desc_num; i++) { + if (handle->dma.desc[i]) { + free(handle->dma.desc[i]); + handle->dma.desc[i] = NULL; + } + } free(handle->dma.desc); handle->dma.desc = NULL; } + if (handle->dma.bufs) { + for (int i = 0; i < handle->dma.desc_num; i++) { + if (handle->dma.bufs[i]) { + free(handle->dma.bufs[i]); + handle->dma.bufs[i] = NULL; + } + } + free(handle->dma.bufs); + handle->dma.bufs = NULL; + } + return ESP_OK; } -esp_err_t i2s_alloc_dma_desc(i2s_chan_handle_t handle, uint32_t num, uint32_t bufsize) +esp_err_t i2s_alloc_dma_desc(i2s_chan_handle_t handle, uint32_t bufsize) { I2S_NULL_POINTER_CHECK(TAG, handle); esp_err_t ret = ESP_OK; ESP_RETURN_ON_FALSE(bufsize <= I2S_DMA_BUFFER_MAX_SIZE, ESP_ERR_INVALID_ARG, TAG, "dma buffer can't be bigger than %d", I2S_DMA_BUFFER_MAX_SIZE); - handle->dma.desc_num = num; + uint32_t num = handle->dma.desc_num; handle->dma.buf_size = bufsize; /* Descriptors must be in the internal RAM */ diff --git a/components/driver/i2s/i2s_pdm.c b/components/driver/i2s/i2s_pdm.c index 60c3ead152..79423fd316 100644 --- a/components/driver/i2s/i2s_pdm.c +++ b/components/driver/i2s/i2s_pdm.c @@ -95,9 +95,8 @@ static esp_err_t i2s_pdm_tx_set_slot(i2s_chan_handle_t handle, const i2s_pdm_tx_ uint32_t buf_size = i2s_get_buf_size(handle, slot_cfg->data_bit_width, handle->dma.frame_num); /* The DMA buffer need to re-allocate if the buffer size changed */ if (handle->dma.buf_size != buf_size) { - handle->dma.buf_size = buf_size; ESP_RETURN_ON_ERROR(i2s_free_dma_desc(handle), TAG, "failed to free the old dma descriptor"); - ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, handle->dma.desc_num, buf_size), + ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, buf_size), TAG, "allocate memory for dma descriptor failed"); } /* Share bck and ws signal in full-duplex mode */ @@ -381,9 +380,8 @@ static esp_err_t i2s_pdm_rx_set_slot(i2s_chan_handle_t handle, const i2s_pdm_rx_ uint32_t buf_size = i2s_get_buf_size(handle, slot_cfg->data_bit_width, handle->dma.frame_num); /* The DMA buffer need to re-allocate if the buffer size changed */ if (handle->dma.buf_size != buf_size) { - handle->dma.buf_size = buf_size; ESP_RETURN_ON_ERROR(i2s_free_dma_desc(handle), TAG, "failed to free the old dma descriptor"); - ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, handle->dma.desc_num, buf_size), + ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, buf_size), TAG, "allocate memory for dma descriptor failed"); } /* Share bck and ws signal in full-duplex mode */ diff --git a/components/driver/i2s/i2s_private.h b/components/driver/i2s/i2s_private.h index dae3aba359..cfc7ec0335 100644 --- a/components/driver/i2s/i2s_private.h +++ b/components/driver/i2s/i2s_private.h @@ -188,7 +188,6 @@ esp_err_t i2s_free_dma_desc(i2s_chan_handle_t handle); * @brief Allocate memory for I2S DMA descriptor and DMA buffer * * @param handle I2S channel handle - * @param num Number of DMA descriptors * @param bufsize The DMA buffer size * * @return @@ -196,7 +195,7 @@ esp_err_t i2s_free_dma_desc(i2s_chan_handle_t handle); * - ESP_ERR_INVALID_ARG NULL pointer or bufsize is too big * - ESP_ERR_NO_MEM No memory for DMA descriptor and DMA buffer */ -esp_err_t i2s_alloc_dma_desc(i2s_chan_handle_t handle, uint32_t num, uint32_t bufsize); +esp_err_t i2s_alloc_dma_desc(i2s_chan_handle_t handle, uint32_t bufsize); /** * @brief Get DMA buffer size diff --git a/components/driver/i2s/i2s_std.c b/components/driver/i2s/i2s_std.c index fe1075aa97..7fd2f8dda3 100644 --- a/components/driver/i2s/i2s_std.c +++ b/components/driver/i2s/i2s_std.c @@ -103,9 +103,8 @@ static esp_err_t i2s_std_set_slot(i2s_chan_handle_t handle, const i2s_std_slot_c uint32_t buf_size = i2s_get_buf_size(handle, slot_cfg->data_bit_width, handle->dma.frame_num); /* The DMA buffer need to re-allocate if the buffer size changed */ if (handle->dma.buf_size != buf_size) { - handle->dma.buf_size = buf_size; ESP_RETURN_ON_ERROR(i2s_free_dma_desc(handle), TAG, "failed to free the old dma descriptor"); - ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, handle->dma.desc_num, buf_size), + ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, buf_size), TAG, "allocate memory for dma descriptor failed"); } bool is_slave = handle->role == I2S_ROLE_SLAVE; diff --git a/components/driver/i2s/i2s_tdm.c b/components/driver/i2s/i2s_tdm.c index 64a68b1189..1889c2511c 100644 --- a/components/driver/i2s/i2s_tdm.c +++ b/components/driver/i2s/i2s_tdm.c @@ -111,9 +111,8 @@ static esp_err_t i2s_tdm_set_slot(i2s_chan_handle_t handle, const i2s_tdm_slot_c uint32_t buf_size = i2s_get_buf_size(handle, slot_cfg->data_bit_width, handle->dma.frame_num); /* The DMA buffer need to re-allocate if the buffer size changed */ if (handle->dma.buf_size != buf_size) { - handle->dma.buf_size = buf_size; ESP_RETURN_ON_ERROR(i2s_free_dma_desc(handle), TAG, "failed to free the old dma descriptor"); - ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, handle->dma.desc_num, buf_size), + ESP_RETURN_ON_ERROR(i2s_alloc_dma_desc(handle, buf_size), TAG, "allocate memory for dma descriptor failed"); } bool is_slave = handle->role == I2S_ROLE_SLAVE; From dd1f36f0a19c488814023b160b4da672eac5e173 Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Fri, 28 Mar 2025 15:23:56 +0800 Subject: [PATCH 2/2] fix(i2s): add check for the minimum sample rate Closes https://github.com/espressif/esp-idf/issues/15659 --- components/driver/i2s/i2s_pdm.c | 6 ++++-- components/driver/i2s/i2s_std.c | 3 ++- components/driver/i2s/i2s_tdm.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/components/driver/i2s/i2s_pdm.c b/components/driver/i2s/i2s_pdm.c index 79423fd316..26b03a1279 100644 --- a/components/driver/i2s/i2s_pdm.c +++ b/components/driver/i2s/i2s_pdm.c @@ -45,7 +45,8 @@ static esp_err_t i2s_pdm_tx_calculate_clock(i2s_chan_handle_t handle, const i2s_ clk_info->mclk_div = clk_info->sclk / clk_info->mclk; /* 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((float)clk_info->sclk > 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"); /* Set up sampling configuration */ i2s_ll_tx_set_pdm_fpfs(handle->controller->hal.dev, pdm_tx_clk->up_sample_fp, pdm_tx_clk->up_sample_fs); i2s_ll_tx_set_pdm_over_sample_ratio(handle->controller->hal.dev, over_sample_ratio); @@ -338,7 +339,8 @@ static esp_err_t i2s_pdm_rx_calculate_clock(i2s_chan_handle_t handle, const i2s_ clk_info->mclk_div = clk_info->sclk / clk_info->mclk; /* 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((float)clk_info->sclk > clk_info->mclk * 1.99, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); + ESP_RETURN_ON_FALSE(clk_info->mclk_div < I2S_LL_CLK_FRAC_DIV_N_MAX, ESP_ERR_INVALID_ARG, TAG, "sample rate is too small"); /* Set down-sampling configuration */ i2s_ll_rx_set_pdm_dsr(handle->controller->hal.dev, pdm_rx_clk->dn_sample_mode); return ESP_OK; diff --git a/components/driver/i2s/i2s_std.c b/components/driver/i2s/i2s_std.c index 7fd2f8dda3..d3468622c6 100644 --- a/components/driver/i2s/i2s_std.c +++ b/components/driver/i2s/i2s_std.c @@ -56,7 +56,8 @@ static esp_err_t i2s_std_calculate_clock(i2s_chan_handle_t handle, const i2s_std clk_info->mclk_div = clk_info->sclk / clk_info->mclk; /* 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"); + ESP_RETURN_ON_FALSE((float)clk_info->sclk > clk_info->mclk * min_mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); + ESP_RETURN_ON_FALSE(clk_info->mclk_div < I2S_LL_CLK_FRAC_DIV_N_MAX, ESP_ERR_INVALID_ARG, TAG, "sample rate is too small"); return ESP_OK; } diff --git a/components/driver/i2s/i2s_tdm.c b/components/driver/i2s/i2s_tdm.c index 1889c2511c..f10d7a60fe 100644 --- a/components/driver/i2s/i2s_tdm.c +++ b/components/driver/i2s/i2s_tdm.c @@ -62,7 +62,8 @@ static esp_err_t i2s_tdm_calculate_clock(i2s_chan_handle_t handle, const i2s_tdm clk_info->mclk_div = clk_info->sclk / clk_info->mclk; /* 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"); + ESP_RETURN_ON_FALSE((float)clk_info->sclk > clk_info->mclk * min_mclk_div, ESP_ERR_INVALID_ARG, TAG, "sample rate is too large"); + ESP_RETURN_ON_FALSE(clk_info->mclk_div < I2S_LL_CLK_FRAC_DIV_N_MAX, ESP_ERR_INVALID_ARG, TAG, "sample rate is too small"); return ESP_OK; }