PM runtime dynamic clock switching implementation

src/arch/xtensa/configs/override/cannonlake_chrome.config

@@ -1,2 +1 @@
 CONFIG_CAVS_LPS=y
-CONFIG_CAVS_LPRO=y

src/arch/xtensa/configs/override/jasperlake_chrome.config

@@ -1,2 +1 @@
 CONFIG_CAVS_LPS=y
-CONFIG_CAVS_LPRO=y

src/arch/xtensa/configs/override/tigerlake_chrome.config

@@ -1,3 +1,2 @@
 CONFIG_CAVS_LPS=y
-CONFIG_CAVS_LPRO=y
 CONFIG_PROBE=y

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
@@ -296,23 +297,22 @@ 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
+	bool "Use low power ring oscillator in WFI"
 	default n
-	depends on CAVS
-	select WAKEUP_HOOK
+	depends on CAVS && !CAVS_LPRO_ONLY
 	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

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
 

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

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
 

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__ */
 

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__ */
 

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)

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);
@@ -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);

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
 

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