Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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.24.0

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

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

 	cpu_cluster_pm_exit();
+
+	return err;
 }

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 6 lines

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

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

arch/arm/mach-tegra/pm.c void tegra_idle_lp2_last(void) · line 211

Quoted 5 lines

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

 	/*
 	 * 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();

If the cpu_suspend fails, does restore_cpu_complex() need to be called ?

21.02.2020 18:16, Daniel Lezcano пишет:

Quoted 24 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 10 lines

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

 	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)
 

Quoted 3 lines

arch/arm/mach-tegra/pm.c void tegra_idle_lp2_last(void) · line 211

Quoted 1 line

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

 	restore_cpu_complex();

If the cpu_suspend fails, does restore_cpu_complex() need to be called ?

Yes, because suspend_cpu_complex() didn't fail. I don't see any reason why restore_cpu_complex() shouldn't be called, please clarify yours thought.

Quoted 29 lines

21.02.2020 18:16, Daniel Lezcano пишет:

Quoted 27 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

arch/arm/mach-tegra/pm.c void tegra_idle_lp2_last(void) · line 211

Quoted 11 lines

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

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

If the cpu_suspend fails, does restore_cpu_complex() need to be called ?

Quoted 2 lines

Yes, because suspend_cpu_complex() didn't fail. I don't see any reason why restore_cpu_complex() shouldn't be called, please clarify yours thought.

If the suspend fails, the power down does not happen, thus the logic is not lost and then it not necessary to restore something which has not been lost.

I don't know the hardware details, so that may be partially correct.

21.02.2020 20:40, Daniel Lezcano пишет:

Quoted 30 lines · Dmitry Osipenko

Quoted 29 lines

21.02.2020 18:16, Daniel Lezcano пишет:

Quoted 27 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

arch/arm/mach-tegra/pm.c void tegra_idle_lp2_last(void) · line 211

Quoted 11 lines

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

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

If the cpu_suspend fails, does restore_cpu_complex() need to be called ?

Quoted 7 lines

Quoted 2 lines

Yes, because suspend_cpu_complex() didn't fail. I don't see any reason why restore_cpu_complex() shouldn't be called, please clarify yours thought.

If the suspend fails, the power down does not happen, thus the logic is not lost and then it not necessary to restore something which has not been lost.

I don't know the hardware details, so that may be partially correct.

At quick glance, the restore_cpu_complex() is only used for restoring the GIC's state on Tegra.

I don't think it's really worth the effort to handle restore_cpu_complex() specially in a case of the error condition because the chance that the error will ever happen is very small and restoring the cluster's state won't cause any trouble in that case.

Let's keep this patch as-is for simplicity :)

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

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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

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

index 9672c619f4bc..bcc158b72e67 100644
@@ -83,14 +83,57 @@ static inline void tegra20_wake_cpu1_from_reset(void)
 }
 #endif
 
+static void tegra20_report_cpus_state(void)
+{
+	unsigned long cpu, lcpu, csr;
+
+	for_each_cpu(lcpu, cpu_possible_mask) {
+		cpu = cpu_logical_map(lcpu);
+		csr = flowctrl_read_cpu_csr(cpu);
+
+		pr_err("cpu%lu: online=%d flowctrl_csr=0x%08lx\n",
+		       cpu, cpu_online(lcpu), csr);
+	}
+}
+
+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));
+
+		pr_err("secondary CPU taking too long to park\n");
+
+		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.24.0

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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

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

index 9672c619f4bc..bcc158b72e67 100644
@@ -83,14 +83,57 @@ static inline void tegra20_wake_cpu1_from_reset(void)
 }
 #endif
 
+static void tegra20_report_cpus_state(void)
+{
+	unsigned long cpu, lcpu, csr;
+
+	for_each_cpu(lcpu, cpu_possible_mask) {
+		cpu = cpu_logical_map(lcpu);
+		csr = flowctrl_read_cpu_csr(cpu);
+
+		pr_err("cpu%lu: online=%d flowctrl_csr=0x%08lx\n",

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

+		pr_err("secondary CPU taking too long to park\n");
+
+		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.24.0

Quoted 34 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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

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

index 9672c619f4bc..bcc158b72e67 100644
@@ -83,14 +83,57 @@ static inline void tegra20_wake_cpu1_from_reset(void)
 }
 #endif
 
+static void tegra20_report_cpus_state(void)
+{
+	unsigned long cpu, lcpu, csr;
+
+	for_each_cpu(lcpu, cpu_possible_mask) {
+		cpu = cpu_logical_map(lcpu);
+		csr = flowctrl_read_cpu_csr(cpu);
+
+		pr_err("cpu%lu: online=%d flowctrl_csr=0x%08lx\n",
+		       cpu, cpu_online(lcpu), csr);
+	}
+}
+

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 104

Quoted 6 lines

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

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 117

Quoted 13 lines

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

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

Quoted 8 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 104

Quoted 6 lines

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

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

Could you please explain what benefits jiffies have over the ktime_get()?

Quoted 20 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 117

Quoted 13 lines

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

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

Quoted 14 lines

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 9 lines

Quoted 8 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 104

Quoted 6 lines

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

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

Quoted 1 line

Could you please explain what benefits jiffies have over the ktime_get()?

ktime_get() is very slow, jiffies is updated every tick.

Quoted 23 lines

Quoted 23 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 117

Quoted 13 lines

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

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

Quoted 4 lines

Quoted 2 lines

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

21.02.2020 20:36, Daniel Lezcano пишет:

Quoted 27 lines · Dmitry Osipenko

Quoted 14 lines

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 9 lines

Quoted 8 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 104

Quoted 6 lines

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

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

Quoted 3 lines

Quoted 1 line

Could you please explain what benefits jiffies have over the ktime_get()?

ktime_get() is very slow, jiffies is updated every tick.

But how jiffies are supposed to be updated if interrupts are disabled?

Aren't jiffies actually slower than ktime_get() because jiffies are updating every 10/1ms (depending on CONFIG_HZ)?

We're kinda interesting here in getting into deep-idling state as quick as possible. I was checking how much time takes the busy-loop below and it takes ~40-150us in average, which is good enough.

Quoted 28 lines

Quoted 27 lines

Quoted 18 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 117

Quoted 13 lines

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

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

Quoted 7 lines

I suggest something like:

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

Quoted 1 line

busy looping instead of register reads with all the hardware things involved behind.

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

Quoted 28 lines

21.02.2020 20:36, Daniel Lezcano пишет:

Quoted 27 lines · Dmitry Osipenko

Quoted 14 lines

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 9 lines

Quoted 8 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 104

Quoted 6 lines

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

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Could you please explain what benefits jiffies have over the ktime_get()?

ktime_get() is very slow, jiffies is updated every tick.

But how jiffies are supposed to be updated if interrupts are disabled?

Yeah, other cpus must not be idle in this.

Quoted 2 lines

Aren't jiffies actually slower than ktime_get() because jiffies are updating every 10/1ms (depending on CONFIG_HZ)?

They are no slower, they have a lower resolution which is 10ms or 4ms.

Given the 500ms timeout, it is fine.

Quoted 3 lines

We're kinda interesting here in getting into deep-idling state as quick as possible. I was checking how much time takes the busy-loop below and it takes ~40-150us in average, which is good enough.

ktime_get() gets a seq lock and it is very slow.

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline void tegra20_wake_cpu1_from_reset(void) · line 117

Quoted 13 lines

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

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

21.02.2020 23:02, Daniel Lezcano пишет:

Quoted 29 lines · Dmitry Osipenko

Quoted 28 lines

21.02.2020 20:36, Daniel Lezcano пишет:

Quoted 27 lines · Dmitry Osipenko

Quoted 14 lines

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 10 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---

[ ... ]

Quoted 9 lines

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+static int tegra20_wait_for_secondary_cpu_parking(void)
+{
+	unsigned int retries = 3;
+
+	while (retries--) {
+		ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Oops I missed this one. Do not use ktime_get() in this code path, use jiffies.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Could you please explain what benefits jiffies have over the ktime_get()?

ktime_get() is very slow, jiffies is updated every tick.

But how jiffies are supposed to be updated if interrupts are disabled?

Yeah, other cpus must not be idle in this.

Okay, then jiffies can't be used here because this function is used for the coupled / power-gated state only. All CPUs are idling in this state.

Quoted 5 lines

Quoted 2 lines

Aren't jiffies actually slower than ktime_get() because jiffies are updating every 10/1ms (depending on CONFIG_HZ)?

They are no slower, they have a lower resolution which is 10ms or 4ms.

Quoted 7 lines

Given the 500ms timeout, it is fine.

Quoted 3 lines

We're kinda interesting here in getting into deep-idling state as quick as possible. I was checking how much time takes the busy-loop below and it takes ~40-150us in average, which is good enough.

ktime_get() gets a seq lock and it is very slow.

Since all CPUs are idling here, the locking isn't a problem.

The wait_for_secondary_cpu_parking() function is called on CPU0, it waits for the secondary CPUs to enter into safe-state before CPU0 could power-gate the whole CPU cluster.

Quoted 30 lines

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 8 lines

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

Yes, my point is that udelay() doesn't bring much benefit for us here because:

1. we want to enter into power-gated state as quick as possible and udelay() just adds an unnecessary delay

2. udelay() spins in a busy-loop until delay is expired, just like we're doing it in this function already

21.02.2020 23:02, Daniel Lezcano пишет:

Quoted 30 lines

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 8 lines

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

Yes, my point is that udelay() doesn't bring much benefit for us here because:

1. we want to enter into power-gated state as quick as possible and udelay() just adds an unnecessary delay

2. udelay() spins in a busy-loop until delay is expired, just like we're doing it in this function already

Quoted 1 line

21.02.2020 23:02, Daniel Lezcano пишет:

[ ... ]

Quoted 42 lines

Quoted 30 lines

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 8 lines

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

Yes, my point is that udelay() doesn't bring much benefit for us here because:

Quoted 5 lines

1. we want to enter into power-gated state as quick as possible and udelay() just adds an unnecessary delay

2. udelay() spins in a busy-loop until delay is expired, just like we're doing it in this function already

In this case why not remove ktime_get() and increase the number of retries?

21.02.2020 23:48, Daniel Lezcano пишет:

Quoted 50 lines · Dmitry Osipenko

Quoted 1 line

21.02.2020 23:02, Daniel Lezcano пишет:

[ ... ]

Quoted 45 lines

Quoted 30 lines

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 8 lines

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

Yes, my point is that udelay() doesn't bring much benefit for us here because:

1. we want to enter into power-gated state as quick as possible and udelay() just adds an unnecessary delay

Quoted 4 lines

Quoted 2 lines

2. udelay() spins in a busy-loop until delay is expired, just like we're doing it in this function already

In this case why not remove ktime_get() and increase the number of retries?

Because the busy-loop performance depends on CPU's frequency, so we can't rely on a bare number of the retries.

Quoted 54 lines

21.02.2020 23:48, Daniel Lezcano пишет:

Quoted 53 lines · Dmitry Osipenko

Quoted 1 line

21.02.2020 23:02, Daniel Lezcano пишет:

[ ... ]

Quoted 47 lines

Quoted 30 lines

Quoted 30 lines

Quoted 29 lines

Quoted 20 lines

Quoted 20 lines

Review

Quoted 13 lines

(quoted diff) +13 -0

+
+		/*
+		 * 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));

So this loop will aggresively call tegra_cpu_rail_off_ready() and retry 3 times. The tegra_cpu_rail_off_ready() function can be called thoushand of times here but the function will hang 1.5s :/

I suggest something like:

Quoted 7 lines

Quoted 5 lines

	while (retries--i && !tegra_cpu_rail_off_ready()) 
		udelay(100);

So <retries> calls to tegra_cpu_rail_off_ready() and 100us x <retries> maximum impact.

But udelay() also results into CPU spinning in a busy-loop, and thus, what's the difference?

busy looping instead of register reads with all the hardware things involved behind.

Quoted 8 lines

Quoted 6 lines

Please notice that this code runs only on an older Cortex-A9/A15, which doesn't support WFE for the delaying, and thus, CPU always busy-loops inside udelay().

What about if I'll add cpu_relax() to the loop? Do you think it it could have any positive effect?

I think udelay() has a call to cpu_relax().

Yes, my point is that udelay() doesn't bring much benefit for us here because:

1. we want to enter into power-gated state as quick as possible and udelay() just adds an unnecessary delay

2. udelay() spins in a busy-loop until delay is expired, just like we're doing it in this function already

Quoted 4 lines

Quoted 1 line

In this case why not remove ktime_get() and increase the number of retries?

Because the busy-loop performance depends on CPU's frequency, so we can't rely on a bare number of the retries.

Why not if computed in the worst case scenario?

Anyway, I'll let you give a try.

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

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

Acked-by Peter De Schrijver <[email protected]>

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

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra20.c | 216 --------------------
 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       | 277 ++++++++++++++++++++++++++

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------

Quoted 9 lines

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.

Quoted 8 lines

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------

Add the -M option when resending please.

21.02.2020 19:29, Daniel Lezcano пишет:

Quoted 19 lines · Dmitry Osipenko

Quoted 17 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------

Quoted 1 line

Add the -M option when resending please.

Okay, thank you very much for taking a look at the patches!

Quoted 20 lines

21.02.2020 19:29, Daniel Lezcano пишет:

Quoted 19 lines · Dmitry Osipenko

Quoted 17 lines

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.

Acked-by Peter De Schrijver <[email protected]>

Tested-by Peter Geis <[email protected]>

Tested-by Jasper Korten <[email protected]>

Tested-by David Heidelberg <[email protected]>

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

---
 arch/arm/mach-tegra/Makefile          |   3 -
 arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------

Quoted 3 lines

Quoted 1 line

Add the -M option when resending please.

Okay, thank you very much for taking a look at the patches!

Yeah, sorry for the delay. Nice cleanup BTW.

LP2 suspending could be unavailable, for example if it is disabled in a device-tree. CC6 cpuidle state won't work in that case.

Acked-by Peter De Schrijver <[email protected]>

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

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

Acked-by Peter De Schrijver <[email protected]>

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 017d65f86eba..7c8a1c310bbb 100644
@@ -113,6 +113,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.24.0

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

Acked-by Peter De Schrijver <[email protected]>

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

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 apply, thanks!

!!!WARNING!!! This series was made on top of the cpufreq patches [1]. But it
should be fine as long as Thierry Reding would pick up this and the cpufreq patchsets via the Tegra tree, otherwise there will one minor merge-conflict.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=158206

Changelog:

v9: - Added acks from Peter De Schrijver.

- Added tested-by from Peter Geis, Jasper Korten and David Heidelberg who tested these patches on Ouya, TF300T and Nexus 7 devices.

- Temporarily dropped the "cpuidle: tegra: Support CPU cluster power-down state on Tegra30" patch because Michał Mirosław reported that it didn't work well on his TF300T. After some testing we found that changing a way in which firmware performs L2 cache maintenance helps, but later on we also found that the current v9 series works just fine without the extra firmware changes using recent linux-next and the reason why v8 didn't work before is still unknown (need more testing). So I decided that it will be better to postpone the dropped patch until we know for sure that it works well for everyone in every possible configuration.

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 apply, thanks!

!!!WARNING!!! This series was made on top of the cpufreq patches [1]. But it
should be fine as long as Thierry Reding would pick up this and the cpufreq patchsets via the Tegra tree, otherwise there will one minor merge-conflict.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=158206

cpuidle: Refactor and move out NVIDIA Tegra20 driver into
drivers/cpuidle
cpuidle: tegra: Squash Tegra30 driver into the common driver
cpuidle: tegra: Squash Tegra114 driver into the common driver
cpuidle: tegra: Disable CC6 state if LP2 unavailable

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 apply, thanks!

!!!WARNING!!! This series was made on top of the cpufreq patches [1]. But it
should be fine as long as Thierry Reding would pick up this and the cpufreq patchsets via the Tegra tree, otherwise there will one minor merge-conflict.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=158206

Changelog:

v9: - Added acks from Peter De Schrijver.

- Added tested-by from Peter Geis, Jasper Korten and David Heidelberg who tested these patches on Ouya, TF300T and Nexus 7 devices.

- Temporarily dropped the "cpuidle: tegra: Support CPU cluster power-down state on Tegra30" patch because Michał Mirosław reported that it didn't work well on his TF300T. After some testing we found that changing a way in which firmware performs L2 cache maintenance helps, but later on we also found that the current v9 series works just fine without the extra firmware changes using recent linux-next and the reason why v8 didn't work before is still unknown (need more testing). So I decided that it will be better to postpone the dropped patch until we know for sure that it works well for everyone in every possible configuration.

- Rebased this series on top of recent linux-next, in a result dropped the "cpuidle: Avoid NULL dereference in cpuidle_driver_state_disabled()" patch because it's not needed anymore.