On 11/10/2019 15:44, Douglas RAILLARD wrote:
[...]
> @@ -66,6 +70,38 @@ static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu);
>
> /************************ Governor internals ***********************/
>
> +#ifdef CONFIG_ENERGY_MODEL
> +static void sugov_policy_attach_pd(struct sugov_policy *sg_policy)
> +{
> + struct em_perf_domain *pd;
> + struct cpufreq_policy *policy = sg_policy->policy;
Shouldn't always order local variable declarations from longest to
shortest line?
> +
> + sg_policy->pd = NULL;
> + pd = em_cpu_get(policy->cpu);
> + if (!pd)
> + return;
> +
> + if (cpumask_equal(policy->related_cpus, to_cpumask(pd->cpus)))
> + sg_policy->pd = pd;
> + else
> + pr_warn("%s: Not all CPUs in schedutil policy %u share the same
> perf domain, no perf domain for that policy will be registered\n",
> + __func__, policy->cpu);
Maybe {} because of 2 lines?
> +}
> +
> +static struct em_perf_domain *sugov_policy_get_pd(
> + struct sugov_policy *sg_policy)
Maybe this way? This format is already used in this file.
static struct em_perf_domain *
sugov_policy_get_pd(struct sugov_policy *sg_policy)
> +{
> + return sg_policy->pd;
> +}
> +#else /* CONFIG_ENERGY_MODEL */
> +static void sugov_policy_attach_pd(struct sugov_policy *sg_policy) {}
> +static struct em_perf_domain *sugov_policy_get_pd(
> + struct sugov_policy *sg_policy)
> +{
> + return NULL;
> +}
> +#endif /* CONFIG_ENERGY_MODEL */
> +
> static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64
> time)
> {
> s64 delta_ns;
> @@ -859,6 +895,9 @@ static int sugov_start(struct cpufreq_policy *policy)
> sugov_update_shared :
> sugov_update_single);
> }
> +
> + sugov_policy_attach_pd(sg_policy);
> +
> return 0;
> }
A sugov_policy_detach_pd() called from sugov_stop() (doing for instance
the g_policy->pd = NULL) is not needed?
