From 43b5fdd5c9daec2357e4a02d87170ae64bb0a688 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 22 Mar 2018 19:02:41 +0800 Subject: [PATCH 1/6] soc/rtc: fix switching between 80/160 and 240MHz Previous code contained a check for PLL frequency to be 240MHz, while in fact 240MHz was a CPU frequency; corresponding PLL frequency is 480MHz. Fixed the comparison and replaced integer MHz values with an enum. --- components/soc/esp32/rtc_clk.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/components/soc/esp32/rtc_clk.c b/components/soc/esp32/rtc_clk.c index 112e68c6bb..dc5f71db21 100644 --- a/components/soc/esp32/rtc_clk.c +++ b/components/soc/esp32/rtc_clk.c @@ -98,9 +98,17 @@ static const char* TAG = "rtc_clk"; #define DIG_DBIAS_XTAL RTC_CNTL_DBIAS_1V10 #define DIG_DBIAS_2M RTC_CNTL_DBIAS_1V00 +/* PLL currently enabled, if any */ +typedef enum { + RTC_PLL_NONE, + RTC_PLL_320M, + RTC_PLL_480M +} rtc_pll_t; +static rtc_pll_t s_cur_pll = RTC_PLL_NONE; +/* Current CPU frequency; saved in a variable for faster freq. switching */ static rtc_cpu_freq_t s_cur_freq = RTC_CPU_FREQ_XTAL; -static int s_pll_freq = 0; + static void rtc_clk_32k_enable_internal(int dac, int dres, int dbias) { @@ -392,8 +400,9 @@ static void rtc_clk_cpu_freq_to_xtal() static void rtc_clk_cpu_freq_to_pll(rtc_cpu_freq_t cpu_freq) { int freq = 0; - if ((cpu_freq == RTC_CPU_FREQ_240M && s_pll_freq == 320) || - (cpu_freq != RTC_CPU_FREQ_240M && s_pll_freq == 240)) { + if (s_cur_pll == RTC_PLL_NONE || + (cpu_freq == RTC_CPU_FREQ_240M && s_cur_pll == RTC_PLL_320M) || + (cpu_freq != RTC_CPU_FREQ_240M && s_cur_pll == RTC_PLL_480M)) { /* need to switch PLLs, fall back to full implementation */ rtc_clk_cpu_freq_set(cpu_freq); return; @@ -451,7 +460,7 @@ void rtc_clk_cpu_freq_set(rtc_cpu_freq_t cpu_freq) SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG, RTC_CNTL_BB_I2C_FORCE_PD | RTC_CNTL_BBPLL_FORCE_PD | RTC_CNTL_BBPLL_I2C_FORCE_PD); - s_pll_freq = 0; + s_cur_pll = RTC_PLL_NONE; rtc_clk_apb_freq_update(xtal_freq * MHZ); /* is APLL under force power down? */ @@ -479,15 +488,15 @@ void rtc_clk_cpu_freq_set(rtc_cpu_freq_t cpu_freq) if (cpu_freq == RTC_CPU_FREQ_80M) { DPORT_REG_SET_FIELD(DPORT_CPU_PER_CONF_REG, DPORT_CPUPERIOD_SEL, 0); ets_update_cpu_frequency(80); - s_pll_freq = 320; + s_cur_pll = RTC_PLL_320M; } else if (cpu_freq == RTC_CPU_FREQ_160M) { DPORT_REG_SET_FIELD(DPORT_CPU_PER_CONF_REG, DPORT_CPUPERIOD_SEL, 1); ets_update_cpu_frequency(160); - s_pll_freq = 320; + s_cur_pll = RTC_PLL_320M; } else if (cpu_freq == RTC_CPU_FREQ_240M) { DPORT_REG_SET_FIELD(DPORT_CPU_PER_CONF_REG, DPORT_CPUPERIOD_SEL, 2); ets_update_cpu_frequency(240); - s_pll_freq = 480; + s_cur_pll = RTC_PLL_480M; } REG_SET_FIELD(RTC_CNTL_CLK_CONF_REG, RTC_CNTL_SOC_CLK_SEL, RTC_CNTL_SOC_CLK_SEL_PLL); rtc_clk_wait_for_slow_cycle(); From 1517aeda6c5df09fbbf8862be5aa495db5a97934 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 27 Mar 2018 10:49:47 +0800 Subject: [PATCH 2/6] pm: fix calculation of maximum of two frequencies The old code calculated MAX() of two enum values, but CPU frequency enum values are not ordered (2MHz goes after 240MHz). This caused incorrect configuration to be set. --- components/esp32/pm_esp32.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/components/esp32/pm_esp32.c b/components/esp32/pm_esp32.c index 5d422f3b8c..6f848f250d 100644 --- a/components/esp32/pm_esp32.c +++ b/components/esp32/pm_esp32.c @@ -148,6 +148,21 @@ pm_mode_t esp_pm_impl_get_mode(esp_pm_lock_type_t type, int arg) } } +/* rtc_cpu_freq_t enum is not ordered by frequency, so convert to MHz, + * figure out the maximum value, then convert back to rtc_cpu_freq_t. + */ +static rtc_cpu_freq_t max_freq_of(rtc_cpu_freq_t f1, rtc_cpu_freq_t f2) +{ + int f1_hz = rtc_clk_cpu_freq_value(f1); + int f2_hz = rtc_clk_cpu_freq_value(f2); + int f_max_hz = MAX(f1_hz, f2_hz); + rtc_cpu_freq_t result = RTC_CPU_FREQ_XTAL; + if (!rtc_clk_cpu_freq_from_mhz(f_max_hz/1000000, &result)) { + assert(false && "unsupported frequency"); + } + return result; +} + esp_err_t esp_pm_configure(const void* vconfig) { #ifndef CONFIG_PM_ENABLE @@ -160,6 +175,11 @@ esp_err_t esp_pm_configure(const void* vconfig) } rtc_cpu_freq_t min_freq = config->min_cpu_freq; rtc_cpu_freq_t max_freq = config->max_cpu_freq; + int min_freq_mhz = rtc_clk_cpu_freq_value(min_freq); + int max_freq_mhz = rtc_clk_cpu_freq_value(max_freq); + if (min_freq_mhz > max_freq_mhz) { + return ESP_ERR_INVALID_ARG; + } rtc_cpu_freq_t apb_max_freq; /* CPU frequency in APB_MAX mode */ if (max_freq == RTC_CPU_FREQ_240M) { @@ -174,7 +194,7 @@ esp_err_t esp_pm_configure(const void* vconfig) apb_max_freq = RTC_CPU_FREQ_80M; } - apb_max_freq = MAX(apb_max_freq, min_freq); + apb_max_freq = max_freq_of(apb_max_freq, min_freq); ESP_LOGI(TAG, "Frequency switching config: " "CPU_MAX: %s, APB_MAX: %s, APB_MIN: %s, Light sleep: %s", From fe5d294f6cd4a1d3a65f09dcb238bf4c69a72397 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 27 Mar 2018 10:51:07 +0800 Subject: [PATCH 3/6] pm: handle changes to the PM_MODE_ACTIVE frequency The issue would manifest itself in cases when switching from PM configuration like {active=160MHz, idle=80MHz} to {active=80MHz, idle=80Mhz}. After the configuration was changed, PM logic would think that current frequency was 80MHz and would not do any further switching. In fact, frequency was still 160MHz. --- components/esp32/pm_esp32.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/components/esp32/pm_esp32.c b/components/esp32/pm_esp32.c index 6f848f250d..c866bd1466 100644 --- a/components/esp32/pm_esp32.c +++ b/components/esp32/pm_esp32.c @@ -110,6 +110,13 @@ static const char* s_freq_names[] __attribute__((unused)) = { /* Whether automatic light sleep is enabled. Currently always false */ static bool s_light_sleep_en = false; +/* When configuration is changed, current frequency may not match the + * newly configured frequency for the current mode. This is an indicator + * to the mode switch code to get the actual current frequency instead of + * relying on the current mode. + */ +static bool s_config_changed = false; + #ifdef WITH_PROFILING /* Time, in microseconds, spent so far in each mode */ static pm_time_t s_time_in_mode[PM_MODE_COUNT]; @@ -209,6 +216,7 @@ esp_err_t esp_pm_configure(const void* vconfig) s_cpu_freq_by_mode[PM_MODE_APB_MIN] = min_freq; s_cpu_freq_by_mode[PM_MODE_LIGHT_SLEEP] = min_freq; s_light_sleep_en = config->light_sleep_enable; + s_config_changed = true; portEXIT_CRITICAL(&s_switch_lock); return ESP_OK; @@ -343,10 +351,17 @@ static void IRAM_ATTR do_switch(pm_mode_t new_mode) } while (true); s_new_mode = new_mode; s_is_switching = true; + bool config_changed = s_config_changed; + s_config_changed = false; portEXIT_CRITICAL_ISR(&s_switch_lock); - rtc_cpu_freq_t old_freq = s_cpu_freq_by_mode[s_mode]; rtc_cpu_freq_t new_freq = s_cpu_freq_by_mode[new_mode]; + rtc_cpu_freq_t old_freq; + if (!config_changed) { + old_freq = s_cpu_freq_by_mode[s_mode]; + } else { + old_freq = rtc_clk_cpu_freq_get(); + } if (new_freq != old_freq) { uint32_t old_ticks_per_us = g_ticks_per_us_pro; From 839c665c92c3d332ff1362f3da507055e825f72f Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 27 Mar 2018 10:52:03 +0800 Subject: [PATCH 4/6] pm: fix initialisation only done in dual core mode Introduced in 9377d4ac. Accidentally put the new code block under an --- components/esp32/pm_esp32.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/esp32/pm_esp32.c b/components/esp32/pm_esp32.c index c866bd1466..a596401155 100644 --- a/components/esp32/pm_esp32.c +++ b/components/esp32/pm_esp32.c @@ -489,14 +489,16 @@ void esp_pm_impl_init() ESP_ERROR_CHECK(esp_pm_lock_create(ESP_PM_CPU_FREQ_MAX, 0, "rtos1", &s_rtos_lock_handle[1])); ESP_ERROR_CHECK(esp_pm_lock_acquire(s_rtos_lock_handle[1])); +#endif // portNUM_PROCESSORS == 2 /* Configure all modes to use the default CPU frequency. * This will be modified later by a call to esp_pm_configure. */ rtc_cpu_freq_t default_freq; - assert(rtc_clk_cpu_freq_from_mhz(CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ, &default_freq)); + if (!rtc_clk_cpu_freq_from_mhz(CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ, &default_freq)) { + assert(false && "unsupported frequency"); + } for (size_t i = 0; i < PM_MODE_COUNT; ++i) { s_cpu_freq_by_mode[i] = default_freq; } -#endif // portNUM_PROCESSORS == 2 } From 94014a9a515ce1a06bf6a528b69b6788f6554fdf Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 27 Mar 2018 10:52:38 +0800 Subject: [PATCH 5/6] pm: handle the case of 2MHz frequency It is not possible to generate 1 MHz REF_TICK from 2 MHz APB clock (this is a limitation of REF_TICK divider circuit). Since switching REF_TICK frequency is something we would like to avoid (to maintain UART output even with DFS), 2 MHz frequency has been marked as unsupported. --- components/esp32/pm_esp32.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/components/esp32/pm_esp32.c b/components/esp32/pm_esp32.c index a596401155..11a4d3c0db 100644 --- a/components/esp32/pm_esp32.c +++ b/components/esp32/pm_esp32.c @@ -180,6 +180,12 @@ esp_err_t esp_pm_configure(const void* vconfig) if (config->light_sleep_enable) { return ESP_ERR_NOT_SUPPORTED; } + + if (config->min_cpu_freq == RTC_CPU_FREQ_2M) { + /* Minimal APB frequency to achieve 1MHz REF_TICK frequency is 5 MHz */ + return ESP_ERR_NOT_SUPPORTED; + } + rtc_cpu_freq_t min_freq = config->min_cpu_freq; rtc_cpu_freq_t max_freq = config->max_cpu_freq; int min_freq_mhz = rtc_clk_cpu_freq_value(min_freq); @@ -188,14 +194,14 @@ esp_err_t esp_pm_configure(const void* vconfig) return ESP_ERR_INVALID_ARG; } - rtc_cpu_freq_t apb_max_freq; /* CPU frequency in APB_MAX mode */ + rtc_cpu_freq_t apb_max_freq = max_freq; /* CPU frequency in APB_MAX mode */ if (max_freq == RTC_CPU_FREQ_240M) { /* We can't switch between 240 and 80/160 without disabling PLL, * so use 240MHz CPU frequency when 80MHz APB frequency is requested. */ apb_max_freq = RTC_CPU_FREQ_240M; - } else { - /* Otherwise (max CPU frequency is 80MHz or 160MHz), can use 80MHz + } else if (max_freq == RTC_CPU_FREQ_160M || max_freq == RTC_CPU_FREQ_80M) { + /* Otherwise, can use 80MHz * CPU frequency when 80MHz APB frequency is requested. */ apb_max_freq = RTC_CPU_FREQ_80M; From db44a719fb76b3365493fccea8888fb1509be5f5 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 27 Mar 2018 10:53:03 +0800 Subject: [PATCH 6/6] pm: add unit test for configuration changes --- components/esp32/test/test_pm.c | 41 ++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/components/esp32/test/test_pm.c b/components/esp32/test/test_pm.c index 2214cec591..7033238a04 100644 --- a/components/esp32/test/test_pm.c +++ b/components/esp32/test/test_pm.c @@ -4,9 +4,48 @@ #include #include "unity.h" #include "esp_pm.h" - +#include "esp_clk.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "esp_log.h" TEST_CASE("Can dump power management lock stats", "[pm]") { esp_pm_dump_locks(stdout); } + +#ifdef CONFIG_PM_ENABLE + +static void switch_freq(int mhz) +{ + rtc_cpu_freq_t max_freq; + assert(rtc_clk_cpu_freq_from_mhz(mhz, &max_freq)); + esp_pm_config_esp32_t pm_config = { + .max_cpu_freq = max_freq, + .min_cpu_freq = RTC_CPU_FREQ_XTAL, + }; + ESP_ERROR_CHECK( esp_pm_configure(&pm_config) ); + printf("Waiting for frequency to be set to %d (%d MHz)...\n", max_freq, mhz); + while (esp_clk_cpu_freq() / 1000000 != mhz) { + vTaskDelay(pdMS_TO_TICKS(1000)); + printf("Frequency is %d MHz\n", esp_clk_cpu_freq()); + } + printf("Frequency is set to %d MHz\n", mhz); +} + +TEST_CASE("Can switch frequency using esp_pm_configure", "[pm]") +{ + int orig_freq_mhz = esp_clk_cpu_freq() / 1000000; + switch_freq(240); + switch_freq(40); + switch_freq(160); + switch_freq(240); + switch_freq(80); + switch_freq(40); + switch_freq(240); + switch_freq(40); + switch_freq(80); + switch_freq(orig_freq_mhz); +} + +#endif // CONFIG_PM_ENABLE