Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-05-28 16:46:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(esp_hw_support): fixed spi buslock multi dev acq/release logic issue

Browse Source
This commit is contained in:
wanckl
2026-04-20 18:31:20 +08:00
committed by Wan Lei
parent cb4a55ace2
commit 2521f3887a
5 changed files with 176 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
components/esp_driver_spi/test_apps/components/spi_bench_mark/include/spi_performance.h
View File

@@ -57,7 +57,7 @@
#elif CONFIG_IDF_TARGET_ESP32C6
#define IDF_TARGET_MAX_SPI_CLK_FREQ 26666*1000
#define IDF_TARGET_MAX_TRANS_TIME_INTR_DMA 35 //TODO: IDF-9551, check perform
#define IDF_TARGET_MAX_TRANS_TIME_INTR_DMA 37 //TODO: IDF-9551, check perform
#define IDF_TARGET_MAX_TRANS_TIME_POLL_DMA 19
#define IDF_TARGET_MAX_TRANS_TIME_INTR_CPU 32
#define IDF_TARGET_MAX_TRANS_TIME_POLL_CPU 15

60
components/esp_driver_spi/test_apps/master/main/test_spi_bus_lock.c
View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -347,3 +347,61 @@ TEST_CASE("spi master can be used on SPI1", "[spi]")
//TODO: add a case when a non-polling transaction happened in the bus-acquiring time and then release the bus then queue a new trans
#endif //!(CONFIG_SPIRAM && CONFIG_IDF_TARGET_ESP32)
#define TEST_LARGE_TRANS_LEN 2048
static void dev2_polling_task(void *arg)
{
task_context_t *ctx = (task_context_t *)arg;
spi_transaction_t t = {
.flags = SPI_TRANS_USE_TXDATA,
.length = 32,
};
while (!ctx->finished) {
TEST_ESP_OK(spi_device_polling_transmit(ctx->handle, &t));
vTaskDelay(pdMS_TO_TICKS(1));
}
vTaskDelete(NULL);
}
TEST_CASE("release_bus during flying is safe to other device acquiring", "[spi]")
{
spi_bus_config_t buscfg = SPI_BUS_TEST_DEFAULT_CONFIG();
TEST_ESP_OK(spi_bus_initialize(TEST_SPI_HOST, &buscfg, SPI_DMA_CH_AUTO));
spi_device_interface_config_t devcfg_p = SPI_DEVICE_TEST_DEFAULT_CONFIG();
spi_device_interface_config_t devcfg_q = SPI_DEVICE_TEST_DEFAULT_CONFIG();
devcfg_q.spics_io_num = -1;
devcfg_q.queue_size = 3;
devcfg_q.clock_speed_hz = 500 * 1000;
spi_device_handle_t dev_q;
task_context_t ctx = {};
TEST_ESP_OK(spi_bus_add_device(TEST_SPI_HOST, &devcfg_p, &ctx.handle));
TEST_ESP_OK(spi_bus_add_device(TEST_SPI_HOST, &devcfg_q, &dev_q));
// polling task with higher priority than the interrupt task
xTaskCreate(dev2_polling_task, "spi17860_p", 4096, &ctx, 6, NULL);
uint8_t *q_txb = heap_caps_malloc(TEST_LARGE_TRANS_LEN, MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL);
spi_transaction_t *ret_trans, trans = {
.length = TEST_LARGE_TRANS_LEN * 8,
.tx_buffer = q_txb,
};
for (int i = 0; i < 30; i++) {
TEST_ESP_OK(spi_device_acquire_bus(dev_q, portMAX_DELAY));
TEST_ESP_OK(spi_device_queue_trans(dev_q, &trans, portMAX_DELAY));
esp_rom_printf("queue trans %d\n", i);
spi_device_release_bus(dev_q);
}
ctx.finished = true;
vTaskDelay(pdMS_TO_TICKS(100)); // wait for all trans finished
for (int i = 0; i < devcfg_q.queue_size; i++) {
spi_device_get_trans_result(dev_q, &ret_trans, 0);
}
free(q_txb);
TEST_ESP_OK(spi_bus_remove_device(ctx.handle));
TEST_ESP_OK(spi_bus_remove_device(dev_q));
TEST_ESP_OK(spi_bus_free(TEST_SPI_HOST));
}

2
components/esp_hw_support/include/esp_private/spi_share_hw_ctrl.h
View File

@@ -23,7 +23,7 @@ extern "C" {
#if BUS_LOCK_DEBUG
#define BUS_LOCK_DEBUG_EXECUTE_CHECK(x) assert(x)
#else
#define BUS_LOCK_DEBUG_EXECUTE_CHECK(x)
#define BUS_LOCK_DEBUG_EXECUTE_CHECK(x) (void)(x)
#endif
#define CHECK_IOMUX_PIN(HOST, PIN_NAME) if (GPIO.func_in_sel_cfg[spi_periph_signal[(HOST)].PIN_NAME##_in].sig_in_sel) return false

74
components/esp_hw_support/spi_bus_lock.c
View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -89,12 +89,14 @@
* -> STATE_ACQ: by `acquire_core`
*
* - STATE_BG:
* * No acquiring device, the ISR is the acquiring processor, there is BG bits active, but no LOCK
* bits
* * No acquiring device, the ISR is the acquiring processor, and there are BG bits active. There
* may also be LOCK bits pending for another device, but the lock owner must wait until BG is
* fully finished.
* * The BG operation should be enabled while turning into this state.
*
* -> STATE_IDLE: by `bg_exit_core` after `clear_pend_core` for all BG bits
* -> STATE_BG_ACQ: by `schedule_core`, when there is new LOCK bit set (by `acquire_core`)
* -> STATE_BG_ACQ: by `schedule_core`, when there is a new LOCK bit set (by `acquire_core`) and
* BG is active for the same device
*
* - STATE_BG_ACQ:
* * There is acquiring device, the ISR is the acquiring processor, there may be BG bits active for
@@ -117,9 +119,11 @@
*
* -> STATE_BG_ACQ: by `req_core`
* -> STATE_BG_ACQ (other device): by `acquire_end_core`, when there is LOCK bit for another
* device, and the new acquiring device has active BG bits.
* -> STATE_ACQ (other device): by `acquire_end_core`, when there is LOCK bit for another devices,
* but the new acquiring device has no active BG bits.
* device, and BG is active for the new acquiring device.
* -> STATE_ACQ (other device): by `acquire_end_core`, when there is LOCK bit for another device,
* and no BG bits are active.
* -> STATE_BG: by `acquire_end_core`, when there is LOCK bit for another device, but BG is still
* active for a different device.
* -> STATE_BG: by `acquire_end_core` when there is no LOCK bit active, but there are active BG
* bits.
* -> STATE_IDLE: by `acquire_end_core` when there is no LOCK bit, nor BG bit active.
@@ -390,19 +394,22 @@ SPI_BUS_LOCK_ISR_ATTR static inline bool acquire_core(spi_bus_lock_dev_t *dev_ha
/**
* Find the next acquiring processor according to the status. Will directly change
* the acquiring device if new one found.
* the acquiring device if the BG can yield to the lock owner, or if BG is active for the
* lock owner.
*
* Cases:
* - BG should still be the acquiring processor (Return false):
* 1. Acquiring device has active BG bits: out_desired_dev = new acquiring device
* 2. No acquiring device, but BG active: out_desired_dev = randomly pick one device with active BG bits
* 2. A new acquiring device exists, but BG is still active for another device:
* out_desired_dev = that BG-active device
* 3. No acquiring device, but BG active: out_desired_dev = randomly pick one device with active BG bits
* - BG should yield to the task (Return true):
* 3. Acquiring device has no active BG bits: out_desired_dev = new acquiring device
* 4. No acquiring device while no active BG bits: out_desired_dev=NULL
* 4. Acquiring device has no active BG bits: out_desired_dev = new acquiring device
* 5. No acquiring device while no active BG bits: out_desired_dev=NULL
*
* Acquiring device task need to be resumed only when case 3.
* Acquiring device task needs to be resumed only when case 4.
*
* This scheduling can happen in either task or ISR, so `in_isr` or `bg_active` not touched.
* This scheduling can happen in either task or ISR, so `in_isr` is not touched.
*
* @param lock
* @param status Current status
@@ -421,12 +428,26 @@ schedule_core(spi_bus_lock_t *lock, uint32_t status, spi_bus_lock_dev_t **out_de
bool bg_yield;
if (lock_bits) {
int dev_id = mask_get_id(lock_bits);
desired_dev = (spi_bus_lock_dev_t *)atomic_load(&lock->dev[dev_id]);
BUS_LOCK_DEBUG_EXECUTE_CHECK(desired_dev);
spi_bus_lock_dev_t *lock_dev = (spi_bus_lock_dev_t *)atomic_load(&lock->dev[dev_id]);
BUS_LOCK_DEBUG_EXECUTE_CHECK(lock_dev);
lock->acquiring_dev = desired_dev;
bg_yield = ((bg_bits & desired_dev->mask) == 0);
lock->acq_dev_bg_active = !bg_yield;
if (bg_bits && ((bg_bits & lock_dev->mask) == 0)) {
int bg_dev_id = mask_get_id(bg_bits);
desired_dev = (spi_bus_lock_dev_t *)atomic_load(&lock->dev[bg_dev_id]);
BUS_LOCK_DEBUG_EXECUTE_CHECK(desired_dev);
// Keep ISR/BG owning the bus until the previous device's in-flight
// interrupt transactions are fully finished. The new lock owner will
// be resumed by a later schedule once BG bits are cleared.
lock->acquiring_dev = NULL;
lock->acq_dev_bg_active = false;
bg_yield = false;
} else {
desired_dev = lock_dev;
lock->acquiring_dev = desired_dev;
bg_yield = ((bg_bits & desired_dev->mask) == 0);
lock->acq_dev_bg_active = !bg_yield;
}
} else {
lock->acq_dev_bg_active = false;
if (bg_bits) {
@@ -534,7 +555,7 @@ SPI_BUS_LOCK_ISR_ATTR static inline bool bg_entry_core(spi_bus_lock_t *lock)
// Handle the conditions of status and interrupt, avoiding the ISR being disabled when there is any new coming BG requests.
// When called with `wip=true`, means the ISR is performing some operations. Will enable the interrupt again and exit unconditionally.
// When called with `wip=false`, will only return `true` when there is no coming BG request. If return value is `false`, the ISR should try again.
// Will not change acquiring device.
// May change acquiring device when BG has finished and there is a pending LOCK bit.
SPI_BUS_LOCK_ISR_ATTR static inline bool bg_exit_core(spi_bus_lock_t *lock, bool wip, BaseType_t *do_yield)
{
//See comments in `bg_entry_core`, re-enable interrupt disabled in entry if we do need the interrupt
@@ -558,7 +579,20 @@ SPI_BUS_LOCK_ISR_ATTR static inline bool bg_exit_core(spi_bus_lock_t *lock, bool
}
} else {
BUS_LOCK_DEBUG_EXECUTE_CHECK(!lock->acq_dev_bg_active);
ret = !(status & BG_MASK);
if (status & BG_MASK) {
ret = false;
} else if (status & LOCK_MASK) {
spi_bus_lock_dev_t *desired_dev = NULL;
bool bg_yield = schedule_core(lock, status, &desired_dev);
// A waiting lock owner must be selected once BG is fully finished.
BUS_LOCK_DEBUG_EXECUTE_CHECK(bg_yield);
BUS_LOCK_DEBUG_EXECUTE_CHECK(desired_dev);
BUS_LOCK_DEBUG_EXECUTE_CHECK(lock->acquiring_dev == desired_dev);
resume_dev_in_isr(lock->acquiring_dev, do_yield);
ret = true;
} else {
ret = true;
}
}
if (ret) {
//when successfully exit, but no transaction done, mark BG as inactive

61
components/esp_lcd/test_apps/spi_lcd/main/test_spi_lcd_panel.c
View File

@@ -302,3 +302,64 @@ TEST_CASE("spi_lcd_send_colors_to_fixed_region", "[lcd]")
TEST_ESP_OK(gpio_reset_pin(TEST_LCD_BK_LIGHT_GPIO));
free(color_data);
}
#define TEST_SPI_LCD_CONCURRENT_COLOR_LEN (120 * 120)
typedef struct {
spi_device_handle_t spi_dev;
TaskHandle_t done_task;
volatile bool stop;
uint32_t trans_count;
} spi_lcd_concurrent_polling_ctx_t;
static void spi_lcd_concurrent_polling_task(void *arg)
{
spi_lcd_concurrent_polling_ctx_t *ctx = arg;
spi_transaction_t trans = {
.length = 4 * 8,
.flags = SPI_TRANS_USE_TXDATA,
};
while (!ctx->stop) {
vTaskDelay(pdMS_TO_TICKS(50));
TEST_ESP_OK(spi_device_polling_transmit(ctx->spi_dev, &trans));
ctx->trans_count++;
}
xTaskNotifyGive(ctx->done_task);
vTaskDelete(NULL);
}
TEST_CASE("spi_lcd_safe_with_another_device_polling_on_same_bus", "[lcd]")
{
void *color_data = malloc(TEST_SPI_LCD_CONCURRENT_COLOR_LEN);
TEST_ASSERT_NOT_NULL(color_data);
esp_lcd_panel_io_handle_t io_handle = NULL;
test_spi_lcd_common_initialize(&io_handle, NULL, NULL, 8, 8, false);
spi_lcd_concurrent_polling_ctx_t polling_ctx = { .done_task = xTaskGetCurrentTaskHandle() };
spi_device_interface_config_t other_dev_config = {
.clock_speed_hz = TEST_LCD_PIXEL_CLOCK_HZ,
.spics_io_num = -1,
.queue_size = 1,
};
TEST_ESP_OK(spi_bus_add_device(TEST_SPI_HOST_ID, &other_dev_config, &polling_ctx.spi_dev));
// polling task with higher priority than the interrupt task
xTaskCreate(spi_lcd_concurrent_polling_task, "spi_polling", 4096, &polling_ctx, 10, NULL);
for (int i = 0; i < 30; i++) {
printf("panel_io_tx_color %d\r\n", i);
TEST_ESP_OK(esp_lcd_panel_io_tx_color(io_handle, -1, color_data, TEST_SPI_LCD_CONCURRENT_COLOR_LEN));
}
polling_ctx.stop = true;
vTaskDelay(pdMS_TO_TICKS(100)); // wait for polling task to stop
TEST_ASSERT_GREATER_THAN(0, (int)ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(5000)));
TEST_ASSERT_GREATER_THAN_UINT32(0, polling_ctx.trans_count);
TEST_ESP_OK(esp_lcd_panel_io_del(io_handle));
TEST_ESP_OK(spi_bus_remove_device(polling_ctx.spi_dev));
TEST_ESP_OK(spi_bus_free(TEST_SPI_HOST_ID));
TEST_ESP_OK(gpio_reset_pin(TEST_LCD_BK_LIGHT_GPIO));
free(color_data);
}