Re: brocken devfreq simple_ondemand for Odroid XU3/4?

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: Kamil Konieczny <k.konieczny@samsung.com>,
	Willy Wolff <willy.mh.wolff.ml@gmail.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	linux-pm@vger.kernel.org,
	"linux-samsung-soc@vger.kernel.org" 
	<linux-samsung-soc@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: brocken devfreq simple_ondemand for Odroid XU3/4?
Date: Wed, 24 Jun 2020 15:13:41 +0200	[thread overview]
Message-ID: <20200624131341.GA20905@pi3> (raw)
In-Reply-To: <55772862-ff8f-1e1d-91ae-7b4d7c3be1b6@arm.com>

On Wed, Jun 24, 2020 at 02:03:03PM +0100, Lukasz Luba wrote:
> 
> 
> On 6/24/20 1:06 PM, Krzysztof Kozlowski wrote:
> > My case was clearly showing wrong behavior. System was idle but not
> > sleeping - network working, SSH connection ongoing.  Therefore at least
> > one CPU was not idle and could adjust the devfreq/DMC... but this did not
> > happen. The system stayed for like a minute in 633 MHz OPP.
> > 
> > Not-waking up idle processors - ok... so why not using power efficient
> > workqueue? It is exactly for this purpose - wake up from time to time on
> > whatever CPU to do the necessary job.
> 
> IIRC I've done this experiment, still keeping in devfreq:
> INIT_DEFERRABLE_WORK()
> just applying patch [1]. It uses a system_wq which should
> be the same as system_power_efficient_wq when
> CONFIG_WQ_POWER_EFFICIENT_DEFAULT is not set (our case).
> This wasn't solving the issue for the deferred work. That's
> why the patch 2/2 following patch 1/2 [1] was needed.
> 
> The deferred work uses TIMER_DEFERRABLE in it's initialization
> and this is the problem. When the deferred work was queued on a CPU,
> next that CPU went idle, the work was not migrated to some other CPU.
> The former cpu is also not woken up according to the documentation [2].

Yes, you need either workqueue.power_efficient kernel param or CONFIG
option to actually enable it.  But at least it could then work on any
CPU.

Another solution is to use directly WQ_UNBOUND.

> That's why Kamil's approach should be continue IMHO. It gives more
> control over important devices like: bus, dmc, gpu, which utilization
> does not strictly correspond to cpu utilization (which might be low or
> even 0 and cpu put into idle).
> 
> I think Kamil was pointing out also some other issues not only dmc
> (buses probably), but I realized too late to help him.

This should not be a configurable option. Why someone would prefer to
use one over another and decide about this during build or run time?
Instead it should be just *right* all the time. Always.

Argument that we want to save power so we will not wake up any CPU is
ridiculous if because of this system stays in high-power mode.

If system is idle and memory going to be idle, someone should be woken
up to save more power and slow down memory controller.

If system is idle but memory going to be busy, the currently busy CPU
(which performs some memory-intensive job) could do the job and ramp up
the devfreq performance.

Best regards,
Krzysztof