Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)
@ 2020-02-12 23:51 Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 01/17] ARM: tegra: Compile sleep-tegra20/30.S unconditionally Dmitry Osipenko
                   ` (19 more replies)
  0 siblings, 20 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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.

v8: - Rebased on recent linux-next, now making use of
      cpuidle_driver_state_disabled(). Added new patch to make this new API
      usable by the updated Tegra cpuidle driver:

        cpuidle: Avoid NULL dereference in cpuidle_driver_state_disabled()

    - Added new patch to handle case where LP2 isn't available:

        cpuidle: tegra: Disable CC6 state if LP2 unavailable

v7: - drivers/cpuidle/cpuidle-tegra.c now includes an explicit comment that
      clarifies the new terminology that is used for naming of the idling
      states. This change was suggested by Peter De Schrijver in the review
      comment to v6. See the comment to struct tegra_idle_driver in the code.

    - (!) This series is now based on top of the "NVIDIA Tegra20 CPUFreq
      driver major update" patchset. The conflict between these two series
      is trivial to resolve, but still it's worth to mention about that.

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 (17):
  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
  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 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
  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               | 389 ++++++++++++++++++
 include/soc/tegra/cpuidle.h                   |   2 +-
 .../mach-tegra => include/soc/tegra}/irq.h    |   8 +-
 include/soc/tegra/pm.h                        |  31 ++
 23 files changed, 482 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%)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 01/17] ARM: tegra: Compile sleep-tegra20/30.S unconditionally
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 02/17] ARM: tegra: Add tegra_pm_park_secondary_cpu() Dmitry Osipenko
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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.

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/Makefile | 6 ++----
 arch/arm/mach-tegra/sleep.h  | 2 --
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 6c1dff2eccc2..965862608ff6 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -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
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 78ef32a907c8..63e2205cbc82 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -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);
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 02/17] ARM: tegra: Add tegra_pm_park_secondary_cpu()
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 01/17] ARM: tegra: Compile sleep-tegra20/30.S unconditionally Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20 Dmitry Osipenko
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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.

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/pm.c | 14 ++++++++++++++
 arch/arm/mach-tegra/pm.h |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 3cab81b82866..f5ff3dd1dd81 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 569151b3edc0..9a790f00237f 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -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_ */
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 01/17] ARM: tegra: Compile sleep-tegra20/30.S unconditionally Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 02/17] ARM: tegra: Add tegra_pm_park_secondary_cpu() Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 14:58   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void Dmitry Osipenko
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 69f3fa270fbe..f7d5041e73cc 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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;
 }
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index f5ff3dd1dd81..1ff499068bb1 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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)
 {
 	/*
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 9a790f00237f..b9cc12222bb1 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -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);
 
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index e3f34815c9da..53123ae4ac3b 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -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
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index a4cfc08159f6..51265592cb1a 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -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 \
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index 9a89f30d53ca..0e00ba8cf646 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -43,9 +43,6 @@
 #define APB_MISC_XM2CFGCPADCTRL2	0x8e4
 #define APB_MISC_XM2CFGDPADCTRL2	0x8e8
 
-#define __tegra20_cpu1_resettable_status_offset \
-	(__tegra_cpu_reset_handler_data_offset + RESET_DATA(RESETTABLE_STATUS))
-
 .macro pll_enable, rd, r_car_base, pll_base
 	ldr	\rd, [\r_car_base, #\pll_base]
 	tst	\rd, #(1 << 30)
@@ -90,10 +87,6 @@ ENDPROC(tegra20_hotplug_shutdown)
 ENTRY(tegra20_cpu_shutdown)
 	cmp	r0, #0
 	reteq	lr			@ must not be called for CPU 0
-	mov32	r1, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r2, =__tegra20_cpu1_resettable_status_offset
-	mov	r12, #CPU_RESETTABLE
-	strb	r12, [r1, r2]
 
 	cpu_to_halt_reg r1, r0
 	ldr	r3, =TEGRA_FLOW_CTRL_VIRT
@@ -116,107 +109,6 @@ ENDPROC(tegra20_cpu_shutdown)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-/*
- * tegra_pen_lock
- *
- * spinlock implementation with no atomic test-and-set and no coherence
- * using Peterson's algorithm on strongly-ordered registers
- * used to synchronize a cpu waking up from wfi with entering lp2 on idle
- *
- * The reference link of Peterson's algorithm:
- * http://en.wikipedia.org/wiki/Peterson's_algorithm
- *
- * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)
- * on cpu 0:
- * r2 = flag[0] (in SCRATCH38)
- * r3 = flag[1] (in SCRATCH39)
- * on cpu1:
- * r2 = flag[1] (in SCRATCH39)
- * r3 = flag[0] (in SCRATCH38)
- *
- * must be called with MMU on
- * corrupts r0-r3, r12
- */
-ENTRY(tegra_pen_lock)
-	mov32	r3, TEGRA_PMC_VIRT
-	cpu_id	r0
-	add	r1, r3, #PMC_SCRATCH37
-	cmp	r0, #0
-	addeq	r2, r3, #PMC_SCRATCH38
-	addeq	r3, r3, #PMC_SCRATCH39
-	addne	r2, r3, #PMC_SCRATCH39
-	addne	r3, r3, #PMC_SCRATCH38
-
-	mov	r12, #1
-	str	r12, [r2]		@ flag[cpu] = 1
-	dsb
-	str	r12, [r1]		@ !turn = cpu
-1:	dsb
-	ldr	r12, [r3]
-	cmp	r12, #1			@ flag[!cpu] == 1?
-	ldreq	r12, [r1]
-	cmpeq	r12, r0			@ !turn == cpu?
-	beq	1b			@ while !turn == cpu && flag[!cpu] == 1
-
-	ret	lr			@ locked
-ENDPROC(tegra_pen_lock)
-
-ENTRY(tegra_pen_unlock)
-	dsb
-	mov32	r3, TEGRA_PMC_VIRT
-	cpu_id	r0
-	cmp	r0, #0
-	addeq	r2, r3, #PMC_SCRATCH38
-	addne	r2, r3, #PMC_SCRATCH39
-	mov	r12, #0
-	str	r12, [r2]
-	ret     lr
-ENDPROC(tegra_pen_unlock)
-
-/*
- * tegra20_cpu_clear_resettable(void)
- *
- * Called to clear the "resettable soon" flag in IRAM variable when
- * it is expected that the secondary CPU will be idle soon.
- */
-ENTRY(tegra20_cpu_clear_resettable)
-	mov32	r1, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r2, =__tegra20_cpu1_resettable_status_offset
-	mov	r12, #CPU_NOT_RESETTABLE
-	strb	r12, [r1, r2]
-	ret	lr
-ENDPROC(tegra20_cpu_clear_resettable)
-
-/*
- * tegra20_cpu_set_resettable_soon(void)
- *
- * Called to set the "resettable soon" flag in IRAM variable when
- * it is expected that the secondary CPU will be idle soon.
- */
-ENTRY(tegra20_cpu_set_resettable_soon)
-	mov32	r1, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r2, =__tegra20_cpu1_resettable_status_offset
-	mov	r12, #CPU_RESETTABLE_SOON
-	strb	r12, [r1, r2]
-	ret	lr
-ENDPROC(tegra20_cpu_set_resettable_soon)
-
-/*
- * tegra20_cpu_is_resettable_soon(void)
- *
- * Returns true if the "resettable soon" flag in IRAM variable has been
- * set because it is expected that the secondary CPU will be idle soon.
- */
-ENTRY(tegra20_cpu_is_resettable_soon)
-	mov32	r1, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r2, =__tegra20_cpu1_resettable_status_offset
-	ldrb	r12, [r1, r2]
-	cmp	r12, #CPU_RESETTABLE_SOON
-	moveq	r0, #1
-	movne	r0, #0
-	ret	lr
-ENDPROC(tegra20_cpu_is_resettable_soon)
-
 /*
  * tegra20_sleep_core_finish(unsigned long v2p)
  *
@@ -242,68 +134,6 @@ ENTRY(tegra20_sleep_core_finish)
 	ret	r3
 ENDPROC(tegra20_sleep_core_finish)
 
-/*
- * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
- *
- * Enters WFI on secondary CPU by exiting coherency.
- */
-ENTRY(tegra20_sleep_cpu_secondary_finish)
-	stmfd	sp!, {r4-r11, lr}
-
-	mrc	p15, 0, r11, c1, c0, 1  @ save actlr before exiting coherency
-
-	/* Flush and disable the L1 data cache */
-	mov	r0, #TEGRA_FLUSH_CACHE_LOUIS
-	bl	tegra_disable_clean_inv_dcache
-
-	mov32	r0, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r4, =__tegra20_cpu1_resettable_status_offset
-	mov	r3, #CPU_RESETTABLE
-	strb	r3, [r0, r4]
-
-	bl	tegra_cpu_do_idle
-
-	/*
-	 * cpu may be reset while in wfi, which will return through
-	 * tegra_resume to cpu_resume
-	 * or interrupt may wake wfi, which will return here
-	 * cpu state is unchanged - MMU is on, cache is on, coherency
-	 * is off, and the data cache is off
-	 *
-	 * r11 contains the original actlr
-	 */
-
-	bl	tegra_pen_lock
-
-	mov32	r0, TEGRA_IRAM_RESET_BASE_VIRT
-	ldr	r4, =__tegra20_cpu1_resettable_status_offset
-	mov	r3, #CPU_NOT_RESETTABLE
-	strb	r3, [r0, r4]
-
-	bl	tegra_pen_unlock
-
-	/* Re-enable the data cache */
-	mrc	p15, 0, r10, c1, c0, 0
-	orr	r10, r10, #CR_C
-	mcr	p15, 0, r10, c1, c0, 0
-	isb
-
-	mcr	p15, 0, r11, c1, c0, 1	@ reenable coherency
-
-	/* Invalidate the TLBs & BTAC */
-	mov	r1, #0
-	mcr	p15, 0, r1, c8, c3, 0	@ invalidate shared TLBs
-	mcr	p15, 0, r1, c7, c1, 6	@ invalidate shared BTAC
-	dsb
-	isb
-
-	/* the cpu was running with coherency disabled,
-	 * caches may be out of date */
-	bl	v7_flush_kern_cache_louis
-
-	ldmfd	sp!, {r4 - r11, pc}
-ENDPROC(tegra20_sleep_cpu_secondary_finish)
-
 /*
  * tegra20_tear_down_cpu
  *
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 63e2205cbc82..4978def9db46 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -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);
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20 Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:04   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last() Dmitry Osipenko
                   ` (15 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index c6128526877d..a3ce8dabfe18 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -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();
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 1ff499068bb1..a72f9a2d3cb7 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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)
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index b9cc12222bb1..2c294f6365c0 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -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);
 
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:16   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver Dmitry Osipenko
                   ` (14 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index f7d5041e73cc..9789541adb7d 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index a3ce8dabfe18..17cbd118abee 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -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
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index a72f9a2d3cb7..a094acaca307 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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(
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 2c294f6365c0..7d72f31dee77 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -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
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last() Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:18   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions Dmitry Osipenko
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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%)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index 5118f777fd66..2d8527837aeb 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -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
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 9789541adb7d..fdfe92068e38 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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"
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 17cbd118abee..3e91c29891f7 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -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
diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index ace7a390b5fe..4e1ee70b2a3f 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -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
 
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 7d72f31dee77..81525f5f4a44 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -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_ */
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 4978def9db46..4718a3cb45a1 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -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
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 1e3b85923ca3..79184a077c84 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -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"
diff --git a/arch/arm/mach-tegra/irq.h b/include/soc/tegra/irq.h
similarity index 59%
rename from arch/arm/mach-tegra/irq.h
rename to include/soc/tegra/irq.h
index 7a94cf121448..8eb11a7109e4 100644
--- a/arch/arm/mach-tegra/irq.h
+++ b/include/soc/tegra/irq.h
@@ -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 */
diff --git a/include/soc/tegra/pm.h b/include/soc/tegra/pm.h
index 951fcd738d55..1974e8405098 100644
--- a/include/soc/tegra/pm.h
+++ b/include/soc/tegra/pm.h
@@ -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__ */
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:19   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 08/17] ARM: tegra: Make outer_disable() open-coded Dmitry Osipenko
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index 2d8527837aeb..858c30cc5dc7 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -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();
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index fdfe92068e38..9672c619f4bc 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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();
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 3e91c29891f7..a4f0add46a27 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -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();
 
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index a094acaca307..7d9ef26e52a7 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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;
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index c3946dbd0240..2667bcdb5dc6 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -265,11 +265,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 */
@@ -281,7 +281,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
diff --git a/include/soc/tegra/pm.h b/include/soc/tegra/pm.h
index 1974e8405098..08477d7bfab9 100644
--- a/include/soc/tegra/pm.h
+++ b/include/soc/tegra/pm.h
@@ -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;
 }
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 08/17] ARM: tegra: Make outer_disable() open-coded
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2 Dmitry Osipenko
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/pm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 7d9ef26e52a7..d1e1a61b12cf 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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:
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 08/17] ARM: tegra: Make outer_disable() open-coded Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 14:55   ` Daniel Lezcano
  2020-02-21 15:43   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic Dmitry Osipenko
                   ` (10 subsequent siblings)
  19 siblings, 2 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 47 +++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 9672c619f4bc..bcc158b72e67 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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();
 
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2 Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:25   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier Dmitry Osipenko
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index bcc158b72e67..ccb8b0aa6c46 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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,
@@ -168,13 +168,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;
 	}
 
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (9 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:25   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle Dmitry Osipenko
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 2 --
 arch/arm/mach-tegra/cpuidle-tegra30.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index ccb8b0aa6c46..f8f14c0b79ff 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -194,8 +194,6 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 
 	local_fiq_enable();
 
-	smp_rmb();
-
 	return entered_lp2 ? index : 0;
 }
 #endif
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index a4f0add46a27..80ae64bcdf50 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -113,8 +113,6 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 
 	local_fiq_enable();
 
-	smp_rmb();
-
 	return (entered_lp2) ? index : 0;
 }
 #endif
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (10 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 15:44   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver Dmitry Osipenko
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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 ++++++++++++++++++++++++++
 include/soc/tegra/cpuidle.h           |   2 +-
 8 files changed, 289 insertions(+), 236 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra20.c
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 965862608ff6..8425bb5608d5 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -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
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
deleted file mode 100644
index f8f14c0b79ff..000000000000
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ /dev/null
@@ -1,216 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * CPU idle driver for Tegra CPUs
- *
- * Copyright (c) 2010-2012, NVIDIA Corporation.
- * Copyright (c) 2011 Google, Inc.
- * Author: Colin Cross <ccross@android.com>
- *         Gary King <gking@nvidia.com>
- *
- * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
- */
-
-#include <linux/clk/tegra.h>
-#include <linux/tick.h>
-#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>
-#include <soc/tegra/pm.h>
-
-#include <asm/cpuidle.h>
-#include <asm/smp_plat.h>
-#include <asm/suspend.h>
-
-#include "cpuidle.h"
-#include "iomap.h"
-#include "irq.h"
-#include "reset.h"
-#include "sleep.h"
-
-#ifdef CONFIG_PM_SLEEP
-static atomic_t abort_flag;
-static atomic_t abort_barrier;
-static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
-				    struct cpuidle_driver *drv,
-				    int index);
-#define TEGRA20_MAX_STATES 2
-#else
-#define TEGRA20_MAX_STATES 1
-#endif
-
-static struct cpuidle_driver tegra_idle_driver = {
-	.name = "tegra_idle",
-	.owner = THIS_MODULE,
-	.states = {
-		ARM_CPUIDLE_WFI_STATE_PWR(600),
-#ifdef CONFIG_PM_SLEEP
-		{
-			.enter            = tegra20_idle_lp2_coupled,
-			.exit_latency     = 5000,
-			.target_residency = 10000,
-			.power_usage      = 0,
-			.flags            = CPUIDLE_FLAG_COUPLED |
-			                    CPUIDLE_FLAG_TIMER_STOP,
-			.name             = "powered-down",
-			.desc             = "CPU power gated",
-		},
-#endif
-	},
-	.state_count = TEGRA20_MAX_STATES,
-	.safe_state_index = 0,
-};
-
-#ifdef CONFIG_PM_SLEEP
-#ifdef CONFIG_SMP
-static void tegra20_wake_cpu1_from_reset(void)
-{
-	/* enable cpu clock on cpu */
-	tegra_enable_cpu_clock(1);
-
-	/* take the CPU out of reset */
-	tegra_cpu_out_of_reset(1);
-
-	/* unhalt the cpu */
-	flowctrl_write_cpu_halt(1, 0);
-}
-#else
-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;
-
-	if (tegra20_wait_for_secondary_cpu_parking())
-		return false;
-
-	ret = !tegra_pm_enter_lp2();
-
-	if (cpu_online(1))
-		tegra20_wake_cpu1_from_reset();
-
-	return ret;
-}
-
-#ifdef CONFIG_SMP
-static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
-					 struct cpuidle_driver *drv,
-					 int index)
-{
-	cpu_suspend(dev->cpu, tegra_pm_park_secondary_cpu);
-
-	return true;
-}
-#else
-static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
-						struct cpuidle_driver *drv,
-						int index)
-{
-	return true;
-}
-#endif
-
-static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
-				    struct cpuidle_driver *drv,
-				    int index)
-{
-	bool entered_lp2 = false;
-
-	if (tegra_pending_sgi())
-		atomic_set(&abort_flag, 1);
-
-	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
-
-	if (atomic_read(&abort_flag)) {
-		cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
-		/* clean flag for next coming */
-		atomic_set(&abort_flag, 0);
-		return -EINTR;
-	}
-
-	local_fiq_disable();
-
-	tegra_pm_set_cpu_in_lp2();
-	cpu_pm_enter();
-
-	if (dev->cpu == 0)
-		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
-	else
-		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
-
-	cpu_pm_exit();
-	tegra_pm_clear_cpu_in_lp2();
-
-	local_fiq_enable();
-
-	return entered_lp2 ? index : 0;
-}
-#endif
-
-/*
- * Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
- * they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around
- * this, simply disable LP2 if the PCI driver and DT node are both enabled.
- */
-void tegra20_cpuidle_pcie_irqs_in_use(void)
-{
-	pr_info_once(
-		"Disabling cpuidle LP2 state, since PCIe IRQs are in use\n");
-	cpuidle_driver_state_disabled(&tegra_idle_driver, 1, true);
-}
-
-int __init tegra20_cpuidle_init(void)
-{
-	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
-}
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index d565c44cfc93..eee85d517783 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -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;
-	}
-}
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index 4e1f459f5bd8..eeb37baf18e1 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -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
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 62272ecfa771..99a2d72ac02b 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -86,3 +86,11 @@ config ARM_MVEBU_V7_CPUIDLE
 	depends on (ARCH_MVEBU || COMPILE_TEST) && !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.
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index cc8c769d7fa9..55a464f6a78b 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ARM_CPUIDLE)		+= cpuidle-arm.o
 obj-$(CONFIG_ARM_PSCI_CPUIDLE)		+= cpuidle_psci.o
 cpuidle_psci-y				:= cpuidle-psci.o
 cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
+obj-$(CONFIG_ARM_TEGRA_CPUIDLE)		+= cpuidle-tegra.o
 
 ###############################################################################
 # MIPS drivers
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
new file mode 100644
index 000000000000..a58d2773d45f
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CPU idle driver for Tegra CPUs
+ *
+ * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2011 Google, Inc.
+ * Author: Colin Cross <ccross@android.com>
+ *         Gary King <gking@nvidia.com>
+ *
+ * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
+ *
+ * Tegra20/124 driver unification by Dmitry Osipenko <digetx@gmail.com>
+ */
+
+#define pr_fmt(fmt)	"tegra-cpuidle: " fmt
+
+#include <linux/atomic.h>
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
+#include <linux/errno.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include <linux/clk/tegra.h>
+
+#include <soc/tegra/cpuidle.h>
+#include <soc/tegra/flowctrl.h>
+#include <soc/tegra/fuse.h>
+#include <soc/tegra/irq.h>
+#include <soc/tegra/pm.h>
+
+#include <asm/cpuidle.h>
+#include <asm/smp_plat.h>
+#include <asm/suspend.h>
+
+enum tegra_state {
+	TEGRA_C1,
+	TEGRA_CC6,
+	TEGRA_STATE_COUNT,
+};
+
+static atomic_t tegra_idle_barrier;
+static atomic_t tegra_abort_flag;
+
+static void tegra_cpuidle_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 tegra_cpuidle_wait_for_secondary_cpus_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");
+
+		tegra_cpuidle_report_cpus_state();
+	}
+
+	pr_err("timed out waiting secondaries to park\n");
+
+	return -ETIMEDOUT;
+}
+
+static void tegra_cpuidle_unpark_secondary_cpus(void)
+{
+	unsigned int cpu, lcpu;
+
+	for_each_cpu(lcpu, cpu_online_mask) {
+		cpu = cpu_logical_map(lcpu);
+
+		if (cpu > 0) {
+			tegra_enable_cpu_clock(cpu);
+			tegra_cpu_out_of_reset(cpu);
+			flowctrl_write_cpu_halt(cpu, 0);
+		}
+	}
+}
+
+static int tegra_cpuidle_cc6_enter(unsigned int cpu)
+{
+	int ret;
+
+	if (cpu > 0) {
+		ret = cpu_suspend(cpu, tegra_pm_park_secondary_cpu);
+	} else {
+		ret = tegra_cpuidle_wait_for_secondary_cpus_parking();
+		if (!ret)
+			ret = tegra_pm_enter_lp2();
+
+		tegra_cpuidle_unpark_secondary_cpus();
+	}
+
+	return ret;
+}
+
+static int tegra_cpuidle_coupled_barrier(struct cpuidle_device *dev)
+{
+	if (tegra_pending_sgi()) {
+		/*
+		 * CPU got local interrupt that will be lost after GIC's
+		 * shutdown because GIC driver doesn't save/restore the
+		 * pending SGI state across CPU cluster PM.  Abort and retry
+		 * next time.
+		 */
+		atomic_set(&tegra_abort_flag, 1);
+	}
+
+	cpuidle_coupled_parallel_barrier(dev, &tegra_idle_barrier);
+
+	if (atomic_read(&tegra_abort_flag)) {
+		cpuidle_coupled_parallel_barrier(dev, &tegra_idle_barrier);
+		atomic_set(&tegra_abort_flag, 0);
+		return -EINTR;
+	}
+
+	return 0;
+}
+
+static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
+				     int index, unsigned int cpu)
+{
+	int ret;
+
+	/*
+	 * CC6 state is the "CPU cluster power-off" state.  In order to
+	 * enter this state, at first the secondary CPU cores need to be
+	 * parked into offline mode, then the last CPU should clean out
+	 * remaining dirty cache lines into DRAM and trigger Flow Controller
+	 * logic that turns off the cluster's power domain (which includes
+	 * CPU cores, GIC and L2 cache).
+	 */
+	if (index == TEGRA_CC6) {
+		ret = tegra_cpuidle_coupled_barrier(dev);
+		if (ret)
+			return ret;
+	}
+
+	local_fiq_disable();
+	tegra_pm_set_cpu_in_lp2();
+	cpu_pm_enter();
+
+	switch (index) {
+	case TEGRA_CC6:
+		ret = tegra_cpuidle_cc6_enter(cpu);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	cpu_pm_exit();
+	tegra_pm_clear_cpu_in_lp2();
+	local_fiq_enable();
+
+	return ret;
+}
+
+static int tegra_cpuidle_enter(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv,
+			       int index)
+{
+	unsigned int cpu = cpu_logical_map(dev->cpu);
+	int err;
+
+	err = tegra_cpuidle_state_enter(dev, index, cpu);
+	if (err && err != -EINTR)
+		pr_err_once("cpu%u failed to enter idle state %d err: %d\n",
+			    cpu, index, err);
+
+	return err ? -1 : index;
+}
+
+/*
+ * The previous versions of Tegra CPUIDLE driver used a different "legacy"
+ * terminology for naming of the idling states, while this driver uses the
+ * new terminology.
+ *
+ * Mapping of the old terms into the new ones:
+ *
+ * Old | New
+ * ---------
+ * LP3 | C1	(CPU core clock gating)
+ * LP2 | C7	(CPU core power gating)
+ * LP2 | CC6	(CPU cluster power gating)
+ *
+ * Note that that the older CPUIDLE driver versions didn't explicitly
+ * differentiate the LP2 states because these states either used the same
+ * code path or because CC6 wasn't supported.
+ */
+static struct cpuidle_driver tegra_idle_driver = {
+	.name = "tegra_idle",
+	.states = {
+		[TEGRA_C1] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+		[TEGRA_CC6] = {
+			.enter			= tegra_cpuidle_enter,
+			.exit_latency		= 5000,
+			.target_residency	= 10000,
+			.power_usage		= 0,
+			.flags			= CPUIDLE_FLAG_TIMER_STOP |
+						  CPUIDLE_FLAG_COUPLED,
+			.name			= "CC6",
+			.desc			= "CPU cluster powered off",
+		},
+	},
+	.state_count = TEGRA_STATE_COUNT,
+	.safe_state_index = TEGRA_C1,
+};
+
+static inline void tegra_cpuidle_disable_state(enum tegra_state state)
+{
+	cpuidle_driver_state_disabled(&tegra_idle_driver, state, true);
+}
+
+/*
+ * Tegra20 HW appears to have a bug such that PCIe device interrupts, whether
+ * they are legacy IRQs or MSI, are lost when CC6 is enabled.  To work around
+ * this, simply disable CC6 if the PCI driver and DT node are both enabled.
+ */
+void tegra_cpuidle_pcie_irqs_in_use(void)
+{
+	struct cpuidle_state *state_cc6 = &tegra_idle_driver.states[TEGRA_CC6];
+
+	if ((state_cc6->flags & CPUIDLE_FLAG_UNUSABLE) ||
+	    tegra_get_chip_id() != TEGRA20)
+		return;
+
+	pr_info("disabling CC6 state, since PCIe IRQs are in use\n");
+	tegra_cpuidle_disable_state(TEGRA_CC6);
+}
+
+static int tegra_cpuidle_probe(struct platform_device *pdev)
+{
+	/*
+	 * Required suspend-resume functionality, which is provided by the
+	 * Tegra-arch core and PMC driver, is unavailable if PM-sleep option
+	 * is disabled.
+	 */
+	if (!IS_ENABLED(CONFIG_PM_SLEEP))
+		tegra_cpuidle_disable_state(TEGRA_CC6);
+
+	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
+}
+
+static struct platform_driver tegra_cpuidle_driver = {
+	.probe = tegra_cpuidle_probe,
+	.driver = {
+		.name = "tegra-cpuidle",
+	},
+};
+builtin_platform_driver(tegra_cpuidle_driver);
diff --git a/include/soc/tegra/cpuidle.h b/include/soc/tegra/cpuidle.h
index 029ba1f4b2cc..5665975015d8 100644
--- a/include/soc/tegra/cpuidle.h
+++ b/include/soc/tegra/cpuidle.h
@@ -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)
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (11 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 16:29   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 14/17] cpuidle: tegra: Squash Tegra114 " Dmitry Osipenko
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Jasper Korten <jja2000@gmail.com>
Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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       |  74 ++++++++++++++--
 5 files changed, 70 insertions(+), 136 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra30.c

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 8425bb5608d5..99c5f4274e5c 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -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
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
deleted file mode 100644
index 80ae64bcdf50..000000000000
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ /dev/null
@@ -1,123 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * CPU idle driver for Tegra CPUs
- *
- * Copyright (c) 2010-2012, NVIDIA Corporation.
- * Copyright (c) 2011 Google, Inc.
- * Author: Colin Cross <ccross@android.com>
- *         Gary King <gking@nvidia.com>
- *
- * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
- */
-
-#include <linux/clk/tegra.h>
-#include <linux/tick.h>
-#include <linux/cpuidle.h>
-#include <linux/cpu_pm.h>
-#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 "sleep.h"
-
-#ifdef CONFIG_PM_SLEEP
-static int tegra30_idle_lp2(struct cpuidle_device *dev,
-			    struct cpuidle_driver *drv,
-			    int index);
-#endif
-
-static struct cpuidle_driver tegra_idle_driver = {
-	.name = "tegra_idle",
-	.owner = THIS_MODULE,
-#ifdef CONFIG_PM_SLEEP
-	.state_count = 2,
-#else
-	.state_count = 1,
-#endif
-	.states = {
-		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
-#ifdef CONFIG_PM_SLEEP
-		[1] = {
-			.enter			= tegra30_idle_lp2,
-			.exit_latency		= 2000,
-			.target_residency	= 2200,
-			.power_usage		= 0,
-			.flags			= CPUIDLE_FLAG_TIMER_STOP,
-			.name			= "powered-down",
-			.desc			= "CPU power gated",
-		},
-#endif
-	},
-};
-
-#ifdef CONFIG_PM_SLEEP
-static bool tegra30_cpu_cluster_power_down(struct cpuidle_device *dev,
-					   struct cpuidle_driver *drv,
-					   int index)
-{
-	/* All CPUs entering LP2 is not working.
-	 * Don't let CPU0 enter LP2 when any secondary CPU is online.
-	 */
-	if (num_online_cpus() > 1 || !tegra_cpu_rail_off_ready()) {
-		cpu_do_idle();
-		return false;
-	}
-
-	return !tegra_pm_enter_lp2();
-}
-
-#ifdef CONFIG_SMP
-static bool tegra30_cpu_core_power_down(struct cpuidle_device *dev,
-					struct cpuidle_driver *drv,
-					int index)
-{
-	smp_wmb();
-
-	cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
-
-	return true;
-}
-#else
-static inline bool tegra30_cpu_core_power_down(struct cpuidle_device *dev,
-					       struct cpuidle_driver *drv,
-					       int index)
-{
-	return true;
-}
-#endif
-
-static int tegra30_idle_lp2(struct cpuidle_device *dev,
-			    struct cpuidle_driver *drv,
-			    int index)
-{
-	bool entered_lp2 = false;
-
-	local_fiq_disable();
-
-	tegra_pm_set_cpu_in_lp2();
-	cpu_pm_enter();
-
-	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_pm_clear_cpu_in_lp2();
-
-	local_fiq_enable();
-
-	return (entered_lp2) ? index : 0;
-}
-#endif
-
-int __init tegra30_cpuidle_init(void)
-{
-	return cpuidle_register(&tegra_idle_driver, NULL);
-}
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index eee85d517783..fa0dcf3c2c45 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -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:
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index eeb37baf18e1..5423a05a69f6 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -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
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index a58d2773d45f..59398c76cf68 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -37,6 +37,7 @@
 
 enum tegra_state {
 	TEGRA_C1,
+	TEGRA_C7,
 	TEGRA_CC6,
 	TEGRA_STATE_COUNT,
 };
@@ -119,6 +120,11 @@ static int tegra_cpuidle_cc6_enter(unsigned int cpu)
 	return ret;
 }
 
+static int tegra_cpuidle_c7_enter(void)
+{
+	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
+}
+
 static int tegra_cpuidle_coupled_barrier(struct cpuidle_device *dev)
 {
 	if (tegra_pending_sgi()) {
@@ -166,6 +172,10 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
 	cpu_pm_enter();
 
 	switch (index) {
+	case TEGRA_C7:
+		ret = tegra_cpuidle_c7_enter();
+		break;
+
 	case TEGRA_CC6:
 		ret = tegra_cpuidle_cc6_enter(cpu);
 		break;
@@ -182,6 +192,24 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
 	return ret;
 }
 
+static int tegra_cpuidle_adjust_state_index(int index, unsigned int cpu)
+{
+	/*
+	 * On Tegra30 CPU0 can't be power-gated separately from secondary
+	 * cores because it gates the whole CPU cluster.
+	 */
+	if (cpu > 0 || index != TEGRA_C7 || tegra_get_chip_id() != TEGRA30)
+		return index;
+
+	/* put CPU0 into C1 if C7 is requested and secondaries are online */
+	if (!IS_ENABLED(CONFIG_PM_SLEEP) || num_online_cpus() > 1)
+		index = TEGRA_C1;
+	else
+		index = TEGRA_CC6;
+
+	return index;
+}
+
 static int tegra_cpuidle_enter(struct cpuidle_device *dev,
 			       struct cpuidle_driver *drv,
 			       int index)
@@ -189,10 +217,17 @@ static int tegra_cpuidle_enter(struct cpuidle_device *dev,
 	unsigned int cpu = cpu_logical_map(dev->cpu);
 	int err;
 
-	err = tegra_cpuidle_state_enter(dev, index, cpu);
-	if (err && err != -EINTR)
-		pr_err_once("cpu%u failed to enter idle state %d err: %d\n",
-			    cpu, index, err);
+	index = tegra_cpuidle_adjust_state_index(index, cpu);
+	if (dev->states_usage[index].disable)
+		return -1;
+
+	if (index == TEGRA_C1)
+		err = arm_cpuidle_simple_enter(dev, drv, index);
+	else
+		err = tegra_cpuidle_state_enter(dev, index, cpu);
+
+	if (err && (err != -EINTR || index != TEGRA_CC6))
+		pr_err_once("failed to enter state %d err: %d\n", index, err);
 
 	return err ? -1 : index;
 }
@@ -218,6 +253,15 @@ static struct cpuidle_driver tegra_idle_driver = {
 	.name = "tegra_idle",
 	.states = {
 		[TEGRA_C1] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+		[TEGRA_C7] = {
+			.enter			= tegra_cpuidle_enter,
+			.exit_latency		= 2000,
+			.target_residency	= 2200,
+			.power_usage		= 100,
+			.flags			= CPUIDLE_FLAG_TIMER_STOP,
+			.name			= "C7",
+			.desc			= "CPU core powered off",
+		},
 		[TEGRA_CC6] = {
 			.enter			= tegra_cpuidle_enter,
 			.exit_latency		= 5000,
@@ -262,8 +306,28 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 * Tegra-arch core and PMC driver, is unavailable if PM-sleep option
 	 * is disabled.
 	 */
-	if (!IS_ENABLED(CONFIG_PM_SLEEP))
+	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
+		tegra_cpuidle_disable_state(TEGRA_C7);
 		tegra_cpuidle_disable_state(TEGRA_CC6);
+	}
+
+	/*
+	 * Generic WFI state (also known as C1 or LP3) and the coupled CPU
+	 * cluster power-off (CC6 or LP2) states are common for all Tegra SoCs.
+	 */
+	switch (tegra_get_chip_id()) {
+	case TEGRA20:
+		/* Tegra20 isn't capable to power-off individual CPU cores */
+		tegra_cpuidle_disable_state(TEGRA_C7);
+		break;
+
+	case TEGRA30:
+		tegra_cpuidle_disable_state(TEGRA_CC6);
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
 }
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 14/17] cpuidle: tegra: Squash Tegra114 driver into the common driver
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (12 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver Dmitry Osipenko
@ 2020-02-12 23:51 ` " Dmitry Osipenko
  2020-02-12 23:51 ` [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable Dmitry Osipenko
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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.

Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 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        | 45 ++++++++++++-
 6 files changed, 48 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

diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 99c5f4274e5c..07572b5373b8 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -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
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
deleted file mode 100644
index 858c30cc5dc7..000000000000
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ /dev/null
@@ -1,90 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
- */
-
-#include <asm/firmware.h>
-#include <linux/tick.h>
-#include <linux/cpuidle.h>
-#include <linux/cpu_pm.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-
-#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 "sleep.h"
-
-#ifdef CONFIG_PM_SLEEP
-#define TEGRA114_MAX_STATES 2
-#else
-#define TEGRA114_MAX_STATES 1
-#endif
-
-#ifdef CONFIG_PM_SLEEP
-static int tegra114_idle_power_down(struct cpuidle_device *dev,
-				    struct cpuidle_driver *drv,
-				    int index)
-{
-	local_fiq_disable();
-
-	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_pm_secondary_cpu_suspend);
-
-	cpu_pm_exit();
-	tegra_pm_clear_cpu_in_lp2();
-
-	local_fiq_enable();
-
-	return index;
-}
-
-static void tegra114_idle_enter_s2idle(struct cpuidle_device *dev,
-				       struct cpuidle_driver *drv,
-				       int index)
-{
-       tegra114_idle_power_down(dev, drv, index);
-}
-#endif
-
-static struct cpuidle_driver tegra_idle_driver = {
-	.name = "tegra_idle",
-	.owner = THIS_MODULE,
-	.state_count = TEGRA114_MAX_STATES,
-	.states = {
-		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
-#ifdef CONFIG_PM_SLEEP
-		[1] = {
-			.enter			= tegra114_idle_power_down,
-			.enter_s2idle		= tegra114_idle_enter_s2idle,
-			.exit_latency		= 500,
-			.target_residency	= 1000,
-			.flags			= CPUIDLE_FLAG_TIMER_STOP,
-			.power_usage		= 0,
-			.name			= "powered-down",
-			.desc			= "CPU power gated",
-		},
-#endif
-	},
-};
-
-int __init tegra114_cpuidle_init(void)
-{
-	if (!psci_smp_available())
-		return cpuidle_register(&tegra_idle_driver, NULL);
-
-	return 0;
-}
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
deleted file mode 100644
index fa0dcf3c2c45..000000000000
--- a/arch/arm/mach-tegra/cpuidle.c
+++ /dev/null
@@ -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 <ccross@android.com>
- *         Gary King <gking@nvidia.com>
- *
- * Rework for 3.3 by Peter De Schrijver <pdeschrijver@nvidia.com>
- */
-
-#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;
-	}
-}
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
deleted file mode 100644
index 5423a05a69f6..000000000000
--- a/arch/arm/mach-tegra/cpuidle.h
+++ /dev/null
@@ -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
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 79184a077c84..eeacff626546 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -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"))
@@ -98,6 +97,9 @@ static void __init tegra_dt_init_late(void)
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
 	    of_machine_is_compatible("nvidia,tegra30"))
 		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[] = {
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index 59398c76cf68..b6cdad8437af 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 
 #include <linux/clk/tegra.h>
+#include <linux/firmware/trusted_foundations.h>
 
 #include <soc/tegra/cpuidle.h>
 #include <soc/tegra/flowctrl.h>
@@ -32,6 +33,7 @@
 #include <soc/tegra/pm.h>
 
 #include <asm/cpuidle.h>
+#include <asm/firmware.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 
@@ -45,6 +47,11 @@ enum tegra_state {
 static atomic_t tegra_idle_barrier;
 static atomic_t tegra_abort_flag;
 
+static inline bool tegra_cpuidle_using_firmware(void)
+{
+	return firmware_ops->prepare_idle && firmware_ops->do_idle;
+}
+
 static void tegra_cpuidle_report_cpus_state(void)
 {
 	unsigned long cpu, lcpu, csr;
@@ -122,6 +129,16 @@ static int tegra_cpuidle_cc6_enter(unsigned int cpu)
 
 static int tegra_cpuidle_c7_enter(void)
 {
+	int err;
+
+	if (tegra_cpuidle_using_firmware()) {
+		err = call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
+		if (err)
+			return err;
+
+		return call_firmware_op(do_idle, 0);
+	}
+
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }
 
@@ -232,6 +249,13 @@ static int tegra_cpuidle_enter(struct cpuidle_device *dev,
 	return err ? -1 : index;
 }
 
+static void tegra114_enter_s2idle(struct cpuidle_device *dev,
+				  struct cpuidle_driver *drv,
+				  int index)
+{
+	tegra_cpuidle_enter(dev, drv, index);
+}
+
 /*
  * The previous versions of Tegra CPUIDLE driver used a different "legacy"
  * terminology for naming of the idling states, while this driver uses the
@@ -299,6 +323,15 @@ void tegra_cpuidle_pcie_irqs_in_use(void)
 	tegra_cpuidle_disable_state(TEGRA_CC6);
 }
 
+static void tegra_cpuidle_setup_tegra114_c7_state(void)
+{
+	struct cpuidle_state *s = &tegra_idle_driver.states[TEGRA_C7];
+
+	s->enter_s2idle = tegra114_enter_s2idle;
+	s->target_residency = 1000;
+	s->exit_latency = 500;
+}
+
 static int tegra_cpuidle_probe(struct platform_device *pdev)
 {
 	/*
@@ -307,7 +340,9 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 * is disabled.
 	 */
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
-		tegra_cpuidle_disable_state(TEGRA_C7);
+		if (!tegra_cpuidle_using_firmware())
+			tegra_cpuidle_disable_state(TEGRA_C7);
+
 		tegra_cpuidle_disable_state(TEGRA_CC6);
 	}
 
@@ -325,6 +360,14 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 		tegra_cpuidle_disable_state(TEGRA_CC6);
 		break;
 
+	case TEGRA114:
+	case TEGRA124:
+		tegra_cpuidle_setup_tegra114_c7_state();
+
+		/* coupled CC6 (LP2) state isn't implemented yet */
+		tegra_cpuidle_disable_state(TEGRA_CC6);
+		break;
+
 	default:
 		return -EINVAL;
 	}
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (13 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 14/17] cpuidle: tegra: Squash Tegra114 " Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 16:39   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver Dmitry Osipenko
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/cpuidle/cpuidle-tegra.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index b6cdad8437af..675b8a4464ba 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -31,6 +31,7 @@
 #include <soc/tegra/fuse.h>
 #include <soc/tegra/irq.h>
 #include <soc/tegra/pm.h>
+#include <soc/tegra/pmc.h>
 
 #include <asm/cpuidle.h>
 #include <asm/firmware.h>
@@ -334,6 +335,10 @@ static void tegra_cpuidle_setup_tegra114_c7_state(void)
 
 static int tegra_cpuidle_probe(struct platform_device *pdev)
 {
+	/* LP2 could be disabled in device-tree */
+	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
+		tegra_cpuidle_disable_state(TEGRA_CC6);
+
 	/*
 	 * Required suspend-resume functionality, which is provided by the
 	 * Tegra-arch core and PMC driver, is unavailable if PM-sleep option
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (14 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 16:30   ` Daniel Lezcano
  2020-02-12 23:51 ` [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig Dmitry Osipenko
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 017d65f86eba..7c8a1c310bbb 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -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
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (15 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver Dmitry Osipenko
@ 2020-02-12 23:51 ` Dmitry Osipenko
  2020-02-21 16:31   ` Daniel Lezcano
  2020-02-13  0:38 ` [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-12 23:51 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/configs/tegra_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index a27592d3b1fa..aa94369bdd0f 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -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
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (16 preceding siblings ...)
  2020-02-12 23:51 ` [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig Dmitry Osipenko
@ 2020-02-13  0:38 ` Dmitry Osipenko
  2020-02-18 14:56 ` Dmitry Osipenko
  2020-02-24 16:44 ` Dmitry Osipenko
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-13  0:38 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis, Nicolas Chauvet
  Cc: linux-pm, linux-tegra, linux-kernel

13.02.2020 02:51, 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 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.

I forgot to mention that both cpufreq and cpuidle patchsets were also
tested on AC100 by Nicolas Chauvet and I forgot to ask for the explicit
t-b. Nicolas, thank you very much for all the testing of the
grate-kernel! Please feel free to give yours t-b :)

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

Michał, please let me know if you'll spot any problems with the recent
version of the patches and please feel free to give yours t-b if it
works well.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (17 preceding siblings ...)
  2020-02-13  0:38 ` [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
@ 2020-02-18 14:56 ` Dmitry Osipenko
  2020-02-24 16:44 ` Dmitry Osipenko
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-18 14:56 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano
  Cc: Michał Mirosław, Jasper Korten, David Heidelberg,
	Peter Geis, linux-pm, linux-tegra, linux-kernel

13.02.2020 02:51, 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 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 Rafael and Daniel,

Could you please let us know whether you're fine with the above patches
by giving an ACK to them? My understanding is that Thierry can't take
the cpuidle patches through the Tegra tree without yours ACK. Thanks in
advance!

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-12 23:51 ` [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2 Dmitry Osipenko
@ 2020-02-21 14:55   ` Daniel Lezcano
  2020-02-21 15:43   ` Daniel Lezcano
  1 sibling, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 14:55 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  arch/arm/mach-tegra/cpuidle-tegra20.c | 47 +++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> index 9672c619f4bc..bcc158b72e67 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> @@ -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();
>  
> -- 
> 2.24.0
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20
  2020-02-12 23:51 ` [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20 Dmitry Osipenko
@ 2020-02-21 14:58   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 14:58 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:20AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void
  2020-02-12 23:51 ` [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void Dmitry Osipenko
@ 2020-02-21 15:04   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:04 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:21AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/cpuidle-tegra30.c | 14 ++++----------

Mind to move to drivers/cpuidle ?

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

>  arch/arm/mach-tegra/pm.c              |  8 +-------
>  arch/arm/mach-tegra/pm.h              |  2 +-
>  3 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
> index c6128526877d..a3ce8dabfe18 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra30.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
> @@ -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();
> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> index 1ff499068bb1..a72f9a2d3cb7 100644
> --- a/arch/arm/mach-tegra/pm.c
> +++ b/arch/arm/mach-tegra/pm.c
> @@ -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)
> diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
> index b9cc12222bb1..2c294f6365c0 100644
> --- a/arch/arm/mach-tegra/pm.h
> +++ b/arch/arm/mach-tegra/pm.h
> @@ -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);
>  
> -- 
> 2.24.0
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-12 23:51 ` [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last() Dmitry Osipenko
@ 2020-02-21 15:16   ` Daniel Lezcano
  2020-02-21 17:21     ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:16 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---

[ ... ]

>  	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 ?

>  	cpu_cluster_pm_exit();
> +
> +	return err;
>  }

[ ... ]

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver
  2020-02-12 23:51 ` [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver Dmitry Osipenko
@ 2020-02-21 15:18   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:18 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:23AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

[ ... ]


-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions
  2020-02-12 23:51 ` [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions Dmitry Osipenko
@ 2020-02-21 15:19   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:19 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:24AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic
  2020-02-12 23:51 ` [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic Dmitry Osipenko
@ 2020-02-21 15:25   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:25 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:27AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier
  2020-02-12 23:51 ` [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier Dmitry Osipenko
@ 2020-02-21 15:25   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:25 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:28AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-12 23:51 ` [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2 Dmitry Osipenko
  2020-02-21 14:55   ` Daniel Lezcano
@ 2020-02-21 15:43   ` Daniel Lezcano
  2020-02-21 16:56     ` Dmitry Osipenko
  1 sibling, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:43 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/cpuidle-tegra20.c | 47 +++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> index 9672c619f4bc..bcc158b72e67 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> @@ -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);

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

> +
> +		/*
> +		 * 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.

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

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle
  2020-02-12 23:51 ` [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle Dmitry Osipenko
@ 2020-02-21 15:44   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 15:44 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:29AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  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 ++++++++++++++++++++++++++

Is it possible to use -M option here to make the review easier?


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver
  2020-02-12 23:51 ` [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver Dmitry Osipenko
@ 2020-02-21 16:29   ` Daniel Lezcano
  2020-02-21 16:59     ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 16:29 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:30AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Jasper Korten <jja2000@gmail.com>
> Tested-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/Makefile          |   3 -
>  arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------

Add the -M option when resending please.


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
  2020-02-12 23:51 ` [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver Dmitry Osipenko
@ 2020-02-21 16:30   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 16:30 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:33AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  arch/arm/configs/multi_v7_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 017d65f86eba..7c8a1c310bbb 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -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
> -- 
> 2.24.0
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
  2020-02-12 23:51 ` [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig Dmitry Osipenko
@ 2020-02-21 16:31   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 16:31 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:34AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable
  2020-02-12 23:51 ` [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable Dmitry Osipenko
@ 2020-02-21 16:39   ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 16:39 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Thu, Feb 13, 2020 at 02:51:32AM +0300, Dmitry Osipenko wrote:
> 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 <pdeschrijver@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 15:43   ` Daniel Lezcano
@ 2020-02-21 16:56     ` Dmitry Osipenko
  2020-02-21 17:36       ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 16:56 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

Hello Daniel,

21.02.2020 18:43, Daniel Lezcano пишет:
> On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
>> 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 <pdeschrijver@nvidia.com>
>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>> Tested-by: Jasper Korten <jja2000@gmail.com>
>> Tested-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/cpuidle-tegra20.c | 47 +++++++++++++++++++++++++--
>>  1 file changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
>> index 9672c619f4bc..bcc158b72e67 100644
>> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
>> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
>> @@ -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);
> 
> 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()?

>> +
>> +		/*
>> +		 * 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.
But udelay() also results into CPU spinning in a busy-loop, and thus,
what's the difference?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver
  2020-02-21 16:29   ` Daniel Lezcano
@ 2020-02-21 16:59     ` Dmitry Osipenko
  2020-02-21 17:41       ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 16:59 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 19:29, Daniel Lezcano пишет:
> On Thu, Feb 13, 2020 at 02:51:30AM +0300, Dmitry Osipenko wrote:
>> 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 <pdeschrijver@nvidia.com>
>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>> Tested-by: Jasper Korten <jja2000@gmail.com>
>> Tested-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/Makefile          |   3 -
>>  arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------
> 
> Add the -M option when resending please.

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

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-21 15:16   ` Daniel Lezcano
@ 2020-02-21 17:21     ` Dmitry Osipenko
  2020-02-21 17:40       ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 17:21 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 18:16, Daniel Lezcano пишет:
> On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
>> 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 <pdeschrijver@nvidia.com>
>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>> Tested-by: Jasper Korten <jja2000@gmail.com>
>> Tested-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
> 
> [ ... ]
> 
>>  	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 ?

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.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 16:56     ` Dmitry Osipenko
@ 2020-02-21 17:36       ` Daniel Lezcano
  2020-02-21 18:19         ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 17:36 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Fri, Feb 21, 2020 at 07:56:51PM +0300, Dmitry Osipenko wrote:
> Hello Daniel,
> 
> 21.02.2020 18:43, Daniel Lezcano пишет:
> > On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
> >> 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 <pdeschrijver@nvidia.com>
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>
> >> Tested-by: Jasper Korten <jja2000@gmail.com>
> >> Tested-by: David Heidelberg <david@ixit.cz>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---

[ ... ]

> >> +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()?

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

> >> +
> >> +		/*
> >> +		 * 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.
> 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.

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-21 17:21     ` Dmitry Osipenko
@ 2020-02-21 17:40       ` Daniel Lezcano
  2020-02-21 18:42         ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 17:40 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Fri, Feb 21, 2020 at 08:21:41PM +0300, Dmitry Osipenko wrote:
> 21.02.2020 18:16, Daniel Lezcano пишет:
> > On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
> >> 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 <pdeschrijver@nvidia.com>
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>
> >> Tested-by: Jasper Korten <jja2000@gmail.com>
> >> Tested-by: David Heidelberg <david@ixit.cz>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> > 
> > [ ... ]
> > 
> >>  	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 ?
> 
> 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.

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver
  2020-02-21 16:59     ` Dmitry Osipenko
@ 2020-02-21 17:41       ` Daniel Lezcano
  2020-02-21 18:20         ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 17:41 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On Fri, Feb 21, 2020 at 07:59:14PM +0300, Dmitry Osipenko wrote:
> 21.02.2020 19:29, Daniel Lezcano пишет:
> > On Thu, Feb 13, 2020 at 02:51:30AM +0300, Dmitry Osipenko wrote:
> >> 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 <pdeschrijver@nvidia.com>
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>
> >> Tested-by: Jasper Korten <jja2000@gmail.com>
> >> Tested-by: David Heidelberg <david@ixit.cz>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  arch/arm/mach-tegra/Makefile          |   3 -
> >>  arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------
> > 
> > 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.

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 17:36       ` Daniel Lezcano
@ 2020-02-21 18:19         ` Dmitry Osipenko
  2020-02-21 20:02           ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 18:19 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 20:36, Daniel Lezcano пишет:
> On Fri, Feb 21, 2020 at 07:56:51PM +0300, Dmitry Osipenko wrote:
>> Hello Daniel,
>>
>> 21.02.2020 18:43, Daniel Lezcano пишет:
>>> On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
>>>> 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 <pdeschrijver@nvidia.com>
>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
> 
> [ ... ]
> 
>>>> +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()?
> 
> 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.

>>>> +
>>>> +		/*
>>>> +		 * 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.
>> 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.

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?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver
  2020-02-21 17:41       ` Daniel Lezcano
@ 2020-02-21 18:20         ` Dmitry Osipenko
  0 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 18:20 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 20:41, Daniel Lezcano пишет:
> On Fri, Feb 21, 2020 at 07:59:14PM +0300, Dmitry Osipenko wrote:
>> 21.02.2020 19:29, Daniel Lezcano пишет:
>>> On Thu, Feb 13, 2020 at 02:51:30AM +0300, Dmitry Osipenko wrote:
>>>> 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 <pdeschrijver@nvidia.com>
>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  arch/arm/mach-tegra/Makefile          |   3 -
>>>>  arch/arm/mach-tegra/cpuidle-tegra30.c | 123 --------------------------
>>>
>>> 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.

No problems, thank you :)

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-21 17:40       ` Daniel Lezcano
@ 2020-02-21 18:42         ` Dmitry Osipenko
  2020-02-21 19:16           ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 18:42 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 20:40, Daniel Lezcano пишет:
> On Fri, Feb 21, 2020 at 08:21:41PM +0300, Dmitry Osipenko wrote:
>> 21.02.2020 18:16, Daniel Lezcano пишет:
>>> On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
>>>> 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 <pdeschrijver@nvidia.com>
>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>
>>> [ ... ]
>>>
>>>>  	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 ?
>>
>> 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 :)

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last()
  2020-02-21 18:42         ` Dmitry Osipenko
@ 2020-02-21 19:16           ` Daniel Lezcano
  0 siblings, 0 replies; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 19:16 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On 21/02/2020 19:42, Dmitry Osipenko wrote:
> 21.02.2020 20:40, Daniel Lezcano пишет:
>> On Fri, Feb 21, 2020 at 08:21:41PM +0300, Dmitry Osipenko wrote:
>>> 21.02.2020 18:16, Daniel Lezcano пишет:
>>>> On Thu, Feb 13, 2020 at 02:51:22AM +0300, Dmitry Osipenko wrote:
>>>>> 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 <pdeschrijver@nvidia.com>
>>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>
>>>> [ ... ]
>>>>
>>>>>  	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 ?
>>>
>>> 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 :)

Yep, not a big deal.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 18:19         ` Dmitry Osipenko
@ 2020-02-21 20:02           ` Daniel Lezcano
  2020-02-21 20:21             ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 20:02 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On 21/02/2020 19:19, Dmitry Osipenko wrote:
> 21.02.2020 20:36, Daniel Lezcano пишет:
>> On Fri, Feb 21, 2020 at 07:56:51PM +0300, Dmitry Osipenko wrote:
>>> Hello Daniel,
>>>
>>> 21.02.2020 18:43, Daniel Lezcano пишет:
>>>> On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
>>>>> 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 <pdeschrijver@nvidia.com>
>>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>
>> [ ... ]
>>
>>>>> +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()?
>>
>> 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.

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

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

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




-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 20:02           ` Daniel Lezcano
@ 2020-02-21 20:21             ` Dmitry Osipenko
  2020-02-21 20:42               ` Dmitry Osipenko
  2020-02-21 20:48               ` Daniel Lezcano
  0 siblings, 2 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 20:21 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 23:02, Daniel Lezcano пишет:
> On 21/02/2020 19:19, Dmitry Osipenko wrote:
>> 21.02.2020 20:36, Daniel Lezcano пишет:
>>> On Fri, Feb 21, 2020 at 07:56:51PM +0300, Dmitry Osipenko wrote:
>>>> Hello Daniel,
>>>>
>>>> 21.02.2020 18:43, Daniel Lezcano пишет:
>>>>> On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
>>>>>> 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 <pdeschrijver@nvidia.com>
>>>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>> ---
>>>
>>> [ ... ]
>>>
>>>>>> +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()?
>>>
>>> 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.

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

>>>>>> +
>>>>>> +		/*
>>>>>> +		 * 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.
>>>> 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.
>>
>> 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

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 20:21             ` Dmitry Osipenko
@ 2020-02-21 20:42               ` Dmitry Osipenko
  2020-02-21 20:48               ` Daniel Lezcano
  1 sibling, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 20:42 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 23:21, Dmitry Osipenko пишет:
> 21.02.2020 23:02, Daniel Lezcano пишет:
>> On 21/02/2020 19:19, Dmitry Osipenko wrote:
>>> 21.02.2020 20:36, Daniel Lezcano пишет:
>>>> On Fri, Feb 21, 2020 at 07:56:51PM +0300, Dmitry Osipenko wrote:
>>>>> Hello Daniel,
>>>>>
>>>>> 21.02.2020 18:43, Daniel Lezcano пишет:
>>>>>> On Thu, Feb 13, 2020 at 02:51:26AM +0300, Dmitry Osipenko wrote:
>>>>>>> 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 <pdeschrijver@nvidia.com>
>>>>>>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>>>>>> Tested-by: Jasper Korten <jja2000@gmail.com>
>>>>>>> Tested-by: David Heidelberg <david@ixit.cz>
>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>> ---
>>>>
>>>> [ ... ]
>>>>
>>>>>>> +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()?
>>>>
>>>> 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.
> 
>>> 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.
>>
>>> 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.
> 
>>>>>>> +
>>>>>>> +		/*
>>>>>>> +		 * 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.
>>>>> 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.
>>>
>>> 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

I'll try the udelay()-loop over the weekend and will see if it makes any
real difference, maybe I'm missing something.

If it doesn't make any difference, I'll leave this patch as-is, okay?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 20:21             ` Dmitry Osipenko
  2020-02-21 20:42               ` Dmitry Osipenko
@ 2020-02-21 20:48               ` Daniel Lezcano
  2020-02-21 20:54                 ` Dmitry Osipenko
  1 sibling, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 20:48 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On 21/02/2020 21:21, Dmitry Osipenko wrote:
> 21.02.2020 23:02, Daniel Lezcano пишет:

[ ... ]

>>>>>>> +
>>>>>>> +		/*
>>>>>>> +		 * 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.
>>>>> 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.
>>>
>>> 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

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

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 20:48               ` Daniel Lezcano
@ 2020-02-21 20:54                 ` Dmitry Osipenko
  2020-02-21 21:11                   ` Daniel Lezcano
  0 siblings, 1 reply; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-21 20:54 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

21.02.2020 23:48, Daniel Lezcano пишет:
> On 21/02/2020 21:21, Dmitry Osipenko wrote:
>> 21.02.2020 23:02, Daniel Lezcano пишет:
> 
> [ ... ]
> 
>>>>>>>> +
>>>>>>>> +		/*
>>>>>>>> +		 * 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.
>>>>>> 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.
>>>>
>>>> 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
> 
> 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.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 20:54                 ` Dmitry Osipenko
@ 2020-02-21 21:11                   ` Daniel Lezcano
  2020-02-24 15:12                     ` Dmitry Osipenko
  0 siblings, 1 reply; 53+ messages in thread
From: Daniel Lezcano @ 2020-02-21 21:11 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

On 21/02/2020 21:54, Dmitry Osipenko wrote:
> 21.02.2020 23:48, Daniel Lezcano пишет:
>> On 21/02/2020 21:21, Dmitry Osipenko wrote:
>>> 21.02.2020 23:02, Daniel Lezcano пишет:
>>
>> [ ... ]
>>
>>>>>>>>> +
>>>>>>>>> +		/*
>>>>>>>>> +		 * 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.
>>>>>>> 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.
>>>>>
>>>>> 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
>>
>> 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.

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2
  2020-02-21 21:11                   ` Daniel Lezcano
@ 2020-02-24 15:12                     ` Dmitry Osipenko
  0 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-24 15:12 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Michał Mirosław, Jasper Korten,
	David Heidelberg, Peter Geis, linux-pm, linux-tegra,
	linux-kernel

22.02.2020 00:11, Daniel Lezcano пишет:
> On 21/02/2020 21:54, Dmitry Osipenko wrote:
>> 21.02.2020 23:48, Daniel Lezcano пишет:
>>> On 21/02/2020 21:21, Dmitry Osipenko wrote:
>>>> 21.02.2020 23:02, Daniel Lezcano пишет:
>>>
>>> [ ... ]
>>>
>>>>>>>>>> +
>>>>>>>>>> +		/*
>>>>>>>>>> +		 * 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.
>>>>>>>> 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.
>>>>>>
>>>>>> 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
>>>
>>> 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?

There are always at least a few dozens of microseconds to wait, so
something like udelay(10) should be a bit better variant anyways.

> Anyway, I'll let you give a try.
Turned out that udelay(10) is noticeably better when system is running
on a lower freqs in comparison to ktime_get(). I'll switch to udelay in
v10, thank you very much for the suggestion!

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)
  2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
                   ` (18 preceding siblings ...)
  2020-02-18 14:56 ` Dmitry Osipenko
@ 2020-02-24 16:44 ` Dmitry Osipenko
  19 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2020-02-24 16:44 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław,
	Jasper Korten, David Heidelberg, Peter Geis
  Cc: linux-pm, linux-tegra, linux-kernel

13.02.2020 02:51, 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 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.

I just noticed that this patchset was versioned wrongly, it should've
been v10. Sorry for the confusion, next version will be v11.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig
  2019-12-18 21:04 Dmitry Osipenko
@ 2019-12-18 21:05 ` Dmitry Osipenko
  0 siblings, 0 replies; 53+ messages in thread
From: Dmitry Osipenko @ 2019-12-18 21:05 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Rafael J. Wysocki, Daniel Lezcano, Michał Mirosław
  Cc: linux-pm, linux-tegra, linux-kernel

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 <pdeschrijver@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/configs/tegra_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index a27592d3b1fa..aa94369bdd0f 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -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
-- 
2.24.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, back to index

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 23:51 [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 01/17] ARM: tegra: Compile sleep-tegra20/30.S unconditionally Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 02/17] ARM: tegra: Add tegra_pm_park_secondary_cpu() Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 03/17] ARM: tegra: Remove pen-locking from cpuidle-tegra20 Dmitry Osipenko
2020-02-21 14:58   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 04/17] ARM: tegra: Change tegra_set_cpu_in_lp2() type to void Dmitry Osipenko
2020-02-21 15:04   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 05/17] ARM: tegra: Propagate error from tegra_idle_lp2_last() Dmitry Osipenko
2020-02-21 15:16   ` Daniel Lezcano
2020-02-21 17:21     ` Dmitry Osipenko
2020-02-21 17:40       ` Daniel Lezcano
2020-02-21 18:42         ` Dmitry Osipenko
2020-02-21 19:16           ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 06/17] ARM: tegra: Expose PM functions required for new cpuidle driver Dmitry Osipenko
2020-02-21 15:18   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 07/17] ARM: tegra: Rename some of the newly exposed PM functions Dmitry Osipenko
2020-02-21 15:19   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 08/17] ARM: tegra: Make outer_disable() open-coded Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 09/17] arm: tegra20: cpuidle: Handle case where secondary CPU hangs on entering LP2 Dmitry Osipenko
2020-02-21 14:55   ` Daniel Lezcano
2020-02-21 15:43   ` Daniel Lezcano
2020-02-21 16:56     ` Dmitry Osipenko
2020-02-21 17:36       ` Daniel Lezcano
2020-02-21 18:19         ` Dmitry Osipenko
2020-02-21 20:02           ` Daniel Lezcano
2020-02-21 20:21             ` Dmitry Osipenko
2020-02-21 20:42               ` Dmitry Osipenko
2020-02-21 20:48               ` Daniel Lezcano
2020-02-21 20:54                 ` Dmitry Osipenko
2020-02-21 21:11                   ` Daniel Lezcano
2020-02-24 15:12                     ` Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 10/17] arm: tegra20: cpuidle: Make abort_flag atomic Dmitry Osipenko
2020-02-21 15:25   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 11/17] arm: tegra20/30: cpuidle: Remove unnecessary memory barrier Dmitry Osipenko
2020-02-21 15:25   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 12/17] cpuidle: Refactor and move out NVIDIA Tegra20 driver into drivers/cpuidle Dmitry Osipenko
2020-02-21 15:44   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 13/17] cpuidle: tegra: Squash Tegra30 driver into the common driver Dmitry Osipenko
2020-02-21 16:29   ` Daniel Lezcano
2020-02-21 16:59     ` Dmitry Osipenko
2020-02-21 17:41       ` Daniel Lezcano
2020-02-21 18:20         ` Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 14/17] cpuidle: tegra: Squash Tegra114 " Dmitry Osipenko
2020-02-12 23:51 ` [PATCH v9 15/17] cpuidle: tegra: Disable CC6 state if LP2 unavailable Dmitry Osipenko
2020-02-21 16:39   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 16/17] ARM: multi_v7_defconfig: Enable Tegra cpuidle driver Dmitry Osipenko
2020-02-21 16:30   ` Daniel Lezcano
2020-02-12 23:51 ` [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig Dmitry Osipenko
2020-02-21 16:31   ` Daniel Lezcano
2020-02-13  0:38 ` [PATCH v9 00/17] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) Dmitry Osipenko
2020-02-18 14:56 ` Dmitry Osipenko
2020-02-24 16:44 ` Dmitry Osipenko
  -- strict thread matches above, loose matches on Subject: below --
2019-12-18 21:04 Dmitry Osipenko
2019-12-18 21:05 ` [PATCH v9 17/17] ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig Dmitry Osipenko

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git