From 0c2c1421343e63a1e13d8db2beaaf1b08dbaae43 Mon Sep 17 00:00:00 2001 From: Chen Jichang Date: Thu, 19 Dec 2024 14:50:36 +0800 Subject: [PATCH] fix(parlio_tx): add clock and fifo reset in disable function --- components/esp_driver_parlio/src/parlio_tx.c | 22 ++++++++++++------- .../test_apps/parlio/main/test_parlio_tx.c | 12 +++++----- .../hal/esp32c5/include/hal/parlio_ll.h | 5 ++++- .../hal/esp32c6/include/hal/parlio_ll.h | 11 ++++++---- .../hal/esp32h2/include/hal/parlio_ll.h | 5 ++++- .../hal/esp32p4/include/hal/parlio_ll.h | 5 ++++- 6 files changed, 39 insertions(+), 21 deletions(-) diff --git a/components/esp_driver_parlio/src/parlio_tx.c b/components/esp_driver_parlio/src/parlio_tx.c index ae19ae3b3a..dcdfd8f073 100644 --- a/components/esp_driver_parlio/src/parlio_tx.c +++ b/components/esp_driver_parlio/src/parlio_tx.c @@ -223,9 +223,6 @@ static esp_err_t parlio_tx_unit_init_dma(parlio_tx_unit_t *tx_unit, const parlio .buffer_alignment = 1, .item_alignment = PARLIO_DMA_DESC_ALIGNMENT, .num_items = dma_nodes_num, - .flags = { - .check_owner = true, - }, }; // throw the error to the caller @@ -478,11 +475,13 @@ static void IRAM_ATTR parlio_tx_do_transaction(parlio_tx_unit_t *tx_unit, parlio } }; gdma_link_mount_buffers(tx_unit->dma_link, 0, &mount_config, 1, NULL); - parlio_ll_tx_reset_fifo(hal->regs); PARLIO_RCC_ATOMIC() { parlio_ll_tx_reset_clock(hal->regs); } + PARLIO_CLOCK_SRC_ATOMIC() { + parlio_ll_tx_enable_clock(hal->regs, false); + } parlio_ll_tx_set_idle_data_value(hal->regs, t->idle_value); parlio_ll_tx_set_trans_bit_len(hal->regs, t->payload_bits); @@ -491,6 +490,9 @@ static void IRAM_ATTR parlio_tx_do_transaction(parlio_tx_unit_t *tx_unit, parlio while (parlio_ll_tx_is_ready(hal->regs) == false); // turn on the core clock after we start the TX unit parlio_ll_tx_start(hal->regs, true); + PARLIO_CLOCK_SRC_ATOMIC() { + parlio_ll_tx_enable_clock(hal->regs, true); + } } esp_err_t parlio_tx_unit_enable(parlio_tx_unit_handle_t tx_unit) @@ -503,14 +505,13 @@ esp_err_t parlio_tx_unit_enable(parlio_tx_unit_handle_t tx_unit) if (tx_unit->pm_lock) { esp_pm_lock_acquire(tx_unit->pm_lock); } - parlio_hal_context_t *hal = &tx_unit->base.group->hal; parlio_ll_enable_interrupt(hal->regs, PARLIO_LL_EVENT_TX_MASK, true); atomic_store(&tx_unit->fsm, PARLIO_TX_FSM_ENABLE); } else { ESP_RETURN_ON_FALSE(false, ESP_ERR_INVALID_STATE, TAG, "unit not in init state"); } - // enable clock output + // the chip may resumes from light-sleep, in which case the register configuration needs to be resynchronized PARLIO_CLOCK_SRC_ATOMIC() { parlio_ll_tx_enable_clock(hal->regs, true); } @@ -555,13 +556,18 @@ esp_err_t parlio_tx_unit_disable(parlio_tx_unit_handle_t tx_unit) } ESP_RETURN_ON_FALSE(valid_state, ESP_ERR_INVALID_STATE, TAG, "unit can't be disabled in state %d", expected_fsm); - // stop the TX engine + // stop the DMA engine, reset the peripheral state parlio_hal_context_t *hal = &tx_unit->base.group->hal; - // disable clock output + // to stop the undergoing transaction, disable and reset clock PARLIO_CLOCK_SRC_ATOMIC() { parlio_ll_tx_enable_clock(hal->regs, false); } + PARLIO_RCC_ATOMIC() { + parlio_ll_tx_reset_clock(hal->regs); + } gdma_stop(tx_unit->dma_chan); + gdma_reset(tx_unit->dma_chan); + parlio_ll_tx_reset_fifo(hal->regs); parlio_ll_tx_start(hal->regs, false); parlio_ll_enable_interrupt(hal->regs, PARLIO_LL_EVENT_TX_MASK, false); diff --git a/components/esp_driver_parlio/test_apps/parlio/main/test_parlio_tx.c b/components/esp_driver_parlio/test_apps/parlio/main/test_parlio_tx.c index 0f96a0b6ad..d2bdfde79e 100644 --- a/components/esp_driver_parlio/test_apps/parlio/main/test_parlio_tx.c +++ b/components/esp_driver_parlio/test_apps/parlio/main/test_parlio_tx.c @@ -143,7 +143,7 @@ TEST_CASE("parallel_tx_unit_enable_disable", "[parlio_tx]") TEST_DATA7_GPIO, }, .output_clk_freq_hz = 1 * 1000 * 1000, - .trans_queue_depth = 64, + .trans_queue_depth = 4, .max_transfer_size = 256, .bit_pack_order = PARLIO_BIT_PACK_ORDER_LSB, .sample_edge = PARLIO_SAMPLE_EDGE_POS, @@ -155,19 +155,19 @@ TEST_CASE("parallel_tx_unit_enable_disable", "[parlio_tx]") parlio_transmit_config_t transmit_config = { .idle_value = 0x00, }; - __attribute__((aligned(64))) uint8_t payload[128] = {0}; - for (int i = 0; i < 128; i++) { + __attribute__((aligned(64))) uint8_t payload[256] = {0}; + for (int i = 0; i < 256; i++) { payload[i] = i; } - for (int j = 0; j < 64; j++) { - TEST_ESP_OK(parlio_tx_unit_transmit(tx_unit, payload, 128 * sizeof(uint8_t) * 8, &transmit_config)); + for (int j = 0; j < 3; j++) { + TEST_ESP_OK(parlio_tx_unit_transmit(tx_unit, payload, 256 * sizeof(uint8_t) * 8, &transmit_config)); } printf("disable the transaction in the middle\r\n"); while (parlio_tx_unit_disable(tx_unit) != ESP_OK) { esp_rom_delay_us(1000); } - vTaskDelay(pdMS_TO_TICKS(100)); + vTaskDelay(pdMS_TO_TICKS(10)); printf("resume the transaction and pending packets should continue\r\n"); TEST_ESP_OK(parlio_tx_unit_enable(tx_unit)); diff --git a/components/hal/esp32c5/include/hal/parlio_ll.h b/components/hal/esp32c5/include/hal/parlio_ll.h index 5f6d7b0242..5f818acd25 100644 --- a/components/hal/esp32c5/include/hal/parlio_ll.h +++ b/components/hal/esp32c5/include/hal/parlio_ll.h @@ -488,8 +488,11 @@ static inline void parlio_ll_tx_enable_clock_gating(parl_io_dev_t *dev, bool en) /** * @brief Start TX unit to transmit data * + * @note The hardware monitors the rising edge of tx_start as the trigger signal. + * Once the transmission starts, it cannot be stopped by clearing tx_start. + * * @param dev Parallel IO register base address - * @param en True to start, False to stop + * @param en True to start, False to reset the reg state (not meaning the TX unit will be stopped) */ __attribute__((always_inline)) static inline void parlio_ll_tx_start(parl_io_dev_t *dev, bool en) diff --git a/components/hal/esp32c6/include/hal/parlio_ll.h b/components/hal/esp32c6/include/hal/parlio_ll.h index 78d126a33f..3eb277d14f 100644 --- a/components/hal/esp32c6/include/hal/parlio_ll.h +++ b/components/hal/esp32c6/include/hal/parlio_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -30,7 +30,7 @@ #define PARLIO_LL_EVENT_TX_FIFO_EMPTY (1 << 0) #define PARLIO_LL_EVENT_RX_FIFO_FULL (1 << 1) #define PARLIO_LL_EVENT_TX_EOF (1 << 2) -#define PARLIO_LL_EVENT_TX_MASK (PARLIO_LL_EVENT_TX_FIFO_EMPTY | PARLIO_LL_EVENT_TX_EOF) +#define PARLIO_LL_EVENT_TX_MASK (PARLIO_LL_EVENT_TX_EOF) // On C6, TX FIFO EMPTY event always comes with TX EOF event. We don't enable it #define PARLIO_LL_EVENT_RX_MASK (PARLIO_LL_EVENT_RX_FIFO_FULL) #define PARLIO_LL_TX_DATA_LINE_AS_VALID_SIG 15 // TXD[15] can be used a valid signal @@ -450,7 +450,7 @@ static inline void parlio_ll_tx_set_trans_bit_len(parl_io_dev_t *dev, uint32_t b } /** - * @brief Wether to enable the TX clock gating + * @brief Whether to enable the TX clock gating * * @note The TXD[7] will be taken as the gating enable signal * @@ -465,8 +465,11 @@ static inline void parlio_ll_tx_enable_clock_gating(parl_io_dev_t *dev, bool en) /** * @brief Start TX unit to transmit data * + * @note The hardware monitors the rising edge of tx_start as the trigger signal. + * Once the transmission starts, it cannot be stopped by clearing tx_start. + * * @param dev Parallel IO register base address - * @param en True to start, False to stop + * @param en True to start, False to reset the reg state (not meaning the TX unit will be stopped) */ __attribute__((always_inline)) static inline void parlio_ll_tx_start(parl_io_dev_t *dev, bool en) diff --git a/components/hal/esp32h2/include/hal/parlio_ll.h b/components/hal/esp32h2/include/hal/parlio_ll.h index ac3f5d7470..dd284cb900 100644 --- a/components/hal/esp32h2/include/hal/parlio_ll.h +++ b/components/hal/esp32h2/include/hal/parlio_ll.h @@ -472,8 +472,11 @@ static inline void parlio_ll_tx_enable_clock_gating(parl_io_dev_t *dev, bool en) /** * @brief Start TX unit to transmit data * + * @note The hardware monitors the rising edge of tx_start as the trigger signal. + * Once the transmission starts, it cannot be stopped by clearing tx_start. + * * @param dev Parallel IO register base address - * @param en True to start, False to stop + * @param en True to start, False to reset the reg state (not meaning the TX unit will be stopped) */ __attribute__((always_inline)) static inline void parlio_ll_tx_start(parl_io_dev_t *dev, bool en) diff --git a/components/hal/esp32p4/include/hal/parlio_ll.h b/components/hal/esp32p4/include/hal/parlio_ll.h index c982049870..94cb605c99 100644 --- a/components/hal/esp32p4/include/hal/parlio_ll.h +++ b/components/hal/esp32p4/include/hal/parlio_ll.h @@ -537,8 +537,11 @@ static inline void parlio_ll_tx_enable_clock_gating(parl_io_dev_t *dev, bool en) /** * @brief Start TX unit to transmit data * + * @note The hardware monitors the rising edge of tx_start as the trigger signal. + * Once the transmission starts, it cannot be stopped by clearing tx_start. + * * @param dev Parallel IO register base address - * @param en True to start, False to stop + * @param en True to start, False to reset the reg state (not meaning the TX unit will be stopped) */ __attribute__((always_inline)) static inline void parlio_ll_tx_start(parl_io_dev_t *dev, bool en)