On Monday, February 11, 2013 01:20:00 PM Viresh Kumar wrote:
> Currently, there can't be multiple instances of single governor_type. If we 
> have
> a multi-package system, where we have multiple instances of struct policy (per
> package), we can't have multiple instances of same governor. i.e. We can't 
> have
> multiple instances of ondemand governor for multiple packages.
> 
> Governors directory in sysfs is created at /sys/devices/system/cpu/cpufreq/
> governor-name/. Which again reflects that there can be only one instance of a
> governor_type in the system.
> 
> This is a bottleneck for multicluster system, where we want different packages
> to use same governor type, but with different tunables.
> 
> This patch is inclined towards providing this infrastructure. Because we are
> required to allocate governor's resources dynamically now, we must do it at
> policy creation and end. And so got CPUFREQ_GOV_POLICY_INIT/EXIT.

Are those new events NOPs now?

> Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 21 ++++++++++++++++++---
>  include/linux/cpufreq.h   |  9 ++++++---
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 04aab05..40f7083 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1081,6 +1081,8 @@ static int __cpufreq_remove_dev(struct device *dev, 
> struct subsys_interface *sif
>  
>       /* If cpu is last user of policy, free policy */
>       if (cpus == 1) {
> +             __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
> +
>               lock_policy_rwsem_read(cpu);
>               kobj = &data->kobj;
>               cmp = &data->kobj_unregister;
> @@ -1669,7 +1671,7 @@ EXPORT_SYMBOL(cpufreq_get_policy);
>  static int __cpufreq_set_policy(struct cpufreq_policy *data,
>                               struct cpufreq_policy *policy)
>  {
> -     int ret = 0;
> +     int ret = 0, failed = 1;
>       struct cpufreq_driver *driver = rcu_dereference(cpufreq_driver);
>  
>       pr_debug("setting new policy for CPU %u: %u - %u kHz\n", policy->cpu,
> @@ -1724,18 +1726,31 @@ static int __cpufreq_set_policy(struct cpufreq_policy 
> *data,
>                       pr_debug("governor switch\n");
>  
>                       /* end old governor */
> -                     if (data->governor)
> +                     if (data->governor) {
>                               __cpufreq_governor(data, CPUFREQ_GOV_STOP);
> +                             __cpufreq_governor(data,
> +                                             CPUFREQ_GOV_POLICY_EXIT);
> +                     }
>  
>                       /* start new governor */
>                       data->governor = policy->governor;
> -                     if (__cpufreq_governor(data, CPUFREQ_GOV_START)) {
> +                     if (!__cpufreq_governor(data, CPUFREQ_GOV_POLICY_INIT)) 
> {
> +                             if (!__cpufreq_governor(data, 
> CPUFREQ_GOV_START))
> +                                     failed = 0;
> +                             else
> +                                     __cpufreq_governor(data,
> +                                                     
> CPUFREQ_GOV_POLICY_EXIT);
> +                     }
> +
> +                     if (failed) {
>                               /* new governor failed, so re-start old one */
>                               pr_debug("starting governor %s failed\n",
>                                                       data->governor->name);
>                               if (old_gov) {
>                                       data->governor = old_gov;
>                                       __cpufreq_governor(data,
> +                                                     
> CPUFREQ_GOV_POLICY_INIT);
> +                                     __cpufreq_governor(data,
>                                                          CPUFREQ_GOV_START);
>                               }
>                               ret = -EINVAL;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index a22944c..3b822ce 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -106,6 +106,7 @@ struct cpufreq_policy {
>                                        * governors are used */
>       unsigned int            policy; /* see above */
>       struct cpufreq_governor *governor; /* see below */
> +     void                    *governor_data;
>  
>       struct work_struct      update; /* if update_policy() needs to be
>                                        * called, but you're in IRQ context */
> @@ -178,9 +179,11 @@ static inline unsigned long cpufreq_scale(unsigned long 
> old, u_int div, u_int mu
>   *                          CPUFREQ GOVERNORS                        *
>   *********************************************************************/
>  
> -#define CPUFREQ_GOV_START  1
> -#define CPUFREQ_GOV_STOP   2
> -#define CPUFREQ_GOV_LIMITS 3
> +#define CPUFREQ_GOV_START    1
> +#define CPUFREQ_GOV_STOP     2
> +#define CPUFREQ_GOV_LIMITS   3
> +#define CPUFREQ_GOV_POLICY_INIT      4
> +#define CPUFREQ_GOV_POLICY_EXIT      4

Why don't you use different values here?

If you need only one value, one #define should be sufficient.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to