Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)

PATCH Applied Dmitry Osipenko 3 participants 28 messages Oct 15, 2019 – Nov 1, 2019

Subsystems (5)

Arm (32-bit) Arm sub-architectures Common clk Tegra architecture support +1 more subsystems

lore

Share & tools

lore

Dmitry Osipenko

author

2019-10-15 16:59 UTC #0 lore raw

v6 cover Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)

Hello,

This series does the following:

1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.

2. Enables CPU cluster power-down idling state on Tegra30.

In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!

Changelog:

v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.

- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.

v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.

- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.

- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.

- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.

v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).

- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.

v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.

Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.

Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.

Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.

Further polished cpuidle driver's code.

The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.

v2: - Added patches to enable the new cpuidle driver in the defconfigs:

ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig

- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:

cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states

- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.

Dmitry Osipenko (18):
ARM: tegra: Compile sleep-tegra20/30.S unconditionally
ARM: tegra: Add tegra_pm_park_secondary_cpu()
ARM: tegra: Remove pen-locking from cpuidle-tegra20
ARM: tegra: Change tegra_set_cpu_in_lp2() type to void
ARM: tegra: Propagate error from tegra_idle_lp2_last()
ARM: tegra: Expose PM functions required for new cpuidle driver
ARM: tegra: Rename some of the newly exposed PM functions
ARM: tegra: Make outer_disable() open-coded
clk: tegra: Add missing stubs for the case of !CONFIG_PM_SLEEP
arm: tegra20: cpuidle: Handle case where secondary CPU hangs on
entering LP2
arm: tegra20: cpuidle: Make abort_flag atomic
arm: tegra20/30: cpuidle: Remove unnecessary memory barrier
cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/
cpuidle: tegra: Squash Tegra30 driver into the common driver
cpuidle: tegra: Support CPU cluster power-down state on Tegra30
cpuidle: tegra: Squash Tegra114 driver into the common driver
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig

 arch/arm/configs/multi_v7_defconfig           |   1 +
 arch/arm/configs/tegra_defconfig              |   1 +
 arch/arm/mach-tegra/Makefile                  |  19 +-
 arch/arm/mach-tegra/cpuidle-tegra114.c        |  89 -----
 arch/arm/mach-tegra/cpuidle-tegra20.c         | 212 -----------
 arch/arm/mach-tegra/cpuidle-tegra30.c         | 132 -------
 arch/arm/mach-tegra/cpuidle.c                 |  50 ---
 arch/arm/mach-tegra/cpuidle.h                 |  21 --
 arch/arm/mach-tegra/irq.c                     |   3 +-
 arch/arm/mach-tegra/pm.c                      |  54 +--
 arch/arm/mach-tegra/pm.h                      |   4 -
 arch/arm/mach-tegra/reset-handler.S           |  11 -
 arch/arm/mach-tegra/reset.h                   |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S           | 170 ---------
 arch/arm/mach-tegra/sleep-tegra30.S           |   6 +-
 arch/arm/mach-tegra/sleep.h                   |  15 -
 arch/arm/mach-tegra/tegra.c                   |   7 +-
 drivers/cpuidle/Kconfig.arm                   |   8 +
 drivers/cpuidle/Makefile                      |   1 +
 drivers/cpuidle/cpuidle-tegra.c               | 357 ++++++++++++++++++
 include/linux/clk/tegra.h                     |  13 +
 include/soc/tegra/cpuidle.h                   |   2 +-
 .../mach-tegra => include/soc/tegra}/irq.h    |   8 +-
 include/soc/tegra/pm.h                        |  31 ++
 24 files changed, 463 insertions(+), 761 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra114.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra20.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra30.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.h
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c
 rename {arch/arm/mach-tegra => include/soc/tegra}/irq.h (59%)

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 16:59 UTC #1 lore raw

PATCH v6 1/18 ARM: tegra: Compile sleep-tegra20/30.S unconditionally

The sleep-tegra*.S provides functionality required for suspend/resume and CPU hotplugging. The new unified CPUIDLE driver will support multiple hardware generations starting from Terga20 and ending with Tegra124, the driver will utilize functions that are provided by the assembly and thus it is cleaner to compile that code without any build-dependencies in order to avoid churning with #ifdef's.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile | 6 ++----
 arch/arm/mach-tegra/sleep.h  | 2 --
 2 files changed, 2 insertions(+), 6 deletions(-)

arch/arm/mach-tegra/Makefile +2 -4

index 6c1dff2eccc2..965862608ff6 100644
@@ -8,13 +8,13 @@ obj-y					+= reset.o
 obj-y					+= reset-handler.o
 obj-y					+= sleep.o
 obj-y					+= tegra.o
+obj-y					+= sleep-tegra20.o
+obj-y					+= sleep-tegra30.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= sleep-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= cpuidle-tegra20.o
 endif
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= cpuidle-tegra30.o

@@ -22,12 +22,10 @@ endif
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
 
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= pm-tegra30.o
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= cpuidle-tegra114.o
 endif
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= pm-tegra30.o
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= cpuidle-tegra114.o

arch/arm/mach-tegra/sleep.h +0 -2

index 78ef32a907c8..63e2205cbc82 100644
@@ -120,10 +120,8 @@ void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
 void tegra_disable_clean_inv_dcache(u32 flag);
 
-#ifdef CONFIG_HOTPLUG_CPU
 void tegra20_hotplug_shutdown(void);
 void tegra30_hotplug_shutdown(void);
-#endif
 
 void tegra20_cpu_shutdown(int cpu);
 int tegra20_cpu_is_resettable_soon(void);

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 16:59 UTC #2 lore raw

PATCH v6 2/18 ARM: tegra: Add tegra_pm_park_secondary_cpu()

In reply to Dmitry Osipenko

This function resembles tegra_cpu_die() of the hotplug code, but this variant is more suitable to be used for CPU PM because it's made specifically to be used by cpu_suspend(). In short this function puts secondary CPU offline, it will be used by the new CPUIDLE driver.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/pm.c | 14 ++++++++++++++
 arch/arm/mach-tegra/pm.h |  5 +++++
 2 files changed, 19 insertions(+)

arch/arm/mach-tegra/pm.c +14 -0

index 3cab81b82866..f5ff3dd1dd81 100644
@@ -436,4 +436,18 @@ void __init tegra_init_suspend(void)
 
 	suspend_set_ops(&tegra_suspend_ops);
 }
+
+int tegra_pm_park_secondary_cpu(unsigned long cpu)
+{
+	if (cpu > 0) {
+		tegra_disable_clean_inv_dcache(TEGRA_FLUSH_CACHE_LOUIS);
+
+		if (tegra_get_chip_id() == TEGRA20)
+			tegra20_hotplug_shutdown();
+		else
+			tegra30_hotplug_shutdown();
+	}
+
+	return -EINVAL;
+}
 #endif

arch/arm/mach-tegra/pm.h +5 -0

index 569151b3edc0..9a790f00237f 100644
@@ -31,8 +31,13 @@ extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP
 void tegra_init_suspend(void);
+int tegra_pm_park_secondary_cpu(unsigned long cpu);
 #else
 static inline void tegra_init_suspend(void) {}
+static inline int tegra_pm_park_secondary_cpu(unsigned long cpu)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #endif /* _MACH_TEGRA_PM_H_ */

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #3 lore raw

PATCH v6 3/18 ARM: tegra: Remove pen-locking from cpuidle-tegra20

In reply to Dmitry Osipenko

Pen-locking is meant to block CPU0 if CPU1 wakes up during of entering into LP2 because of some interrupt firing up, preventing unnecessary LP2 enter that will be resumed immediately. Apparently this case doesn't happen often in practice, I checked how often it takes place and found that after ~20 hours of browsing web, managing email, watching videos and idling (15+ hours) there is only a dozen of early LP2 entering abortions and they all happened while device was idling. Thus let's remove the pen-locking and make LP2 entering uninterruptible, simplifying code quite a lot. This will also become very handy for the upcoming unified cpuidle driver, allowing to have a common LP2 code-path across of different hardware generations.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c |  46 +------
 arch/arm/mach-tegra/pm.c              |   7 --
 arch/arm/mach-tegra/pm.h              |   1 -
 arch/arm/mach-tegra/reset-handler.S   |  11 --
 arch/arm/mach-tegra/reset.h           |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S   | 170 --------------------------
 arch/arm/mach-tegra/sleep.h           |  12 --
 7 files changed, 5 insertions(+), 251 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +3 ~~-43~~

index 2447427cb4a8..4da5d72ae030 100644
@@ -15,6 +15,7 @@
 #include <linux/cpuidle.h>
 #include <linux/cpu_pm.h>
 #include <linux/kernel.h>
+#include <linux/ktime.h>
 #include <linux/module.h>
 
 #include <soc/tegra/flowctrl.h>

@@ -65,28 +66,8 @@ static struct cpuidle_driver tegra_idle_driver = {
 
 #ifdef CONFIG_PM_SLEEP
 #ifdef CONFIG_SMP
-static int tegra20_reset_sleeping_cpu_1(void)
-{
-	int ret = 0;
-
-	tegra_pen_lock();
-
-	if (readb(tegra20_cpu1_resettable_status) == CPU_RESETTABLE)
-		tegra20_cpu_shutdown(1);
-	else
-		ret = -EINVAL;
-
-	tegra_pen_unlock();
-
-	return ret;
-}
-
 static void tegra20_wake_cpu1_from_reset(void)
 {
-	tegra_pen_lock();
-
-	tegra20_cpu_clear_resettable();
-
 	/* enable cpu clock on cpu */
 	tegra_enable_cpu_clock(1);

@@ -95,39 +76,20 @@ static void tegra20_wake_cpu1_from_reset(void)
 
 	/* unhalt the cpu */
 	flowctrl_write_cpu_halt(1, 0);
-
-	tegra_pen_unlock();
-}
-
-static int tegra20_reset_cpu_1(void)
-{
-	if (!cpu_online(1) || !tegra20_reset_sleeping_cpu_1())
-		return 0;
-
-	tegra20_wake_cpu1_from_reset();
-	return -EBUSY;
 }
 #else
 static inline void tegra20_wake_cpu1_from_reset(void)
 {
 }
-
-static inline int tegra20_reset_cpu_1(void)
-{
-	return 0;
-}
 #endif
 
 static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
 					   struct cpuidle_driver *drv,
 					   int index)
 {
-	while (tegra20_cpu_is_resettable_soon())
+	while (!tegra_cpu_rail_off_ready())
 		cpu_relax();
 
-	if (tegra20_reset_cpu_1() || !tegra_cpu_rail_off_ready())
-		return false;
-
 	tegra_idle_lp2_last();
 
 	if (cpu_online(1))

@@ -141,9 +103,7 @@ static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 					 struct cpuidle_driver *drv,
 					 int index)
 {
-	cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
-
-	tegra20_cpu_clear_resettable();
+	cpu_suspend(dev->cpu, tegra_pm_park_secondary_cpu);
 
 	return true;
 }

arch/arm/mach-tegra/pm.c +0 -7

index f5ff3dd1dd81..1ff499068bb1 100644
@@ -137,18 +137,11 @@ bool tegra_set_cpu_in_lp2(void)
 
 	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
 		last_cpu = true;
-	else if (tegra_get_chip_id() == TEGRA20 && phy_cpu_id == 1)
-		tegra20_cpu_set_resettable_soon();
 
 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;
 }
 
-int tegra_cpu_do_idle(void)
-{
-	return cpu_do_idle();
-}
-
 static int tegra_sleep_cpu(unsigned long v2p)
 {
 	/*

arch/arm/mach-tegra/pm.h +0 -1

index 9a790f00237f..b9cc12222bb1 100644
@@ -25,7 +25,6 @@ void tegra30_sleep_core_init(void);
 
 void tegra_clear_cpu_in_lp2(void);
 bool tegra_set_cpu_in_lp2(void);
-int tegra_cpu_do_idle(void);
 void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);

arch/arm/mach-tegra/reset-handler.S +0 ~~-11~~

index 67b763fea005..df44828a34d3 100644
@@ -183,17 +183,6 @@ after_errata:
 	bleq	__die				@ CPU not present (to OS)
 #endif
 
-#ifdef CONFIG_ARCH_TEGRA_2x_SOC
-	/* Are we on Tegra20? */
-	cmp	r6, #TEGRA20
-	bne	1f
-	/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
-	mov	r0, #CPU_NOT_RESETTABLE
-	cmp	r10, #0
-	strbne	r0, [r12, #RESET_DATA(RESETTABLE_STATUS)]
-1:
-#endif
-
 	/* Waking up from LP1? */
 	ldr	r8, [r12, #RESET_DATA(MASK_LP1)]
 	tst	r8, r11				@ if in_lp1

arch/arm/mach-tegra/reset.h +2 -7

index a4cfc08159f6..51265592cb1a 100644
@@ -16,9 +16,8 @@
 #define TEGRA_RESET_STARTUP_SECONDARY	3
 #define TEGRA_RESET_STARTUP_LP2		4
 #define TEGRA_RESET_STARTUP_LP1		5
-#define TEGRA_RESET_RESETTABLE_STATUS	6
-#define TEGRA_RESET_TF_PRESENT		7
-#define TEGRA_RESET_DATA_SIZE		8
+#define TEGRA_RESET_TF_PRESENT		6
+#define TEGRA_RESET_DATA_SIZE		7
 
 #define RESET_DATA(x)	((TEGRA_RESET_##x)*4)

@@ -42,10 +41,6 @@ void __tegra_cpu_reset_handler_end(void);
 	(IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
 	((u32)&__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_LP2] - \
 	 (u32)__tegra_cpu_reset_handler_start)))
-#define tegra20_cpu1_resettable_status \
-	(IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
-	((u32)&__tegra_cpu_reset_handler_data[TEGRA_RESET_RESETTABLE_STATUS] - \
-	 (u32)__tegra_cpu_reset_handler_start)))
 #endif
 
 #define tegra_cpu_reset_handler_offset \

arch/arm/mach-tegra/sleep-tegra20.S +0 ~~-170~~

194 lines not on this page · raw message

arch/arm/mach-tegra/sleep.h +0 ~~-12~~

index 63e2205cbc82..4978def9db46 100644
@@ -114,8 +114,6 @@
 .endm
 
 #else
-void tegra_pen_lock(void);
-void tegra_pen_unlock(void);
 void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
 void tegra_disable_clean_inv_dcache(u32 flag);

@@ -123,16 +121,6 @@ void tegra_disable_clean_inv_dcache(u32 flag);
 void tegra20_hotplug_shutdown(void);
 void tegra30_hotplug_shutdown(void);
 
-void tegra20_cpu_shutdown(int cpu);
-int tegra20_cpu_is_resettable_soon(void);
-void tegra20_cpu_clear_resettable(void);
-#ifdef CONFIG_ARCH_TEGRA_2x_SOC
-void tegra20_cpu_set_resettable_soon(void);
-#else
-static inline void tegra20_cpu_set_resettable_soon(void) {}
-#endif
-
-int tegra20_sleep_cpu_secondary_finish(unsigned long);
 void tegra20_tear_down_cpu(void);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #4 lore raw

PATCH v6 4/18 ARM: tegra: Change tegra_set_cpu_in_lp2() type to void

In reply to Dmitry Osipenko

The Tegra30 CPUIDLE driver has intention to check whether primary CPU was the last CPU that entered LP2 (CC6) idle-state, but that functionality never got utilized because driver never supported the CC6 state for the case where any secondary CPU is online. The new cpuidle driver will properly support CC6 on Tegra30, including the case where secondary CPUs are online, and that knowledge about what CPUs entered into CC6 won't be needed at all because new driver will use different approach by making use of the coupled idle-state and explicitly parking secondary CPUs before entering into CC6. Thus this patch is just a minor cleanup change.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra30.c | 14 ++++----------
 arch/arm/mach-tegra/pm.c              |  8 +-------
 arch/arm/mach-tegra/pm.h              |  2 +-
 3 files changed, 6 insertions(+), 18 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra30.c +4 ~~-10~~

index c6128526877d..a3ce8dabfe18 100644
@@ -98,22 +98,16 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 			    int index)
 {
 	bool entered_lp2 = false;
-	bool last_cpu;
 
 	local_fiq_disable();
 
-	last_cpu = tegra_set_cpu_in_lp2();
+	tegra_set_cpu_in_lp2();
 	cpu_pm_enter();
 
-	if (dev->cpu == 0) {
-		if (last_cpu)
-			entered_lp2 = tegra30_cpu_cluster_power_down(dev, drv,
-								     index);
-		else
-			cpu_do_idle();
-	} else {
+	if (dev->cpu == 0)
+		entered_lp2 = tegra30_cpu_cluster_power_down(dev, drv, index);
+	else
 		entered_lp2 = tegra30_cpu_core_power_down(dev, drv, index);
-	}
 
 	cpu_pm_exit();
 	tegra_clear_cpu_in_lp2();

arch/arm/mach-tegra/pm.c +1 -7

index 1ff499068bb1..a72f9a2d3cb7 100644
@@ -123,11 +123,9 @@ void tegra_clear_cpu_in_lp2(void)
 	spin_unlock(&tegra_lp2_lock);
 }
 
-bool tegra_set_cpu_in_lp2(void)
+void tegra_set_cpu_in_lp2(void)
 {
 	int phy_cpu_id = cpu_logical_map(smp_processor_id());
-	bool last_cpu = false;
-	cpumask_t *cpu_lp2_mask = tegra_cpu_lp2_mask;
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;
 
 	spin_lock(&tegra_lp2_lock);

@@ -135,11 +133,7 @@ bool tegra_set_cpu_in_lp2(void)
 	BUG_ON((*cpu_in_lp2 & BIT(phy_cpu_id)));
 	*cpu_in_lp2 |= BIT(phy_cpu_id);
 
-	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
-		last_cpu = true;
-
 	spin_unlock(&tegra_lp2_lock);
-	return last_cpu;
 }
 
 static int tegra_sleep_cpu(unsigned long v2p)

arch/arm/mach-tegra/pm.h +1 -1

index b9cc12222bb1..2c294f6365c0 100644
@@ -24,7 +24,7 @@ void tegra30_lp1_iram_hook(void);
 void tegra30_sleep_core_init(void);
 
 void tegra_clear_cpu_in_lp2(void);
-bool tegra_set_cpu_in_lp2(void);
+void tegra_set_cpu_in_lp2(void);
 void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #5 lore raw

PATCH v6 5/18 ARM: tegra: Propagate error from tegra_idle_lp2_last()

In reply to Dmitry Osipenko

Technically cpu_suspend() may fail and it's never good to lose information about failure. For example things like cpuidle core could correctly sample idling time in the case of failure.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 6 ++++--
 arch/arm/mach-tegra/cpuidle-tegra30.c | 4 +---
 arch/arm/mach-tegra/pm.c              | 8 ++++++--
 arch/arm/mach-tegra/pm.h              | 2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +4 -2

index 4da5d72ae030..74ba37992259 100644
@@ -87,15 +87,17 @@ static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
 					   struct cpuidle_driver *drv,
 					   int index)
 {
+	bool ret;
+
 	while (!tegra_cpu_rail_off_ready())
 		cpu_relax();
 
-	tegra_idle_lp2_last();
+	ret = !tegra_idle_lp2_last();
 
 	if (cpu_online(1))
 		tegra20_wake_cpu1_from_reset();
 
-	return true;
+	return ret;
 }
 
 #ifdef CONFIG_SMP

arch/arm/mach-tegra/cpuidle-tegra30.c +1 -3

index a3ce8dabfe18..17cbd118abee 100644
@@ -68,9 +68,7 @@ static bool tegra30_cpu_cluster_power_down(struct cpuidle_device *dev,
 		return false;
 	}
 
-	tegra_idle_lp2_last();
-
-	return true;
+	return !tegra_idle_lp2_last();
 }
 
 #ifdef CONFIG_SMP

arch/arm/mach-tegra/pm.c +6 -2

index a72f9a2d3cb7..a094acaca307 100644
@@ -189,14 +189,16 @@ static void tegra_pm_set(enum tegra_suspend_mode mode)
 	tegra_pmc_enter_suspend_mode(mode);
 }
 
-void tegra_idle_lp2_last(void)
+int tegra_idle_lp2_last(void)
 {
+	int err;
+
 	tegra_pm_set(TEGRA_SUSPEND_LP2);
 
 	cpu_cluster_pm_enter();
 	suspend_cpu_complex();
 
-	cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
+	err = cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
 
 	/*
 	 * Resume L2 cache if it wasn't re-enabled early during resume,

@@ -208,6 +210,8 @@ void tegra_idle_lp2_last(void)
 
 	restore_cpu_complex();
 	cpu_cluster_pm_exit();
+
+	return err;
 }
 
 enum tegra_suspend_mode tegra_pm_validate_suspend_mode(

arch/arm/mach-tegra/pm.h +1 -1

index 2c294f6365c0..7d72f31dee77 100644
@@ -25,7 +25,7 @@ void tegra30_sleep_core_init(void);
 
 void tegra_clear_cpu_in_lp2(void);
 void tegra_set_cpu_in_lp2(void);
-void tegra_idle_lp2_last(void);
+int tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #6 lore raw

PATCH v6 6/18 ARM: tegra: Expose PM functions required for new cpuidle driver

In reply to Dmitry Osipenko

The upcoming unified CPUIDLE driver will be added to the drivers/cpuidle/ directory and it will require all these exposed Tegra PM-core functions.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra114.c        |  3 +-
 arch/arm/mach-tegra/cpuidle-tegra20.c         |  2 +-
 arch/arm/mach-tegra/cpuidle-tegra30.c         |  3 +-
 arch/arm/mach-tegra/irq.c                     |  3 +-
 arch/arm/mach-tegra/pm.h                      |  8 -----
 arch/arm/mach-tegra/sleep.h                   |  1 -
 arch/arm/mach-tegra/tegra.c                   |  1 -
 .../mach-tegra => include/soc/tegra}/irq.h    |  8 +++--
 include/soc/tegra/pm.h                        | 31 +++++++++++++++++++
 9 files changed, 43 insertions(+), 17 deletions(-)
 rename {arch/arm/mach-tegra => include/soc/tegra}/irq.h (59%)

arch/arm/mach-tegra/cpuidle-tegra114.c +2 -1

index 5118f777fd66..2d8527837aeb 100644
@@ -12,13 +12,14 @@
 
 #include <linux/firmware/trusted_foundations.h>
 
+#include <soc/tegra/pm.h>
+
 #include <asm/cpuidle.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 #include <asm/psci.h>
 
 #include "cpuidle.h"
-#include "pm.h"
 #include "sleep.h"
 
 #ifdef CONFIG_PM_SLEEP

arch/arm/mach-tegra/cpuidle-tegra20.c +1 -1

index 74ba37992259..7de2151a2e0c 100644
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 
 #include <soc/tegra/flowctrl.h>
+#include <soc/tegra/pm.h>
 
 #include <asm/cpuidle.h>
 #include <asm/smp_plat.h>

@@ -27,7 +28,6 @@
 #include "cpuidle.h"
 #include "iomap.h"
 #include "irq.h"
-#include "pm.h"
 #include "reset.h"
 #include "sleep.h"

arch/arm/mach-tegra/cpuidle-tegra30.c +2 -1

index 17cbd118abee..3e91c29891f7 100644
@@ -17,12 +17,13 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 
+#include <soc/tegra/pm.h>
+
 #include <asm/cpuidle.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 
 #include "cpuidle.h"
-#include "pm.h"
 #include "sleep.h"
 
 #ifdef CONFIG_PM_SLEEP

arch/arm/mach-tegra/irq.c +2 -1

index ace7a390b5fe..4e1ee70b2a3f 100644
@@ -18,9 +18,10 @@
 #include <linux/of.h>
 #include <linux/syscore_ops.h>
 
+#include <soc/tegra/irq.h>
+
 #include "board.h"
 #include "iomap.h"
-#include "irq.h"
 
 #define SGI_MASK 0xFFFF

arch/arm/mach-tegra/pm.h +0 -8

index 7d72f31dee77..81525f5f4a44 100644
@@ -23,20 +23,12 @@ void tegra20_sleep_core_init(void);
 void tegra30_lp1_iram_hook(void);
 void tegra30_sleep_core_init(void);
 
-void tegra_clear_cpu_in_lp2(void);
-void tegra_set_cpu_in_lp2(void);
-int tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP
 void tegra_init_suspend(void);
-int tegra_pm_park_secondary_cpu(unsigned long cpu);
 #else
 static inline void tegra_init_suspend(void) {}
-static inline int tegra_pm_park_secondary_cpu(unsigned long cpu)
-{
-	return -ENOTSUPP;
-}
 #endif
 
 #endif /* _MACH_TEGRA_PM_H_ */

arch/arm/mach-tegra/sleep.h +0 -1

index 4978def9db46..4718a3cb45a1 100644
@@ -122,7 +122,6 @@ void tegra20_hotplug_shutdown(void);
 void tegra30_hotplug_shutdown(void);
 
 void tegra20_tear_down_cpu(void);
-int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
 #endif

arch/arm/mach-tegra/tegra.c +0 -1

index e512e606eabd..00aaf495bbf7 100644
@@ -42,7 +42,6 @@
 #include "common.h"
 #include "cpuidle.h"
 #include "iomap.h"
-#include "irq.h"
 #include "pm.h"
 #include "reset.h"
 #include "sleep.h"

include/soc/tegra/irq.h renamed from arch/arm/mach-tegra/irq.h +5 -3

similarity index 59%
rename from arch/arm/mach-tegra/irq.h
rename to include/soc/tegra/irq.h
index 7a94cf121448..8eb11a7109e4 100644
@@ -3,9 +3,11 @@
  * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
  */
 
-#ifndef __TEGRA_IRQ_H
-#define __TEGRA_IRQ_H
+#ifndef __SOC_TEGRA_IRQ_H
+#define __SOC_TEGRA_IRQ_H
 
+#if defined(CONFIG_ARM)
 bool tegra_pending_sgi(void);
-
 #endif
+
+#endif /* __SOC_TEGRA_IRQ_H */

include/soc/tegra/pm.h +31 -0

index 951fcd738d55..1974e8405098 100644
@@ -6,6 +6,8 @@
 #ifndef __SOC_TEGRA_PM_H__
 #define __SOC_TEGRA_PM_H__
 
+#include <linux/errno.h>
+
 enum tegra_suspend_mode {
 	TEGRA_SUSPEND_NONE = 0,
 	TEGRA_SUSPEND_LP2, /* CPU voltage off */

@@ -20,6 +22,12 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode);
 
 /* low-level resume entry point */
 void tegra_resume(void);
+
+int tegra30_sleep_cpu_secondary_finish(unsigned long arg);
+void tegra_clear_cpu_in_lp2(void);
+void tegra_set_cpu_in_lp2(void);
+int tegra_idle_lp2_last(void);
+int tegra_pm_park_secondary_cpu(unsigned long cpu);
 #else
 static inline enum tegra_suspend_mode
 tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode)

@@ -30,6 +38,29 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode)
 static inline void tegra_resume(void)
 {
 }
+
+static inline int tegra30_sleep_cpu_secondary_finish(unsigned long arg)
+{
+	return -ENOTSUPP;
+}
+
+static inline void tegra_clear_cpu_in_lp2(void)
+{
+}
+
+static inline void tegra_set_cpu_in_lp2(void)
+{
+}
+
+static inline int tegra_idle_lp2_last(void)
+{
+	return -ENOTSUPP;
+}
+
+static inline int tegra_pm_park_secondary_cpu(unsigned long cpu)
+{
+	return -ENOTSUPP;
+}
 #endif /* CONFIG_PM_SLEEP */
 
 #endif /* __SOC_TEGRA_PM_H__ */

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #7 lore raw

PATCH v6 7/18 ARM: tegra: Rename some of the newly exposed PM functions

In reply to Dmitry Osipenko

Rename some of the recently exposed PM functions, prefixing them with "tegra_pm_" in order to make the naming of the PM functions consistent.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra114.c |  6 +++---
 arch/arm/mach-tegra/cpuidle-tegra20.c  |  6 +++---
 arch/arm/mach-tegra/cpuidle-tegra30.c  |  8 ++++----
 arch/arm/mach-tegra/pm.c               | 10 +++++-----
 arch/arm/mach-tegra/sleep-tegra30.S    |  6 +++---
 include/soc/tegra/pm.h                 | 16 ++++++++--------
 6 files changed, 26 insertions(+), 26 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra114.c +3 -3

index 2d8527837aeb..858c30cc5dc7 100644
@@ -35,17 +35,17 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
 {
 	local_fiq_disable();
 
-	tegra_set_cpu_in_lp2();
+	tegra_pm_set_cpu_in_lp2();
 	cpu_pm_enter();
 
 	call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
 
 	/* Do suspend by ourselves if the firmware does not implement it */
 	if (call_firmware_op(do_idle, 0) == -ENOSYS)
-		cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+		cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 
 	cpu_pm_exit();
-	tegra_clear_cpu_in_lp2();
+	tegra_pm_clear_cpu_in_lp2();
 
 	local_fiq_enable();

arch/arm/mach-tegra/cpuidle-tegra20.c +3 -3

index 7de2151a2e0c..77702dd42479 100644
@@ -92,7 +92,7 @@ static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
 	while (!tegra_cpu_rail_off_ready())
 		cpu_relax();
 
-	ret = !tegra_idle_lp2_last();
+	ret = !tegra_pm_enter_lp2();
 
 	if (cpu_online(1))
 		tegra20_wake_cpu1_from_reset();

@@ -137,7 +137,7 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 
 	local_fiq_disable();
 
-	tegra_set_cpu_in_lp2();
+	tegra_pm_set_cpu_in_lp2();
 	cpu_pm_enter();
 
 	if (dev->cpu == 0)

@@ -146,7 +146,7 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
 
 	cpu_pm_exit();
-	tegra_clear_cpu_in_lp2();
+	tegra_pm_clear_cpu_in_lp2();
 
 	local_fiq_enable();

arch/arm/mach-tegra/cpuidle-tegra30.c +4 -4

index 3e91c29891f7..a4f0add46a27 100644
@@ -69,7 +69,7 @@ static bool tegra30_cpu_cluster_power_down(struct cpuidle_device *dev,
 		return false;
 	}
 
-	return !tegra_idle_lp2_last();
+	return !tegra_pm_enter_lp2();
 }
 
 #ifdef CONFIG_SMP

@@ -79,7 +79,7 @@ static bool tegra30_cpu_core_power_down(struct cpuidle_device *dev,
 {
 	smp_wmb();
 
-	cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+	cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 
 	return true;
 }

@@ -100,7 +100,7 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 
 	local_fiq_disable();
 
-	tegra_set_cpu_in_lp2();
+	tegra_pm_set_cpu_in_lp2();
 	cpu_pm_enter();
 
 	if (dev->cpu == 0)

@@ -109,7 +109,7 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 		entered_lp2 = tegra30_cpu_core_power_down(dev, drv, index);
 
 	cpu_pm_exit();
-	tegra_clear_cpu_in_lp2();
+	tegra_pm_clear_cpu_in_lp2();
 
 	local_fiq_enable();

arch/arm/mach-tegra/pm.c +5 -5

index a094acaca307..7d9ef26e52a7 100644
@@ -110,7 +110,7 @@ static void suspend_cpu_complex(void)
 	flowctrl_cpu_suspend_enter(cpu);
 }
 
-void tegra_clear_cpu_in_lp2(void)
+void tegra_pm_clear_cpu_in_lp2(void)
 {
 	int phy_cpu_id = cpu_logical_map(smp_processor_id());
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;

@@ -123,7 +123,7 @@ void tegra_clear_cpu_in_lp2(void)
 	spin_unlock(&tegra_lp2_lock);
 }
 
-void tegra_set_cpu_in_lp2(void)
+void tegra_pm_set_cpu_in_lp2(void)
 {
 	int phy_cpu_id = cpu_logical_map(smp_processor_id());
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;

@@ -189,7 +189,7 @@ static void tegra_pm_set(enum tegra_suspend_mode mode)
 	tegra_pmc_enter_suspend_mode(mode);
 }
 
-int tegra_idle_lp2_last(void)
+int tegra_pm_enter_lp2(void)
 {
 	int err;

@@ -356,7 +356,7 @@ static int tegra_suspend_enter(suspend_state_t state)
 		tegra_suspend_enter_lp1();
 		break;
 	case TEGRA_SUSPEND_LP2:
-		tegra_set_cpu_in_lp2();
+		tegra_pm_set_cpu_in_lp2();
 		break;
 	default:
 		break;

@@ -377,7 +377,7 @@ static int tegra_suspend_enter(suspend_state_t state)
 		tegra_suspend_exit_lp1();
 		break;
 	case TEGRA_SUSPEND_LP2:
-		tegra_clear_cpu_in_lp2();
+		tegra_pm_clear_cpu_in_lp2();
 		break;
 	default:
 		break;

arch/arm/mach-tegra/sleep-tegra30.S +3 -3

index b408fa56eb89..386319a3d2d2 100644
@@ -262,11 +262,11 @@ ENTRY(tegra30_sleep_core_finish)
 ENDPROC(tegra30_sleep_core_finish)
 
 /*
- * tegra30_sleep_cpu_secondary_finish(unsigned long v2p)
+ * tegra30_pm_secondary_cpu_suspend(unsigned long unused_arg)
  *
  * Enters LP2 on secondary CPU by exiting coherency and powergating the CPU.
  */
-ENTRY(tegra30_sleep_cpu_secondary_finish)
+ENTRY(tegra30_pm_secondary_cpu_suspend)
 	mov	r7, lr
 
 	/* Flush and disable the L1 data cache */

@@ -278,7 +278,7 @@ ENTRY(tegra30_sleep_cpu_secondary_finish)
 	bl	tegra30_cpu_shutdown
 	mov	r0, #1                          @ never return here
 	ret	r7
-ENDPROC(tegra30_sleep_cpu_secondary_finish)
+ENDPROC(tegra30_pm_secondary_cpu_suspend)
 
 /*
  * tegra30_tear_down_cpu

include/soc/tegra/pm.h +8 -8

index 1974e8405098..08477d7bfab9 100644
@@ -23,10 +23,10 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode);
 /* low-level resume entry point */
 void tegra_resume(void);
 
-int tegra30_sleep_cpu_secondary_finish(unsigned long arg);
-void tegra_clear_cpu_in_lp2(void);
-void tegra_set_cpu_in_lp2(void);
-int tegra_idle_lp2_last(void);
+int tegra30_pm_secondary_cpu_suspend(unsigned long arg);
+void tegra_pm_clear_cpu_in_lp2(void);
+void tegra_pm_set_cpu_in_lp2(void);
+int tegra_pm_enter_lp2(void);
 int tegra_pm_park_secondary_cpu(unsigned long cpu);
 #else
 static inline enum tegra_suspend_mode

@@ -39,20 +39,20 @@ static inline void tegra_resume(void)
 {
 }
 
-static inline int tegra30_sleep_cpu_secondary_finish(unsigned long arg)
+static inline int tegra30_pm_secondary_cpu_suspend(unsigned long arg)
 {
 	return -ENOTSUPP;
 }
 
-static inline void tegra_clear_cpu_in_lp2(void)
+static inline void tegra_pm_clear_cpu_in_lp2(void)
 {
 }
 
-static inline void tegra_set_cpu_in_lp2(void)
+static inline void tegra_pm_set_cpu_in_lp2(void)
 {
 }
 
-static inline int tegra_idle_lp2_last(void)
+static inline int tegra_pm_enter_lp2(void)
 {
 	return -ENOTSUPP;
 }

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #8 lore raw

PATCH v6 8/18 ARM: tegra: Make outer_disable() open-coded

In reply to Dmitry Osipenko

The outer_disable() of Tegra's suspend code is open-coded now since that helper produces spurious warning message about secondary CPUs being online when CPU enters into LP2 from cpuidle. The secondaries are actually halted by the cpuidle driver on entering into LP2 idle-state, but the online status is not touched by the cpuidle. This fixes a storm of warnings once LP2 idling state is enabled on Tegra30. The outer_disable() helper has sanity checks for interrupts and secondary CPUs being disabled and we are pretty confident about the interrupts state during of CPU idling / system suspend. The rail-off status check is added in this patch as equivalent for the "num_online_cpus() > 1".

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/pm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

arch/arm/mach-tegra/pm.c +8 -3

index 7d9ef26e52a7..d1e1a61b12cf 100644
@@ -138,6 +138,10 @@ void tegra_pm_set_cpu_in_lp2(void)
 
 static int tegra_sleep_cpu(unsigned long v2p)
 {
+	if (tegra_cpu_car_ops->rail_off_ready &&
+	    WARN_ON(!tegra_cpu_rail_off_ready()))
+		return -EBUSY;
+
 	/*
 	 * L2 cache disabling using kernel API only allowed when all
 	 * secondary CPU's are offline. Cache have to be disabled with

@@ -146,9 +150,10 @@ static int tegra_sleep_cpu(unsigned long v2p)
 	 * if any of secondary CPU's is online and this is the LP2-idle
 	 * code-path only for Tegra20/30.
 	 */
-	if (trusted_foundations_registered())
-		outer_disable();
-
+#ifdef CONFIG_OUTER_CACHE
+	if (trusted_foundations_registered() && outer_cache.disable)
+		outer_cache.disable();
+#endif
 	/*
 	 * Note that besides of setting up CPU reset vector this firmware
 	 * call may also do the following, depending on the FW version:

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #9 lore raw

PATCH v6 9/18 clk: tegra: Add missing stubs for the case of !CONFIG_PM_SLEEP

In reply to Dmitry Osipenko

The new CPUIDLE driver uses the Tegra's CLK API and that driver won't strictly depend on CONFIG_PM_SLEEP, hence add the required stubs in order to allow compiling of the new driver with the CONFIG_PM_SLEEP=n.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 include/linux/clk/tegra.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

include/linux/clk/tegra.h +13 -0

index 6a7cbc3cfadc..2b1b35240074 100644
@@ -108,6 +108,19 @@ static inline void tegra_cpu_clock_resume(void)
 
 	tegra_cpu_car_ops->resume();
 }
+#else
+static inline bool tegra_cpu_rail_off_ready(void)
+{
+	return false;
+}
+
+static inline void tegra_cpu_clock_suspend(void)
+{
+}
+
+static inline void tegra_cpu_clock_resume(void)
+{
+}
 #endif
 
 extern void tegra210_xusb_pll_hw_control_enable(void);

Signature · 1 line

2.23.0

Thierry Reding

Applied

2019-10-29 13:19 UTC #10 lore raw

Re: [PATCH v6 09/18] clk: tegra: Add missing stubs for the case of !CONFIG_PM_SLEEP

Quoted 8 lines · Dmitry Osipenko

The new CPUIDLE driver uses the Tegra's CLK API and that driver won't strictly depend on CONFIG_PM_SLEEP, hence add the required stubs in order to allow compiling of the new driver with the CONFIG_PM_SLEEP=n.
Signed-off-by Dmitry Osipenko <[email protected]>
---
 include/linux/clk/tegra.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Applied to for-5.5/clk, thanks.

Thierry

Dmitry Osipenko

author

2019-10-15 17:00 UTC #11 lore raw

PATCH v6 10/18 arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2

In reply to Dmitry Osipenko

It is possible that something may go wrong with the secondary CPU, in that case it is much nicer to get a dump of the flow-controller state before hanging machine.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 46 +++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +44 -2

index 77702dd42479..bc5873e92af5 100644
@@ -83,14 +83,56 @@ static inline void tegra20_wake_cpu1_from_reset(void)
 }
 #endif
 
+static void tegra20_report_cpus_state(void)
+{
+	unsigned int cpu, lcpu;
+
+	pr_err("secondary CPU taking too long to park\n");
+
+	for_each_cpu(lcpu, cpu_possible_mask) {
+		cpu = cpu_logical_map(lcpu);
+
+		pr_err("cpu%u: online=%d flowctrl_csr=0x%08x\n",
+		       cpu, cpu_online(lcpu), flowctrl_read_cpu_csr(cpu));
+	}
+}
+
+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);
+
+		/*
+		 * The primary CPU0 core shall wait for the secondaries
+		 * shutdown in order to power-off CPU's cluster safely.
+		 * The timeout value depends on the current CPU frequency,
+		 * it takes about 40-150us  in average and over 1000us in
+		 * a worst case scenario.
+		 */
+		do {
+			if (tegra_cpu_rail_off_ready())
+				return 0;
+
+		} while (ktime_before(ktime_get(), timeout));
+
+		tegra20_report_cpus_state();
+	}
+
+	pr_err("timed out waiting secondaries to park\n");
+
+	return -ETIMEDOUT;
+}
+
 static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
 					   struct cpuidle_driver *drv,
 					   int index)
 {
 	bool ret;
 
-	while (!tegra_cpu_rail_off_ready())
-		cpu_relax();
+	if (tegra20_wait_for_secondary_cpu_parking())
+		return false;
 
 	ret = !tegra_pm_enter_lp2();

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #12 lore raw

PATCH v6 11/18 arm: tegra20: cpuidle: Make abort_flag atomic

In reply to Dmitry Osipenko

Replace memory accessors with atomic API just to make code consistent with the abort_barrier. The new variant may be even more correct now since atomic_read() will prevent compiler from generating wrong things like carrying abort_flag value in a register instead of re-fetching it from memory.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +5 -4

index bc5873e92af5..fe80f1988120 100644
@@ -32,7 +32,7 @@
 #include "sleep.h"
 
 #ifdef CONFIG_PM_SLEEP
-static bool abort_flag;
+static atomic_t abort_flag;
 static atomic_t abort_barrier;
 static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 				    struct cpuidle_driver *drv,

@@ -167,13 +167,14 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 	bool entered_lp2 = false;
 
 	if (tegra_pending_sgi())
-		WRITE_ONCE(abort_flag, true);
+		atomic_set(&abort_flag, 1);
 
 	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
 
-	if (abort_flag) {
+	if (atomic_read(&abort_flag)) {
 		cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
-		abort_flag = false;	/* clean flag for next coming */
+		/* clean flag for next coming */
+		atomic_set(&abort_flag, 0);
 		return -EINTR;
 	}

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #13 lore raw

PATCH v6 12/18 arm: tegra20/30: cpuidle: Remove unnecessary memory barrier

In reply to Dmitry Osipenko

There is no good justification for smp_rmb() after returning from LP2 because there are no memory operations that require SMP synchronization. Thus remove the confusing barrier.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 2 --
 arch/arm/mach-tegra/cpuidle-tegra30.c | 2 --
 2 files changed, 4 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +0 -2

index fe80f1988120..af2cd339db43 100644
@@ -193,8 +193,6 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 
 	local_fiq_enable();
 
-	smp_rmb();
-
 	return entered_lp2 ? index : 0;
 }
 #endif

arch/arm/mach-tegra/cpuidle-tegra30.c +0 -2

index a4f0add46a27..80ae64bcdf50 100644
@@ -113,8 +113,6 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 
 	local_fiq_enable();
 
-	smp_rmb();
-
 	return (entered_lp2) ? index : 0;
 }
 #endif

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #14 lore raw

PATCH v6 13/18 cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/

In reply to Dmitry Osipenko

The driver's code is refactored in a way that will make it easy to support Tegra30/114/124 SoCs by this driver later on. The functionality is equal to the old Tegra20 driver, only the code's structure changed a tad and it's a platform driver now.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra20.c | 215 ----------------------
 arch/arm/mach-tegra/cpuidle.c         |  14 +-
 arch/arm/mach-tegra/cpuidle.h         |   4 -
 drivers/cpuidle/Kconfig.arm           |   8 +
 drivers/cpuidle/Makefile              |   1 +
 drivers/cpuidle/cpuidle-tegra.c       | 251 ++++++++++++++++++++++++++
 include/soc/tegra/cpuidle.h           |   2 +-
 8 files changed, 263 insertions(+), 235 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra20.c
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c

arch/arm/mach-tegra/Makefile +0 -3

index 965862608ff6..8425bb5608d5 100644
@@ -12,9 +12,6 @@ obj-y					+= sleep-tegra20.o
 obj-y					+= sleep-tegra30.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= cpuidle-tegra20.o
-endif
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
 ifeq ($(CONFIG_CPU_IDLE),y)
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= cpuidle-tegra30.o

arch/arm/mach-tegra/cpuidle-tegra20.c deleted +0 ~~-215~~

215 lines not on this page · raw message

arch/arm/mach-tegra/cpuidle.c +2 ~~-12~~

index d565c44cfc93..eee85d517783 100644
@@ -14,6 +14,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <soc/tegra/fuse.h>

@@ -23,8 +24,7 @@ void __init tegra_cpuidle_init(void)
 {
 	switch (tegra_get_chip_id()) {
 	case TEGRA20:
-		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
-			tegra20_cpuidle_init();
+		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
 		break;
 	case TEGRA30:
 		if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC))

@@ -38,13 +38,3 @@ void __init tegra_cpuidle_init(void)
 		break;
 	}
 }
-
-void tegra_cpuidle_pcie_irqs_in_use(void)
-{
-	switch (tegra_get_chip_id()) {
-	case TEGRA20:
-		if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
-			tegra20_cpuidle_pcie_irqs_in_use();
-		break;
-	}
-}

arch/arm/mach-tegra/cpuidle.h +0 -4

index 4e1f459f5bd8..eeb37baf18e1 100644
@@ -7,15 +7,11 @@
 #define __MACH_TEGRA_CPUIDLE_H
 
 #ifdef CONFIG_CPU_IDLE
-int tegra20_cpuidle_init(void);
-void tegra20_cpuidle_pcie_irqs_in_use(void);
 int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
 void tegra_cpuidle_init(void);
-void tegra_cpuidle_pcie_irqs_in_use(void);
 #else
 static inline void tegra_cpuidle_init(void) {}
-static inline void tegra_cpuidle_pcie_irqs_in_use(void) {}
 #endif
 
 #endif

drivers/cpuidle/Kconfig.arm +8 -0

index d8530475493c..6952bf7bb260 100644
@@ -86,3 +86,11 @@ config ARM_MVEBU_V7_CPUIDLE
 	depends on ARCH_MVEBU && !ARM64
 	help
 	  Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_TEGRA_CPUIDLE
+	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
+	depends on ARCH_TEGRA && !ARM64
+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
+	select ARM_CPU_SUSPEND
+	help
+	  Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.

drivers/cpuidle/Makefile +1 -0

index ee70d5cc5b99..a15e4808d295 100644
@@ -22,6 +22,7 @@ obj-$(CONFIG_ARM_AT91_CPUIDLE)          += cpuidle-at91.o
 obj-$(CONFIG_ARM_EXYNOS_CPUIDLE)        += cpuidle-exynos.o
 obj-$(CONFIG_ARM_CPUIDLE)		+= cpuidle-arm.o
 obj-$(CONFIG_ARM_PSCI_CPUIDLE)		+= cpuidle-psci.o
+obj-$(CONFIG_ARM_TEGRA_CPUIDLE)		+= cpuidle-tegra.o
 
 ###############################################################################
 # MIPS drivers

drivers/cpuidle/cpuidle-tegra.c new file +251 -0

251 lines not on this page · raw message

include/soc/tegra/cpuidle.h +1 -1

index 029ba1f4b2cc..5665975015d8 100644
@@ -6,7 +6,7 @@
 #ifndef __SOC_TEGRA_CPUIDLE_H__
 #define __SOC_TEGRA_CPUIDLE_H__
 
-#if defined(CONFIG_ARM) && defined(CONFIG_ARCH_TEGRA) && defined(CONFIG_CPU_IDLE)
+#ifdef CONFIG_ARM_TEGRA_CPUIDLE
 void tegra_cpuidle_pcie_irqs_in_use(void);
 #else
 static inline void tegra_cpuidle_pcie_irqs_in_use(void)

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #15 lore raw

PATCH v6 14/18 cpuidle: tegra: Squash Tegra30 driver into the common driver

In reply to Dmitry Osipenko

Tegra20 and Terga30 SoCs have common C1 and CC6 idling states and thus share the same code paths, there is no point in having separate drivers for a similar hardware. This patch merely moves functionality of the old driver into the new, although the CC6 state is kept disabled for now since old driver had a rudimentary support for this state (allowing to enter into CC6 only when secondary CPUs are put offline), while new driver can provide a full-featured support. The new feature will be enabled by another patch.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------
 arch/arm/mach-tegra/cpuidle.c         |   5 +-
 arch/arm/mach-tegra/cpuidle.h         |   1 -
 drivers/cpuidle/cpuidle-tegra.c       |  75 ++++++++++++++--
 5 files changed, 71 insertions(+), 136 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra30.c

arch/arm/mach-tegra/Makefile +0 -3

index 8425bb5608d5..99c5f4274e5c 100644
@@ -13,9 +13,6 @@ obj-y					+= sleep-tegra30.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= cpuidle-tegra30.o
-endif
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o

arch/arm/mach-tegra/cpuidle-tegra30.c deleted +0 ~~-123~~

123 lines not on this page · raw message

arch/arm/mach-tegra/cpuidle.c +1 -4

index eee85d517783..fa0dcf3c2c45 100644
@@ -24,11 +24,8 @@ void __init tegra_cpuidle_init(void)
 {
 	switch (tegra_get_chip_id()) {
 	case TEGRA20:
-		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
-		break;
 	case TEGRA30:
-		if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC))
-			tegra30_cpuidle_init();
+		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
 		break;
 	case TEGRA114:
 	case TEGRA124:

arch/arm/mach-tegra/cpuidle.h +0 -1

index eeb37baf18e1..5423a05a69f6 100644
@@ -7,7 +7,6 @@
 #define __MACH_TEGRA_CPUIDLE_H
 
 #ifdef CONFIG_CPU_IDLE
-int tegra30_cpuidle_init(void);
 int tegra114_cpuidle_init(void);
 void tegra_cpuidle_init(void);
 #else

drivers/cpuidle/cpuidle-tegra.c +70 -5

118 lines not on this page · raw message

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #16 lore raw

PATCH v6 15/18 cpuidle: tegra: Support CPU cluster power-down state on Tegra30

In reply to Dmitry Osipenko

The new Tegra CPU Idle driver now has a unified code path for the coupled CC6 (LP2) state, this allows to enable the deepest idling state on Tegra30 SoC where the whole CPU cluster is power-gated.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/cpuidle/cpuidle-tegra.c | 1 -
 1 file changed, 1 deletion(-)

drivers/cpuidle/cpuidle-tegra.c +0 -1

index 1a2cff5d4ff3..e38a2f6c11e1 100644
@@ -298,7 +298,6 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 		tegra_idle_driver.states[TEGRA_C7].disabled = true;
 		break;
 	case TEGRA30:
-		tegra_idle_driver.states[TEGRA_CC6].disabled = true;
 		break;
 	default:
 		return -EINVAL;

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #17 lore raw

PATCH v6 16/18 cpuidle: tegra: Squash Tegra114 driver into the common driver

In reply to Dmitry Osipenko

Tegra20/30/114/124 SoCs have common idling states, thus there is no much point in having separate drivers for a similar hardware. This patch moves Tegra114/124 arch/ drivers into the common driver without any functional changes. The CC6 state is kept disabled on Tegra114/124 because the core Tegra PM code needs some more work in order to support that state.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  7 --
 arch/arm/mach-tegra/cpuidle-tegra114.c | 90 --------------------------
 arch/arm/mach-tegra/cpuidle.c          | 37 -----------
 arch/arm/mach-tegra/cpuidle.h          | 16 -----
 arch/arm/mach-tegra/tegra.c            |  6 +-
 drivers/cpuidle/cpuidle-tegra.c        | 44 ++++++++++++-
 6 files changed, 47 insertions(+), 153 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra114.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.h

arch/arm/mach-tegra/Makefile +0 -7

index 99c5f4274e5c..07572b5373b8 100644
@@ -10,19 +10,12 @@ obj-y					+= sleep.o
 obj-y					+= tegra.o
 obj-y					+= sleep-tegra20.o
 obj-y					+= sleep-tegra30.o
-obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
 
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= cpuidle-tegra114.o
-endif
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= pm-tegra30.o
-ifeq ($(CONFIG_CPU_IDLE),y)
-obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= cpuidle-tegra114.o
-endif
 
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-paz00.o

arch/arm/mach-tegra/cpuidle-tegra114.c deleted +0 ~~-90~~

90 lines not on this page · raw message

arch/arm/mach-tegra/cpuidle.c deleted +0 ~~-37~~

deleted file mode 100644
index fa0dcf3c2c45..000000000000
@@ -1,37 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * arch/arm/mach-tegra/cpuidle.c
- *
- * CPU idle driver for Tegra CPUs
- *
- * Copyright (c) 2010-2012, NVIDIA Corporation.
- * Copyright (c) 2011 Google, Inc.
- * Author: Colin Cross <[email protected]>
- *         Gary King <[email protected]>
- *
- * Rework for 3.3 by Peter De Schrijver <[email protected]>
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-
-#include <soc/tegra/fuse.h>
-
-#include "cpuidle.h"
-
-void __init tegra_cpuidle_init(void)
-{
-	switch (tegra_get_chip_id()) {
-	case TEGRA20:
-	case TEGRA30:
-		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
-		break;
-	case TEGRA114:
-	case TEGRA124:
-		if (IS_ENABLED(CONFIG_ARCH_TEGRA_114_SOC) ||
-		    IS_ENABLED(CONFIG_ARCH_TEGRA_124_SOC))
-			tegra114_cpuidle_init();
-		break;
-	}
-}

arch/arm/mach-tegra/cpuidle.h deleted +0 ~~-16~~

deleted file mode 100644
index 5423a05a69f6..000000000000
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
- */
-
-#ifndef __MACH_TEGRA_CPUIDLE_H
-#define __MACH_TEGRA_CPUIDLE_H
-
-#ifdef CONFIG_CPU_IDLE
-int tegra114_cpuidle_init(void);
-void tegra_cpuidle_init(void);
-#else
-static inline void tegra_cpuidle_init(void) {}
-#endif
-
-#endif

arch/arm/mach-tegra/tegra.c +4 -2

index 00aaf495bbf7..f1ce2857a251 100644
@@ -36,11 +36,11 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
 #include <asm/mach-types.h>
+#include <asm/psci.h>
 #include <asm/setup.h>
 
 #include "board.h"
 #include "common.h"
-#include "cpuidle.h"
 #include "iomap.h"
 #include "pm.h"
 #include "reset.h"

@@ -85,7 +85,6 @@ static void __init tegra_dt_init(void)
 static void __init tegra_dt_init_late(void)
 {
 	tegra_init_suspend();
-	tegra_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
 	    of_machine_is_compatible("compal,paz00"))

@@ -94,6 +93,9 @@ static void __init tegra_dt_init_late(void)
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
 	    of_machine_is_compatible("nvidia,tegra20"))
 		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
+
+	if (IS_ENABLED(CONFIG_ARM_TEGRA_CPUIDLE) && !psci_smp_available())
+		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
 }
 
 static const char * const tegra_dt_board_compat[] = {

drivers/cpuidle/cpuidle-tegra.c +43 -1

92 lines not on this page · raw message

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #18 lore raw

PATCH v6 17/18 ARM: multi_v7_defconfig: Enable Tegra cpuidle driver

In reply to Dmitry Osipenko

The Tegra CPU Idle driver was moved out into driver/cpuidle/ directory and it is now a proper platform driver.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

arch/arm/configs/multi_v7_defconfig +1 -0

index e039680d3c72..e15cc31aeaf3 100644
@@ -110,6 +110,7 @@ CONFIG_CPU_IDLE=y
 CONFIG_ARM_CPUIDLE=y
 CONFIG_ARM_ZYNQ_CPUIDLE=y
 CONFIG_ARM_EXYNOS_CPUIDLE=y
+CONFIG_ARM_TEGRA_CPUIDLE=y
 CONFIG_KERNEL_MODE_NEON=y
 CONFIG_RASPBERRYPI_FIRMWARE=y
 CONFIG_TRUSTED_FOUNDATIONS=y

Signature · 1 line

2.23.0

Dmitry Osipenko

author

2019-10-15 17:00 UTC #19 lore raw

PATCH v6 18/18 ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig

In reply to Dmitry Osipenko

The Tegra CPU Idle driver was moved out into driver/cpuidle/ directory and it is now a proper platform driver.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/configs/tegra_defconfig | 1 +
 1 file changed, 1 insertion(+)

arch/arm/configs/tegra_defconfig +1 -0

index 8f5c6a5b444c..9a2f11a780a8 100644
@@ -25,6 +25,7 @@ CONFIG_CPU_FREQ=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_CPU_IDLE=y
+CONFIG_ARM_TEGRA_CPUIDLE=y
 CONFIG_VFP=y
 CONFIG_NEON=y
 CONFIG_TRUSTED_FOUNDATIONS=y

Signature · 1 line

2.23.0

Peter De Schrijver

2019-10-16 19:21 UTC #20 lore raw

In reply to Dmitry Osipenko

Quoted 70 lines · Dmitry Osipenko

Hello,
This series does the following:
1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.
2. Enables CPU cluster power-down idling state on Tegra30.
In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!
Changelog:
v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.
- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.
v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.
- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.
- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.
- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.
v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).
- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.
v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.
Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.
Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.
Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.
Further polished cpuidle driver's code.
The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.
v2: - Added patches to enable the new cpuidle driver in the defconfigs:
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:

Quoted 7 lines

cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states
- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.

It would be useful to switch the power state terminology to the one used for later chips:

LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7

Meaning of these states is as follows

C is a core state:

C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating

CC is a CPU cluster C state:

CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating

SC is a System C state:

SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off

Cheers,

Peter.

Dmitry Osipenko

author

2019-10-16 19:47 UTC #21 lore raw

16.10.2019 22:21, Peter De Schrijver пишет:

Quoted 110 lines · Dmitry Osipenko

Quoted 70 lines
Hello,
This series does the following:
1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.
2. Enables CPU cluster power-down idling state on Tegra30.
In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!
Changelog:
v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.
- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.
v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.
- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.
- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.
- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.
v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).
- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.
v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.
Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.
Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.
Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.
Further polished cpuidle driver's code.
The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.
v2: - Added patches to enable the new cpuidle driver in the defconfigs:
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:
Quoted 7 lines
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states
- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:

Quoted 7 lines

SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off

Hello Peter,

But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?

Dmitry Osipenko

author

2019-10-16 20:14 UTC #22 lore raw

16.10.2019 22:47, Dmitry Osipenko пишет:

Quoted 119 lines

16.10.2019 22:21, Peter De Schrijver пишет:
Quoted 110 lines · Dmitry Osipenko
Quoted 70 lines
Hello,
This series does the following:
1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.
2. Enables CPU cluster power-down idling state on Tegra30.
In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!
Changelog:
v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.
- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.
v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.
- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.
- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.
- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.
v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).
- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.
v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.
Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.
Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.
Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.
Further polished cpuidle driver's code.
The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.
v2: - Added patches to enable the new cpuidle driver in the defconfigs:
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:
Quoted 7 lines
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states
- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off

Quoted 7 lines

Hello Peter,
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?

Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)

Peter De Schrijver

2019-10-28 14:04 UTC #23 lore raw

Quoted 122 lines · Dmitry Osipenko

16.10.2019 22:47, Dmitry Osipenko пишет:
Quoted 121 lines
16.10.2019 22:21, Peter De Schrijver пишет:
Quoted 110 lines · Dmitry Osipenko
Quoted 70 lines
Hello,
This series does the following:
1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.
2. Enables CPU cluster power-down idling state on Tegra30.
In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!
Changelog:
v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.
- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.
v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.
- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.
- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.
- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.
v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).
- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.
v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.
Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.
Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.
Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.
Further polished cpuidle driver's code.
The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.
v2: - Added patches to enable the new cpuidle driver in the defconfigs:
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:
Quoted 7 lines
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states
- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off
Hello Peter,

Quoted 8 lines

Quoted 5 lines
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?
Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)

At least some of the variable and function names still say lp2?

Peter.

Dmitry Osipenko

author

2019-10-29 00:47 UTC #24 lore raw

28.10.2019 17:04, Peter De Schrijver пишет:

Quoted 129 lines · Dmitry Osipenko

Quoted 128 lines
16.10.2019 22:47, Dmitry Osipenko пишет:
Quoted 126 lines
16.10.2019 22:21, Peter De Schrijver пишет:
Quoted 110 lines · Dmitry Osipenko
Quoted 70 lines
Hello,
This series does the following:
1. Unifies Tegra20/30/114 drivers into a single driver and moves it out into common drivers/cpuidle/ directory.
2. Enables CPU cluster power-down idling state on Tegra30.
In the end there is a quite nice clean up of the Tegra CPUIDLE drivers and of the Tegra's arch code in general. Please review, thanks!
Changelog:
v6: - Addressed request from Thierry Reding to change the way patches are
organized by making changes in a more incremental manner.
- tegra_sleep_cpu() now checks for the secondary CPUs to be offline in the "Make outer_disable() open-coded" patch.
v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.
- Improved commit's message of the "Support CPU cluster power-down state on Tegra30" patch.
- The "Support CPU cluster power-down state on Tegra30" patch is also got split and now there is additional "Make outer_disable() open-coded" patch.
- Made minor cosmetic changes to the "Introduce unified driver for NVIDIA Tegra SoCs" patch by improving error message and renaming one variable.
v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
works).
- Replaced ktime_compare() with ktime_before() in the new driver, for consistency.
v3: - Addressed review comments that were made by Jon Hunter to v2 by
splitting patches into smaller (and simpler) chunks, better documenting changes in the commit messages and using proper error codes in the code.
Warnings are replaced with a useful error messages in the code of "Introduce unified driver for NVIDIA Tegra SoCs" patch.
Secondary CPUs parking timeout increased to 100ms because I found that it actually may happen to take more than 1ms if CPU is running on a *very* low frequency.
Added diagnostic messages that are reporting Flow Controller state when CPU parking fails.
Further polished cpuidle driver's code.
The coupled state entering is now aborted if there is a pending SGI (Software Generated Interrupt) because it will be lost after GIC's power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.
v2: - Added patches to enable the new cpuidle driver in the defconfigs:
ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
- Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling states because that flag actually doesn't have any negative effects, but still is correct for the case of a local CPU timer on older Tegra SoCs:
Quoted 7 lines
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states
- The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish. Tegra30 and Terga114 states are now squashed into a single common C7 state (following Parker TRM terminology, see 17.2.2.2 Power Management States), more comments added, etc minor changes.
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off
Hello Peter,
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?

Quoted 4 lines

Quoted 2 lines
Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)
At least some of the variable and function names still say lp2?

The cpuidle driver uses LP2 terminology for everything that comes from the external arch / firmware includes. But it says CC6 for everything that is internal to the driver. So yes, there is a bit of new/old terminology mixing in the code.

The arch code / PMC driver / TF firmware are all saying LP2. The LP2 naming is also a part of the device-tree binding.

It will be a lot of mess to rename the mach-tegra/pm.c code. I guess eventually it could be moved to drivers/soc/, so maybe it will be better to postpone the renaming until then?

Peter De Schrijver

2019-11-01 12:33 UTC #25 lore raw

On Tue, Oct 29, 2019 at 03:47:56AM +0300, Dmitry Osipenko wrote:
..

Quoted 57 lines

Quoted 47 lines
Quoted 47 lines
Quoted 46 lines
Quoted 31 lines
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off
Hello Peter,
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?
Quoted 4 lines
Quoted 2 lines
Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)
At least some of the variable and function names still say lp2?
The cpuidle driver uses LP2 terminology for everything that comes from the external arch / firmware includes. But it says CC6 for everything that is internal to the driver. So yes, there is a bit of new/old terminology mixing in the code.

Quoted 6 lines

The arch code / PMC driver / TF firmware are all saying LP2. The LP2 naming is also a part of the device-tree binding.
It will be a lot of mess to rename the mach-tegra/pm.c code. I guess eventually it could be moved to drivers/soc/, so maybe it will be better to postpone the renaming until then?

Or maybe add a comment somewhere indicating:

LP2 = CC6
LP1 = C1
LP0 = SC7

TF predates the new naming, so that may make some sense.

Peter.

Dmitry Osipenko

author

2019-11-01 13:22 UTC #26 lore raw

01.11.2019 15:33, Peter De Schrijver пишет:

Quoted 67 lines · Dmitry Osipenko

..
Quoted 57 lines
Quoted 47 lines
Quoted 47 lines
Quoted 46 lines
Quoted 31 lines
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off
Hello Peter,
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?
Quoted 4 lines
Quoted 2 lines
Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)
At least some of the variable and function names still say lp2?
The cpuidle driver uses LP2 terminology for everything that comes from the external arch / firmware includes. But it says CC6 for everything that is internal to the driver. So yes, there is a bit of new/old terminology mixing in the code.
Quoted 6 lines
The arch code / PMC driver / TF firmware are all saying LP2. The LP2 naming is also a part of the device-tree binding.
It will be a lot of mess to rename the mach-tegra/pm.c code. I guess eventually it could be moved to drivers/soc/, so maybe it will be better to postpone the renaming until then?

Quoted 7 lines

Or maybe add a comment somewhere indicating:
LP2 = CC6
LP1 = C1
LP0 = SC7
TF predates the new naming, so that may make some sense.

Today it should make more sense just to add an explicit comment to the cpuidle driver that clarifies the new naming (IMHO). I'll prepare v7 with that change.

Maybe later on, once more code will be consolidated in drivers/soc/tegra/, it will become useful to duplicate the clarification there as well.

Please let me know if you disagree or think that something better could be done.

Dmitry Osipenko

author

2019-11-01 21:30 UTC #27 lore raw

01.11.2019 16:22, Dmitry Osipenko пишет:

Quoted 80 lines

01.11.2019 15:33, Peter De Schrijver пишет:
Quoted 67 lines · Dmitry Osipenko
..
Quoted 57 lines
Quoted 47 lines
Quoted 47 lines
Quoted 46 lines
Quoted 31 lines
It would be useful to switch the power state terminology to the one used for later chips:
LP0 becomes SC7
LP1 becomes C1
LP2 becomes CC7
Meaning of these states is as follows
C is a core state:
C1 clock gating
C2 not defined
C3 not defined
C4 not defined
C5 not defined
C6 not defined for ARM cores
C7 power-gating
CC is a CPU cluster C state:
CC1 cluster clock gated
CC2 not defined
CC3 fmax@Vmin: not used prior to Tegra186
CC4: cluster retention: no longer supported
CC5: not defined
CC6: cluster power gating
CC7: cluster rail gating
SC is a System C state:
Quoted 7 lines
SC1: not defined
SC2: not defined
SC3: not defined
SC4: not defined
SC5: not defined
SC6: not defined
SC7: VDD_SOC off
Hello Peter,
But new "drivers/cpuidle/cpuidle-tegra.c" uses exactly that terminology, please see "cpuidle: Refactor and move NVIDIA Tegra20 driver into drivers/cpuidle/" and further patches. Am I missing something? Or do you want the renaming to be a separate patch?
Quoted 4 lines
Quoted 2 lines
Or maybe you're suggesting to change the names everywhere and not only in the cpuidle driver? Please clarify :)
At least some of the variable and function names still say lp2?
The cpuidle driver uses LP2 terminology for everything that comes from the external arch / firmware includes. But it says CC6 for everything that is internal to the driver. So yes, there is a bit of new/old terminology mixing in the code.
Quoted 6 lines
The arch code / PMC driver / TF firmware are all saying LP2. The LP2 naming is also a part of the device-tree binding.
It will be a lot of mess to rename the mach-tegra/pm.c code. I guess eventually it could be moved to drivers/soc/, so maybe it will be better to postpone the renaming until then?
Quoted 7 lines
Or maybe add a comment somewhere indicating:
LP2 = CC6
LP1 = C1
LP0 = SC7
TF predates the new naming, so that may make some sense.
Today it should make more sense just to add an explicit comment to the cpuidle driver that clarifies the new naming (IMHO). I'll prepare v7 with that change.

Quoted 6 lines

Maybe later on, once more code will be consolidated in drivers/soc/tegra/, it will become useful to duplicate the clarification there as well.
Please let me know if you disagree or think that something better could be done.

BTW, LP3 = C1.

I don't think that the new terminology has equivalent to LP1 (CPU cluster power gating + DRAM in self-refresh).