From 73791ff4e02ea1eb63a3d5e9adc0f4bfdf4d0478 Mon Sep 17 00:00:00 2001 From: Armando Date: Tue, 16 May 2023 14:23:19 +0800 Subject: [PATCH 1/2] adc: fix adc continuous driver conv_frame_size not bigger than 4092 issue Closes https://github.com/espressif/esp-idf/issues/11385 --- components/driver/deprecated/adc_dma_legacy.c | 14 ++-- components/esp_adc/adc_continuous.c | 18 +++-- components/hal/adc_hal.c | 65 ++++++++++++++----- components/hal/include/hal/adc_hal.h | 11 ++-- components/hal/include/hal/dma_types.h | 2 + 5 files changed, 76 insertions(+), 34 deletions(-) diff --git a/components/driver/deprecated/adc_dma_legacy.c b/components/driver/deprecated/adc_dma_legacy.c index d03bc67e24..bd84931171 100644 --- a/components/driver/deprecated/adc_dma_legacy.c +++ b/components/driver/deprecated/adc_dma_legacy.c @@ -207,7 +207,9 @@ esp_err_t adc_digi_initialize(const adc_digi_init_config_t *init_config) } //malloc dma descriptor - s_adc_digi_ctx->hal.rx_desc = heap_caps_calloc(1, (sizeof(dma_descriptor_t)) * INTERNAL_BUF_NUM, MALLOC_CAP_DMA); + uint32_t dma_desc_num_per_frame = (init_config->conv_num_each_intr + DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED - 1) / DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED; + uint32_t dma_desc_max_num = dma_desc_num_per_frame * INTERNAL_BUF_NUM; + s_adc_digi_ctx->hal.rx_desc = heap_caps_calloc(1, (sizeof(dma_descriptor_t)) * dma_desc_max_num, MALLOC_CAP_DMA); if (!s_adc_digi_ctx->hal.rx_desc) { ret = ESP_ERR_NO_MEM; goto cleanup; @@ -310,7 +312,8 @@ esp_err_t adc_digi_initialize(const adc_digi_init_config_t *init_config) #elif CONFIG_IDF_TARGET_ESP32 .dev = (void *)I2S_LL_GET_HW(s_adc_digi_ctx->i2s_host), #endif - .desc_max_num = INTERNAL_BUF_NUM, + .eof_desc_num = INTERNAL_BUF_NUM, + .eof_step = dma_desc_num_per_frame, .dma_chan = dma_chan, .eof_num = init_config->conv_num_each_intr / SOC_ADC_DIGI_DATA_BYTES_PER_CONV }; @@ -367,15 +370,16 @@ static IRAM_ATTR bool s_adc_dma_intr(adc_digi_context_t *adc_digi_ctx) portBASE_TYPE taskAwoken = 0; BaseType_t ret; adc_hal_dma_desc_status_t status = false; - dma_descriptor_t *current_desc = NULL; + uint8_t *finished_buffer = NULL; + uint32_t finished_size = 0; while (1) { - status = adc_hal_get_reading_result(&adc_digi_ctx->hal, adc_digi_ctx->rx_eof_desc_addr, ¤t_desc); + status = adc_hal_get_reading_result(&adc_digi_ctx->hal, adc_digi_ctx->rx_eof_desc_addr, &finished_buffer, &finished_size); if (status != ADC_HAL_DMA_DESC_VALID) { break; } - ret = xRingbufferSendFromISR(adc_digi_ctx->ringbuf_hdl, current_desc->buffer, current_desc->dw0.length, &taskAwoken); + ret = xRingbufferSendFromISR(adc_digi_ctx->ringbuf_hdl, finished_buffer, finished_size, &taskAwoken); if (ret == pdFALSE) { //ringbuffer overflow adc_digi_ctx->ringbuf_overflow_flag = 1; diff --git a/components/esp_adc/adc_continuous.c b/components/esp_adc/adc_continuous.c index 90d5c78c47..6b08be7eae 100644 --- a/components/esp_adc/adc_continuous.c +++ b/components/esp_adc/adc_continuous.c @@ -134,7 +134,9 @@ esp_err_t adc_continuous_new_handle(const adc_continuous_handle_cfg_t *hdl_confi } //malloc dma descriptor - adc_ctx->hal.rx_desc = heap_caps_calloc(1, (sizeof(dma_descriptor_t)) * INTERNAL_BUF_NUM, MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA); + uint32_t dma_desc_num_per_frame = (hdl_config->conv_frame_size + DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED - 1) / DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED; + uint32_t dma_desc_max_num = dma_desc_num_per_frame * INTERNAL_BUF_NUM; + adc_ctx->hal.rx_desc = heap_caps_calloc(1, (sizeof(dma_descriptor_t)) * dma_desc_max_num, MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA); if (!adc_ctx->hal.rx_desc) { ret = ESP_ERR_NO_MEM; goto cleanup; @@ -224,7 +226,8 @@ esp_err_t adc_continuous_new_handle(const adc_continuous_handle_cfg_t *hdl_confi #elif CONFIG_IDF_TARGET_ESP32 .dev = (void *)I2S_LL_GET_HW(adc_ctx->i2s_host), #endif - .desc_max_num = INTERNAL_BUF_NUM, + .eof_desc_num = INTERNAL_BUF_NUM, + .eof_step = dma_desc_num_per_frame, .dma_chan = dma_chan, .eof_num = hdl_config->conv_frame_size / SOC_ADC_DIGI_DATA_BYTES_PER_CONV }; @@ -286,21 +289,22 @@ static IRAM_ATTR bool s_adc_dma_intr(adc_continuous_ctx_t *adc_digi_ctx) bool need_yield = false; BaseType_t ret; adc_hal_dma_desc_status_t status = false; - dma_descriptor_t *current_desc = NULL; + uint8_t *finished_buffer = NULL; + uint32_t finished_size = 0; while (1) { - status = adc_hal_get_reading_result(&adc_digi_ctx->hal, adc_digi_ctx->rx_eof_desc_addr, ¤t_desc); + status = adc_hal_get_reading_result(&adc_digi_ctx->hal, adc_digi_ctx->rx_eof_desc_addr, &finished_buffer, &finished_size); if (status != ADC_HAL_DMA_DESC_VALID) { break; } - ret = xRingbufferSendFromISR(adc_digi_ctx->ringbuf_hdl, current_desc->buffer, current_desc->dw0.length, &taskAwoken); + ret = xRingbufferSendFromISR(adc_digi_ctx->ringbuf_hdl, finished_buffer, finished_size, &taskAwoken); need_yield |= (taskAwoken == pdTRUE); if (adc_digi_ctx->cbs.on_conv_done) { adc_continuous_evt_data_t edata = { - .conv_frame_buffer = current_desc->buffer, - .size = current_desc->dw0.length, + .conv_frame_buffer = finished_buffer, + .size = finished_size, }; if (adc_digi_ctx->cbs.on_conv_done(adc_digi_ctx, &edata, adc_digi_ctx->user_data)) { need_yield |= true; diff --git a/components/hal/adc_hal.c b/components/hal/adc_hal.c index 4d47a3da0b..824d30457b 100644 --- a/components/hal/adc_hal.c +++ b/components/hal/adc_hal.c @@ -96,7 +96,8 @@ void adc_hal_dma_ctx_config(adc_hal_dma_ctx_t *hal, const adc_hal_dma_config_t * { hal->desc_dummy_head.next = hal->rx_desc; hal->dev = config->dev; - hal->desc_max_num = config->desc_max_num; + hal->eof_desc_num = config->eof_desc_num; + hal->eof_step = config->eof_step; hal->dma_chan = config->dma_chan; hal->eof_num = config->eof_num; } @@ -228,23 +229,33 @@ void adc_hal_digi_controller_config(adc_hal_dma_ctx_t *hal, const adc_hal_digi_c adc_hal_digi_sample_freq_config(hal, cfg->clk_src, cfg->clk_src_freq_hz, cfg->sample_freq_hz); } -static void adc_hal_digi_dma_link_descriptors(dma_descriptor_t *desc, uint8_t *data_buf, uint32_t size, uint32_t num) +static void adc_hal_digi_dma_link_descriptors(dma_descriptor_t *desc, uint8_t *data_buf, uint32_t per_eof_size, uint32_t eof_step, uint32_t eof_num) { HAL_ASSERT(((uint32_t)data_buf % 4) == 0); - HAL_ASSERT((size % 4) == 0); + HAL_ASSERT((per_eof_size % 4) == 0); uint32_t n = 0; - while (num--) { - desc[n] = (dma_descriptor_t) { - .dw0.size = size, - .dw0.length = 0, - .dw0.suc_eof = 0, - .dw0.owner = 1, - .buffer = data_buf, - .next = &desc[n+1] - }; - data_buf += size; - n++; + while (eof_num--) { + uint32_t eof_size = per_eof_size; + + for (int i = 0; i < eof_step; i++) { + uint32_t this_len = eof_size; + if (this_len > DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED) { + this_len = DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED; + } + + desc[n] = (dma_descriptor_t) { + .dw0.size = this_len, + .dw0.length = 0, + .dw0.suc_eof = 0, + .dw0.owner = 1, + .buffer = data_buf, + .next = &desc[n+1] + }; + eof_size -= this_len; + data_buf += this_len; + n++; + } } desc[n-1].next = NULL; } @@ -261,7 +272,7 @@ void adc_hal_digi_start(adc_hal_dma_ctx_t *hal, uint8_t *data_buf) //reset the current descriptor address hal->cur_desc_ptr = &hal->desc_dummy_head; - adc_hal_digi_dma_link_descriptors(hal->rx_desc, data_buf, hal->eof_num * SOC_ADC_DIGI_DATA_BYTES_PER_CONV, hal->desc_max_num); + adc_hal_digi_dma_link_descriptors(hal->rx_desc, data_buf, hal->eof_num * SOC_ADC_DIGI_DATA_BYTES_PER_CONV, hal->eof_step, hal->eof_desc_num); //start DMA adc_dma_ll_rx_start(hal->dev, hal->dma_chan, (lldesc_t *)hal->rx_desc); @@ -283,18 +294,36 @@ bool adc_hal_check_event(adc_hal_dma_ctx_t *hal, uint32_t mask) } #endif //#if !SOC_GDMA_SUPPORTED -adc_hal_dma_desc_status_t adc_hal_get_reading_result(adc_hal_dma_ctx_t *hal, const intptr_t eof_desc_addr, dma_descriptor_t **cur_desc) +adc_hal_dma_desc_status_t adc_hal_get_reading_result(adc_hal_dma_ctx_t *hal, const intptr_t eof_desc_addr, uint8_t **buffer, uint32_t *len) { HAL_ASSERT(hal->cur_desc_ptr); + if (!hal->cur_desc_ptr->next) { return ADC_HAL_DMA_DESC_NULL; } + if ((intptr_t)hal->cur_desc_ptr == eof_desc_addr) { return ADC_HAL_DMA_DESC_WAITING; } - hal->cur_desc_ptr = hal->cur_desc_ptr->next; - *cur_desc = hal->cur_desc_ptr; + uint8_t *buffer_start = NULL; + uint32_t eof_len = 0; + dma_descriptor_t *eof_desc = hal->cur_desc_ptr; + + //Find the eof list start + eof_desc = eof_desc->next; + buffer_start = eof_desc->buffer; + eof_len += eof_desc->dw0.length; + + //Find the eof list end + for (int i = 1; i < hal->eof_step; i++) { + eof_desc = eof_desc->next; + eof_len += eof_desc->dw0.length; + } + + hal->cur_desc_ptr = eof_desc; + *buffer = buffer_start; + *len = eof_len; return ADC_HAL_DMA_DESC_VALID; } diff --git a/components/hal/include/hal/adc_hal.h b/components/hal/include/hal/adc_hal.h index 5bbb0c81bd..d023ff7dd6 100644 --- a/components/hal/include/hal/adc_hal.h +++ b/components/hal/include/hal/adc_hal.h @@ -55,7 +55,8 @@ typedef enum adc_hal_dma_desc_status_t { */ typedef struct adc_hal_dma_config_t { void *dev; ///< DMA peripheral address - uint32_t desc_max_num; ///< Number of the descriptors linked once + uint32_t eof_desc_num; ///< Number of dma descriptors that is eof + uint32_t eof_step; ///< Number of linked descriptors that is one eof uint32_t dma_chan; ///< DMA channel to be used uint32_t eof_num; ///< Bytes between 2 in_suc_eof interrupts } adc_hal_dma_config_t; @@ -73,7 +74,8 @@ typedef struct adc_hal_dma_ctx_t { /**< these need to be configured by `adc_hal_dma_config_t` via driver layer*/ void *dev; ///< DMA address - uint32_t desc_max_num; ///< Number of the descriptors linked once + uint32_t eof_desc_num; ///< Number of dma descriptors that is eof + uint32_t eof_step; ///< Number of linked descriptors that is one eof uint32_t dma_chan; ///< DMA channel to be used uint32_t eof_num; ///< Words between 2 in_suc_eof interrupts } adc_hal_dma_ctx_t; @@ -175,11 +177,12 @@ bool adc_hal_check_event(adc_hal_dma_ctx_t *hal, uint32_t mask); * * @param hal Context of the HAL * @param eof_desc_addr The last descriptor that is finished by HW. Should be got from DMA - * @param[out] cur_desc The descriptor with ADC reading result (from the 1st one to the last one (``eof_desc_addr``)) + * @param[out] buffer ADC reading result buffer + * @param[out] len ADC reading result len * * @return See ``adc_hal_dma_desc_status_t`` */ -adc_hal_dma_desc_status_t adc_hal_get_reading_result(adc_hal_dma_ctx_t *hal, const intptr_t eof_desc_addr, dma_descriptor_t **cur_desc); +adc_hal_dma_desc_status_t adc_hal_get_reading_result(adc_hal_dma_ctx_t *hal, const intptr_t eof_desc_addr, uint8_t **buffer, uint32_t *len); /** * @brief Clear interrupt diff --git a/components/hal/include/hal/dma_types.h b/components/hal/include/hal/dma_types.h index ad323deb00..dead785c2c 100644 --- a/components/hal/include/hal/dma_types.h +++ b/components/hal/include/hal/dma_types.h @@ -36,6 +36,8 @@ ESP_STATIC_ASSERT(sizeof(dma_descriptor_t) == 12, "dma_descriptor_t should occup #define DMA_DESCRIPTOR_BUFFER_OWNER_CPU (0) /*!< DMA buffer is allowed to be accessed by CPU */ #define DMA_DESCRIPTOR_BUFFER_OWNER_DMA (1) /*!< DMA buffer is allowed to be accessed by DMA engine */ #define DMA_DESCRIPTOR_BUFFER_MAX_SIZE (4095) /*!< Maximum size of the buffer that can be attached to descriptor */ +#define DMA_DESCRIPTOR_BUFFER_MAX_SIZE_4B_ALIGNED (4095-3) /*!< Maximum size of the buffer that can be attached to descriptor, and aligned to 4B */ + #ifdef __cplusplus } From bd6cd85b8edda5c798a4860b00cdb228ff4b3eae Mon Sep 17 00:00:00 2001 From: Armando Date: Tue, 16 May 2023 15:34:28 +0800 Subject: [PATCH 2/2] adc: added big conv_frame test --- .../test_apps/adc/main/test_adc_driver.c | 96 +++++++++++++++++-- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/components/esp_adc/test_apps/adc/main/test_adc_driver.c b/components/esp_adc/test_apps/adc/main/test_adc_driver.c index 886ecf7274..e163fa0e83 100644 --- a/components/esp_adc/test_apps/adc/main/test_adc_driver.c +++ b/components/esp_adc/test_apps/adc/main/test_adc_driver.c @@ -34,7 +34,8 @@ const __attribute__((unused)) static char *TAG = "TEST_ADC"; ---------------------------------------------------------------*/ typedef struct { TaskHandle_t task_handle; //Task handle - adc_oneshot_unit_handle_t adc_handle; //ADC handle + adc_oneshot_unit_handle_t oneshot_handle; //oneshot handle + adc_continuous_handle_t continuous_handle; //continuous handle bool level; //ADC level } test_adc_isr_ctx_t; @@ -50,7 +51,7 @@ static bool IRAM_ATTR s_alarm_callback(gptimer_handle_t timer, const gptimer_ala */ esp_rom_printf("alarm isr count=%llu\r\n", edata->count_value); - TEST_ESP_OK(adc_oneshot_read_isr(test_ctx->adc_handle, ADC1_TEST_CHAN0, &adc_raw)); + TEST_ESP_OK(adc_oneshot_read_isr(test_ctx->oneshot_handle, ADC1_TEST_CHAN0, &adc_raw)); esp_rom_printf("adc raw: %d\r\n", adc_raw); if (test_ctx->level) { TEST_ASSERT_INT_WITHIN(ADC_TEST_HIGH_THRESH, ADC_TEST_HIGH_VAL, adc_raw); @@ -68,7 +69,7 @@ static bool IRAM_ATTR s_alarm_callback(gptimer_handle_t timer, const gptimer_ala TEST_CASE("ADC oneshot fast work with ISR", "[adc_oneshot]") { static test_adc_isr_ctx_t isr_test_ctx = {}; - isr_test_ctx.adc_handle = NULL; + isr_test_ctx.oneshot_handle = NULL; isr_test_ctx.task_handle = xTaskGetCurrentTaskHandle(); //-------------ADC1 Init---------------// @@ -76,14 +77,14 @@ TEST_CASE("ADC oneshot fast work with ISR", "[adc_oneshot]") .unit_id = ADC_UNIT_1, .ulp_mode = ADC_ULP_MODE_DISABLE, }; - TEST_ESP_OK(adc_oneshot_new_unit(&init_config1, &isr_test_ctx.adc_handle)); + TEST_ESP_OK(adc_oneshot_new_unit(&init_config1, &isr_test_ctx.oneshot_handle)); //-------------ADC1 TEST Channel 0 Config---------------// adc_oneshot_chan_cfg_t config = { .bitwidth = ADC_BITWIDTH_DEFAULT, .atten = ADC_ATTEN_DB_11, }; - TEST_ESP_OK(adc_oneshot_config_channel(isr_test_ctx.adc_handle, ADC1_TEST_CHAN0, &config)); + TEST_ESP_OK(adc_oneshot_config_channel(isr_test_ctx.oneshot_handle, ADC1_TEST_CHAN0, &config)); //-------------GPTimer Init & Config---------------// gptimer_handle_t timer = NULL; @@ -127,10 +128,93 @@ TEST_CASE("ADC oneshot fast work with ISR", "[adc_oneshot]") //Tear Down TEST_ESP_OK(gptimer_disable(timer)); TEST_ESP_OK(gptimer_del_timer(timer)); - TEST_ESP_OK(adc_oneshot_del_unit(isr_test_ctx.adc_handle)); + TEST_ESP_OK(adc_oneshot_del_unit(isr_test_ctx.oneshot_handle)); } #if SOC_ADC_DMA_SUPPORTED +#if (SOC_ADC_DIGI_RESULT_BYTES == 2) +#define ADC_DRIVER_TEST_OUTPUT_TYPE ADC_DIGI_OUTPUT_FORMAT_TYPE1 +#define ADC_DRIVER_TEST_GET_CHANNEL(p_data) ((p_data)->type1.channel) +#define ADC_DRIVER_TEST_GET_DATA(p_data) ((p_data)->type1.data) +#else +#define ADC_DRIVER_TEST_OUTPUT_TYPE ADC_DIGI_OUTPUT_FORMAT_TYPE2 +#define ADC_DRIVER_TEST_GET_CHANNEL(p_data) ((p_data)->type2.channel) +#define ADC_DRIVER_TEST_GET_DATA(p_data) ((p_data)->type2.data) +#endif + +#define ADC_FRAME_TEST_SIZE 8192 + + +static bool IRAM_ATTR NOINLINE_ATTR s_conv_done_cb_frame_size_test(adc_continuous_handle_t handle, const adc_continuous_evt_data_t *edata, void *user_data) +{ + test_adc_isr_ctx_t *test_ctx = (test_adc_isr_ctx_t *)user_data; + BaseType_t high_task_wakeup; + + vTaskNotifyGiveFromISR(test_ctx->task_handle, &high_task_wakeup); + + return high_task_wakeup == pdTRUE; +} + +TEST_CASE("ADC continuous big conv_frame_size test", "[adc_continuous]") +{ + static test_adc_isr_ctx_t isr_test_ctx = {}; + isr_test_ctx.continuous_handle = NULL; + isr_test_ctx.task_handle = xTaskGetCurrentTaskHandle(); + + adc_continuous_handle_t handle = NULL; + adc_continuous_handle_cfg_t adc_config = { + .max_store_buf_size = ADC_FRAME_TEST_SIZE, + .conv_frame_size = ADC_FRAME_TEST_SIZE, + }; + TEST_ESP_OK(adc_continuous_new_handle(&adc_config, &handle)); + isr_test_ctx.continuous_handle = handle; + + adc_continuous_config_t dig_cfg = { + .sample_freq_hz = 50 * 1000, + .conv_mode = ADC_CONV_SINGLE_UNIT_1, + .format = ADC_DRIVER_TEST_OUTPUT_TYPE, + }; + adc_digi_pattern_config_t adc_pattern[SOC_ADC_PATT_LEN_MAX] = {0}; + adc_pattern[0].atten = ADC_ATTEN_DB_11; + adc_pattern[0].channel = ADC1_TEST_CHAN0; + adc_pattern[0].unit = ADC_UNIT_1; + adc_pattern[0].bit_width = SOC_ADC_DIGI_MAX_BITWIDTH; + dig_cfg.adc_pattern = adc_pattern; + dig_cfg.pattern_num = 1; + TEST_ESP_OK(adc_continuous_config(handle, &dig_cfg)); + + adc_continuous_evt_cbs_t cbs = { + .on_conv_done = s_conv_done_cb_frame_size_test, + }; + TEST_ESP_OK(adc_continuous_register_event_callbacks(handle, &cbs, &isr_test_ctx)); + + uint8_t* result = malloc(ADC_FRAME_TEST_SIZE); + TEST_ASSERT(result); + + test_adc_set_io_level(ADC_UNIT_1, ADC1_TEST_CHAN0, 0); + TEST_ESP_OK(adc_continuous_start(handle)); + + for (int i = 0; i < 5; i++) { + uint32_t ret_num = 0; + uint32_t sum = 0; + uint32_t cnt = 0; + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); + TEST_ESP_OK(adc_continuous_read(handle, result, ADC_FRAME_TEST_SIZE, &ret_num, ADC_MAX_DELAY)); + esp_rom_printf("ret_num: %d\n", ret_num); + for (int i = 0; i < ret_num; i += SOC_ADC_DIGI_RESULT_BYTES) { + adc_digi_output_data_t *p = (adc_digi_output_data_t*)&result[i]; + sum += ADC_DRIVER_TEST_GET_DATA(p); + cnt++; + } + esp_rom_printf("avg: %d\n", sum/cnt); + TEST_ASSERT_INT_WITHIN(ADC_TEST_LOW_THRESH, ADC_TEST_LOW_VAL, sum/cnt); + } + + TEST_ESP_OK(adc_continuous_stop(handle)); + TEST_ESP_OK(adc_continuous_deinit(handle)); + free(result); +} + #if SOC_ADC_DIG_IIR_FILTER_SUPPORTED TEST_CASE("ADC filter exhausted allocation", "[adc_oneshot]") {