From 76f97f3abd9c2cb0bd3466acc29957a631e60cda Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Tue, 25 Jun 2019 19:23:10 +0800 Subject: [PATCH 1/2] esp32: Dis interrupts up to 5 lvl for DPORT Disable interrupts for both DPORT workarounds up to 5 lvl. Closes: https://esp32.com/viewtopic.php?f=2&t=10981&sid=d125cec233070ed4d2c5410bf5d3d74a Closes: IDF-728 --- components/esp32/Kconfig | 8 +++++++- components/esp32/dport_access.c | 2 +- components/esp32/dport_panic_highint_hdl.S | 9 +++++++-- components/esp32/include/esp_dport_access.h | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/components/esp32/Kconfig b/components/esp32/Kconfig index ab092603d..d6d65ae3d 100644 --- a/components/esp32/Kconfig +++ b/components/esp32/Kconfig @@ -1022,6 +1022,13 @@ config ESP32_RTCDATA_IN_FAST_MEM This option depends on the CONFIG_FREERTOS_UNICORE option because RTC fast memory can be accessed only by PRO_CPU core. +config ESP32_DPORT_DIS_INTERRUPT_LVL + int "Disable the interrupt level for the DPORT workarounds" + default 5 + help + To prevent interrupting DPORT workarounds, + need to disable interrupt with a maximum used level in the system. + endmenu # ESP32-Specific menu Wi-Fi @@ -1306,7 +1313,6 @@ config ESP32_PHY_MAX_TX_POWER endmenu # PHY - menu "Power Management" config PM_ENABLE diff --git a/components/esp32/dport_access.c b/components/esp32/dport_access.c index 731bdb7ec..1a3348105 100644 --- a/components/esp32/dport_access.c +++ b/components/esp32/dport_access.c @@ -256,7 +256,7 @@ uint32_t IRAM_ATTR esp_dport_access_reg_read(uint32_t reg) unsigned int intLvl; __asm__ __volatile__ (\ "movi %[APB], "XTSTR(0x3ff40078)"\n"\ - "rsil %[LVL], "XTSTR(3)"\n"\ + "rsil %[LVL], "XTSTR(CONFIG_ESP32_DPORT_DIS_INTERRUPT_LVL)"\n"\ "l32i %[APB], %[APB], 0\n"\ "l32i %[REG], %[REG], 0\n"\ "wsr %[LVL], "XTSTR(PS)"\n"\ diff --git a/components/esp32/dport_panic_highint_hdl.S b/components/esp32/dport_panic_highint_hdl.S index bddde3cdf..35412deba 100644 --- a/components/esp32/dport_panic_highint_hdl.S +++ b/components/esp32/dport_panic_highint_hdl.S @@ -31,9 +31,10 @@ Interrupt , a high-priority interrupt, is used for several things: */ -#define L4_INTR_STACK_SIZE 8 +#define L4_INTR_STACK_SIZE 12 #define L4_INTR_A2_OFFSET 0 #define L4_INTR_A3_OFFSET 4 +#define L4_INTR_A4_OFFSET 8 .data _l4_intr_stack: .space L4_INTR_STACK_SIZE @@ -145,10 +146,11 @@ xt_highint4: movi a0, (1< Date: Fri, 28 Jun 2019 13:34:19 +0800 Subject: [PATCH 2/2] esp32: Add UTs for DPORT and Hi-interrupt --- components/esp32/test/CMakeLists.txt | 1 + components/esp32/test/component.mk | 3 +- components/esp32/test/test_dport.c | 95 ++++++++++++++++++- .../esp32/test/test_dport_xt_highint5.S | 80 ++++++++++++++++ 4 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 components/esp32/test/test_dport_xt_highint5.S diff --git a/components/esp32/test/CMakeLists.txt b/components/esp32/test/CMakeLists.txt index 0beee4041..b9648401d 100644 --- a/components/esp32/test/CMakeLists.txt +++ b/components/esp32/test/CMakeLists.txt @@ -49,3 +49,4 @@ add_definitions(-DWIFI_ESP_WIFI_MD5=\"${WIFI_ESP_WIFI_MD5}\") add_custom_target(esp32_test_logo DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/test_tjpgd_logo.h") add_dependencies(${COMPONENT_NAME} esp32_test_logo) + diff --git a/components/esp32/test/component.mk b/components/esp32/test/component.mk index c35427b3f..be9b04544 100644 --- a/components/esp32/test/component.mk +++ b/components/esp32/test/component.mk @@ -4,7 +4,8 @@ COMPONENT_EXTRA_CLEAN := test_tjpgd_logo.h -COMPONENT_ADD_LDFLAGS = -Wl,--whole-archive -l$(COMPONENT_NAME) -Wl,--no-whole-archive +COMPONENT_ADD_LDFLAGS = -Wl,--whole-archive -l$(COMPONENT_NAME) -Wl,--no-whole-archive \ + -u ld_include_test_dport_xt_highint5 \ COMPONENT_SRCDIRS := . test_vectors diff --git a/components/esp32/test/test_dport.c b/components/esp32/test/test_dport.c index 96fa0b109..96e483b9a 100644 --- a/components/esp32/test/test_dport.c +++ b/components/esp32/test/test_dport.c @@ -1,5 +1,7 @@ #include #include +#include "xtensa/core-macros.h" +#include "xtensa/hal.h" #include "esp_types.h" #include "esp_clk.h" @@ -13,10 +15,13 @@ #include "soc/uart_reg.h" #include "soc/dport_reg.h" #include "soc/rtc.h" +#include "esp_intr_alloc.h" +#include "driver/timer.h" #define MHZ (1000000) static volatile bool exit_flag; static bool dport_test_result; static bool apb_test_result; +uint32_t volatile apb_intr_test_result; static void accessDPORT(void *pvParameters) { @@ -58,6 +63,7 @@ static void accessAPB(void *pvParameters) void run_tasks(const char *task1_description, void (* task1_func)(void *), const char *task2_description, void (* task2_func)(void *), uint32_t delay_ms) { + apb_intr_test_result = 1; int i; TaskHandle_t th[2]; xSemaphoreHandle exit_sema[2]; @@ -93,7 +99,7 @@ void run_tasks(const char *task1_description, void (* task1_func)(void *), const vSemaphoreDelete(exit_sema[i]); } } - TEST_ASSERT(dport_test_result == true && apb_test_result == true); + TEST_ASSERT(dport_test_result == true && apb_test_result == true && apb_intr_test_result == 1); } TEST_CASE("access DPORT and APB at same time", "[esp32]") @@ -323,3 +329,90 @@ TEST_CASE("BENCHMARK for DPORT access performance", "[freertos]") } BENCHMARK_END("_DPORT_REG_READ"); } + +uint32_t xt_highint5_read_apb; + +#ifndef CONFIG_FREERTOS_UNICORE +timer_isr_handle_t inth; +xSemaphoreHandle sync_sema; + +static void init_hi_interrupt(void *arg) +{ + printf("init hi_interrupt on CPU%d \n", xPortGetCoreID()); + TEST_ESP_OK(esp_intr_alloc(ETS_INTERNAL_TIMER2_INTR_SOURCE, ESP_INTR_FLAG_LEVEL5 | ESP_INTR_FLAG_IRAM, NULL, NULL, &inth)); + while (exit_flag == false); + esp_intr_free(inth); + printf("disable hi_interrupt on CPU%d \n", xPortGetCoreID()); + vTaskDelete(NULL); +} + +static void accessDPORT2_stall_other_cpu(void *pvParameters) +{ + xSemaphoreHandle *sema = (xSemaphoreHandle *) pvParameters; + dport_test_result = true; + while (exit_flag == false) { + DPORT_STALL_OTHER_CPU_START(); + XTHAL_SET_CCOMPARE(2, XTHAL_GET_CCOUNT()); + xt_highint5_read_apb = 1; + for (int i = 0; i < 200; ++i) { + if (_DPORT_REG_READ(DPORT_DATE_REG) != _DPORT_REG_READ(DPORT_DATE_REG)) { + apb_test_result = false; + break; + } + } + xt_highint5_read_apb = 0; + DPORT_STALL_OTHER_CPU_END(); + } + printf("accessDPORT2_stall_other_cpu finish\n"); + + xSemaphoreGive(*sema); + vTaskDelete(NULL); +} + +TEST_CASE("Check stall workaround DPORT and Hi-interrupt", "[esp32]") +{ + xt_highint5_read_apb = 0; + dport_test_result = false; + apb_test_result = true; + TEST_ASSERT(xTaskCreatePinnedToCore(&init_hi_interrupt, "init_hi_intr", 2048, NULL, 6, NULL, 1) == pdTRUE); + // Access DPORT(stall other cpu method) - CPU0 + // STALL - CPU1 + // Hi-interrupt - CPU1 + run_tasks("accessDPORT2_stall_other_cpu", accessDPORT2_stall_other_cpu, " - ", NULL, 10000); +} + +static void accessDPORT2(void *pvParameters) +{ + xSemaphoreHandle *sema = (xSemaphoreHandle *) pvParameters; + dport_test_result = true; + + TEST_ESP_OK(esp_intr_alloc(ETS_INTERNAL_TIMER2_INTR_SOURCE, ESP_INTR_FLAG_LEVEL5 | ESP_INTR_FLAG_IRAM, NULL, NULL, &inth)); + + XTHAL_SET_CCOMPARE(2, XTHAL_GET_CCOUNT() + 21); + int sync = 0; + while (exit_flag == false) { + ets_delay_us(++sync % 10); + for (int i = 0; i < 200; ++i) { + if (DPORT_REG_READ(DPORT_DATE_REG) != DPORT_REG_READ(DPORT_DATE_REG)) { + dport_test_result = false; + break; + } + } + } + esp_intr_free(inth); + printf("accessDPORT2 finish\n"); + + xSemaphoreGive(*sema); + vTaskDelete(NULL); +} + +TEST_CASE("Check pre-read workaround DPORT and Hi-interrupt", "[esp32]") +{ + xt_highint5_read_apb = 0; + dport_test_result = false; + apb_test_result = true; + // Access DPORT(pre-read method) - CPU1 + // Hi-interrupt - CPU1 + run_tasks("accessAPB", accessAPB, "accessDPORT2", accessDPORT2, 10000); +} +#endif // CONFIG_FREERTOS_UNICORE diff --git a/components/esp32/test/test_dport_xt_highint5.S b/components/esp32/test/test_dport_xt_highint5.S new file mode 100644 index 000000000..965c7385a --- /dev/null +++ b/components/esp32/test/test_dport_xt_highint5.S @@ -0,0 +1,80 @@ +#include +#include +#include +#include "freertos/xtensa_context.h" + +#include "sdkconfig.h" +#include "soc/soc.h" +#include "soc/dport_reg.h" + +#ifndef CONFIG_FREERTOS_UNICORE + +#define L5_INTR_STACK_SIZE 12 +#define L5_INTR_A2_OFFSET 0 +#define L5_INTR_A3_OFFSET 4 +#define L5_INTR_A4_OFFSET 8 + .data +_l5_intr_stack: + .space L5_INTR_STACK_SIZE + + .section .iram1,"ax" + .global xt_highint5 + .type xt_highint5,@function + .align 4 +xt_highint5: + + movi a0, xt_highint5_read_apb + l32i a0, a0, 0 + bnez a0, .read_apb_reg + +// Short interrupt + esync + rsr a0, CCOUNT + addi a0, a0, 27 + wsr a0, CCOMPARE2 + esync + + rsr a0, EXCSAVE_5 // restore a0 + rfi 5 + + + +// read APB reg 10 time. +.read_apb_reg: + movi a0, _l5_intr_stack + s32i a2, a0, L5_INTR_A2_OFFSET + s32i a3, a0, L5_INTR_A3_OFFSET + s32i a4, a0, L5_INTR_A4_OFFSET + + movi a4, 10 // count of reading + movi a0, 0x3ff40078 // read APB reg + l32i a2, a0, 0 +.loop_read_apb_reg: + l32i a3, a0, 0 + bne a3, a2, .need_set_apb_test_result + addi a4, a4, -1 + l32i a2, a0, 0 + bnez a4, .loop_read_apb_reg + j 1f +.need_set_apb_test_result: + movi a0, apb_intr_test_result // set fail + movi a2, 0 + s32i a2, a0, 0 + memw +1: + movi a0, _l5_intr_stack + l32i a2, a0, L5_INTR_A2_OFFSET + l32i a3, a0, L5_INTR_A3_OFFSET + l32i a4, a0, L5_INTR_A4_OFFSET + rsync +.L_xt_highint5_exit: + rsr a0, EXCSAVE_5 // restore a0 + rfi 5 + +/* The linker has no reason to link in this file; all symbols it exports are already defined + (weakly!) in the default int handler. Define a symbol here so we can use it to have the + linker inspect this anyway. */ + + .global ld_include_test_dport_xt_highint5 +ld_include_test_dport_xt_highint5: +#endif // CONFIG_FREERTOS_UNICORE