[PATCH v5 0/4] PM / devfreq: Add cpu based scaling support to passive governor

Chanwoo Choi posted 4 patches 3 years, 11 months ago
drivers/devfreq/devfreq.c          |  20 +-
drivers/devfreq/governor.h         |  27 ++
drivers/devfreq/governor_passive.c | 403 ++++++++++++++++++++++++-----
include/linux/devfreq.h            |  17 +-
4 files changed, 389 insertions(+), 78 deletions(-)
[PATCH v5 0/4] PM / devfreq: Add cpu based scaling support to passive governor
Posted by Chanwoo Choi 3 years, 11 months ago
The devfreq passive governor has already supported the devfreq parent device
for coupling the frequency change if some hardware have the constraints
such as power sharing and so on.

Add cpu based scaling support to passive governor with required-opp property.
It uses the cpufreq notifier to catch the frequency change timing of cpufreq
and get the next frequency according to new cpu frequency by using required-opp
property. It is based on patch[1] and then just code clean-up by myself.

Make the common code for both passive_devfreq and passive_cpufreq
parent type to remove the duplicate code.

[1] [RFC,v2] PM / devfreq: Add cpu based scaling support to passive_governor
- https://lore.kernel.org/patchwork/patch/1101049/

Changes from v4:
: https://patchwork.kernel.org/project/linux-pm/cover/20220511093554.17535-1-cw00.choi@samsung.com/
- Fix issue[2] reported by Marek Szyprowski
[2] https://patchwork.kernel.org/project/linux-pm/patch/20220511093554.17535-3-cw00.choi@samsung.com/

Changes from v3:
: ttps://patchwork.kernel.org/project/linux-pm/cover/20220509120337.92472-1-cw00.choi@samsung.com/
- Add tested-by tag of both Chen-Yu Tsai and Johnson Wang
- Fix some typo

Changes from v2:
: https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/
- Drop the following patch ("PM / devfreq: passive: Update frequency when start governor")
- Move p_data->this initialization into cpufreq_passive_regiser_notifier()

Changes from v1:
: https://patchwork.kernel.org/project/linux-pm/cover/20210617060546.26933-1-cw00.choi@samsung.com/
- Rename cpu_data variable to parent_cpu_data to avoid build fail
- Use for_each_possible_cpu macro when register cpufreq transition notifier
- Add missing exception handling when cpufreq_passive_register_notifier is failed
- Keep cpufreq_policy for posible cpus instead of NR_CPU in order to avoid
  the memory waste when NR_CPU is too high.
- Add reviewed-by tag of Matthias Kaehlcke for patch1

Chanwoo Choi (3):
  PM / devfreq: Export devfreq_get_freq_range symbol within devfreq
  PM / devfreq: passive: Reduce duplicate code when passive_devfreq case
  PM / devfreq: passive: Keep cpufreq_policy for possible cpus

Saravana Kannan (1):
  PM / devfreq: Add cpu based scaling support to passive governor

 drivers/devfreq/devfreq.c          |  20 +-
 drivers/devfreq/governor.h         |  27 ++
 drivers/devfreq/governor_passive.c | 403 ++++++++++++++++++++++++-----
 include/linux/devfreq.h            |  17 +-
 4 files changed, 389 insertions(+), 78 deletions(-)

-- 
2.17.1
Re: [PATCH v5 0/4] PM / devfreq: Add cpu based scaling support to passive governor
Posted by Marek Szyprowski 3 years, 11 months ago
Hi,

On 17.05.2022 11:21, Chanwoo Choi wrote:
> The devfreq passive governor has already supported the devfreq parent device
> for coupling the frequency change if some hardware have the constraints
> such as power sharing and so on.
>
> Add cpu based scaling support to passive governor with required-opp property.
> It uses the cpufreq notifier to catch the frequency change timing of cpufreq
> and get the next frequency according to new cpu frequency by using required-opp
> property. It is based on patch[1] and then just code clean-up by myself.
>
> Make the common code for both passive_devfreq and passive_cpufreq
> parent type to remove the duplicate code.

I've just tested it on Exynos based boards and on all I see the 
following issues after system suspend/resume cycle:

# time rtcwake -s10 -mmem
rtcwake: wakeup from "mem" using /dev/rtc0 at Wed May 18 11:13:16 2022
PM: suspend entry (deep)
Filesystems sync: 0.001 seconds
Freezing user space processes ... (elapsed 0.002 seconds) done.
OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.

...

devfreq soc:bus-disp1-fimd: failed to suspend devfreq device
devfreq soc:bus-jpeg-apb: failed to suspend devfreq device
devfreq soc:bus-jpeg: failed to suspend devfreq device
devfreq soc:bus-g2d-acp: failed to suspend devfreq device
devfreq soc:bus-g2d: failed to suspend devfreq device
devfreq soc:bus-peri: failed to suspend devfreq device
devfreq soc:bus-gen: failed to suspend devfreq device
devfreq soc:bus-mfc: failed to suspend devfreq device
devfreq soc:bus-fsys2: failed to suspend devfreq device
devfreq soc:bus-fsys-apb: failed to suspend devfreq device
devfreq soc:bus-noc: failed to suspend devfreq device
...
Disabling non-boot CPUs ...
Enabling non-boot CPUs ...
...
devfreq soc:bus-mscl: failed to resume devfreq device
devfreq soc:bus-gscl-scaler: failed to resume devfreq device
devfreq soc:bus-disp1: failed to resume devfreq device
devfreq soc:bus-disp1-fimd: failed to resume devfreq device
devfreq soc:bus-jpeg-apb: failed to resume devfreq device
devfreq soc:bus-jpeg: failed to resume devfreq device
devfreq soc:bus-g2d-acp: failed to resume devfreq device
devfreq soc:bus-g2d: failed to resume devfreq device
devfreq soc:bus-peri: failed to resume devfreq device
devfreq soc:bus-gen: failed to resume devfreq device
devfreq soc:bus-mfc: failed to resume devfreq device
devfreq soc:bus-fsys2: failed to resume devfreq device
devfreq soc:bus-fsys-apb: failed to resume devfreq device
devfreq soc:bus-noc: failed to resume devfreq device

Some boards (like Trats2) after suspend/resume cycle reveals random crashes.

All those issues were not observed before applying this patchset.

> [1] [RFC,v2] PM / devfreq: Add cpu based scaling support to passive_governor
> - https://lore.kernel.org/patchwork/patch/1101049/
>
> Changes from v4:
> : https://patchwork.kernel.org/project/linux-pm/cover/20220511093554.17535-1-cw00.choi@samsung.com/
> - Fix issue[2] reported by Marek Szyprowski
> [2] https://patchwork.kernel.org/project/linux-pm/patch/20220511093554.17535-3-cw00.choi@samsung.com/
>
> Changes from v3:
> : ttps://patchwork.kernel.org/project/linux-pm/cover/20220509120337.92472-1-cw00.choi@samsung.com/
> - Add tested-by tag of both Chen-Yu Tsai and Johnson Wang
> - Fix some typo
>
> Changes from v2:
> : https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/
> - Drop the following patch ("PM / devfreq: passive: Update frequency when start governor")
> - Move p_data->this initialization into cpufreq_passive_regiser_notifier()
>
> Changes from v1:
> : https://patchwork.kernel.org/project/linux-pm/cover/20210617060546.26933-1-cw00.choi@samsung.com/
> - Rename cpu_data variable to parent_cpu_data to avoid build fail
> - Use for_each_possible_cpu macro when register cpufreq transition notifier
> - Add missing exception handling when cpufreq_passive_register_notifier is failed
> - Keep cpufreq_policy for posible cpus instead of NR_CPU in order to avoid
>    the memory waste when NR_CPU is too high.
> - Add reviewed-by tag of Matthias Kaehlcke for patch1
>
> Chanwoo Choi (3):
>    PM / devfreq: Export devfreq_get_freq_range symbol within devfreq
>    PM / devfreq: passive: Reduce duplicate code when passive_devfreq case
>    PM / devfreq: passive: Keep cpufreq_policy for possible cpus
>
> Saravana Kannan (1):
>    PM / devfreq: Add cpu based scaling support to passive governor
>
>   drivers/devfreq/devfreq.c          |  20 +-
>   drivers/devfreq/governor.h         |  27 ++
>   drivers/devfreq/governor_passive.c | 403 ++++++++++++++++++++++++-----
>   include/linux/devfreq.h            |  17 +-
>   4 files changed, 389 insertions(+), 78 deletions(-)
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH v5 0/4] PM / devfreq: Add cpu based scaling support to passive governor
Posted by Chanwoo Choi 3 years, 11 months ago
Hi Marek,


I send the previous mail without detailed my reply
because of my mistake.

Thanks for the report.
I fix this issue as following: I'll send patch. Thanks.

diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 7306e943a234..72c67979ebe1 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -402,7 +402,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 {
 	struct devfreq_passive_data *p_data
 			= (struct devfreq_passive_data *)devfreq->data;
-	int ret = -EINVAL;
+	int ret = 0;
 
 	if (!p_data)
 		return -EINVAL;
-- 
2.17.1



On 5/18/22 7:56 PM, Marek Szyprowski wrote:
> Hi,
> 
> On 17.05.2022 11:21, Chanwoo Choi wrote:
>> The devfreq passive governor has already supported the devfreq parent device
>> for coupling the frequency change if some hardware have the constraints
>> such as power sharing and so on.
>>
>> Add cpu based scaling support to passive governor with required-opp property.
>> It uses the cpufreq notifier to catch the frequency change timing of cpufreq
>> and get the next frequency according to new cpu frequency by using required-opp
>> property. It is based on patch[1] and then just code clean-up by myself.
>>
>> Make the common code for both passive_devfreq and passive_cpufreq
>> parent type to remove the duplicate code.
> 
> I've just tested it on Exynos based boards and on all I see the 
> following issues after system suspend/resume cycle:
> 
> # time rtcwake -s10 -mmem
> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed May 18 11:13:16 2022
> PM: suspend entry (deep)
> Filesystems sync: 0.001 seconds
> Freezing user space processes ... (elapsed 0.002 seconds) done.
> OOM killer disabled.
> Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
> 
> ...
> 
> devfreq soc:bus-disp1-fimd: failed to suspend devfreq device
> devfreq soc:bus-jpeg-apb: failed to suspend devfreq device
> devfreq soc:bus-jpeg: failed to suspend devfreq device
> devfreq soc:bus-g2d-acp: failed to suspend devfreq device
> devfreq soc:bus-g2d: failed to suspend devfreq device
> devfreq soc:bus-peri: failed to suspend devfreq device
> devfreq soc:bus-gen: failed to suspend devfreq device
> devfreq soc:bus-mfc: failed to suspend devfreq device
> devfreq soc:bus-fsys2: failed to suspend devfreq device
> devfreq soc:bus-fsys-apb: failed to suspend devfreq device
> devfreq soc:bus-noc: failed to suspend devfreq device
> ...
> Disabling non-boot CPUs ...
> Enabling non-boot CPUs ...
> ...
> devfreq soc:bus-mscl: failed to resume devfreq device
> devfreq soc:bus-gscl-scaler: failed to resume devfreq device
> devfreq soc:bus-disp1: failed to resume devfreq device
> devfreq soc:bus-disp1-fimd: failed to resume devfreq device
> devfreq soc:bus-jpeg-apb: failed to resume devfreq device
> devfreq soc:bus-jpeg: failed to resume devfreq device
> devfreq soc:bus-g2d-acp: failed to resume devfreq device
> devfreq soc:bus-g2d: failed to resume devfreq device
> devfreq soc:bus-peri: failed to resume devfreq device
> devfreq soc:bus-gen: failed to resume devfreq device
> devfreq soc:bus-mfc: failed to resume devfreq device
> devfreq soc:bus-fsys2: failed to resume devfreq device
> devfreq soc:bus-fsys-apb: failed to resume devfreq device
> devfreq soc:bus-noc: failed to resume devfreq device
> 
> Some boards (like Trats2) after suspend/resume cycle reveals random crashes.
> 
> All those issues were not observed before applying this patchset.
> 
>> [1] [RFC,v2] PM / devfreq: Add cpu based scaling support to passive_governor
>> - https://lore.kernel.org/patchwork/patch/1101049/
>>
>> Changes from v4:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20220511093554.17535-1-cw00.choi@samsung.com/
>> - Fix issue[2] reported by Marek Szyprowski
>> [2] https://patchwork.kernel.org/project/linux-pm/patch/20220511093554.17535-3-cw00.choi@samsung.com/
>>
>> Changes from v3:
>> : ttps://patchwork.kernel.org/project/linux-pm/cover/20220509120337.92472-1-cw00.choi@samsung.com/
>> - Add tested-by tag of both Chen-Yu Tsai and Johnson Wang
>> - Fix some typo
>>
>> Changes from v2:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/
>> - Drop the following patch ("PM / devfreq: passive: Update frequency when start governor")
>> - Move p_data->this initialization into cpufreq_passive_regiser_notifier()
>>
>> Changes from v1:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20210617060546.26933-1-cw00.choi@samsung.com/
>> - Rename cpu_data variable to parent_cpu_data to avoid build fail
>> - Use for_each_possible_cpu macro when register cpufreq transition notifier
>> - Add missing exception handling when cpufreq_passive_register_notifier is failed
>> - Keep cpufreq_policy for posible cpus instead of NR_CPU in order to avoid
>>    the memory waste when NR_CPU is too high.
>> - Add reviewed-by tag of Matthias Kaehlcke for patch1
>>
>> Chanwoo Choi (3):
>>    PM / devfreq: Export devfreq_get_freq_range symbol within devfreq
>>    PM / devfreq: passive: Reduce duplicate code when passive_devfreq case
>>    PM / devfreq: passive: Keep cpufreq_policy for possible cpus
>>
>> Saravana Kannan (1):
>>    PM / devfreq: Add cpu based scaling support to passive governor
>>
>>   drivers/devfreq/devfreq.c          |  20 +-
>>   drivers/devfreq/governor.h         |  27 ++
>>   drivers/devfreq/governor_passive.c | 403 ++++++++++++++++++++++++-----
>>   include/linux/devfreq.h            |  17 +-
>>   4 files changed, 389 insertions(+), 78 deletions(-)
>>
> Best regards
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH v5 0/4] PM / devfreq: Add cpu based scaling support to passive governor
Posted by Chanwoo Choi 3 years, 11 months ago
Hi Marek,

Thanks for the report.
It 


On 5/18/22 7:56 PM, Marek Szyprowski wrote:
> Hi,
> 
> On 17.05.2022 11:21, Chanwoo Choi wrote:
>> The devfreq passive governor has already supported the devfreq parent device
>> for coupling the frequency change if some hardware have the constraints
>> such as power sharing and so on.
>>
>> Add cpu based scaling support to passive governor with required-opp property.
>> It uses the cpufreq notifier to catch the frequency change timing of cpufreq
>> and get the next frequency according to new cpu frequency by using required-opp
>> property. It is based on patch[1] and then just code clean-up by myself.
>>
>> Make the common code for both passive_devfreq and passive_cpufreq
>> parent type to remove the duplicate code.
> 
> I've just tested it on Exynos based boards and on all I see the 
> following issues after system suspend/resume cycle:
> 
> # time rtcwake -s10 -mmem
> rtcwake: wakeup from "mem" using /dev/rtc0 at Wed May 18 11:13:16 2022
> PM: suspend entry (deep)
> Filesystems sync: 0.001 seconds
> Freezing user space processes ... (elapsed 0.002 seconds) done.
> OOM killer disabled.
> Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done.
> 
> ...
> 
> devfreq soc:bus-disp1-fimd: failed to suspend devfreq device
> devfreq soc:bus-jpeg-apb: failed to suspend devfreq device
> devfreq soc:bus-jpeg: failed to suspend devfreq device
> devfreq soc:bus-g2d-acp: failed to suspend devfreq device
> devfreq soc:bus-g2d: failed to suspend devfreq device
> devfreq soc:bus-peri: failed to suspend devfreq device
> devfreq soc:bus-gen: failed to suspend devfreq device
> devfreq soc:bus-mfc: failed to suspend devfreq device
> devfreq soc:bus-fsys2: failed to suspend devfreq device
> devfreq soc:bus-fsys-apb: failed to suspend devfreq device
> devfreq soc:bus-noc: failed to suspend devfreq device
> ...
> Disabling non-boot CPUs ...
> Enabling non-boot CPUs ...
> ...
> devfreq soc:bus-mscl: failed to resume devfreq device
> devfreq soc:bus-gscl-scaler: failed to resume devfreq device
> devfreq soc:bus-disp1: failed to resume devfreq device
> devfreq soc:bus-disp1-fimd: failed to resume devfreq device
> devfreq soc:bus-jpeg-apb: failed to resume devfreq device
> devfreq soc:bus-jpeg: failed to resume devfreq device
> devfreq soc:bus-g2d-acp: failed to resume devfreq device
> devfreq soc:bus-g2d: failed to resume devfreq device
> devfreq soc:bus-peri: failed to resume devfreq device
> devfreq soc:bus-gen: failed to resume devfreq device
> devfreq soc:bus-mfc: failed to resume devfreq device
> devfreq soc:bus-fsys2: failed to resume devfreq device
> devfreq soc:bus-fsys-apb: failed to resume devfreq device
> devfreq soc:bus-noc: failed to resume devfreq device
> 
> Some boards (like Trats2) after suspend/resume cycle reveals random crashes.
> 
> All those issues were not observed before applying this patchset.
> 
>> [1] [RFC,v2] PM / devfreq: Add cpu based scaling support to passive_governor
>> - https://lore.kernel.org/patchwork/patch/1101049/
>>
>> Changes from v4:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20220511093554.17535-1-cw00.choi@samsung.com/
>> - Fix issue[2] reported by Marek Szyprowski
>> [2] https://patchwork.kernel.org/project/linux-pm/patch/20220511093554.17535-3-cw00.choi@samsung.com/
>>
>> Changes from v3:
>> : ttps://patchwork.kernel.org/project/linux-pm/cover/20220509120337.92472-1-cw00.choi@samsung.com/
>> - Add tested-by tag of both Chen-Yu Tsai and Johnson Wang
>> - Fix some typo
>>
>> Changes from v2:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/
>> - Drop the following patch ("PM / devfreq: passive: Update frequency when start governor")
>> - Move p_data->this initialization into cpufreq_passive_regiser_notifier()
>>
>> Changes from v1:
>> : https://patchwork.kernel.org/project/linux-pm/cover/20210617060546.26933-1-cw00.choi@samsung.com/
>> - Rename cpu_data variable to parent_cpu_data to avoid build fail
>> - Use for_each_possible_cpu macro when register cpufreq transition notifier
>> - Add missing exception handling when cpufreq_passive_register_notifier is failed
>> - Keep cpufreq_policy for posible cpus instead of NR_CPU in order to avoid
>>    the memory waste when NR_CPU is too high.
>> - Add reviewed-by tag of Matthias Kaehlcke for patch1
>>
>> Chanwoo Choi (3):
>>    PM / devfreq: Export devfreq_get_freq_range symbol within devfreq
>>    PM / devfreq: passive: Reduce duplicate code when passive_devfreq case
>>    PM / devfreq: passive: Keep cpufreq_policy for possible cpus
>>
>> Saravana Kannan (1):
>>    PM / devfreq: Add cpu based scaling support to passive governor
>>
>>   drivers/devfreq/devfreq.c          |  20 +-
>>   drivers/devfreq/governor.h         |  27 ++
>>   drivers/devfreq/governor_passive.c | 403 ++++++++++++++++++++++++-----
>>   include/linux/devfreq.h            |  17 +-
>>   4 files changed, 389 insertions(+), 78 deletions(-)
>>
> Best regards
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics