[PATCH v3 10/15] xen/cpufreq: only set gov NULL when cpufreq_driver.setpolicy is NULL

Penny Zheng posted 15 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v3 10/15] xen/cpufreq: only set gov NULL when cpufreq_driver.setpolicy is NULL
Posted by Penny Zheng 11 months, 1 week ago
From: Penny Zheng <penny.zheng@amd.com>

amd-cppc on active mode bypasses the scaling governor layer, and
provides its own P-state selection algorithms in hardware. Consequently,
when it is used, the driver's -> setpolicy() callback is invoked
to register per-CPU utilization update callbacks, not the ->target()
callback.

So, only when cpufreq_driver.setpolicy is NULL, we need to deliberately
set old gov as NULL to trigger the according gov starting.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/cpufreq/cpufreq.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 792e4dc02c..8fc6e527c2 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -353,7 +353,13 @@ int cpufreq_add_cpu(unsigned int cpu)
     if (hw_all || (cpumask_weight(cpufreq_dom->map) ==
                    pmpt->domain_info.num_processors)) {
         memcpy(&new_policy, policy, sizeof(struct cpufreq_policy));
-        policy->governor = NULL;
+
+       /*
+        * Only when cpufreq_driver.setpolicy == NULL, we need to deliberately
+        * set old gov as NULL to trigger the according gov starting.
+        */
+       if ( cpufreq_driver.setpolicy == NULL )
+            policy->governor = NULL;
 
         cpufreq_cmdline_common_para(&new_policy);
 
-- 
2.34.1
Re: [PATCH v3 10/15] xen/cpufreq: only set gov NULL when cpufreq_driver.setpolicy is NULL
Posted by Jan Beulich 10 months, 2 weeks ago
On 06.03.2025 09:39, Penny Zheng wrote:
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -353,7 +353,13 @@ int cpufreq_add_cpu(unsigned int cpu)
>      if (hw_all || (cpumask_weight(cpufreq_dom->map) ==
>                     pmpt->domain_info.num_processors)) {
>          memcpy(&new_policy, policy, sizeof(struct cpufreq_policy));
> -        policy->governor = NULL;
> +
> +       /*
> +        * Only when cpufreq_driver.setpolicy == NULL, we need to deliberately
> +        * set old gov as NULL to trigger the according gov starting.
> +        */
> +       if ( cpufreq_driver.setpolicy == NULL )
> +            policy->governor = NULL;
>  
>          cpufreq_cmdline_common_para(&new_policy);

Indentation looks off-by-1 here.

Also (I may have asked this before, but couldn't find an indication in this
submission, including in the cover letter): Is this independent of all earlier
patches in the series, and could hence go in right away?

Jan