cpuidle: tegra: Fix C7 idling state on Tegra114

PATCH RESEND Applied Dmitry Osipenko 3 participants 12 messages Mar 3, 2021 – Mar 4, 2021

Subsystems (2)

Tegra architecture support CPU idle time management framework

lore

Share & tools

lore

Dmitry Osipenko

author

2021-03-03 03:36 UTC #0 lore raw

PATCH RESEND v2 1/2 cpuidle: tegra: Fix C7 idling state on Tegra114

Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.

Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")

Cc [email protected] # 5.7+

Reported-by Anton Bambura <[email protected]> # TF701 T114

Tested-by Anton Bambura <[email protected]> # TF701 T114

Tested-by Matt Merhar <[email protected]> # Ouya T30

Tested-by Peter Geis <[email protected]> # Ouya T30

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

---

Changelog:

v2: - No changes. V1 got no attention, hence re-sending.

 drivers/cpuidle/cpuidle-tegra.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/cpuidle/cpuidle-tegra.c +6 -6

index 191966dc8d02..29c5e83500d3 100644
@@ -135,13 +135,13 @@ 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;
+	err = call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
+	if (err && err != -ENOSYS)
+		return err;
 
-		return call_firmware_op(do_idle, 0);
-	}
+	err = call_firmware_op(do_idle, 0);
+	if (err != -ENOSYS)
+		return err;
 
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }

Signature · 1 line

2.29.2

Dmitry Osipenko

author

2021-03-03 03:36 UTC #1 lore raw

PATCH RESEND v2 2/2 cpuidle: tegra: Remove do_idle firmware call

The do_idle firmware call is unused by all Tegra SoCs, hence remove it in order to keep driver's code clean.

Tested-by Anton Bambura <[email protected]> # TF701 T114

Tested-by Matt Merhar <[email protected]> # Ouya T30

Tested-by Peter Geis <[email protected]> # Ouya T30

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

---

Changelog:

v2: - No changes. V1 got no attention, hence re-sending.

 drivers/cpuidle/cpuidle-tegra.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

drivers/cpuidle/cpuidle-tegra.c +1 ~~-12~~

index 29c5e83500d3..508bd9f23792 100644
@@ -48,11 +48,6 @@ 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;

@@ -139,10 +134,6 @@ static int tegra_cpuidle_c7_enter(void)
 	if (err && err != -ENOSYS)
 		return err;
 
-	err = call_firmware_op(do_idle, 0);
-	if (err != -ENOSYS)
-		return err;
-
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }

@@ -356,9 +347,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 * is disabled.
 	 */
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
-		if (!tegra_cpuidle_using_firmware())
-			tegra_cpuidle_disable_state(TEGRA_C7);
-
+		tegra_cpuidle_disable_state(TEGRA_C7);
 		tegra_cpuidle_disable_state(TEGRA_CC6);
 	}

Signature · 1 line

2.29.2

Daniel Lezcano

2021-03-03 03:36 UTC #2 lore raw

Re: [PATCH RESEND v2 2/2] cpuidle: tegra: Remove do_idle firmware call

Quoted 44 lines · Dmitry Osipenko

The do_idle firmware call is unused by all Tegra SoCs, hence remove it in order to keep driver's code clean.
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
---
Changelog:
v2: - No changes. V1 got no attention, hence re-sending.
 drivers/cpuidle/cpuidle-tegra.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/cpuidle/cpuidle-tegra.c +0 -9
index 29c5e83500d3..508bd9f23792 100644
@@ -48,11 +48,6 @@ 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;
@@ -139,10 +134,6 @@ static int tegra_cpuidle_c7_enter(void)
 	if (err && err != -ENOSYS)
 		return err;
 
-	err = call_firmware_op(do_idle, 0);
-	if (err != -ENOSYS)
-		return err;
-
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }
 

drivers/cpuidle/cpuidle-tegra.c static int tegra_cpuidle_probe(struct platform_device *pdev) · line 360

Quoted 5 lines

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

@@ -356,9 +347,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 * is disabled.
 	 */
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
-		if (!tegra_cpuidle_using_firmware())
-			tegra_cpuidle_disable_state(TEGRA_C7);

So firmware_ops->do_idle is always NULL, thus tegra_cpuidle_using_firmware() is always false and tegra_cpuidle_disable_state() always called, right ?

Quoted 5 lines

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

+		tegra_cpuidle_disable_state(TEGRA_C7);
 		tegra_cpuidle_disable_state(TEGRA_CC6);
 	}

Signature · 5 lines

<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

Dmitry Osipenko

author

2021-03-03 03:36 UTC #3 lore raw

Re: [PATCH RESEND v2 2/2] cpuidle: tegra: Remove do_idle firmware call

02.03.2021 15:45, Daniel Lezcano пишет:

Quoted 44 lines · Dmitry Osipenko

Quoted 43 lines
The do_idle firmware call is unused by all Tegra SoCs, hence remove it in order to keep driver's code clean.
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
---
Changelog:
v2: - No changes. V1 got no attention, hence re-sending.
 drivers/cpuidle/cpuidle-tegra.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)
drivers/cpuidle/cpuidle-tegra.c +0 -9
index 29c5e83500d3..508bd9f23792 100644
@@ -48,11 +48,6 @@ 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;
@@ -139,10 +134,6 @@ static int tegra_cpuidle_c7_enter(void)
 	if (err && err != -ENOSYS)
 		return err;
 
-	err = call_firmware_op(do_idle, 0);
-	if (err != -ENOSYS)
-		return err;
-
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }
 

Quoted 10 lines

drivers/cpuidle/cpuidle-tegra.c static int tegra_cpuidle_probe(struct platform_device *pdev) · line 360
Quoted 6 lines
drivers/cpuidle/cpuidle-tegra.c +0 -2
@@ -356,9 +347,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 * is disabled.
 	 */
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
-		if (!tegra_cpuidle_using_firmware())
-			tegra_cpuidle_disable_state(TEGRA_C7);
So firmware_ops->do_idle is always NULL, thus tegra_cpuidle_using_firmware() is always false and tegra_cpuidle_disable_state() always called, right ?

Yes, the tegra_cpuidle_disable_state(TEGRA_C7) is always called if CONFIG_PM_SLEEP is disabled in kernel config.

Daniel Lezcano

Reviewed-by

2021-03-03 03:36 UTC #4 lore raw

Re: [PATCH RESEND v2 2/2] cpuidle: tegra: Remove do_idle firmware call

Quoted 10 lines · Dmitry Osipenko

02.03.2021 15:45, Daniel Lezcano пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 8 lines
The do_idle firmware call is unused by all Tegra SoCs, hence remove it in order to keep driver's code clean.
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
---

[ ... ]

Quoted 10 lines

Quoted 7 lines
drivers/cpuidle/cpuidle-tegra.c static int tegra_cpuidle_probe(struct platform_device *pdev) · line 360
Quoted 3 lines
drivers/cpuidle/cpuidle-tegra.c +0 -2
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
-		if (!tegra_cpuidle_using_firmware())
-			tegra_cpuidle_disable_state(TEGRA_C7);
So firmware_ops->do_idle is always NULL, thus tegra_cpuidle_using_firmware() is always false and tegra_cpuidle_disable_state() always called, right ?
Yes, the tegra_cpuidle_disable_state(TEGRA_C7) is always called if CONFIG_PM_SLEEP is disabled in kernel config.

Ok, thanks.

Reviewed-by Daniel Lezcano <[email protected]>

Signature · 5 lines

<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

Daniel Lezcano

Applied

2021-03-04 14:23 UTC #5 lore raw

Re: [PATCH RESEND v2 2/2] cpuidle: tegra: Remove do_idle firmware call

In reply to Dmitry Osipenko

Quoted 8 lines · Dmitry Osipenko

The do_idle firmware call is unused by all Tegra SoCs, hence remove it in order to keep driver's code clean.
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
---

Applied, thanks

Signature · 5 lines

<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

Daniel Lezcano

Reviewed-by on v2 1/2

2021-03-03 03:36 UTC #6 lore raw

In reply to Dmitry Osipenko

Quoted 6 lines · Dmitry Osipenko

Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.

Quoted 7 lines

Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>

Reviewed-by Daniel Lezcano <[email protected]>

Quoted 34 lines

---

Changelog:

v2: - No changes. V1 got no attention, hence re-sending.

 drivers/cpuidle/cpuidle-tegra.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/cpuidle/cpuidle-tegra.c +6 -6

index 191966dc8d02..29c5e83500d3 100644
@@ -135,13 +135,13 @@ 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;
+	err = call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
+	if (err && err != -ENOSYS)
+		return err;
 
-		return call_firmware_op(do_idle, 0);
-	}
+	err = call_firmware_op(do_idle, 0);
+	if (err != -ENOSYS)
+		return err;
 
 	return cpu_suspend(0, tegra30_pm_secondary_cpu_suspend);
 }

Signature · 5 lines

<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

Rafael J. Wysocki

maintainer

2021-03-04 13:57 UTC #7 lore raw

Quoted 16 lines · Daniel Lezcano

Quoted 6 lines · Dmitry Osipenko
Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.
Quoted 7 lines
Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 1 line

Reviewed-by Daniel Lezcano <[email protected]>

So do I gather correctly that I am expected to pick up this series?

Daniel Lezcano

2021-03-04 14:06 UTC #8 lore raw

Quoted 17 lines · Rafael J. Wysocki

Quoted 16 lines · Daniel Lezcano
Quoted 13 lines · Dmitry Osipenko
Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.
Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 3 lines

Quoted 1 line
Reviewed-by Daniel Lezcano <[email protected]>
So do I gather correctly that I am expected to pick up this series?

I had picked the cpuidle related patches in the past. As the traffic became low, I assumed you directly pick them.

But I can take care of them and send a PR at -rc5 like before, especially that we have new driver coming. It is not a problem.

Let me know what is you preferred way.

Signature · 5 lines

<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

Rafael J. Wysocki

maintainer

2021-03-04 14:10 UTC #9 lore raw

Quoted 23 lines · Daniel Lezcano

Quoted 17 lines · Rafael J. Wysocki
Quoted 16 lines · Daniel Lezcano
Quoted 13 lines · Dmitry Osipenko
Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.
Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
Quoted 3 lines
Quoted 1 line
Reviewed-by Daniel Lezcano <[email protected]>
So do I gather correctly that I am expected to pick up this series?

Quoted 7 lines

I had picked the cpuidle related patches in the past. As the traffic became low, I assumed you directly pick them.
But I can take care of them and send a PR at -rc5 like before, especially that we have new driver coming. It is not a problem.
Let me know what is you preferred way.

If you can take care of ARM-specific cpuidle changes, that'll help.

Thanks!

Daniel Lezcano

2021-03-04 14:10 UTC #10 lore raw

Quoted 27 lines · Rafael J. Wysocki

Quoted 26 lines · Daniel Lezcano
Quoted 17 lines · Rafael J. Wysocki
Quoted 16 lines · Daniel Lezcano
Quoted 13 lines · Dmitry Osipenko
Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.
Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>
Quoted 3 lines
Quoted 1 line
Reviewed-by Daniel Lezcano <[email protected]>
So do I gather correctly that I am expected to pick up this series?
I had picked the cpuidle related patches in the past. As the traffic became low, I assumed you directly pick them.

Quoted 6 lines

Quoted 4 lines
But I can take care of them and send a PR at -rc5 like before, especially that we have new driver coming. It is not a problem.
Let me know what is you preferred way.
If you can take care of ARM-specific cpuidle changes, that'll help.

No problem, thanks

Signature · 5 lines

<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

Daniel Lezcano

Applied

2021-03-04 14:23 UTC #11 lore raw

In reply to Dmitry Osipenko

Quoted 6 lines · Dmitry Osipenko

Trusted Foundation firmware doesn't implement the do_idle call and in this case suspending should fall back to the common suspend path. In order to fix this issue we will unconditionally set the NOFLUSH_L2 mode via firmware call, which is a NO-OP on Tegra30/124, and then proceed to the C7 idling, like it was done by the older Tegra114 cpuidle driver.

Quoted 7 lines

Fixes 14e086baca50 ("cpuidle: tegra: Squash Tegra114 driver into the common driver")
Cc [email protected] # 5.7+
Reported-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Anton Bambura <[email protected]> # TF701 T114
Tested-by Matt Merhar <[email protected]> # Ouya T30
Tested-by Peter Geis <[email protected]> # Ouya T30
Signed-off-by Dmitry Osipenko <[email protected]>

Applied, thanks

Signature · 5 lines

<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