From ef4c02dda2a0b2ebd19bf0cbc9e7f7ce2c8f5695 Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Wed, 25 Sep 2024 20:18:33 +0800 Subject: [PATCH 1/2] revert(esp_hw_support): stall another core during cpu/mem/apb freq switching This reverts commit c2bb64fbe847f6d81fefe260194db51b5b7c2436. --- .../esp_hw_support/port/esp32p4/rtc_clk.c | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/components/esp_hw_support/port/esp32p4/rtc_clk.c b/components/esp_hw_support/port/esp32p4/rtc_clk.c index 63fb3c3ca9..f5940277e5 100644 --- a/components/esp_hw_support/port/esp32p4/rtc_clk.c +++ b/components/esp_hw_support/port/esp32p4/rtc_clk.c @@ -14,7 +14,6 @@ #include "soc/rtc.h" #include "esp_private/rtc_clk.h" #include "esp_attr.h" -#include "esp_cpu.h" #include "esp_hw_log.h" #include "esp_rom_sys.h" #include "hal/clk_tree_ll.h" @@ -183,13 +182,7 @@ static void rtc_clk_cpu_freq_to_xtal(int cpu_freq, int div, bool to_default) clk_ll_mem_set_divider(mem_divider); clk_ll_sys_set_divider(sys_divider); clk_ll_apb_set_divider(apb_divider); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - esp_cpu_stall(1 - esp_cpu_get_core_id()); -#endif clk_ll_bus_update(); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - esp_cpu_unstall(1 - esp_cpu_get_core_id()); -#endif esp_rom_set_cpu_ticks_per_us(cpu_freq); } @@ -201,13 +194,7 @@ static void rtc_clk_cpu_freq_to_8m(void) clk_ll_sys_set_divider(1); clk_ll_apb_set_divider(1); clk_ll_cpu_set_src(SOC_CPU_CLK_SRC_RC_FAST); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - esp_cpu_stall(1 - esp_cpu_get_core_id()); -#endif clk_ll_bus_update(); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - esp_cpu_unstall(1 - esp_cpu_get_core_id()); -#endif esp_rom_set_cpu_ticks_per_us(20); } @@ -253,22 +240,14 @@ static void rtc_clk_cpu_freq_to_cpll_mhz(int cpu_freq_mhz, hal_utils_clk_div_t * // Update bit does not control CPU clock sel mux. Therefore, there may be a middle state during the switch (CPU rises) // Since this is upscaling, we need to configure the frequency division coefficient before switching the clock source. // Otherwise, an intermediate state will occur, in the intermediate state, the frequency of APB/MEM does not meet the - // timing requirements. If there are periperals access that depend on these two clocks at this moment, some exception + // timing requirements. If there are periperals/CPU access that depend on these two clocks at this moment, some exception // might occur. clk_ll_cpu_set_divider(div->integer, div->numerator, div->denominator); clk_ll_mem_set_divider(mem_divider); clk_ll_sys_set_divider(sys_divider); clk_ll_apb_set_divider(apb_divider); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - // During frequency switching, non-frequency switching cores may have ongoing memory accesses, which may cause access - // failures, stalling non-frequency switching cores here can avoid such failures. - esp_cpu_stall(1 - esp_cpu_get_core_id()); -#endif clk_ll_bus_update(); clk_ll_cpu_set_src(SOC_CPU_CLK_SRC_PLL); -#if (!defined(BOOTLOADER_BUILD) && (CONFIG_FREERTOS_NUMBER_OF_CORES == 2)) - esp_cpu_unstall(1 - esp_cpu_get_core_id()); -#endif esp_rom_set_cpu_ticks_per_us(cpu_freq_mhz); } From e1a341455a011248c289a180c46bfbd878e5f33e Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Wed, 25 Sep 2024 20:55:39 +0800 Subject: [PATCH 2/2] fix(esp_hw_support): fix esp32p4 CPU frequency switching timing --- .../esp_hw_support/port/esp32p4/rtc_clk.c | 63 +++++++++++++------ 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/components/esp_hw_support/port/esp32p4/rtc_clk.c b/components/esp_hw_support/port/esp32p4/rtc_clk.c index f5940277e5..aa3bfd9675 100644 --- a/components/esp_hw_support/port/esp32p4/rtc_clk.c +++ b/components/esp_hw_support/port/esp32p4/rtc_clk.c @@ -205,15 +205,18 @@ static void rtc_clk_cpu_freq_to_8m(void) */ static void rtc_clk_cpu_freq_to_cpll_mhz(int cpu_freq_mhz, hal_utils_clk_div_t *div) { - // CPLL -> CPU_CLK -> MEM_CLK -> SYS_CLK -> APB_CLK - // Constraint: MEM_CLK <= 200MHz, APB_CLK <= 100MHz - // This implies that when clock source is CPLL, - // If cpu_divider < 2, mem_divider must be larger or equal to 2 - // If cpu_divider < 2, mem_divider = 2, sys_divider < 2, apb_divider must be larger or equal to 2 - // Current available configurations: - // 360 - 360 - 180 - 180 - 90 - // 360 - 180 - 180 - 180 - 90 - // 360 - 90 - 90 - 90 - 90 + /** + * Constraint: MEM_CLK <= 200MHz, APB_CLK <= 100MHz + * This implies that when clock source is CPLL, + * If cpu_divider < 2, mem_divider must be larger or equal to 2 + * If cpu_divider < 2, mem_divider = 2, sys_divider < 2, apb_divider must be larger or equal to 2 + * + * Current available configurations: + * CPLL -> CPU_CLK -> MEM_CLK -> SYS_CLK -> APB_CLK + * 360 div1 360 div2 180 div1 180 div2 90 + * 360 div2 180 div1 180 div1 180 div2 90 + * 360 div4 90 div1 90 div1 90 div1 90 + */ uint32_t mem_divider = 1; uint32_t sys_divider = 1; // We are not going to change this uint32_t apb_divider = 1; @@ -237,16 +240,38 @@ static void rtc_clk_cpu_freq_to_cpll_mhz(int cpu_freq_mhz, hal_utils_clk_div_t * // To avoid such case, we will strictly do abort here. abort(); } - // Update bit does not control CPU clock sel mux. Therefore, there may be a middle state during the switch (CPU rises) - // Since this is upscaling, we need to configure the frequency division coefficient before switching the clock source. - // Otherwise, an intermediate state will occur, in the intermediate state, the frequency of APB/MEM does not meet the - // timing requirements. If there are periperals/CPU access that depend on these two clocks at this moment, some exception - // might occur. - clk_ll_cpu_set_divider(div->integer, div->numerator, div->denominator); - clk_ll_mem_set_divider(mem_divider); - clk_ll_sys_set_divider(sys_divider); - clk_ll_apb_set_divider(apb_divider); - clk_ll_bus_update(); + + // If it's upscaling, the divider of MEM/SYS/APB needs to be increased, to avoid illegal intermediate states, + // the clock divider should be updated in the order from the APB_CLK to CPU_CLK. + // And if it's downscaling, the divider of MEM/SYS/APB needs to be decreased, the clock divider should be updated + // in the order from the CPU_CLK to APB_CLK. + // Otherwise, an intermediate state will occur, in the intermediate state, the frequency of APB/MEM does not meet + // the timing requirements. If there are periperals/CPU access that depend on these two clocks at this moment, some + // exception might occur. + if (cpu_freq_mhz >= esp_rom_get_cpu_ticks_per_us()) { + // Frequency Upscaling + clk_ll_apb_set_divider(apb_divider); + clk_ll_bus_update(); + clk_ll_sys_set_divider(sys_divider); + clk_ll_bus_update(); + clk_ll_mem_set_divider(mem_divider); + clk_ll_bus_update(); + clk_ll_cpu_set_divider(div->integer, div->numerator, div->denominator); + clk_ll_bus_update(); + } else { + // Frequency Downscaling + clk_ll_cpu_set_divider(div->integer, div->numerator, div->denominator); + clk_ll_bus_update(); + clk_ll_mem_set_divider(mem_divider); + clk_ll_bus_update(); + clk_ll_sys_set_divider(sys_divider); + clk_ll_bus_update(); + clk_ll_apb_set_divider(apb_divider); + clk_ll_bus_update(); + } + + // Update bit does not control CPU clock sel mux, the clock source needs to be switched at + // last to avoid intermediate states. clk_ll_cpu_set_src(SOC_CPU_CLK_SRC_PLL); esp_rom_set_cpu_ticks_per_us(cpu_freq_mhz); }