From: Rafael J. Wysocki <[email protected]>
Add a new field to be set when the CPUFREQ_GOV_FLAG_STRICT_TARGET
flag is set for the current governor to struct cpufreq_policy, so
that the drivers needing to check CPUFREQ_GOV_FLAG_STRICT_TARGET do
not have to access the governor object during every frequency
transition.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/cpufreq/cpufreq.c | 2 ++
include/linux/cpufreq.h | 6 ++++++
2 files changed, 8 insertions(+)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2280,6 +2280,8 @@ static int cpufreq_init_governor(struct
}
}
+ policy->strict_target = !!(policy->governor->flags & CPUFREQ_GOV_FLAG_STRICT_TARGET);
+
return 0;
}
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -109,6 +109,12 @@ struct cpufreq_policy {
bool fast_switch_enabled;
/*
+ * Set if the CPUFREQ_GOV_FLAG_STRICT_TARGET flag is set for the
+ * current governor.
+ */
+ bool strict_target;
+
+ /*
* Preferred average time interval between consecutive invocations of
* the driver to set the frequency for this policy. To be set by the
* scaling driver (0, which is the default, means no preference).
On 09-11-20, 17:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Add a new field to be set when the CPUFREQ_GOV_FLAG_STRICT_TARGET
> flag is set for the current governor to struct cpufreq_policy, so
> that the drivers needing to check CPUFREQ_GOV_FLAG_STRICT_TARGET do
> not have to access the governor object during every frequency
> transition.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/cpufreq/cpufreq.c | 2 ++
> include/linux/cpufreq.h | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2280,6 +2280,8 @@ static int cpufreq_init_governor(struct
> }
> }
>
> + policy->strict_target = !!(policy->governor->flags & CPUFREQ_GOV_FLAG_STRICT_TARGET);
> +
> return 0;
> }
>
> Index: linux-pm/include/linux/cpufreq.h
> ===================================================================
> --- linux-pm.orig/include/linux/cpufreq.h
> +++ linux-pm/include/linux/cpufreq.h
> @@ -109,6 +109,12 @@ struct cpufreq_policy {
> bool fast_switch_enabled;
>
> /*
> + * Set if the CPUFREQ_GOV_FLAG_STRICT_TARGET flag is set for the
> + * current governor.
> + */
> + bool strict_target;
> +
> + /*
> * Preferred average time interval between consecutive invocations of
> * the driver to set the frequency for this policy. To be set by the
> * scaling driver (0, which is the default, means no preference).
I was kind of hoping to avoid adding a field here when I proposed updating the
gov structure. I do understand the performance related penalty of accessing the
gov structure for fast switch case though and so wonder if we really need this,
then should we avoid changing the gov structure at all ? I mean there is only
one user of that field now, do we really need a flag for it ? We can just do the
string comparison here with powersave and performance to set strict_target.
Whatever you feel is better though.
Acked-by: Viresh Kumar <[email protected]>
--
viresh
On Tue, Nov 10, 2020 at 3:47 AM Viresh Kumar <[email protected]> wrote:
>
> On 09-11-20, 17:53, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Add a new field to be set when the CPUFREQ_GOV_FLAG_STRICT_TARGET
> > flag is set for the current governor to struct cpufreq_policy, so
> > that the drivers needing to check CPUFREQ_GOV_FLAG_STRICT_TARGET do
> > not have to access the governor object during every frequency
> > transition.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/cpufreq/cpufreq.c | 2 ++
> > include/linux/cpufreq.h | 6 ++++++
> > 2 files changed, 8 insertions(+)
> >
> > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > @@ -2280,6 +2280,8 @@ static int cpufreq_init_governor(struct
> > }
> > }
> >
> > + policy->strict_target = !!(policy->governor->flags & CPUFREQ_GOV_FLAG_STRICT_TARGET);
> > +
> > return 0;
> > }
> >
> > Index: linux-pm/include/linux/cpufreq.h
> > ===================================================================
> > --- linux-pm.orig/include/linux/cpufreq.h
> > +++ linux-pm/include/linux/cpufreq.h
> > @@ -109,6 +109,12 @@ struct cpufreq_policy {
> > bool fast_switch_enabled;
> >
> > /*
> > + * Set if the CPUFREQ_GOV_FLAG_STRICT_TARGET flag is set for the
> > + * current governor.
> > + */
> > + bool strict_target;
> > +
> > + /*
> > * Preferred average time interval between consecutive invocations of
> > * the driver to set the frequency for this policy. To be set by the
> > * scaling driver (0, which is the default, means no preference).
>
> I was kind of hoping to avoid adding a field here when I proposed updating the
> gov structure. I do understand the performance related penalty of accessing the
> gov structure for fast switch case though and so wonder if we really need this,
> then should we avoid changing the gov structure at all ? I mean there is only
> one user of that field now, do we really need a flag for it ? We can just do the
> string comparison here with powersave and performance to set strict_target.
>
> Whatever you feel is better though.
The cost of having the flag is zero and it allows things to be
documented a bit better IMV.
> Acked-by: Viresh Kumar <[email protected]>
Thanks!