diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index 7cae4b05b6..157b9156f7 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -79,6 +79,8 @@ extern "C" { #include #include /* required for XSHAL_CLIB */ #include +#include "esp_crosscore_int.h" + //#include "xtensa_context.h" @@ -261,6 +263,18 @@ void vPortYield( void ); void _frxt_setup_switch( void ); #define portYIELD() vPortYield() #define portYIELD_FROM_ISR() _frxt_setup_switch() + +static inline uint32_t xPortGetCoreID(); + +/* Yielding within an API call (when interrupts are off), means the yield should be delayed + until interrupts are re-enabled. + + To do this, we use the "cross-core" interrupt as a trigger to yield on this core when interrupts are re-enabled.This + is the same interrupt & code path which is used to trigger a yield between CPUs, although in this case the yield is + happening on the same CPU. +*/ +#define portYIELD_WITHIN_API() esp_crosscore_int_send_yield(xPortGetCoreID()) + /*-----------------------------------------------------------*/ /* Task function macros as described on the FreeRTOS.org WEB site. */ diff --git a/components/freertos/queue.c b/components/freertos/queue.c index 7c491f6952..66a7ed0497 100644 --- a/components/freertos/queue.c +++ b/components/freertos/queue.c @@ -123,15 +123,9 @@ zero. */ #if( configUSE_PREEMPTION == 0 ) /* If the cooperative scheduler is being used then a yield should not be performed just because a higher priority task has been woken. */ - #define queueYIELD_IF_USING_PREEMPTION_MUX() #define queueYIELD_IF_USING_PREEMPTION() #else #define queueYIELD_IF_USING_PREEMPTION() portYIELD_WITHIN_API() - #define queueYIELD_IF_USING_PREEMPTION_MUX(mux) { \ - taskEXIT_CRITICAL(mux); \ - portYIELD_WITHIN_API(); \ - taskENTER_CRITICAL(mux); \ - } while(0) #endif /* @@ -290,7 +284,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; { if( xTaskRemoveFromEventList( &( pxQueue->xTasksWaitingToSend ) ) == pdTRUE ) { - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -753,7 +747,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; /* The queue is a member of a queue set, and posting to the queue set caused a higher priority task to unblock. A context switch is required. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -772,7 +766,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; our own so yield immediately. Yes it is ok to do this from within the critical section - the kernel takes care of that. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -785,7 +779,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; executed if the task was holding multiple mutexes and the mutexes were given back in an order that is different to that in which they were taken. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -805,7 +799,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; our own so yield immediately. Yes it is ok to do this from within the critical section - the kernel takes care of that. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -818,7 +812,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; executed if the task was holding multiple mutexes and the mutexes were given back in an order that is different to that in which they were taken. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -1493,7 +1487,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; { if( xTaskRemoveFromEventList( &( pxQueue->xTasksWaitingToSend ) ) == pdTRUE ) { - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { @@ -1522,7 +1516,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; if( xTaskRemoveFromEventList( &( pxQueue->xTasksWaitingToReceive ) ) != pdFALSE ) { /* The task waiting has a higher priority than this task. */ - queueYIELD_IF_USING_PREEMPTION_MUX(&pxQueue->mux); + queueYIELD_IF_USING_PREEMPTION(); } else { diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index 145554a740..ff407d9baf 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -120,14 +120,8 @@ functions but without including stdio.h here. */ /* If the cooperative scheduler is being used then a yield should not be performed just because a higher priority task has been woken. */ #define taskYIELD_IF_USING_PREEMPTION() - #define queueYIELD_IF_USING_PREEMPTION_MUX(mux) #else #define taskYIELD_IF_USING_PREEMPTION() portYIELD_WITHIN_API() - #define taskYIELD_IF_USING_PREEMPTION_MUX(mux) { \ - taskEXIT_CRITICAL(mux); \ - portYIELD_WITHIN_API(); \ - taskENTER_CRITICAL(mux); \ - } while(0) #endif @@ -1166,7 +1160,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode { if( xCoreID == xPortGetCoreID() ) { - taskYIELD_IF_USING_PREEMPTION_MUX(&xTaskQueueMutex); + taskYIELD_IF_USING_PREEMPTION(); } else { taskYIELD_OTHER_CORE(xCoreID, pxNewTCB->uxPriority); @@ -1703,7 +1697,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode if( xYieldRequired == pdTRUE ) { - taskYIELD_IF_USING_PREEMPTION_MUX(&xTaskQueueMutex); + taskYIELD_IF_USING_PREEMPTION(); } else { @@ -1895,7 +1889,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode /* This yield may not cause the task just resumed to run, but will leave the lists in the correct state for the next yield. */ - taskYIELD_IF_USING_PREEMPTION_MUX(&xTaskQueueMutex); + taskYIELD_IF_USING_PREEMPTION(); } else if( pxTCB->xCoreID != xPortGetCoreID() ) { @@ -2206,7 +2200,7 @@ BaseType_t xAlreadyYielded = pdFALSE; xAlreadyYielded = pdTRUE; } #endif - taskYIELD_IF_USING_PREEMPTION_MUX(&xTaskQueueMutex); + taskYIELD_IF_USING_PREEMPTION(); } else { diff --git a/components/freertos/test/test_freertos_eventgroups.c b/components/freertos/test/test_freertos_eventgroups.c index f32c20ade9..8d5a5fb326 100644 --- a/components/freertos/test/test_freertos_eventgroups.c +++ b/components/freertos/test/test_freertos_eventgroups.c @@ -52,14 +52,15 @@ TEST_CASE("FreeRTOS Event Groups", "[freertos]") } /* Tasks all start instantly, but this task will resume running at the same time as the higher priority tasks on the - other processor may still be setting up, so give a tick for them to also block on BIT_CALL... */ - vTaskDelay(1); + other processor may still be setting up, so allow time for them to also block on BIT_CALL... */ + vTaskDelay(10); for (int i = 0; i < COUNT; i++) { /* signal all tasks with "CALL" bit... */ xEventGroupSetBits(eg, BIT_CALL); - TEST_ASSERT_EQUAL_HEX16(ALL_RESPONSE_BITS, xEventGroupWaitBits(eg, ALL_RESPONSE_BITS, true, true, 100 / portMAX_DELAY)); + /* Only wait for 1 tick, the wakeup should be immediate... */ + TEST_ASSERT_EQUAL_HEX16(ALL_RESPONSE_BITS, xEventGroupWaitBits(eg, ALL_RESPONSE_BITS, true, true, 1)); } /* Ensure all tasks cleaned up correctly */ diff --git a/components/freertos/test/test_preemption.c b/components/freertos/test/test_preemption.c new file mode 100644 index 0000000000..09dbcc003a --- /dev/null +++ b/components/freertos/test/test_preemption.c @@ -0,0 +1,108 @@ +/* + Unit tests for FreeRTOS preemption +*/ + +#include +#include +#include "rom/ets_sys.h" + +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "freertos/semphr.h" +#include "freertos/queue.h" +#include "freertos/xtensa_api.h" +#include "unity.h" +#include "soc/cpu.h" + +static volatile bool trigger; +static volatile bool flag; + +/* Task: + - Waits for 'trigger' variable to be set + - Reads the cycle count on this CPU + - Pushes it into a queue supplied as a param + - Busy-waits until the main task terminates it +*/ +static void task_send_to_queue(void *param) +{ + QueueHandle_t queue = (QueueHandle_t) param; + uint32_t ccount; + + while(!trigger) {} + + RSR(CCOUNT, ccount); + flag = true; + xQueueSendToBack(queue, &ccount, 0); + /* This is to ensure that higher priority task + won't wake anyhow, due to this task terminating. + + The task runs until terminated by the main task. + */ + while(1) {} +} + +TEST_CASE("Yield from lower priority task, same CPU", "[freertos]") +{ + /* Do this 3 times, mostly for the benchmark value - the first + run includes a cache miss so uses more cycles than it should. */ + for (int i = 0; i < 3; i++) { + TaskHandle_t sender_task; + QueueHandle_t queue = xQueueCreate(1, sizeof(uint32_t)); + flag = false; + trigger = false; + + /* "yield" task sits on our CPU, lower priority to us */ + xTaskCreatePinnedToCore(task_send_to_queue, "YIELD", 2048, (void *)queue, UNITY_FREERTOS_PRIORITY - 1, &sender_task, UNITY_FREERTOS_CPU); + + vTaskDelay(1); /* make sure everything is set up */ + trigger = true; + + uint32_t yield_ccount, now_ccount, delta; + TEST_ASSERT( xQueueReceive(queue, &yield_ccount, 100 / portTICK_PERIOD_MS) ); + RSR(CCOUNT, now_ccount); + TEST_ASSERT( flag ); + + delta = now_ccount - yield_ccount; + printf("Yielding from lower priority task took %u cycles\n", delta); + TEST_ASSERT(delta < 10000); + + vQueueDelete(queue); + vTaskDelete(sender_task); + } +} + + +TEST_CASE("Yield from lower priority task, other CPU", "[freertos]") +{ + uint32_t trigger_ccount, yield_ccount, now_ccount, delta; + + /* Do this 3 times, mostly for the benchmark value - the first + run includes a cache miss so uses more cycles than it should. */ + for (int i = 0; i < 3; i++) { + TaskHandle_t sender_task; + QueueHandle_t queue = xQueueCreate(1, sizeof(uint32_t)); + trigger = false; + flag = false; + + /* "send_to_queue" task sits on the other CPU, lower priority to us */ + xTaskCreatePinnedToCore(task_send_to_queue, "YIELD", 2048, (void *)queue, UNITY_FREERTOS_PRIORITY - 1, + &sender_task, !UNITY_FREERTOS_CPU); + + vTaskDelay(2); /* make sure everything is set up */ + trigger = true; + RSR(CCOUNT, trigger_ccount); + + // yield_ccount is not useful in this test as it's the other core's CCOUNT + // so we use trigger_ccount instead + TEST_ASSERT( xQueueReceive(queue, &yield_ccount, 100 / portTICK_PERIOD_MS) ); + RSR(CCOUNT, now_ccount); + TEST_ASSERT( flag ); + + delta = now_ccount - trigger_ccount; + printf("Yielding from task on other core took %u cycles\n", delta); + TEST_ASSERT(delta < 10000); + + vQueueDelete(queue); + vTaskDelete(sender_task); + } +} diff --git a/tools/unit-test-app/components/unity/include/unity_config.h b/tools/unit-test-app/components/unity/include/unity_config.h index 07df5b3058..bacb79e00b 100644 --- a/tools/unit-test-app/components/unity/include/unity_config.h +++ b/tools/unit-test-app/components/unity/include/unity_config.h @@ -9,6 +9,10 @@ #include +/* Some definitions applicable to Unity running in FreeRTOS */ +#define UNITY_FREERTOS_PRIORITY 5 +#define UNITY_FREERTOS_CPU 0 + #define UNITY_EXCLUDE_FLOAT #define UNITY_EXCLUDE_DOUBLE diff --git a/tools/unit-test-app/main/app_main.c b/tools/unit-test-app/main/app_main.c index 58a1951720..c5df02b943 100644 --- a/tools/unit-test-app/main/app_main.c +++ b/tools/unit-test-app/main/app_main.c @@ -2,9 +2,9 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "unity.h" +#include "unity_config.h" - -void unityTask(void *pvParameters) +void unityTask(void *pvParameters) { vTaskDelay(1000 / portTICK_PERIOD_MS); unity_run_menu(); @@ -15,6 +15,6 @@ void app_main() { // Note: if unpinning this task, change the way run times are calculated in // unity_platform - xTaskCreatePinnedToCore(unityTask, "unityTask", 4096, NULL, 5, NULL, 0); + xTaskCreatePinnedToCore(unityTask, "unityTask", 4096, NULL, + UNITY_FREERTOS_PRIORITY, NULL, UNITY_FREERTOS_CPU); } -