linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Yuan, Perry" <Perry.Yuan@amd.com>
To: Oleksandr Natalenko <oleksandr@natalenko.name>,
	"rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
	"Limonciello, Mario" <Mario.Limonciello@amd.com>,
	"viresh.kumar@linaro.org" <viresh.kumar@linaro.org>,
	"Huang, Ray" <Ray.Huang@amd.com>,
	"Shenoy, Gautham Ranjal" <gautham.shenoy@amd.com>,
	"Petkov, Borislav" <Borislav.Petkov@amd.com>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Huang, Shimmer" <Shimmer.Huang@amd.com>,
	"Du, Xiaojian" <Xiaojian.Du@amd.com>,
	"Meng, Li (Jassmine)" <Li.Meng@amd.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v10 3/7] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control
Date: Wed, 8 May 2024 11:42:03 +0000	[thread overview]
Message-ID: <CYYPR12MB86553309AB7030D199C9D9159CE52@CYYPR12MB8655.namprd12.prod.outlook.com> (raw)
In-Reply-To: <4563575.LvFx2qVVIh@natalenko.name>

[AMD Official Use Only - General]

 Hi Oleksandr

> -----Original Message-----
> From: Oleksandr Natalenko <oleksandr@natalenko.name>
> Sent: Wednesday, May 8, 2024 5:39 PM
> To: rafael.j.wysocki@intel.com; Limonciello, Mario
> <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray
> <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@amd.com>; Petkov, Borislav
> <Borislav.Petkov@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer
> <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng,
> Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v10 3/7] cpufreq: amd-pstate: implement cpb_boost
> sysfs entry for boost control
>
> Hello.
>
> On středa 8. května 2024 9:21:08, SELČ Perry Yuan wrote:
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > With this new sysfs entry `cpb_boost`created, user can change CPU
> > boost state dynamically under `active`, `guided` and `passive` modes.
> > And the highest perf and frequency will also be updated as the boost
> > state changing.
> >
> > 0): check current boost state
> > cat /sys/devices/system/cpu/amd_pstate/cpb_boost
> >
> > 1): disable CPU boost
> > sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
> >
> > 2): enable CPU boost
> > sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost"
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217931
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217618
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate-ut.c |   2 +-
> >  drivers/cpufreq/amd-pstate.c    | 114
> +++++++++++++++++++++++++++++++-
> >  include/linux/amd-pstate.h      |   1 +
> >  3 files changed, 115 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate-ut.c
> > b/drivers/cpufreq/amd-pstate-ut.c index f04ae67dda37..b3601b0e6dd3
> > 100644
> > --- a/drivers/cpufreq/amd-pstate-ut.c
> > +++ b/drivers/cpufreq/amd-pstate-ut.c
> > @@ -226,7 +226,7 @@ static void amd_pstate_ut_check_freq(u32 index)
> >                     goto skip_test;
> >             }
> >
> > -           if (cpudata->boost_supported) {
> > +           if (amd_pstate_global_params.cpb_boost) {
> >                     if ((policy->max == cpudata->max_freq) ||
> >                                     (policy->max == cpudata-
> >nominal_freq))
> >                             amd_pstate_ut_cases[index].result =
> AMD_PSTATE_UT_RESULT_PASS;
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index f7dab0f7b452..f81fd61dd2a9 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -686,8 +686,10 @@ static int amd_pstate_boost_init(struct
> amd_cpudata *cpudata)
> >     }
> >
> >     amd_pstate_global_params.cpb_supported = !(boost_val &
> MSR_K7_HWCR_CPB_DIS);
> > -   if (amd_pstate_global_params.cpb_supported)
> > +   if (amd_pstate_global_params.cpb_supported) {
> >             current_pstate_driver->boost_enabled = true;
> > +           cpudata->boost_state = true;
> > +   }
> >
> >     amd_pstate_global_params.cpb_boost =
> > amd_pstate_global_params.cpb_supported;
> >
> > @@ -1293,6 +1295,114 @@ static ssize_t prefcore_show(struct device
> *dev,
> >     return sysfs_emit(buf, "%s\n",
> > str_enabled_disabled(amd_pstate_prefcore));
> >  }
> >
> > +static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy,
> > +bool on) {
> > +   struct amd_cpudata *cpudata = policy->driver_data;
> > +   struct cppc_perf_ctrls perf_ctrls;
> > +   u32 highest_perf, nominal_perf, nominal_freq, max_freq;
> > +   int ret;
> > +
> > +   if (!policy) {
> > +           pr_err("policy is null\n");
> > +           return -ENODATA;
> > +   }
> > +
> > +   highest_perf = READ_ONCE(cpudata->highest_perf);
> > +   nominal_perf = READ_ONCE(cpudata->nominal_perf);
> > +   nominal_freq = READ_ONCE(cpudata->nominal_freq);
> > +   max_freq = READ_ONCE(cpudata->max_freq);
> > +
> > +   if (boot_cpu_has(X86_FEATURE_CPPC)) {
> > +           u64 value = READ_ONCE(cpudata->cppc_req_cached);
> > +
> > +           value &= ~GENMASK_ULL(7, 0);
> > +           value |= on ? highest_perf : nominal_perf;
> > +           WRITE_ONCE(cpudata->cppc_req_cached, value);
> > +
> > +           wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ,
> value);
> > +   } else {
> > +           perf_ctrls.max_perf = on ? highest_perf : nominal_perf;
> > +           ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
> > +           if (ret) {
> > +                   cpufreq_cpu_release(policy);
> > +                   pr_debug("failed to set energy perf value (%d)\n",
> ret);
> > +                   return ret;
> > +           }
> > +   }
> > +
> > +   if (on)
> > +           policy->cpuinfo.max_freq = max_freq;
> > +   else
> > +           policy->cpuinfo.max_freq = nominal_freq * 1000;
> > +
> > +   policy->max = policy->cpuinfo.max_freq;
> > +
> > +   if (cppc_state == AMD_PSTATE_PASSIVE) {
> > +           ret = freq_qos_update_request(&cpudata->req[1],
> > +                                 policy->cpuinfo.max_freq);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> > +static int amd_pstate_cpu_boost(int cpu, bool state) {
> > +   int ret;
> > +   struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> > +   struct amd_cpudata *cpudata = policy->driver_data;
> > +
> > +   if (!policy) {
> > +           pr_err("policy is NULL\n");
> > +           ret = -ENODATA;
> > +           goto err_exit;
> > +   }
> > +
> > +   ret = amd_pstate_cpu_boost_update(policy, state);
> > +   refresh_frequency_limits(policy);
> > +   cpudata->boost_state = !!state;
> > +
> > +err_exit:
> > +   cpufreq_cpu_put(policy);
> > +   return ret < 0 ? ret : 0;
> > +}
> > +
> > +static ssize_t cpb_boost_show(struct device *dev,
> > +                      struct device_attribute *attr, char *buf) {
> > +   return sysfs_emit(buf, "%u\n",
> amd_pstate_global_params.cpb_boost);
> > +}
> > +
> > +static ssize_t cpb_boost_store(struct device *dev, struct device_attribute
> *b,
> > +                       const char *buf, size_t count) {
> > +   bool new_state;
> > +   ssize_t ret;
> > +   int cpu;
> > +
> > +   if (!amd_pstate_global_params.cpb_supported) {
> > +           pr_err("Boost mode is not supported by this processor or
> SBIOS\n");
> > +           return -EINVAL;
> > +   }
> > +
> > +   ret = kstrtobool(buf, &new_state);
> > +   if (ret)
> > +           return ret;
> > +
> > +   mutex_lock(&amd_pstate_driver_lock);
> > +   for_each_present_cpu(cpu) {
> > +           ret = amd_pstate_cpu_boost(cpu, new_state);
> > +           if (ret < 0) {
> > +                   pr_warn("failed to update cpu boost for CPU%d
> (%d)\n", cpu, ret);
>
> Note ret is ssize_t, so %d should be %ld (or %z/%zd/%zi?) instead.

Yes, it should be %ld,  looks like the ret is less than zero,  it is not needed to print out looks like.
Will remove it in v11 otherwise it will cause build warning.

Thanks for the review!

Perry.

>
> > +                   goto err_exit;
> > +           }
> > +   }
> > +   amd_pstate_global_params.cpb_boost = !!new_state;
> > +
> > +err_exit:
> > +   mutex_unlock(&amd_pstate_driver_lock);
> > +   return ret < 0 ? ret : count;
> > +}
> > +
> >  cpufreq_freq_attr_ro(amd_pstate_max_freq);
> >  cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
> >
> > @@ -1303,6 +1413,7 @@
> > cpufreq_freq_attr_rw(energy_performance_preference);
> >  cpufreq_freq_attr_ro(energy_performance_available_preferences);
> >  static DEVICE_ATTR_RW(status);
> >  static DEVICE_ATTR_RO(prefcore);
> > +static DEVICE_ATTR_RW(cpb_boost);
> >
> >  static struct freq_attr *amd_pstate_attr[] = {
> >     &amd_pstate_max_freq,
> > @@ -1327,6 +1438,7 @@ static struct freq_attr *amd_pstate_epp_attr[] =
> > {  static struct attribute *pstate_global_attributes[] = {
> >     &dev_attr_status.attr,
> >     &dev_attr_prefcore.attr,
> > +   &dev_attr_cpb_boost.attr,
> >     NULL
> >  };
> >
> > diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
> > index 8ba5dd4d3405..56f616a5cd0f 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/include/linux/amd-pstate.h
> > @@ -106,6 +106,7 @@ struct amd_cpudata {
> >     u32     policy;
> >     u64     cppc_cap1_cached;
> >     bool    suspended;
> > +   bool    boost_state;
> >  };
> >
> >  /*
> >
>
>
> --
> Oleksandr Natalenko (post-factum)

  reply	other threads:[~2024-05-08 11:42 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-08  7:21 [PATCH v10 0/7] AMD Pstate Driver Core Performance Boost Perry Yuan
2024-05-08  7:21 ` [PATCH v10 1/7] cpufreq: acpi: move MSR_K7_HWCR_CPB_DIS_BIT into msr-index.h Perry Yuan
2024-05-08  7:21 ` [PATCH v10 2/7] cpufreq: amd-pstate: initialize new core precision boost state Perry Yuan
2024-05-08  7:21 ` [PATCH v10 3/7] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control Perry Yuan
2024-05-08  9:39   ` Oleksandr Natalenko
2024-05-08 11:42     ` Yuan, Perry [this message]
2024-05-08  7:21 ` [PATCH v10 4/7] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off Perry Yuan
2024-05-08  7:21 ` [PATCH v10 5/7] Documentation: cpufreq: amd-pstate: introduce the new cpu boost control method Perry Yuan
2024-05-08  7:21 ` [PATCH v10 6/7] cpufreq: amd-pstate: introduce per CPU frequency boost control Perry Yuan
2024-05-08  9:34   ` Oleksandr Natalenko
2024-05-08  9:43     ` Oleksandr Natalenko
2024-05-08 12:20       ` Yuan, Perry
2024-05-08  7:21 ` [PATCH v10 7/7] Documentation: cpufreq: amd-pstate: update doc for Per CPU boost control method Perry Yuan
2024-05-08  8:18 ` [PATCH v10 0/7] AMD Pstate Driver Core Performance Boost Borislav Petkov
2024-05-09 16:01   ` Yuan, Perry
2024-05-09 16:12     ` Borislav Petkov
2024-05-08 15:11 ` Oleksandr Natalenko
2024-05-08 19:13   ` Oleksandr Natalenko
2024-05-08 19:21     ` Oleksandr Natalenko
2024-05-08 21:31       ` Oleksandr Natalenko
2024-05-08 22:13         ` Mario Limonciello
2024-05-09 12:01           ` Oleksandr Natalenko
2024-05-09 16:03             ` Yuan, Perry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CYYPR12MB86553309AB7030D199C9D9159CE52@CYYPR12MB8655.namprd12.prod.outlook.com \
    --to=perry.yuan@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Borislav.Petkov@amd.com \
    --cc=Li.Meng@amd.com \
    --cc=Mario.Limonciello@amd.com \
    --cc=Ray.Huang@amd.com \
    --cc=Shimmer.Huang@amd.com \
    --cc=Xiaojian.Du@amd.com \
    --cc=gautham.shenoy@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=oleksandr@natalenko.name \
    --cc=rafael.j.wysocki@intel.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).