| Message ID | 20240104171553.2080674-3-lukasz.luba@arm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Introduce runtime modifiable Energy Model | expand |
The word "refactor" appears to be quite loaded in your patch descriptions, but it is not always the best one to use IMV. For instance, this patch simply extends the argument list of em_cpufreq_update_efficiencies(), so I would say just that in the subject: "Extend em_cpufreq_update_efficiencies() argument list" On Thu, Jan 4, 2024 at 6:14 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > In order to prepare the code for the modifiable EM perf_state table, > refactor existing function em_cpufreq_update_efficiencies(). "make em_cpufreq_update_efficiencies() take a pointer to the EM table as its second argument and modify it to use that new argument instead of the "table" member of dev->em_pd" or something like this. > The function now takes the ptr to the EM table as its argument. > > No functional impact. > > Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> > --- > kernel/power/energy_model.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c > index 8b9dd4a39f63..42486674b834 100644 > --- a/kernel/power/energy_model.c > +++ b/kernel/power/energy_model.c > @@ -237,10 +237,10 @@ static int em_create_pd(struct device *dev, int nr_states, > return 0; > } > > -static void em_cpufreq_update_efficiencies(struct device *dev) > +static void > +em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table) > { > struct em_perf_domain *pd = dev->em_pd; > - struct em_perf_state *table; > struct cpufreq_policy *policy; > int found = 0; > int i; > @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct device *dev) > return; > } > > - table = pd->table; > - > for (i = 0; i < pd->nr_perf_states; i++) { > if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT)) > continue; > @@ -397,7 +395,7 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states, > > dev->em_pd->flags |= flags; > > - em_cpufreq_update_efficiencies(dev); > + em_cpufreq_update_efficiencies(dev, dev->em_pd->table); > > em_debug_create_pd(dev); > dev_info(dev, "EM: created perf domain\n"); > -- The code change itself LGTM.
Hi Rafael, On 1/4/24 19:07, Rafael J. Wysocki wrote: > The word "refactor" appears to be quite loaded in your patch > descriptions, but it is not always the best one to use IMV. Fair enough, I'll change those patches according to your comments. > > For instance, this patch simply extends the argument list of > em_cpufreq_update_efficiencies(), so I would say just that in the > subject: "Extend em_cpufreq_update_efficiencies() argument list" > > On Thu, Jan 4, 2024 at 6:14 PM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> In order to prepare the code for the modifiable EM perf_state table, >> refactor existing function em_cpufreq_update_efficiencies(). > > "make em_cpufreq_update_efficiencies() take a pointer to the EM table > as its second argument and modify it to use that new argument instead > of the "table" member of dev->em_pd" > > or something like this. I think I got the point, will change it. > >> The function now takes the ptr to the EM table as its argument. >> >> No functional impact. >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> kernel/power/energy_model.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c >> index 8b9dd4a39f63..42486674b834 100644 >> --- a/kernel/power/energy_model.c >> +++ b/kernel/power/energy_model.c >> @@ -237,10 +237,10 @@ static int em_create_pd(struct device *dev, int nr_states, >> return 0; >> } >> >> -static void em_cpufreq_update_efficiencies(struct device *dev) >> +static void >> +em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table) >> { >> struct em_perf_domain *pd = dev->em_pd; >> - struct em_perf_state *table; >> struct cpufreq_policy *policy; >> int found = 0; >> int i; >> @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct device *dev) >> return; >> } >> >> - table = pd->table; >> - >> for (i = 0; i < pd->nr_perf_states; i++) { >> if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT)) >> continue; >> @@ -397,7 +395,7 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states, >> >> dev->em_pd->flags |= flags; >> >> - em_cpufreq_update_efficiencies(dev); >> + em_cpufreq_update_efficiencies(dev, dev->em_pd->table); >> >> em_debug_create_pd(dev); >> dev_info(dev, "EM: created perf domain\n"); >> -- > > The code change itself LGTM. Thanks
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c index 8b9dd4a39f63..42486674b834 100644 --- a/kernel/power/energy_model.c +++ b/kernel/power/energy_model.c @@ -237,10 +237,10 @@ static int em_create_pd(struct device *dev, int nr_states, return 0; } -static void em_cpufreq_update_efficiencies(struct device *dev) +static void +em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table) { struct em_perf_domain *pd = dev->em_pd; - struct em_perf_state *table; struct cpufreq_policy *policy; int found = 0; int i; @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct device *dev) return; } - table = pd->table; - for (i = 0; i < pd->nr_perf_states; i++) { if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT)) continue; @@ -397,7 +395,7 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states, dev->em_pd->flags |= flags; - em_cpufreq_update_efficiencies(dev); + em_cpufreq_update_efficiencies(dev, dev->em_pd->table); em_debug_create_pd(dev); dev_info(dev, "EM: created perf domain\n");
In order to prepare the code for the modifiable EM perf_state table, refactor existing function em_cpufreq_update_efficiencies(). The function now takes the ptr to the EM table as its argument. No functional impact. Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- kernel/power/energy_model.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)