linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] Revert "cpufreq: dt: Identify cpu-sharing for platforms without operating-points-v2"
@ 2016-06-16  5:35 Alexandre Courbot
  2016-06-16  6:35 ` Viresh Kumar
  0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Courbot @ 2016-06-16  5:35 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linux-pm, linux-tegra, linux-kernel, gnurou, Alexandre Courbot

This is not an actual request for revert, but rather for comments about
the observed behavior since I am not really familiar with cpufreq.

I am observing a serious performance regression on Jetson TK1 since 4.7-rc1:
namely, moving windows under X would become unsufferably slow, and graphical
performance under X in general is seriously degraded. After bisecting, I
found the regression to be visible since commit 1530b9963eeb ("cpufreq: dt: 
Identify cpu-sharing for platforms without operating-points-v2")

If I revert this commit, I noticed that the CPU frequency immediately jumps to
a higher frequency once I start moving windows (resulting in a smooth and
responsive action), whereas enabling this commit causes the CPU frequency to
remain low (typically 204 Mhz) in that case, resulting in CPU-bound slowness.

What happens is that with 1530b9963eeb applied, dev_pm_opp_get_sharing_cpus()
returns zero causing the fallback variable to remain false, whereas without it
opp_v1 is set to true.

It is not clear to me whether this is a cpufreq issue or a Tegra issue, so I am
posting this in the hope to get clarifications from either side.

Thanks!

---
 drivers/cpufreq/cpufreq-dt.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 3957de801ae8..eef245cd4b7f 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -146,7 +146,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	struct clk *cpu_clk;
 	struct dev_pm_opp *suspend_opp;
 	unsigned int transition_latency;
-	bool fallback = false;
+	bool opp_v1 = false;
 	const char *name;
 	int ret;
 
@@ -166,16 +166,14 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	/* Get OPP-sharing information from "operating-points-v2" bindings */
 	ret = dev_pm_opp_of_get_sharing_cpus(cpu_dev, policy->cpus);
 	if (ret) {
-		if (ret != -ENOENT)
-			goto out_put_clk;
-
 		/*
 		 * operating-points-v2 not supported, fallback to old method of
-		 * finding shared-OPPs for backward compatibility if the
-		 * platform hasn't set sharing CPUs.
+		 * finding shared-OPPs for backward compatibility.
 		 */
-		if (dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus))
-			fallback = true;
+		if (ret == -ENOENT)
+			opp_v1 = true;
+		else
+			goto out_put_clk;
 	}
 
 	/*
@@ -215,7 +213,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 		goto out_free_opp;
 	}
 
-	if (fallback) {
+	if (opp_v1) {
 		cpumask_setall(policy->cpus);
 
 		/*
-- 
2.8.3

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

* Re: [PATCH RFC] Revert "cpufreq: dt: Identify cpu-sharing for platforms without operating-points-v2"
  2016-06-16  5:35 [PATCH RFC] Revert "cpufreq: dt: Identify cpu-sharing for platforms without operating-points-v2" Alexandre Courbot
@ 2016-06-16  6:35 ` Viresh Kumar
  2016-06-16  6:57   ` Alexandre Courbot
  0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2016-06-16  6:35 UTC (permalink / raw)
  To: Alexandre Courbot, Rafael J. Wysocki
  Cc: linux-pm, linux-tegra, linux-kernel, gnurou

+Rafael

On 16-06-16, 14:35, Alexandre Courbot wrote:
> This is not an actual request for revert, but rather for comments about
> the observed behavior since I am not really familiar with cpufreq.
> 
> I am observing a serious performance regression on Jetson TK1 since 4.7-rc1:
> namely, moving windows under X would become unsufferably slow, and graphical
> performance under X in general is seriously degraded. After bisecting, I
> found the regression to be visible since commit 1530b9963eeb ("cpufreq: dt: 
> Identify cpu-sharing for platforms without operating-points-v2")
> 
> If I revert this commit, I noticed that the CPU frequency immediately jumps to
> a higher frequency once I start moving windows (resulting in a smooth and
> responsive action), whereas enabling this commit causes the CPU frequency to
> remain low (typically 204 Mhz) in that case, resulting in CPU-bound slowness.
> 
> What happens is that with 1530b9963eeb applied, dev_pm_opp_get_sharing_cpus()
> returns zero causing the fallback variable to remain false, whereas without it
> opp_v1 is set to true.
> 
> It is not clear to me whether this is a cpufreq issue or a Tegra issue, so I am
> posting this in the hope to get clarifications from either side.
> 
> Thanks!

Yeah, you just hit a corner case. Sorry about that :(

Can you try a patch that I sent to you instead? Its subject is:

[PATCH] PM / OPP: 'UNKNOWN' status of opp-table->shared

Also I would like to know one more thing. You can hit this bug only if
some other piece of code for your platform is creating OPP table for
the CPUs. Which code is doing that ?

-- 
viresh

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

* Re: [PATCH RFC] Revert "cpufreq: dt: Identify cpu-sharing for platforms without operating-points-v2"
  2016-06-16  6:35 ` Viresh Kumar
@ 2016-06-16  6:57   ` Alexandre Courbot
  0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Courbot @ 2016-06-16  6:57 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, Thierry Reding
  Cc: linux-pm, linux-tegra, linux-kernel, gnurou

On 06/16/2016 03:35 PM, Viresh Kumar wrote:
> +Rafael
>
> On 16-06-16, 14:35, Alexandre Courbot wrote:
>> This is not an actual request for revert, but rather for comments about
>> the observed behavior since I am not really familiar with cpufreq.
>>
>> I am observing a serious performance regression on Jetson TK1 since 4.7-rc1:
>> namely, moving windows under X would become unsufferably slow, and graphical
>> performance under X in general is seriously degraded. After bisecting, I
>> found the regression to be visible since commit 1530b9963eeb ("cpufreq: dt:
>> Identify cpu-sharing for platforms without operating-points-v2")
>>
>> If I revert this commit, I noticed that the CPU frequency immediately jumps to
>> a higher frequency once I start moving windows (resulting in a smooth and
>> responsive action), whereas enabling this commit causes the CPU frequency to
>> remain low (typically 204 Mhz) in that case, resulting in CPU-bound slowness.
>>
>> What happens is that with 1530b9963eeb applied, dev_pm_opp_get_sharing_cpus()
>> returns zero causing the fallback variable to remain false, whereas without it
>> opp_v1 is set to true.
>>
>> It is not clear to me whether this is a cpufreq issue or a Tegra issue, so I am
>> posting this in the hope to get clarifications from either side.
>>
>> Thanks!
>
> Yeah, you just hit a corner case. Sorry about that :(
>
> Can you try a patch that I sent to you instead? Its subject is:
>
> [PATCH] PM / OPP: 'UNKNOWN' status of opp-table->shared

Thanks, this seems to solve the issue.

> Also I would like to know one more thing. You can hit this bug only if
> some other piece of code for your platform is creating OPP table for
> the CPUs. Which code is doing that ?

Good question. I am still educating myself about OPP - adding Thierry in 
the meantime who may have an answer.

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

end of thread, other threads:[~2016-06-16  6:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16  5:35 [PATCH RFC] Revert "cpufreq: dt: Identify cpu-sharing for platforms without operating-points-v2" Alexandre Courbot
2016-06-16  6:35 ` Viresh Kumar
2016-06-16  6:57   ` Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).