From 59c3315a4f6fb329104540a6c957bc2901ff0e91 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Fri, 17 Jul 2020 17:01:25 +0800 Subject: [PATCH 1/5] platform: rename CONFIG_CAVS_LPRO to CONFIG_CAVS_LPRO_ONLY Today how we are using CONFIG_CAVS_LPRO is actually using LPRO only, rename to make the naming accurate. Signed-off-by: Keyon Jie --- src/platform/Kconfig | 8 ++++---- src/platform/cannonlake/include/platform/lib/clk.h | 2 +- src/platform/intel/cavs/platform.c | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/platform/Kconfig b/src/platform/Kconfig index bb902b8812fa..3762b90bbf75 100644 --- a/src/platform/Kconfig +++ b/src/platform/Kconfig @@ -296,13 +296,13 @@ config CAVS_LPS This option is required to support S0ix/D0ix mode on cAVS platforms. -config CAVS_LPRO - bool "Enable low power ring oscillator as the default clock" +config CAVS_LPRO_ONLY + bool "Use low power ring oscillator always" default n depends on CAVS && !CAVS_VERSION_1_5 help - Select if you want to use 120MHz LPRO as the DSP clock source. - This option is required to support S0ix/D0ix mode. + Select if you want to use only the 120MHz LPRO as the DSP clock source. + This option is for debugging only at the moment, choose n if unclear. config CAVS_USE_LPRO_IN_WAITI bool diff --git a/src/platform/cannonlake/include/platform/lib/clk.h b/src/platform/cannonlake/include/platform/lib/clk.h index 61fbbd53fa55..1db3d554a9c3 100644 --- a/src/platform/cannonlake/include/platform/lib/clk.h +++ b/src/platform/cannonlake/include/platform/lib/clk.h @@ -20,7 +20,7 @@ #define CPU_HPRO_FREQ_IDX 1 -#if CONFIG_CAVS_LPRO +#if CONFIG_CAVS_LPRO_ONLY #define CPU_DEFAULT_IDX CPU_LPRO_FREQ_IDX #else #define CPU_DEFAULT_IDX CPU_HPRO_FREQ_IDX diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index ca7a4ccc4fc2..a0140edc7678 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -146,7 +146,7 @@ EXT_MAN_PORT( #endif #if CONFIG_CANNONLAKE || CONFIG_ICELAKE || CONFIG_TIGERLAKE -#if CONFIG_CAVS_LPRO +#if CONFIG_CAVS_LPRO_ONLY #define CAVS_DEFAULT_RO SHIM_CLKCTL_RLROSCC #define CAVS_DEFAULT_RO_FOR_MEM SHIM_CLKCTL_OCS_LP_RING #else @@ -293,7 +293,7 @@ int platform_boot_complete(uint32_t boot_message) { uint32_t mb_offset = 0; -#if CONFIG_TIGERLAKE && !CONFIG_CAVS_LPRO +#if CONFIG_TIGERLAKE && !CONFIG_CAVS_LPRO_ONLY cpu_enable_core(PLATFORM_CORE_COUNT - 1); #endif @@ -460,7 +460,7 @@ int platform_init(struct sof *sof) pm_runtime_get(PM_RUNTIME_DSP, PLATFORM_MASTER_CORE_ID); #if CONFIG_DSP_RESIDENCY_COUNTERS -#if CONFIG_CAVS_LPRO +#if CONFIG_CAVS_LPRO_ONLY init_dsp_r_state(r1_r_state); #else init_dsp_r_state(r0_r_state); From 59aa292326885dec864292afed47c7d2a5c66008 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Fri, 17 Jul 2020 17:04:43 +0800 Subject: [PATCH 2/5] config: override: chrome: not enable CONFIG_CAVS_LPRO as it is for debug only The config item CONFIG_CAVS_LPRO (CONFIG_CAVS_LPRO_ONLY now) should be used for debugging only at the moment, remove it from Chrome configs. Signed-off-by: Keyon Jie --- src/arch/xtensa/configs/override/cannonlake_chrome.config | 1 - src/arch/xtensa/configs/override/jasperlake_chrome.config | 1 - src/arch/xtensa/configs/override/tigerlake_chrome.config | 1 - 3 files changed, 3 deletions(-) diff --git a/src/arch/xtensa/configs/override/cannonlake_chrome.config b/src/arch/xtensa/configs/override/cannonlake_chrome.config index 3ac57df516da..79cee681a82b 100644 --- a/src/arch/xtensa/configs/override/cannonlake_chrome.config +++ b/src/arch/xtensa/configs/override/cannonlake_chrome.config @@ -1,2 +1 @@ CONFIG_CAVS_LPS=y -CONFIG_CAVS_LPRO=y diff --git a/src/arch/xtensa/configs/override/jasperlake_chrome.config b/src/arch/xtensa/configs/override/jasperlake_chrome.config index 3ac57df516da..79cee681a82b 100644 --- a/src/arch/xtensa/configs/override/jasperlake_chrome.config +++ b/src/arch/xtensa/configs/override/jasperlake_chrome.config @@ -1,2 +1 @@ CONFIG_CAVS_LPS=y -CONFIG_CAVS_LPRO=y diff --git a/src/arch/xtensa/configs/override/tigerlake_chrome.config b/src/arch/xtensa/configs/override/tigerlake_chrome.config index fedf5820299d..e9c72b979a48 100644 --- a/src/arch/xtensa/configs/override/tigerlake_chrome.config +++ b/src/arch/xtensa/configs/override/tigerlake_chrome.config @@ -1,3 +1,2 @@ CONFIG_CAVS_LPS=y -CONFIG_CAVS_LPRO=y CONFIG_PROBE=y From 813ae3393d847a9a5bf81d75c3770056ec640273 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Mon, 20 Jul 2020 16:24:16 +0800 Subject: [PATCH 3/5] platform: kconfig: only enable CAVS_USE_LPRO_IN_WAITI selection when LPRO_ONLY is not set When LPRO_ONLY is set, the item CAVS_USE_LPRO_IN_WAITI is not configurable, and no clock switching is possible. Change to enable CAVS_USE_LPRO_IN_WAITI selection only when LPRO_ONLY is not set. Signed-off-by: Keyon Jie --- src/platform/Kconfig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/platform/Kconfig b/src/platform/Kconfig index 3762b90bbf75..656e6730e9a1 100644 --- a/src/platform/Kconfig +++ b/src/platform/Kconfig @@ -305,14 +305,14 @@ config CAVS_LPRO_ONLY This option is for debugging only at the moment, choose n if unclear. config CAVS_USE_LPRO_IN_WAITI - bool + bool "Use low power ring oscillator in WFI" default n - depends on CAVS + depends on CAVS && !CAVS_LPRO_ONLY select WAKEUP_HOOK help - Select if platform requires DSP clock source to be switched to LPRO - while in waiti. - After waiti clock source is restored. + Select if we want to use LPRO clock in waiti. + After waiti exit clock source will be restored. + Choose n if unclear. # TODO: it should just take manifest version and offsets config FIRMWARE_SHORT_NAME From ceeababd9d42441393ae21e3fccad6d5bbe6c2ba Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 21 Jul 2020 08:21:02 +0800 Subject: [PATCH 4/5] cAVS: clk: configure default CPU clocks Define CPU_LPRO_FREQ_IDX, CPU_HPRO_FREQ_IDX and CPU_DEFAULT_IDX for all cAVS platforms. Signed-off-by: Keyon Jie --- src/platform/apollolake/include/platform/lib/clk.h | 10 +++++++++- src/platform/icelake/include/platform/lib/clk.h | 4 ++++ src/platform/suecreek/include/platform/lib/clk.h | 4 ++++ src/platform/tigerlake/include/platform/lib/clk.h | 4 ++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/platform/apollolake/include/platform/lib/clk.h b/src/platform/apollolake/include/platform/lib/clk.h index b07098d2722a..e16e942289af 100644 --- a/src/platform/apollolake/include/platform/lib/clk.h +++ b/src/platform/apollolake/include/platform/lib/clk.h @@ -15,7 +15,15 @@ #define CLK_MAX_CPU_HZ 400000000 -#define CPU_DEFAULT_IDX 2 +#define CPU_LPRO_FREQ_IDX 0 + +#define CPU_HPRO_FREQ_IDX 2 + +#if CONFIG_CAVS_LPRO_ONLY +#define CPU_DEFAULT_IDX CPU_LPRO_FREQ_IDX +#else +#define CPU_DEFAULT_IDX CPU_HPRO_FREQ_IDX +#endif #define SSP_DEFAULT_IDX 0 diff --git a/src/platform/icelake/include/platform/lib/clk.h b/src/platform/icelake/include/platform/lib/clk.h index dc13be94b6c1..eefa79c191d6 100644 --- a/src/platform/icelake/include/platform/lib/clk.h +++ b/src/platform/icelake/include/platform/lib/clk.h @@ -20,7 +20,11 @@ #define CPU_HPRO_FREQ_IDX 1 +#if CONFIG_CAVS_LPRO_ONLY +#define CPU_DEFAULT_IDX CPU_LPRO_FREQ_IDX +#else #define CPU_DEFAULT_IDX CPU_HPRO_FREQ_IDX +#endif #define SSP_DEFAULT_IDX 1 diff --git a/src/platform/suecreek/include/platform/lib/clk.h b/src/platform/suecreek/include/platform/lib/clk.h index eb39bb31f549..1db3d554a9c3 100644 --- a/src/platform/suecreek/include/platform/lib/clk.h +++ b/src/platform/suecreek/include/platform/lib/clk.h @@ -20,7 +20,11 @@ #define CPU_HPRO_FREQ_IDX 1 +#if CONFIG_CAVS_LPRO_ONLY +#define CPU_DEFAULT_IDX CPU_LPRO_FREQ_IDX +#else #define CPU_DEFAULT_IDX CPU_HPRO_FREQ_IDX +#endif #define SSP_DEFAULT_IDX 0 diff --git a/src/platform/tigerlake/include/platform/lib/clk.h b/src/platform/tigerlake/include/platform/lib/clk.h index dc13be94b6c1..eefa79c191d6 100644 --- a/src/platform/tigerlake/include/platform/lib/clk.h +++ b/src/platform/tigerlake/include/platform/lib/clk.h @@ -20,7 +20,11 @@ #define CPU_HPRO_FREQ_IDX 1 +#if CONFIG_CAVS_LPRO_ONLY +#define CPU_DEFAULT_IDX CPU_LPRO_FREQ_IDX +#else #define CPU_DEFAULT_IDX CPU_HPRO_FREQ_IDX +#endif #define SSP_DEFAULT_IDX 1 From f47228a9e9d58f65b58c0c8b56253fc30280ce43 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Mon, 20 Jul 2020 17:03:17 +0800 Subject: [PATCH 5/5] cavs: implement PM runtime dynamic clock switching Let's select WAKEUP_HOOK for all cAVS platforms, and just do nothing inside if that's the true. 1. When CONFIG_CAVS_LPRO_ONLY=y, use LPRO only, no clock switching. 2. When CONFIG_CAVS_LPRO_ONLY=n, dynamic clock switching will be enabled, and the clock switching will be performed during platform_wait_for_interrupt() only. In platform_clock_on_waiti(), perform dynamic clock switching if needed (active_freq_idx will be updated to denote the active non-wfi clock frequecy index), and then put CORE_HP_CLK to perform possible HPRO->LPRO switching if CONFIG_CAVS_USE_LPRO_IN_WAITI is selected. In platform_clock_on_wakeup(), when CONFIG_CAVS_USE_LPRO_IN_WAITI is selected, get CORE_HP_CLK to perform switching back to HPRO if HPRO is the saved non-wfi active clock, and no more switching needed. Otherwise, need to restore the active cpu freq_idx manually. Signed-off-by: Keyon Jie --- src/platform/Kconfig | 2 +- .../cavs/include/cavs/drivers/interrupt.h | 2 - .../intel/cavs/include/cavs/lib/clk.h | 2 - src/platform/intel/cavs/lib/clk.c | 149 ++++++++++++++++-- src/platform/intel/cavs/platform.c | 3 +- 5 files changed, 136 insertions(+), 22 deletions(-) diff --git a/src/platform/Kconfig b/src/platform/Kconfig index 656e6730e9a1..737fd2a1abb3 100644 --- a/src/platform/Kconfig +++ b/src/platform/Kconfig @@ -227,6 +227,7 @@ config CAVS select INTERRUPT_LEVEL_5 select INTEL_HDA select INTEL_MN + select WAKEUP_HOOK config CAVS_VERSION_1_5 depends on CAVS @@ -308,7 +309,6 @@ config CAVS_USE_LPRO_IN_WAITI bool "Use low power ring oscillator in WFI" default n depends on CAVS && !CAVS_LPRO_ONLY - select WAKEUP_HOOK help Select if we want to use LPRO clock in waiti. After waiti exit clock source will be restored. diff --git a/src/platform/intel/cavs/include/cavs/drivers/interrupt.h b/src/platform/intel/cavs/include/cavs/drivers/interrupt.h index 82ce1ec4f6f0..b4d0a82e612d 100644 --- a/src/platform/intel/cavs/include/cavs/drivers/interrupt.h +++ b/src/platform/intel/cavs/include/cavs/drivers/interrupt.h @@ -17,12 +17,10 @@ extern const char irq_name_level3[]; extern const char irq_name_level4[]; extern const char irq_name_level5[]; -#if CONFIG_CAVS_USE_LPRO_IN_WAITI static inline void platform_interrupt_on_wakeup(void) { platform_clock_on_wakeup(); } -#endif #endif /* __CAVS_DRIVERS_INTERRUPT_H__ */ diff --git a/src/platform/intel/cavs/include/cavs/lib/clk.h b/src/platform/intel/cavs/include/cavs/lib/clk.h index 459d84847b11..b415530103d0 100644 --- a/src/platform/intel/cavs/include/cavs/lib/clk.h +++ b/src/platform/intel/cavs/include/cavs/lib/clk.h @@ -43,10 +43,8 @@ extern const uint32_t cpu_freq_status_mask[]; void platform_clock_init(struct sof *sof); -#if CONFIG_CAVS_USE_LPRO_IN_WAITI void platform_clock_on_waiti(void); void platform_clock_on_wakeup(void); -#endif #endif /* __CAVS_LIB_CLK_H__ */ diff --git a/src/platform/intel/cavs/lib/clk.c b/src/platform/intel/cavs/lib/clk.c index 173a73478950..e0c51b5f67cc 100644 --- a/src/platform/intel/cavs/lib/clk.c +++ b/src/platform/intel/cavs/lib/clk.c @@ -94,22 +94,46 @@ static inline void select_cpu_clock(int freq_idx, bool release_unused) sizeof(*clk_info) * PLATFORM_CORE_COUNT); } -#if CONFIG_CAVS_USE_LPRO_IN_WAITI -/* Store clock source that was active before going to waiti, - * so it can be restored on wake up. +/* LPRO_ONLY mode */ +#if CONFIG_CAVS_LPRO_ONLY + +/* + * This is call from public API, there is no clock switch for core waiti, so + * value of value frequency index in 'active' state doesn't need to be saved in + * any additional variable and restored in future. */ -static SHARED_DATA int active_freq_idx = CPU_DEFAULT_IDX; +static inline void set_cpu_current_freq_idx(int freq_idx, bool release_unused) +{ + select_cpu_clock(freq_idx, true); +} -static inline int get_current_freq_idx(int clock) +static void platform_clock_low_power_mode(int clock, bool enable) { - struct clock_info *clk_info = clocks_get() + clock; + /* do nothing for LPRO_ONLY mode */ +} - return clk_info->current_freq_idx; +void platform_clock_on_waiti(void) +{ +/* do nothing for LPRO_ONLY mode */ +} + +void platform_clock_on_wakeup(void) +{ +/* do nothing for LPRO_ONLY mode */ } +/* USE_LPRO_IN_WAITI mode */ +#elif CONFIG_CAVS_USE_LPRO_IN_WAITI + +/* Store clock source that was active before going to waiti, + * so it can be restored on wake up. + */ +static SHARED_DATA int active_freq_idx = CPU_DEFAULT_IDX; + /* * This is call from public API, so overwrite currently used frequency index * in 'active' state with new value. This value will be used in each wakeup. + * Caller should hold the pm_runtime lock. */ static inline void set_cpu_current_freq_idx(int freq_idx, bool release_unused) { @@ -119,6 +143,13 @@ static inline void set_cpu_current_freq_idx(int freq_idx, bool release_unused) *uncached_freq_idx = freq_idx; } +static inline int get_current_freq_idx(int clock) +{ + struct clock_info *clk_info = clocks_get() + clock; + + return clk_info->current_freq_idx; +} + static void platform_clock_low_power_mode(int clock, bool enable) { int current_freq_idx = get_current_freq_idx(clock); @@ -135,30 +166,118 @@ static void platform_clock_low_power_mode(int clock, bool enable) select_cpu_clock(freq_idx, false); } -void platform_clock_on_wakeup(void) +void platform_clock_on_waiti(void) { - pm_runtime_get(CORE_HP_CLK, cpu_get_id()); + struct pm_runtime_data *prd = pm_runtime_data_get(); + uint32_t flags; + int freq_idx; + bool pm_is_active; + + /* hold the prd->lock for possible active_freq_idx switching */ + spin_lock_irq(&prd->lock, flags); + + freq_idx = *cache_to_uncache(&active_freq_idx); + pm_is_active = pm_runtime_is_active(PM_RUNTIME_DSP, PLATFORM_MASTER_CORE_ID); + + if (pm_is_active) { + /* set HPRO clock if not already enabled */ + if (freq_idx != CPU_HPRO_FREQ_IDX) + set_cpu_current_freq_idx(CPU_HPRO_FREQ_IDX, true); + } else { + /* set LPRO clock if not already enabled */ + if (freq_idx != CPU_LPRO_FREQ_IDX) + set_cpu_current_freq_idx(CPU_LPRO_FREQ_IDX, true); + } + + spin_unlock_irq(&prd->lock, flags); + + /* check if waiti HPRO->LPRO switching is needed */ + pm_runtime_put(CORE_HP_CLK, cpu_get_id()); } -void platform_clock_on_waiti(void) +void platform_clock_on_wakeup(void) { - pm_runtime_put(CORE_HP_CLK, cpu_get_id()); + /* check if LPRO->HPRO switching back is needed */ + pm_runtime_get(CORE_HP_CLK, cpu_get_id()); } #else + +/* Store clock source that was active before going to waiti, + * so it can be restored on wake up. + */ +static SHARED_DATA int active_freq_idx = CPU_DEFAULT_IDX; + /* - * This is call from public API, there is no clock switch for core waiti, so - * value of value frequency index in 'active' state doesn't need to be saved in - * any additional variable and restored in future. + * This is call from public API, so overwrite currently used frequency index + * in 'active' state with new value. This value will be used in each wakeup. + * Caller should hold the pm_runtime lock. */ static inline void set_cpu_current_freq_idx(int freq_idx, bool release_unused) { - select_cpu_clock(freq_idx, true); + int *uncached_freq_idx = cache_to_uncache(&active_freq_idx); + + select_cpu_clock(freq_idx, release_unused); + *uncached_freq_idx = freq_idx; +} + +static inline int get_current_freq_idx(int clock) +{ + struct clock_info *clk_info = clocks_get() + clock; + + return clk_info->current_freq_idx; } static void platform_clock_low_power_mode(int clock, bool enable) { } + +void platform_clock_on_waiti(void) +{ + struct pm_runtime_data *prd = pm_runtime_data_get(); + uint32_t flags; + int freq_idx; + bool pm_is_active; + + /* hold the prd->lock for possible active_freq_idx switching */ + spin_lock_irq(&prd->lock, flags); + + freq_idx = *cache_to_uncache(&active_freq_idx); + pm_is_active = pm_runtime_is_active(PM_RUNTIME_DSP, PLATFORM_MASTER_CORE_ID); + + if (pm_is_active) { + /* set HPRO clock if not already enabled */ + if (freq_idx != CPU_HPRO_FREQ_IDX) + set_cpu_current_freq_idx(CPU_HPRO_FREQ_IDX, true); + } else { + /* set LPRO clock if not already enabled */ + if (freq_idx != CPU_LPRO_FREQ_IDX) + set_cpu_current_freq_idx(CPU_LPRO_FREQ_IDX, true); + } + + spin_unlock_irq(&prd->lock, flags); +} + +void platform_clock_on_wakeup(void) +{ + struct pm_runtime_data *prd = pm_runtime_data_get(); + uint32_t flags; + int current_idx; + int target_idx; + + /* hold the prd->lock for possible active_freq_idx switching */ + spin_lock_irq(&prd->lock, flags); + + current_idx = get_current_freq_idx(CLK_CPU(cpu_get_id())); + target_idx = *cache_to_uncache(&active_freq_idx); + + /* restore the active cpu freq_idx manually */ + if (current_idx != target_idx) + set_cpu_current_freq_idx(target_idx, true); + + spin_unlock_irq(&prd->lock, flags); +} + #endif static int clock_platform_set_cpu_freq(int clock, int freq_idx) diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index a0140edc7678..120930602037 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -545,9 +545,8 @@ int platform_init(struct sof *sof) #ifndef __ZEPHYR__ void platform_wait_for_interrupt(int level) { -#if CONFIG_CAVS_USE_LPRO_IN_WAITI platform_clock_on_waiti(); -#endif + #if (CONFIG_CAVS_LPS) if (pm_runtime_is_active(PM_RUNTIME_DSP, PLATFORM_MASTER_CORE_ID)) arch_wait_for_interrupt(level);