mbox series

[v4,0/9] Implement AMD Pstate EPP Driver

Message ID 20221110175847.3098728-1-Perry.Yuan@amd.com
Headers show
Series Implement AMD Pstate EPP Driver | expand

Message

Perry Yuan Nov. 10, 2022, 5:58 p.m. UTC
Hi all,

This patchset implements one new AMD CPU frequency driver
"amd-pstate-epp” instance for better performance and power control.
CPPC has a parameter called energy preference performance (EPP).
The EPP is used in the CCLK DPM controller to drive the frequency that a core
is going to operate during short periods of activity.
EPP values will be utilized for different OS profiles (balanced, performance, power savings).

AMD Energy Performance Preference (EPP) provides a hint to the hardware
if software wants to bias toward performance (0x0) or energy efficiency (0xff)
The lowlevel power firmware will calculate the runtime frequency according to the EPP preference 
value. So the EPP hint will impact the CPU cores frequency responsiveness.

We use the RAPL interface with "perf" tool to get the energy data of the package power.
Performance Per Watt (PPW) Calculation:

The PPW calculation is referred by below paper:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fdam%2Fdevelop%2Fexternal%2Fus%2Fen%2Fdocuments%2Fperformance-per-what-paper.pdf&data=04%7C01%7CPerry.Yuan%40amd.com%7Cac66e8ce98044e9b062708d9ab47c8d8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729147708574423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TPOvCE%2Frbb0ptBreWNxHqOi9YnVhcHGKG88vviDLb00%3D&reserved=0

Below formula is referred from below spec to measure the PPW:

(F / t) / P = F * t / (t * E) = F / E,

"F" is the number of frames per second.
"P" is power measured in watts.
"E" is energy measured in joules.

Gitsouce Benchmark Data on ROME Server CPU
+------------------------------+------------------------------+------------+------------------+
| Kernel Module                | PPW (1 / s * J)              |Energy(J) | PPW Improvement (%)|
+==============================+==============================+============+==================+
| acpi-cpufreq:schedutil       | 5.85658E-05                  | 17074.8    | base             |
+------------------------------+------------------------------+------------+------------------+
| acpi-cpufreq:ondemand        | 5.03079E-05                  | 19877.6    | -14.10%          |
+------------------------------+------------------------------+------------+------------------+
| acpi-cpufreq:performance     | 5.88132E-05                  | 17003      | 0.42%            |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:ondemand          | 4.60295E-05                  | 21725.2    | -21.41%          |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:schedutil         | 4.70026E-05                  | 21275.4    | -19.7%           |
+------------------------------+------------------------------+------------+------------------+
| amd-pstate:performance       | 5.80094E-05                  | 17238.6    | -0.95%           |
+------------------------------+------------------------------+------------+------------------+
| EPP:performance              | 5.8292E-05                   | 17155      | -0.47%           |
+------------------------------+------------------------------+------------+------------------+
| EPP: balance performance:    | 6.71709E-05                  | 14887.4    | 14.69%           |
+------------------------------+------------------------------+------------+------------------+
| EPP:power                    | 6.66951E-05                  | 4993.6     | 13.88%           |
+------------------------------+------------------------------+------------+------------------+

Tbench Benchmark Data on ROME Server CPU
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| Kernel Module                               | PPW MB / (s * J)  |Throughput(MB/s)| Energy (J)|PPW Improvement(%)|
+=============================================+===================+==============+=============+==================+
| acpi_cpufreq: schedutil                     | 46.39             | 17191        | 37057.3     | base             |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| acpi_cpufreq: ondemand                      | 51.51             | 19269.5      | 37406.5     | 11.04 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| acpi_cpufreq: performance                   | 45.96             | 17063.7      | 37123.7     | -0.74 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: performance(0)               | 54.46             | 20263.1      | 37205       | 17.87 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: balance performance          | 55.03             | 20481.9      | 37221.5     | 19.14 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: balance_power                | 54.43             | 20245.9      | 37194.2     | 17.77 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| EPP:powersave: power(255)                   | 54.26             | 20181.7      | 37197.4     | 17.40 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: schedutil                       | 48.22             | 17844.9      | 37006.6     | 3.80 %           |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: ondemand                        | 61.30             | 22988        | 37503.4     | 33.72 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+
| amd-pstate: performance                     | 54.52             | 20252.6      | 37147.8     | 17.81 %          |
+---------------------------------------------+-------------------+--------------+-------------+------------------+

changes from v3:
 * add one more document update patch for the active and passive mode
   introducion.
 * drive most of the feedbacks from Mario
 * drive feedback from Rafael for the cppc_acpi driver.
 * remove the epp raw data set/get function
 * set the amd-pstate drive by passing kernel parameter
 * set amd-pstate driver disabled by default if no kernel parameter
   input from booting
 * get cppc_set_auto_epp and cppc_set_epp_perf combined
 * pick up reviewed by flag from Mario

changes from v2:
 * change pstate driver as builtin type from module
 * drop patch "export cpufreq cpu release and acquire"
 * squash patch of shared mem into single patch of epp implementation
 * add one new patch to support frequency boost control
 * add patch to expose driver working status checking
 * rebase driver into v6.1-rc4 kernel release
 * move some declaration to amd-pstate.h
 * drive feedback from Mario for the online/offline patch
 * drive feedback from Mario for the suspend/resume patch
 * drive feedback from Ray for the cppc_acpi and some other patches
 * drive feedback from Nathan for the epp patch

changes from v1:
 * rebased to v6.0
 * drive feedbacks from Mario for the suspend/resume patch
 * drive feedbacks from Nathan for the EPP support on msr type
 * fix some typos and code style indent problems
 * update commit comments for patch 4/7
 * change the `epp_enabled` module param name to `epp`
 * set the default epp mode to be false
 * add testing for the x86_energy_perf_policy utility patchset(will
   send that utility patchset with another thread)

v3: https://lore.kernel.org/all/20221107175705.2207842-1-Perry.Yuan@amd.com/
v2: https://lore.kernel.org/all/20221010162248.348141-1-Perry.Yuan@amd.com/
v1: https://lore.kernel.org/all/20221009071033.21170-1-Perry.Yuan@amd.com/

*** BLURB HERE ***

Perry Yuan (9):
  ACPI: CPPC: Add AMD pstate energy performance preference cppc control
  Documentation: amd-pstate: add EPP profiles introduction
  cpufreq: amd-pstate: change amd-pstate driver to be built-in type
  cpufreq: amd_pstate: implement Pstate EPP support for the AMD
    processors
  cpufreq: amd_pstate: implement amd pstate cpu online and offline
    callback
  cpufreq: amd-pstate: implement suspend and resume callbacks
  cpufreq: amd-pstate: add frequency dynamic boost sysfs control
  cpufreq: amd_pstate: add driver working mode status sysfs entry
  Documentation: amd-pstate: add amd pstate driver mode introduction

 Documentation/admin-guide/pm/amd-pstate.rst |  66 +-
 drivers/acpi/cppc_acpi.c                    | 114 ++-
 drivers/cpufreq/Kconfig.x86                 |   2 +-
 drivers/cpufreq/amd-pstate.c                | 912 +++++++++++++++++++-
 include/acpi/cppc_acpi.h                    |  12 +
 include/linux/amd-pstate.h                  |  82 ++
 6 files changed, 1161 insertions(+), 27 deletions(-)

Comments

Mario Limonciello Nov. 14, 2022, 10:29 p.m. UTC | #1
Please run this through a spell checker before v5.

On 11/10/2022 11:58, Perry Yuan wrote:
> Introduce ``amd-pstate`` CPPC has two operation modes:
> * CPPC Autonomous (active) mode
> * CPPC non-autonomous (passive) mode.
> active mode and passive mode can be choosed by whith different kernel parameters.

can be chosen by different kernel parameters

> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   Documentation/admin-guide/pm/amd-pstate.rst | 47 +++++++++++++++++++--
>   1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index e7488891b12f..6ba02a658b31 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -302,11 +302,11 @@ efficiency frequency management method on AMD processors.
>   Kernel Module Options for ``amd-pstate``
>   =========================================
>   
> -.. _shared_mem:
> +.. legacy_cppc:
>   
> -``shared_mem``
> -Use a module param (shared_mem) to enable related processors manually with
> -**amd_pstate.shared_mem=1**.
> +``legacy_cppc``
> +Use a module param (legacy_cppc) to enable related processors manually with
> +**amd_pstate=legacy_cppc**.
>   Due to the performance issue on the processors with `Shared Memory Support
>   <perf_cap_>`_, we disable it presently and will re-enable this by default
>   once we address performance issue with this solution.
> @@ -321,6 +321,45 @@ If the CPU flags have ``cppc``, then this processor supports `Full MSR Support
>   <perf_cap_>`_. Otherwise, it supports `Shared Memory Support <perf_cap_>`_.
>   
>   
> +AMD Pstae Driver Operation Modes

Pstate

> +=================================
> +
> +``amd-pstate`` CPPC has two operation modes: CPPC Autonomous(active) mode and
> +CPPC non-autonomous(passive) mode.
> +active mode and passive mode can be choosed by whith different kernel parameters.

can be chosen by different kernel parameters

> +When in Autonomous mode, CPPC ignores requests done in the Desired Performance
> +Target register and takes into account only the values set to the Minimum requested
> +performance, Maximum requested performance, and Energy Performance Preference
> +registers. When Autonomous is disabled, it only considers the Desired Performance Target.
> +
> +Active Mode
> +------------
> +
> +``amd-pstate-epp``
> +
> +This is the low-level firmware control mode which is implemented by ``amd-pstate-epp``
> +driver with ``amd-pstate=active`` passed to the kernel in the command line.
> +In this mode, ``amd-pstate-epp`` driver provides a hint to the hardware if software
> +wants to bias toward performance (0x0) or energy efficiency (0xff) to the CPPC firmware.
> +then CPPC power algorithm will calculate the runtime workload and adjust the realtime
> +cores frequency according to the power supply and thermal, core voltage and some other
> +hardware conditions.
> +
> +Passive Mode
> +------------
> +
> +``amd-pstate``
> +
> +It will be enabled if the ``amd_pstate=passive`` is passed to the kernel in the command line.
> +In this mode, ``amd-pstate``driver software specifies a desired QoS target in the CPPC
> +performance scale as a relative number. This can be expressed as percentage of nominal
> +performance (infrastructure max). Below the nominal sustained performance level,
> +desired performance expresses the average performance level of the processor subject
> +to the Performance Reduction Tolerance register. Above the nominal performance level,
> +processor must provide at least nominal performance requested and go higher if current
> +operating conditions allow.
> +
> +
>   ``cpupower`` tool support for ``amd-pstate``
>   ===============================================
>
Mario Limonciello Nov. 14, 2022, 10:30 p.m. UTC | #2
On 11/10/2022 11:58, Perry Yuan wrote:
> Add one sysfs entry to control the CPU cores frequency boost state
> The attribute file can allow user to set max performance boosted or
> keeping at normal perf level.
> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 66 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 645706d65173..b71bfbbb7639 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -765,12 +765,46 @@ static ssize_t show_energy_performance_preference(
>   	return  sprintf(buf, "%s\n", energy_perf_strings[preference]);
>   }
>   
> +static void amd_pstate_update_policies(void)
> +{
> +	int cpu;
> +
> +	for_each_possible_cpu(cpu)
> +		cpufreq_update_policy(cpu);
> +}
> +
> +static ssize_t show_boost(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%u\n", cppc_boost);

For new code I believe you should use the new helper sysfs_emit() instead.

> +}
> +
> +static ssize_t store_boost(struct kobject *a,
> +				       struct kobj_attribute *b,
> +				       const char *buf, size_t count)
> +{
> +	bool new_state;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &new_state);
> +	if (ret)
> +		return -EINVAL;
> +
> +	mutex_lock(&amd_pstate_driver_lock);
> +	cppc_boost = !!new_state;
> +	amd_pstate_update_policies();
> +	mutex_unlock(&amd_pstate_driver_lock);
> +
> +	return count;
> +}
> +
>   cpufreq_freq_attr_ro(amd_pstate_max_freq);
>   cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
>   
>   cpufreq_freq_attr_ro(amd_pstate_highest_perf);
>   cpufreq_freq_attr_rw(energy_performance_preference);
>   cpufreq_freq_attr_ro(energy_performance_available_preferences);
> +define_one_global_rw(boost);
>   
>   static struct freq_attr *amd_pstate_attr[] = {
>   	&amd_pstate_max_freq,
> @@ -788,6 +822,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
>   	NULL,
>   };
>   
> +static struct attribute *pstate_global_attributes[] = {
> +	&boost.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group amd_pstate_global_attr_group = {
> +	.attrs = pstate_global_attributes,
> +};
> +
>   static inline void update_boost_state(void)
>   {
>   	u64 misc_en;
> @@ -1398,9 +1441,28 @@ static int __init amd_pstate_init(void)
>   
>   	ret = cpufreq_register_driver(default_pstate_driver);
>   	if (ret)
> -		pr_err("failed to register amd pstate driver with return %d\n",
> -		       ret);
> +		pr_err("failed to register driver with return %d\n", ret);
> +
> +	amd_pstate_kobj = kobject_create_and_add("amd-pstate", &cpu_subsys.dev_root->kobj);
> +	if (!amd_pstate_kobj) {
> +		ret = -EINVAL;
> +		pr_err("global sysfs registration failed.\n");
> +		goto kobject_free;
> +	}
> +
> +	ret = sysfs_create_group(amd_pstate_kobj, &amd_pstate_global_attr_group);
> +	if (ret) {
> +		pr_err("sysfs attribute export failed with error %d.\n", ret);
> +		goto global_attr_free;
> +	}
> +
> +	return ret;
>   
> +global_attr_free:
> +	kobject_put(amd_pstate_kobj);
> +kobject_free:
> +	kfree(cpudata);
> +	cpufreq_unregister_driver(default_pstate_driver);
>   	return ret;
>   }
>   device_initcall(amd_pstate_init);
Mario Limonciello Nov. 14, 2022, 10:46 p.m. UTC | #3
On 11/10/2022 11:58, Perry Yuan wrote:
> Change the `amd-pstate` driver as the built-in type which can help to
> load the driver before the acpi_cpufreq driver as the default pstate
> driver for the AMD processors.
> 
> for the processors do not have the dedicated MSR functions, add
> `amd-pstate=legacy_cppc` to grub which enable shared memory interface
> to communicate with cppc_acpi module to control pstate hints.

Did you sync with Wyes already as I had suggested?  Was this the outcome?

I was a bit surprised to see this still as legacy_cppc when reviewing v4.

> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/Kconfig.x86  |  2 +-
>   drivers/cpufreq/amd-pstate.c | 25 +++++++++++++++----------
>   2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 310779b07daf..00476e94db90 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -35,7 +35,7 @@ config X86_PCC_CPUFREQ
>   	  If in doubt, say N.
>   
>   config X86_AMD_PSTATE
> -	tristate "AMD Processor P-State driver"
> +	bool "AMD Processor P-State driver"
>   	depends on X86 && ACPI
>   	select ACPI_PROCESSOR
>   	select ACPI_CPPC_LIB if X86_64
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index ace7d50cf2ac..85a0b3fb56c2 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -59,10 +59,7 @@
>    * we disable it by default to go acpi-cpufreq on these processors and add a
>    * module parameter to be able to enable it manually for debugging.
>    */
> -static bool shared_mem = false;
> -module_param(shared_mem, bool, 0444);
> -MODULE_PARM_DESC(shared_mem,
> -		 "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
> +static bool shared_mem __read_mostly;
>   
>   static struct cpufreq_driver amd_pstate_driver;
>   
> @@ -653,16 +650,24 @@ static int __init amd_pstate_init(void)
>   
>   	return ret;
>   }
> +device_initcall(amd_pstate_init);
>   
> -static void __exit amd_pstate_exit(void)
> +static int __init amd_pstate_param(char *str)
>   {
> -	cpufreq_unregister_driver(&amd_pstate_driver);
> +	if (!str)
> +		return -EINVAL;
>   
> -	amd_pstate_enable(false);
> -}
> +	/*
> +	 * support shared memory type CPPC which has no MSR function.
> +	 * enable amd-pstate on processors with shared memory solution
> +	 * (amd-pstate=legacy_cppc enabled), it is disabled by default.
> +	 */
> +	if (!strcmp(str, "legacy_cppc"))
> +		shared_mem = true;
>   
> -module_init(amd_pstate_init);
> -module_exit(amd_pstate_exit);
> +	return 0;
> +}
> +early_param("amd-pstate", amd_pstate_param);

Documentation/kernel-parameters.txt needs to be updated for this early 
parameter support.

>   
>   MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
>   MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
Perry Yuan Nov. 30, 2022, 5:13 p.m. UTC | #4
[AMD Official Use Only - General]



> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Tuesday, November 15, 2022 6:30 AM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Fontenot, Nathan
> <Nathan.Fontenot@amd.com>; 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>; Karny, Wyes <Wyes.Karny@amd.com>;
> linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 7/9] cpufreq: amd-pstate: add frequency dynamic
> boost sysfs control
> 
> On 11/10/2022 11:58, Perry Yuan wrote:
> > Add one sysfs entry to control the CPU cores frequency boost state The
> > attribute file can allow user to set max performance boosted or
> > keeping at normal perf level.
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > ---
> >   drivers/cpufreq/amd-pstate.c | 66
> ++++++++++++++++++++++++++++++++++--
> >   1 file changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 645706d65173..b71bfbbb7639
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -765,12 +765,46 @@ static ssize_t
> show_energy_performance_preference(
> >   	return  sprintf(buf, "%s\n", energy_perf_strings[preference]);
> >   }
> >
> > +static void amd_pstate_update_policies(void) {
> > +	int cpu;
> > +
> > +	for_each_possible_cpu(cpu)
> > +		cpufreq_update_policy(cpu);
> > +}
> > +
> > +static ssize_t show_boost(struct kobject *kobj,
> > +				struct kobj_attribute *attr, char *buf) {
> > +	return sprintf(buf, "%u\n", cppc_boost);
> 
> For new code I believe you should use the new helper sysfs_emit() instead.

Make sense, changed in v5.
Thanks .

Perry. 

> 
> > +}
> > +
> > +static ssize_t store_boost(struct kobject *a,
> > +				       struct kobj_attribute *b,
> > +				       const char *buf, size_t count) {
> > +	bool new_state;
> > +	int ret;
> > +
> > +	ret = kstrtobool(buf, &new_state);
> > +	if (ret)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&amd_pstate_driver_lock);
> > +	cppc_boost = !!new_state;
> > +	amd_pstate_update_policies();
> > +	mutex_unlock(&amd_pstate_driver_lock);
> > +
> > +	return count;
> > +}
> > +
> >   cpufreq_freq_attr_ro(amd_pstate_max_freq);
> >   cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
> >
> >   cpufreq_freq_attr_ro(amd_pstate_highest_perf);
> >   cpufreq_freq_attr_rw(energy_performance_preference);
> >   cpufreq_freq_attr_ro(energy_performance_available_preferences);
> > +define_one_global_rw(boost);
> >
> >   static struct freq_attr *amd_pstate_attr[] = {
> >   	&amd_pstate_max_freq,
> > @@ -788,6 +822,15 @@ static struct freq_attr *amd_pstate_epp_attr[] = {
> >   	NULL,
> >   };
> >
> > +static struct attribute *pstate_global_attributes[] = {
> > +	&boost.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group amd_pstate_global_attr_group = {
> > +	.attrs = pstate_global_attributes,
> > +};
> > +
> >   static inline void update_boost_state(void)
> >   {
> >   	u64 misc_en;
> > @@ -1398,9 +1441,28 @@ static int __init amd_pstate_init(void)
> >
> >   	ret = cpufreq_register_driver(default_pstate_driver);
> >   	if (ret)
> > -		pr_err("failed to register amd pstate driver with
> return %d\n",
> > -		       ret);
> > +		pr_err("failed to register driver with return %d\n", ret);
> > +
> > +	amd_pstate_kobj = kobject_create_and_add("amd-pstate",
> &cpu_subsys.dev_root->kobj);
> > +	if (!amd_pstate_kobj) {
> > +		ret = -EINVAL;
> > +		pr_err("global sysfs registration failed.\n");
> > +		goto kobject_free;
> > +	}
> > +
> > +	ret = sysfs_create_group(amd_pstate_kobj,
> &amd_pstate_global_attr_group);
> > +	if (ret) {
> > +		pr_err("sysfs attribute export failed with error %d.\n", ret);
> > +		goto global_attr_free;
> > +	}
> > +
> > +	return ret;
> >
> > +global_attr_free:
> > +	kobject_put(amd_pstate_kobj);
> > +kobject_free:
> > +	kfree(cpudata);
> > +	cpufreq_unregister_driver(default_pstate_driver);
> >   	return ret;
> >   }
> >   device_initcall(amd_pstate_init);