[v2,1/4] cpufreq: Introduce governor flags
diff mbox series

Message ID 1876249.M1ZxxmeKtZ@kreacher
State New, archived
Headers show
Series
  • cpufreq: intel_pstate: Handle powersave governor correctly in the passive mode with HWP
Related show

Commit Message

Rafael J. Wysocki Nov. 9, 2020, 4:51 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

A new cpufreq governor flag will be added subsequently, so replace
the bool dynamic_switching fleid in struct cpufreq_governor with a
flags field and introduce CPUFREQ_GOV_FLAG_DYN_SWITCH to set for
the "dynamic switching" governors instead of it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c          |    2 +-
 drivers/cpufreq/cpufreq_governor.h |    2 +-
 include/linux/cpufreq.h            |    9 +++++++--
 kernel/sched/cpufreq_schedutil.c   |    2 +-
 4 files changed, 10 insertions(+), 5 deletions(-)

Comments

Viresh Kumar Nov. 10, 2020, 2:41 a.m. UTC | #1
On 09-11-20, 17:51, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> A new cpufreq governor flag will be added subsequently, so replace
> the bool dynamic_switching fleid in struct cpufreq_governor with a
> flags field and introduce CPUFREQ_GOV_FLAG_DYN_SWITCH to set for
> the "dynamic switching" governors instead of it.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c          |    2 +-
>  drivers/cpufreq/cpufreq_governor.h |    2 +-
>  include/linux/cpufreq.h            |    9 +++++++--
>  kernel/sched/cpufreq_schedutil.c   |    2 +-
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2254,7 +2254,7 @@ static int cpufreq_init_governor(struct
>  		return -EINVAL;
>  
>  	/* Platform doesn't want dynamic frequency switching ? */
> -	if (policy->governor->dynamic_switching &&

I completely forgot that we had something like this :)

> +	if (policy->governor->flags & CPUFREQ_GOV_FLAG_DYN_SWITCH &&
>  	    cpufreq_driver->flags & CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING) {
>  		struct cpufreq_governor *gov = cpufreq_fallback_governor();
>  
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> @@ -156,7 +156,7 @@ void cpufreq_dbs_governor_limits(struct
>  #define CPUFREQ_DBS_GOVERNOR_INITIALIZER(_name_)			\
>  	{								\
>  		.name = _name_,						\
> -		.dynamic_switching = true,				\
> +		.flags = CPUFREQ_GOV_FLAG_DYN_SWITCH,			\
>  		.owner = THIS_MODULE,					\
>  		.init = cpufreq_dbs_governor_init,			\
>  		.exit = cpufreq_dbs_governor_exit,			\
> Index: linux-pm/include/linux/cpufreq.h
> ===================================================================
> --- linux-pm.orig/include/linux/cpufreq.h
> +++ linux-pm/include/linux/cpufreq.h
> @@ -565,12 +565,17 @@ struct cpufreq_governor {
>  					 char *buf);
>  	int	(*store_setspeed)	(struct cpufreq_policy *policy,
>  					 unsigned int freq);
> -	/* For governors which change frequency dynamically by themselves */
> -	bool			dynamic_switching;
>  	struct list_head	governor_list;
>  	struct module		*owner;
> +	u8			flags;
>  };
>  
> +/* Governor flags */
> +
> +/* For governors which change frequency dynamically by themselves */
> +#define CPUFREQ_GOV_FLAG_DYN_SWITCH	BIT(0)

Maybe just drop the FLAG_ part as we don't use it for other cpufreq related
flags as well. That will also give us space to write DYN as DYNAMIC (it may be
better as we use the full name in CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING).

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Rafael J. Wysocki Nov. 10, 2020, 12:36 p.m. UTC | #2
On Tue, Nov 10, 2020 at 3:41 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 09-11-20, 17:51, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > A new cpufreq governor flag will be added subsequently, so replace
> > the bool dynamic_switching fleid in struct cpufreq_governor with a
> > flags field and introduce CPUFREQ_GOV_FLAG_DYN_SWITCH to set for
> > the "dynamic switching" governors instead of it.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/cpufreq/cpufreq.c          |    2 +-
> >  drivers/cpufreq/cpufreq_governor.h |    2 +-
> >  include/linux/cpufreq.h            |    9 +++++++--
> >  kernel/sched/cpufreq_schedutil.c   |    2 +-
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> >
> > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > @@ -2254,7 +2254,7 @@ static int cpufreq_init_governor(struct
> >               return -EINVAL;
> >
> >       /* Platform doesn't want dynamic frequency switching ? */
> > -     if (policy->governor->dynamic_switching &&
>
> I completely forgot that we had something like this :)
>
> > +     if (policy->governor->flags & CPUFREQ_GOV_FLAG_DYN_SWITCH &&
> >           cpufreq_driver->flags & CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING) {
> >               struct cpufreq_governor *gov = cpufreq_fallback_governor();
> >
> > Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> > +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> > @@ -156,7 +156,7 @@ void cpufreq_dbs_governor_limits(struct
> >  #define CPUFREQ_DBS_GOVERNOR_INITIALIZER(_name_)                     \
> >       {                                                               \
> >               .name = _name_,                                         \
> > -             .dynamic_switching = true,                              \
> > +             .flags = CPUFREQ_GOV_FLAG_DYN_SWITCH,                   \
> >               .owner = THIS_MODULE,                                   \
> >               .init = cpufreq_dbs_governor_init,                      \
> >               .exit = cpufreq_dbs_governor_exit,                      \
> > Index: linux-pm/include/linux/cpufreq.h
> > ===================================================================
> > --- linux-pm.orig/include/linux/cpufreq.h
> > +++ linux-pm/include/linux/cpufreq.h
> > @@ -565,12 +565,17 @@ struct cpufreq_governor {
> >                                        char *buf);
> >       int     (*store_setspeed)       (struct cpufreq_policy *policy,
> >                                        unsigned int freq);
> > -     /* For governors which change frequency dynamically by themselves */
> > -     bool                    dynamic_switching;
> >       struct list_head        governor_list;
> >       struct module           *owner;
> > +     u8                      flags;
> >  };
> >
> > +/* Governor flags */
> > +
> > +/* For governors which change frequency dynamically by themselves */
> > +#define CPUFREQ_GOV_FLAG_DYN_SWITCH  BIT(0)
>
> Maybe just drop the FLAG_ part as we don't use it for other cpufreq related
> flags as well. That will also give us space to write DYN as DYNAMIC (it may be
> better as we use the full name in CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING).

OK, I'll rename the flag (and the new one too).

> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks!

Patch
diff mbox series

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2254,7 +2254,7 @@  static int cpufreq_init_governor(struct
 		return -EINVAL;
 
 	/* Platform doesn't want dynamic frequency switching ? */
-	if (policy->governor->dynamic_switching &&
+	if (policy->governor->flags & CPUFREQ_GOV_FLAG_DYN_SWITCH &&
 	    cpufreq_driver->flags & CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING) {
 		struct cpufreq_governor *gov = cpufreq_fallback_governor();
 
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -156,7 +156,7 @@  void cpufreq_dbs_governor_limits(struct
 #define CPUFREQ_DBS_GOVERNOR_INITIALIZER(_name_)			\
 	{								\
 		.name = _name_,						\
-		.dynamic_switching = true,				\
+		.flags = CPUFREQ_GOV_FLAG_DYN_SWITCH,			\
 		.owner = THIS_MODULE,					\
 		.init = cpufreq_dbs_governor_init,			\
 		.exit = cpufreq_dbs_governor_exit,			\
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -565,12 +565,17 @@  struct cpufreq_governor {
 					 char *buf);
 	int	(*store_setspeed)	(struct cpufreq_policy *policy,
 					 unsigned int freq);
-	/* For governors which change frequency dynamically by themselves */
-	bool			dynamic_switching;
 	struct list_head	governor_list;
 	struct module		*owner;
+	u8			flags;
 };
 
+/* Governor flags */
+
+/* For governors which change frequency dynamically by themselves */
+#define CPUFREQ_GOV_FLAG_DYN_SWITCH	BIT(0)
+
+
 /* Pass a target to the cpufreq driver */
 unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
 					unsigned int target_freq);
Index: linux-pm/kernel/sched/cpufreq_schedutil.c
===================================================================
--- linux-pm.orig/kernel/sched/cpufreq_schedutil.c
+++ linux-pm/kernel/sched/cpufreq_schedutil.c
@@ -881,7 +881,7 @@  static void sugov_limits(struct cpufreq_
 struct cpufreq_governor schedutil_gov = {
 	.name			= "schedutil",
 	.owner			= THIS_MODULE,
-	.dynamic_switching	= true,
+	.flags			= CPUFREQ_GOV_FLAG_DYN_SWITCH,
 	.init			= sugov_init,
 	.exit			= sugov_exit,
 	.start			= sugov_start,