From 5bca458dbcea3ad64c5cc86993c3a5e3837d9fbe Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Wed, 28 Aug 2024 10:02:47 +0800 Subject: [PATCH 1/2] fix(i2s): fixed alignment of max DMA buffer length on P4 Closes https://github.com/espressif/esp-idf/issues/14448 --- components/esp_driver_i2s/i2s_common.c | 13 +++++++++---- .../test_apps/i2s/main/test_i2s.c | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/components/esp_driver_i2s/i2s_common.c b/components/esp_driver_i2s/i2s_common.c index 55bb984661..e4b63a05f3 100644 --- a/components/esp_driver_i2s/i2s_common.c +++ b/components/esp_driver_i2s/i2s_common.c @@ -25,6 +25,7 @@ #include "soc/soc_caps.h" #include "hal/gpio_hal.h" #include "hal/i2s_hal.h" +#include "hal/dma_types.h" #if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE #include "hal/cache_hal.h" #include "hal/cache_ll.h" @@ -62,8 +63,12 @@ #include "esp_memory_utils.h" /* The actual max size of DMA buffer is 4095 - * Set 4092 here to align with 4-byte, so that the position of the slot data in the buffer will be relatively fixed */ -#define I2S_DMA_BUFFER_MAX_SIZE (4092) + * Reserve several bytes for alignment, so that the position of the slot data in the buffer will be relatively fixed */ +#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE +#define I2S_DMA_BUFFER_MAX_SIZE DMA_DESCRIPTOR_BUFFER_MAX_SIZE_64B_ALIGNED +#else +#define I2S_DMA_BUFFER_MAX_SIZE DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED +#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE static const char *TAG = "i2s_common"; @@ -357,7 +362,7 @@ uint32_t i2s_get_buf_size(i2s_chan_handle_t handle, uint32_t data_bit_width, uin for (int sign = 1; bufsize % alignment != 0; aligned_frame_num += sign) { bufsize = aligned_frame_num * bytes_per_frame; /* If the buffer size exceed the max dma size */ - if (bufsize > I2S_DMA_BUFFER_MAX_SIZE) { + if (bufsize > I2S_DMA_BUFFER_MAX_SIZE && sign == 1) { sign = -1; // toggle the search sign aligned_frame_num = dma_frame_num; // Reset the frame num bufsize = aligned_frame_num * bytes_per_frame; // Reset the bufsize @@ -368,7 +373,7 @@ uint32_t i2s_get_buf_size(i2s_chan_handle_t handle, uint32_t data_bit_width, uin ", bufsize = %"PRIu32, bufsize / bytes_per_frame, alignment, bufsize); } #endif - /* Limit DMA buffer size if it is out of range (DMA buffer limitation is 4092 bytes) */ + /* Limit DMA buffer size if it is out of range */ if (bufsize > I2S_DMA_BUFFER_MAX_SIZE) { uint32_t frame_num = I2S_DMA_BUFFER_MAX_SIZE / bytes_per_frame; bufsize = frame_num * bytes_per_frame; 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 93aca31233..6a1f7d302d 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 @@ -14,6 +14,7 @@ #include "sdkconfig.h" #include "driver/gpio.h" #include "hal/gpio_hal.h" +#include "hal/dma_types.h" #include "esp_err.h" #include "esp_attr.h" #include "unity.h" @@ -866,17 +867,25 @@ TEST_CASE("I2S_package_lost_test", "[i2s]") { /* Steps of calculate appropriate parameters of I2S buffer: * Known by user: sample_rate = 144k, data_bit_width = 32, slot_num = 2, polling_cycle = 10 ms - * 1. dma_buffer_size = dma_frame_num * slot_num * data_bit_width / 8 <= 4092 - * dma_frame_num <= 511, dma_frame_num is as big as possible. + * 1. dma_buffer_size = dma_frame_num * slot_num * data_bit_width / 8 <= DMA_MAX_ALIGNED_SIZE + * dma_frame_num <= DMA_MAX_ALIGNED_SIZE / data_bit_width / slot_num * 8, dma_frame_num is as big as possible. * interrupt_interval = dma_frame_num / sample_rate = 3.549 ms * 2. dma_desc_num > polling_cycle / interrupt_interval = cell(2.818) = 3 - * 3. recv_buffer_size > dma_desc_num * dma_buffer_size = 3 * 4092 = 12276 bytes */ -#define TEST_RECV_BUF_LEN 12276 + * 3. recv_buffer_size > dma_desc_num * dma_buffer_size = 3 * DMA_MAX_ALIGNED_SIZE */ +#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE +#define TEST_RECV_BUF_LEN (3 * DMA_DESCRIPTOR_BUFFER_MAX_SIZE_64B_ALIGNED) +#else +#define TEST_RECV_BUF_LEN (3 * DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED) +#endif i2s_chan_handle_t rx_handle; i2s_chan_config_t chan_cfg = I2S_CHANNEL_DEFAULT_CONFIG(I2S_NUM_AUTO, I2S_ROLE_MASTER); chan_cfg.dma_desc_num = 3; +#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE + chan_cfg.dma_frame_num = 504; +#else chan_cfg.dma_frame_num = 511; +#endif i2s_std_config_t std_cfg = { .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(SAMPLE_RATE), .slot_cfg = I2S_STD_PHILIPS_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_32BIT, I2S_SLOT_MODE_STEREO), From c518c281dd41a7645910d46e92a557039c0fca75 Mon Sep 17 00:00:00 2001 From: laokaiyao Date: Wed, 28 Aug 2024 11:05:08 +0800 Subject: [PATCH 2/2] change(i2s): add warning for inaccurate sample rate --- components/esp_driver_i2s/i2s_std.c | 3 +++ components/esp_driver_i2s/i2s_tdm.c | 3 +++ tools/idf_py_actions/hints.yml | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/components/esp_driver_i2s/i2s_std.c b/components/esp_driver_i2s/i2s_std.c index c3ef0c0469..109aaa0bc5 100644 --- a/components/esp_driver_i2s/i2s_std.c +++ b/components/esp_driver_i2s/i2s_std.c @@ -39,6 +39,9 @@ static esp_err_t i2s_std_calculate_clock(i2s_chan_handle_t handle, const i2s_std clk_info->bclk = rate * handle->total_slot * slot_bits; clk_info->mclk = rate * clk_cfg->mclk_multiple; clk_info->bclk_div = clk_info->mclk / clk_info->bclk; + if (clk_info->mclk % clk_info->bclk != 0) { + ESP_LOGW(TAG, "the current mclk multiple cannot perform integer division (slot_num: %"PRIu32", slot_bits: %"PRIu32")", handle->total_slot, slot_bits); + } } else { /* For slave mode, mclk >= bclk * 8, so fix bclk_div to 2 first */ clk_info->bclk_div = 8; diff --git a/components/esp_driver_i2s/i2s_tdm.c b/components/esp_driver_i2s/i2s_tdm.c index 584e75881c..6d579105ed 100644 --- a/components/esp_driver_i2s/i2s_tdm.c +++ b/components/esp_driver_i2s/i2s_tdm.c @@ -46,6 +46,9 @@ static esp_err_t i2s_tdm_calculate_clock(i2s_chan_handle_t handle, const i2s_tdm clk_info->mclk = clk_info->bclk * clk_info->bclk_div; ESP_LOGW(TAG, "the current mclk multiple is too small, adjust the mclk multiple to %"PRIu32, clk_info->mclk / rate); } + if (clk_info->mclk % clk_info->bclk != 0) { + ESP_LOGW(TAG, "the current mclk multiple cannot perform integer division (slot_num: %"PRIu32", slot_bits: %"PRIu32")", handle->total_slot, slot_bits); + } } else { if (clk_cfg->bclk_div < 8) { ESP_LOGW(TAG, "the current bclk division is too small, adjust the bclk division to 8"); diff --git a/tools/idf_py_actions/hints.yml b/tools/idf_py_actions/hints.yml index f3bd56bf21..44d0bfc521 100644 --- a/tools/idf_py_actions/hints.yml +++ b/tools/idf_py_actions/hints.yml @@ -430,3 +430,7 @@ - re: "unplaced orphan section" hint: "Avoid creating custom sections. Please refer to the 'Linker Script Generation' article in the IDF documentation to address this. Or set option CONFIG_COMPILER_ORPHAN_SECTIONS_PLACE (not recommended)." + +- + re: "the current mclk multiple cannot perform integer division" + hint: "Please adjust the mclk multiple to get the accurate sample rate.\nFor example, if you're using 24-bit slot width or enabled 3 slots, then the mclk multiple should be a multiple of 3, otherwise the sample rate will be inaccurate."