On 30-03-16, 04:00, Rafael J. Wysocki wrote:
> +static int sugov_init(struct cpufreq_policy *policy)
> +{
> + struct sugov_policy *sg_policy;
> + struct sugov_tunables *tunables;
> + unsigned int lat;
> + int ret = 0;
> +
> + /* State should be equivalent to EXIT */
> + if (policy->governor_data)
> + return -EBUSY;
> +
> + sg_policy = sugov_policy_alloc(policy);
> + if (!sg_policy)
> + return -ENOMEM;
> +
> + mutex_lock(&global_tunables_lock);
> +
> + if (global_tunables) {
> + if (WARN_ON(have_governor_per_policy())) {
> + ret = -EINVAL;
> + goto free_sg_policy;
> + }
> + policy->governor_data = sg_policy;
> + sg_policy->tunables = global_tunables;
> +
> + gov_attr_set_get(&global_tunables->attr_set,
> &sg_policy->tunables_hook);
> + goto out;
> + }
> +
> + tunables = sugov_tunables_alloc(sg_policy);
> + if (!tunables) {
> + ret = -ENOMEM;
> + goto free_sg_policy;
> + }
> +
> + tunables->rate_limit_us = LATENCY_MULTIPLIER;
> + lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
> + if (lat)
> + tunables->rate_limit_us *= lat;
> +
> + policy->governor_data = sg_policy;
> + sg_policy->tunables = tunables;
> +
> + ret = kobject_init_and_add(&tunables->attr_set.kobj,
> &sugov_tunables_ktype,
> + get_governor_parent_kobj(policy), "%s",
> + schedutil_gov.name);
> + if (ret)
> + goto fail;
> +
> + out:
> + mutex_unlock(&global_tunables_lock);
> +
> + cpufreq_enable_fast_switch(policy);
> + return 0;
> +
> + fail:
> + policy->governor_data = NULL;
> + sugov_tunables_free(tunables);
> +
> + free_sg_policy:
> + mutex_unlock(&global_tunables_lock);
> +
> + sugov_policy_free(sg_policy);
> + pr_err("cpufreq: schedutil governor initialization failed (error
> %d)\n", ret);
> + return ret;
> +}
The current version of this looks good to me and takes care of all the issues I
raised earlier. Thanks.
> +static int sugov_limits(struct cpufreq_policy *policy)
> +{
> + struct sugov_policy *sg_policy = policy->governor_data;
> +
> + if (!policy->fast_switch_enabled) {
> + mutex_lock(&sg_policy->work_lock);
> +
> + if (policy->max < policy->cur)
> + __cpufreq_driver_target(policy, policy->max,
> + CPUFREQ_RELATION_H);
> + else if (policy->min > policy->cur)
> + __cpufreq_driver_target(policy, policy->min,
> + CPUFREQ_RELATION_L);
> +
> + mutex_unlock(&sg_policy->work_lock);
> + }
> +
> + sg_policy->need_freq_update = true;
I am wondering why we need to do this for !fast_switch_enabled case?
> + return 0;
> +}
Apart from that:
Acked-by: Viresh Kumar <viresh.ku...@linaro.org>
--
viresh
