Re: [PATCH 2/2] PM / devfreq: Generic cpufreq governor

From: Saravana Kannan
Date: Tue Jun 05 2018 - 19:26:40 EST




On 05/27/2018 11:00 PM, MyungJoo Ham wrote:
Many CPU architectures have caches that can scale independent of the CPUs.
Frequency scaling of the caches is necessary to make sure the cache is not
a performance bottleneck that leads to poor performance and power. The same
idea applies for RAM/DDR.

To achieve this, this patch series adds a generic devfreq governor that can
listen to the frequency transitions of each CPU frequency domain and then
adjusts the frequency of the cache (or any devfreq device) based on the
frequency of the CPUs.
I agree that we have some hardware pieces that want to configure
frequencies based on the CPUfreq.

Creating a devfreq governor that configures devfreq-freq
based on incoming events (CPUFreq-transition-event in this case)
is indeed a good idea.

However, I would like to ask the followings:
The overall code appears to be overly complex compared what you need.
- Do you really need to revive "CPUFREQ POLICY" events for this?
especially when you are going to look at "first CPU" only?


Cheers,
MyungJoo

Sorry, didn't notice this email earlier. My message filters seem to be messed up.

The POLICY notifiers are necessary for cases when all CPUs in a policy are hotplugged off -- we need to ignore their frequencies to avoid getting the devfreq device stuck at a high frequency. Looking at "first CPU" is just an optimization to ignore multiple transition notifiers for the each CPU in a policy -- we'd want to do that even if we don't have policy notifiers. Not having policy notifier won't really simplify the code by much. We'd be forced to check for policy->related_cpus for every transition notifier call if the CPU state hasn't been already initialized.

-Saravana

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project