| Message ID | 20240826211358.2694603-8-superm1@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Adjustments for preferred core detection | expand |
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Mario Limonciello <superm1@kernel.org> > Sent: Tuesday, August 27, 2024 5:14 AM > To: Borislav Petkov <bp@alien8.de>; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Yuan, Perry <Perry.Yuan@amd.com> > Cc: maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT) <x86@kernel.org>; > Rafael J . Wysocki <rafael@kernel.org>; open list:X86 ARCHITECTURE (32-BIT > AND 64-BIT) <linux-kernel@vger.kernel.org>; open list:ACPI <linux- > acpi@vger.kernel.org>; open list:CPU FREQUENCY SCALING FRAMEWORK > <linux-pm@vger.kernel.org>; Limonciello, Mario > <Mario.Limonciello@amd.com> > Subject: [PATCH 7/8] cpufreq: amd-pstate: Optimize > amd_pstate_update_limits() > > From: Mario Limonciello <mario.limonciello@amd.com> > > Don't take and release the mutex when prefcore isn't present and avoid > initialization of variables that will be initially set in the function. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 75568d0f84623..ed05d7a0add10 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int > cpu) > int ret; > bool highest_perf_changed = false; > > - mutex_lock(&amd_pstate_driver_lock); > - if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore)) > - goto free_cpufreq_put; > + if (!amd_pstate_prefcore) > + return; > > + mutex_lock(&amd_pstate_driver_lock); > ret = amd_get_highest_perf(cpu, &cur_high); > if (ret) > goto free_cpufreq_put; > > prev_high = READ_ONCE(cpudata->prefcore_ranking); > - if (prev_high != cur_high) { > - highest_perf_changed = true; > + highest_perf_changed = (prev_high != cur_high); > + if (highest_perf_changed) { > WRITE_ONCE(cpudata->prefcore_ranking, cur_high); > > if (cur_high < CPPC_MAX_PERF) > -- > 2.43.0 > Reviewed-by: Perry Yuan <perry.yuan@amd.com> Best Regards. Perry.
On Mon, Aug 26, 2024 at 04:13:57PM -0500, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > Don't take and release the mutex when prefcore isn't present and > avoid initialization of variables that will be initially set > in the function. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 75568d0f84623..ed05d7a0add10 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu) > int ret; > bool highest_perf_changed = false; > > - mutex_lock(&amd_pstate_driver_lock); > - if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore)) > - goto free_cpufreq_put; > + if (!amd_pstate_prefcore) > + return; Looks good to me. Wondering if it is worth maintaining a static key for amd_pstate_prefcore. Anyway it doesn't change after boot. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham.
On 8/27/2024 11:57, Gautham R. Shenoy wrote: > On Mon, Aug 26, 2024 at 04:13:57PM -0500, Mario Limonciello wrote: >> From: Mario Limonciello <mario.limonciello@amd.com> >> >> Don't take and release the mutex when prefcore isn't present and >> avoid initialization of variables that will be initially set >> in the function. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > >> --- >> drivers/cpufreq/amd-pstate.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c >> index 75568d0f84623..ed05d7a0add10 100644 >> --- a/drivers/cpufreq/amd-pstate.c >> +++ b/drivers/cpufreq/amd-pstate.c >> @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu) >> int ret; >> bool highest_perf_changed = false; >> >> - mutex_lock(&amd_pstate_driver_lock); >> - if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore)) >> - goto free_cpufreq_put; >> + if (!amd_pstate_prefcore) >> + return; > > Looks good to me. > > Wondering if it is worth maintaining a static key for > amd_pstate_prefcore. Anyway it doesn't change after boot. As there is a kernel command line option how would you pass the early param parsing result over without a static variable? > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > Thanks! > > -- > Thanks and Regards > gautham.
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 75568d0f84623..ed05d7a0add10 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu) int ret; bool highest_perf_changed = false; - mutex_lock(&amd_pstate_driver_lock); - if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore)) - goto free_cpufreq_put; + if (!amd_pstate_prefcore) + return; + mutex_lock(&amd_pstate_driver_lock); ret = amd_get_highest_perf(cpu, &cur_high); if (ret) goto free_cpufreq_put; prev_high = READ_ONCE(cpudata->prefcore_ranking); - if (prev_high != cur_high) { - highest_perf_changed = true; + highest_perf_changed = (prev_high != cur_high); + if (highest_perf_changed) { WRITE_ONCE(cpudata->prefcore_ranking, cur_high); if (cur_high < CPPC_MAX_PERF)