diff --git a/components/esp_hw_support/dma/dw_gdma.c b/components/esp_hw_support/dma/dw_gdma.c index 33c44c4f89..765d931c0d 100644 --- a/components/esp_hw_support/dma/dw_gdma.c +++ b/components/esp_hw_support/dma/dw_gdma.c @@ -42,8 +42,10 @@ static const char *TAG = "dw-gdma"; #if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE #define DW_GDMA_GET_NON_CACHE_ADDR(addr) ((addr) ? CACHE_LL_L2MEM_NON_CACHE_ADDR(addr) : 0) +#define DW_GDMA_GET_CACHE_ADDRESS(nc_addr) ((nc_addr) ? CACHE_LL_L2MEM_CACHE_ADDR(nc_addr) : 0) #else #define DW_GDMA_GET_NON_CACHE_ADDR(addr) (addr) +#define DW_GDMA_GET_CACHE_ADDRESS(nc_addr) (nc_addr) #endif #if CONFIG_DW_GDMA_ISR_IRAM_SAFE || CONFIG_DW_GDMA_CTRL_FUNC_IN_IRAM || DW_GDMA_SETTER_FUNC_IN_IRAM @@ -385,22 +387,16 @@ esp_err_t dw_gdma_new_link_list(const dw_gdma_link_list_config_t *config, dw_gdm uint32_t num_items = config->num_items; list = heap_caps_calloc(1, sizeof(dw_gdma_link_list_t), DW_GDMA_MEM_ALLOC_CAPS); ESP_GOTO_ON_FALSE(list, ESP_ERR_NO_MEM, err, TAG, "no mem for link list"); - // allocate memory for link list items, from SRAM - // the link list items has itw own alignment requirement - // also we should respect the data cache line size - uint32_t data_cache_line_size = cache_hal_get_cache_line_size(CACHE_LL_LEVEL_INT_MEM, CACHE_TYPE_DATA); - uint32_t alignment = MAX(DW_GDMA_LL_LINK_LIST_ALIGNMENT, data_cache_line_size); - // because we want to access the link list items via non-cache address, so the memory size should also align to the cache line size - uint32_t lli_size = num_items * sizeof(dw_gdma_link_list_item_t); - if (data_cache_line_size) { - lli_size = ALIGN_UP(lli_size, data_cache_line_size); - } - items = heap_caps_aligned_calloc(alignment, 1, lli_size, MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA); + // allocate memory for link list items, from internal memory + // the link list items has its own alignment requirement, the heap allocator can help handle the cache alignment as well + items = heap_caps_aligned_calloc(DW_GDMA_LL_LINK_LIST_ALIGNMENT, num_items, sizeof(dw_gdma_link_list_item_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA); ESP_GOTO_ON_FALSE(items, ESP_ERR_NO_MEM, err, TAG, "no mem for link list items"); - if (data_cache_line_size) { // do memory sync only when the cache exists - // write back and then invalidate the cache, we won't use the cache to operate the link list items afterwards - // even the cache auto-write back happens, there's no risk the link list items will be overwritten - ESP_GOTO_ON_ERROR(esp_cache_msync(items, lli_size, ESP_CACHE_MSYNC_FLAG_DIR_C2M | ESP_CACHE_MSYNC_FLAG_INVALIDATE), + // do memory sync when the link list items are cached + uint32_t data_cache_line_size = cache_hal_get_cache_line_size(CACHE_LL_LEVEL_INT_MEM, CACHE_TYPE_DATA); + if (data_cache_line_size) { + // write back and then invalidate the cache, because later we will read/write the link list items by non-cacheable address + ESP_GOTO_ON_ERROR(esp_cache_msync(items, num_items * sizeof(dw_gdma_link_list_item_t), + ESP_CACHE_MSYNC_FLAG_DIR_C2M | ESP_CACHE_MSYNC_FLAG_INVALIDATE | ESP_CACHE_MSYNC_FLAG_UNALIGNED), err, TAG, "cache sync failed"); } @@ -466,6 +462,7 @@ dw_gdma_lli_handle_t dw_gdma_link_list_get_item(dw_gdma_link_list_handle_t list, { ESP_RETURN_ON_FALSE_ISR(list, NULL, TAG, "invalid argument"); ESP_RETURN_ON_FALSE_ISR(item_index < list->num_items, NULL, TAG, "invalid item index"); + // Note: the returned address is non-cached dw_gdma_link_list_item_t *lli = list->items_nc + item_index; return lli; } @@ -474,8 +471,8 @@ esp_err_t dw_gdma_lli_set_next(dw_gdma_lli_handle_t lli, dw_gdma_lli_handle_t ne { ESP_RETURN_ON_FALSE(lli && next, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); - // the next field must use a cached address - dw_gdma_ll_lli_set_next_item_addr(lli, CACHE_LL_L2MEM_CACHE_ADDR(next)); + // the next field must use a cached address, so convert it to a cached address + dw_gdma_ll_lli_set_next_item_addr(lli, DW_GDMA_GET_CACHE_ADDRESS(next)); return ESP_OK; } diff --git a/components/esp_hw_support/dma/include/esp_private/dw_gdma.h b/components/esp_hw_support/dma/include/esp_private/dw_gdma.h index 6d1ad024c0..dd9105e9d3 100644 --- a/components/esp_hw_support/dma/include/esp_private/dw_gdma.h +++ b/components/esp_hw_support/dma/include/esp_private/dw_gdma.h @@ -169,7 +169,7 @@ esp_err_t dw_gdma_channel_suspend_ctrl(dw_gdma_channel_handle_t chan, bool enter /** * @brief Abort the DMA channel * - * @note If the channel is aborted, it will be diabled immediately, which may cause AXI bus protocol violation. + * @note If the channel is aborted, it will be disabled immediately, which may cause AXI bus protocol violation. * @note This function is recommended to only be used when the channel hangs. Recommend to try `dw_gdma_channel_enable_ctrl` first, then opt for aborting. * * @param[in] chan DMA channel handle, allocated by `dw_gdma_new_channel` @@ -366,8 +366,8 @@ esp_err_t dw_gdma_lli_config_transfer(dw_gdma_lli_handle_t lli, dw_gdma_block_tr /** * @brief Set the next link list item for a given DMA link list item * - * @param[in] lli Link list item - * @param[in] next Next link list item + * @param[in] lli Current link list item, can be obtained from `dw_gdma_link_list_get_item` + * @param[in] next Next link list item, can be obtained from `dw_gdma_link_list_get_item` * @return * - ESP_OK: Set next link list item successfully * - ESP_ERR_INVALID_ARG: Set next link list item failed because of invalid argument diff --git a/components/hal/esp32p4/include/hal/dw_gdma_ll.h b/components/hal/esp32p4/include/hal/dw_gdma_ll.h index 021e929cf5..6afcca79b3 100644 --- a/components/hal/esp32p4/include/hal/dw_gdma_ll.h +++ b/components/hal/esp32p4/include/hal/dw_gdma_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 */ @@ -932,7 +932,7 @@ static inline uint32_t dw_gdma_ll_channel_get_dst_periph_status(dw_gdma_dev_t *d /** * @brief Type of DW-DMA link list item */ -typedef struct dw_gdma_link_list_item_t { +struct dw_gdma_link_list_item_t { dmac_chn_sar0_reg_t sar_lo; /*!< Source address low 32 bits */ dmac_chn_sar1_reg_t sar_hi; /*!< Source address high 32 bits */ dmac_chn_dar0_reg_t dar_lo; /*!< Destination address low 32 bits */ @@ -949,8 +949,9 @@ typedef struct dw_gdma_link_list_item_t { dmac_chn_status1_reg_t status_hi; /*!< Channel status high 32 bits */ uint32_t reserved_38; uint32_t reserved_3c; -} dw_gdma_link_list_item_t __attribute__((aligned(DW_GDMA_LL_LINK_LIST_ALIGNMENT))); +} __attribute__((aligned(DW_GDMA_LL_LINK_LIST_ALIGNMENT))); +typedef struct dw_gdma_link_list_item_t dw_gdma_link_list_item_t; ESP_STATIC_ASSERT(sizeof(dw_gdma_link_list_item_t) == DW_GDMA_LL_LINK_LIST_ALIGNMENT, "Invalid size of dw_gdma_link_list_item_t structure"); /** diff --git a/components/hal/include/hal/gdma_hal_axi.h b/components/hal/include/hal/gdma_hal_axi.h index c9f5a8f318..e7705217f3 100644 --- a/components/hal/include/hal/gdma_hal_axi.h +++ b/components/hal/include/hal/gdma_hal_axi.h @@ -30,8 +30,6 @@ void gdma_axi_hal_enable_burst(gdma_hal_context_t *hal, int chan_id, gdma_channe void gdma_axi_hal_set_burst_size(gdma_hal_context_t *hal, int chan_id, gdma_channel_direction_t dir, uint32_t burst_sz); -void gdma_axi_hal_set_ext_mem_align(gdma_hal_context_t *hal, int chan_id, gdma_channel_direction_t dir, uint8_t align); - void gdma_axi_hal_set_strategy(gdma_hal_context_t *hal, int chan_id, gdma_channel_direction_t dir, bool en_owner_check, bool en_desc_write_back); void gdma_axi_hal_enable_intr(gdma_hal_context_t *hal, int chan_id, gdma_channel_direction_t dir, uint32_t intr_event_mask, bool en_or_dis);