| Message ID | 9b31fbcdfd4e4f00c3302f45e655aa43589b224c.1715356532.git.perry.yuan@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | AMD Pstate Driver Fixes and Improvements | expand |
Hello Perry, On 5/13/2024 7:37 AM, Perry Yuan wrote: > If the `amd-pstate` driver is not loaded automatically by default, > it is because the kernel command line parameter has not been added. > To resolve this issue, it is necessary to call the `amd_pstate_set_driver()` > function to enable the desired mode (passive/active/guided) before registering > the driver instance. > This ensures that the driver is loaded correctly without relying on the kernel > command line parameter. > > [ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd > [ 0.982579] amd_pstate: failed to register with return -22 > > Reported-by: Andrei Amuraritei <andamu@posteo.net> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705 > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++----------------- > 1 file changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index dce901a403c9..03342fef7d94 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -1785,28 +1785,30 @@ static int __init amd_pstate_init(void) > /* check if this machine need CPPC quirks */ > dmi_check_system(amd_pstate_quirks_table); > > + /* get default driver mode for loading */ > + if (cppc_state == AMD_PSTATE_UNDEFINED) > + cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE; > + > + /* Disable on the following configs by default: > + * 1. Undefined platforms > + * 2. Server platforms > + */ > + if (amd_pstate_acpi_pm_profile_undefined() || > + amd_pstate_acpi_pm_profile_server()) { > + pr_info("driver load is disabled for server or undefined platform\n"); > + return -ENODEV; > + } > + Wont this change make it impossible to use amd-pstate on server platforms? It wont work even if we pass "amd-pstate=active" in cmdline, because this check is before the switch case. Thanks, Dhananjay > switch (cppc_state) { > - case AMD_PSTATE_UNDEFINED: > - /* Disable on the following configs by default: > - * 1. Undefined platforms > - * 2. Server platforms > - * 3. Shared memory designs > - */ > - if (amd_pstate_acpi_pm_profile_undefined() || > - amd_pstate_acpi_pm_profile_server() || > - !cpu_feature_enabled(X86_FEATURE_CPPC)) { > - pr_info("driver load is disabled, boot with specific mode to enable this\n"); > - return -ENODEV; > - } > - ret = amd_pstate_set_driver(CONFIG_X86_AMD_PSTATE_DEFAULT_MODE); > - if (ret) > - return ret; > - break; > case AMD_PSTATE_DISABLE: > + pr_info("driver load is disabled, boot with specific mode to enable this\n"); > return -ENODEV; > case AMD_PSTATE_PASSIVE: > case AMD_PSTATE_ACTIVE: > case AMD_PSTATE_GUIDED: > + ret = amd_pstate_set_driver(cppc_state); > + if (ret) > + return ret; > break; > default: > return -EINVAL; > @@ -1827,7 +1829,7 @@ static int __init amd_pstate_init(void) > /* enable amd pstate feature */ > ret = amd_pstate_enable(true); > if (ret) { > - pr_err("failed to enable with return %d\n", ret); > + pr_err("failed to enable driver mode(%d)\n", cppc_state); > return ret; > } >
[AMD Official Use Only - AMD Internal Distribution Only] Hi Dhananjay > -----Original Message----- > From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com> > Sent: Monday, May 13, 2024 5:37 PM > To: Yuan, Perry <Perry.Yuan@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Petkov, > Borislav <Borislav.Petkov@amd.com> > Cc: rafael.j.wysocki@intel.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>; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate > driver by default > > Hello Perry, > > On 5/13/2024 7:37 AM, Perry Yuan wrote: > > If the `amd-pstate` driver is not loaded automatically by default, it > > is because the kernel command line parameter has not been added. > > To resolve this issue, it is necessary to call the > > `amd_pstate_set_driver()` function to enable the desired mode > > (passive/active/guided) before registering the driver instance. > > This ensures that the driver is loaded correctly without relying on > > the kernel command line parameter. > > > > [ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix- > v1 xhci-hcd > > [ 0.982579] amd_pstate: failed to register with return -22 > > > > Reported-by: Andrei Amuraritei <andamu@posteo.net> > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705 > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 36 > > +++++++++++++++++++----------------- > > 1 file changed, 19 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index dce901a403c9..03342fef7d94 > 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -1785,28 +1785,30 @@ static int __init amd_pstate_init(void) > > /* check if this machine need CPPC quirks */ > > dmi_check_system(amd_pstate_quirks_table); > > > > + /* get default driver mode for loading */ > > + if (cppc_state == AMD_PSTATE_UNDEFINED) > > + cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE; > > + > > + /* Disable on the following configs by default: > > + * 1. Undefined platforms > > + * 2. Server platforms > > + */ > > + if (amd_pstate_acpi_pm_profile_undefined() || > > + amd_pstate_acpi_pm_profile_server()) { > > + pr_info("driver load is disabled for server or undefined > platform\n"); > > + return -ENODEV; > > + } > > + > > Wont this change make it impossible to use amd-pstate on server platforms? It > wont work even if we pass "amd-pstate=active" in cmdline, because this check > is before the switch case. > > Thanks, > Dhananjay > Good catch, the original checking is disabling server platform by default, it should not block the server platform when using "amd-pstate=active" in cmdline, will fix it in v3 to allow parameter input for server platfrm from command line. Regards. Perry > > switch (cppc_state) { > > - case AMD_PSTATE_UNDEFINED: > > - /* Disable on the following configs by default: > > - * 1. Undefined platforms > > - * 2. Server platforms > > - * 3. Shared memory designs > > - */ > > - if (amd_pstate_acpi_pm_profile_undefined() || > > - amd_pstate_acpi_pm_profile_server() || > > - !cpu_feature_enabled(X86_FEATURE_CPPC)) { > > - pr_info("driver load is disabled, boot with specific > mode to enable this\n"); > > - return -ENODEV; > > - } > > - ret = > amd_pstate_set_driver(CONFIG_X86_AMD_PSTATE_DEFAULT_MODE); > > - if (ret) > > - return ret; > > - break; > > case AMD_PSTATE_DISABLE: > > + pr_info("driver load is disabled, boot with specific mode to > enable > > +this\n"); > > return -ENODEV; > > case AMD_PSTATE_PASSIVE: > > case AMD_PSTATE_ACTIVE: > > case AMD_PSTATE_GUIDED: > > + ret = amd_pstate_set_driver(cppc_state); > > + if (ret) > > + return ret; > > break; > > default: > > return -EINVAL; > > @@ -1827,7 +1829,7 @@ static int __init amd_pstate_init(void) > > /* enable amd pstate feature */ > > ret = amd_pstate_enable(true); > > if (ret) { > > - pr_err("failed to enable with return %d\n", ret); > > + pr_err("failed to enable driver mode(%d)\n", cppc_state); > > return ret; > > } > >
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index dce901a403c9..03342fef7d94 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -1785,28 +1785,30 @@ static int __init amd_pstate_init(void) /* check if this machine need CPPC quirks */ dmi_check_system(amd_pstate_quirks_table); + /* get default driver mode for loading */ + if (cppc_state == AMD_PSTATE_UNDEFINED) + cppc_state = CONFIG_X86_AMD_PSTATE_DEFAULT_MODE; + + /* Disable on the following configs by default: + * 1. Undefined platforms + * 2. Server platforms + */ + if (amd_pstate_acpi_pm_profile_undefined() || + amd_pstate_acpi_pm_profile_server()) { + pr_info("driver load is disabled for server or undefined platform\n"); + return -ENODEV; + } + switch (cppc_state) { - case AMD_PSTATE_UNDEFINED: - /* Disable on the following configs by default: - * 1. Undefined platforms - * 2. Server platforms - * 3. Shared memory designs - */ - if (amd_pstate_acpi_pm_profile_undefined() || - amd_pstate_acpi_pm_profile_server() || - !cpu_feature_enabled(X86_FEATURE_CPPC)) { - pr_info("driver load is disabled, boot with specific mode to enable this\n"); - return -ENODEV; - } - ret = amd_pstate_set_driver(CONFIG_X86_AMD_PSTATE_DEFAULT_MODE); - if (ret) - return ret; - break; case AMD_PSTATE_DISABLE: + pr_info("driver load is disabled, boot with specific mode to enable this\n"); return -ENODEV; case AMD_PSTATE_PASSIVE: case AMD_PSTATE_ACTIVE: case AMD_PSTATE_GUIDED: + ret = amd_pstate_set_driver(cppc_state); + if (ret) + return ret; break; default: return -EINVAL; @@ -1827,7 +1829,7 @@ static int __init amd_pstate_init(void) /* enable amd pstate feature */ ret = amd_pstate_enable(true); if (ret) { - pr_err("failed to enable with return %d\n", ret); + pr_err("failed to enable driver mode(%d)\n", cppc_state); return ret; }
If the `amd-pstate` driver is not loaded automatically by default, it is because the kernel command line parameter has not been added. To resolve this issue, it is necessary to call the `amd_pstate_set_driver()` function to enable the desired mode (passive/active/guided) before registering the driver instance. This ensures that the driver is loaded correctly without relying on the kernel command line parameter. [ 0.917789] usb usb6: Manufacturer: Linux 6.9.0-rc6-amd-pstate-new-fix-v1 xhci-hcd [ 0.982579] amd_pstate: failed to register with return -22 Reported-by: Andrei Amuraritei <andamu@posteo.net> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705 Signed-off-by: Perry Yuan <perry.yuan@amd.com> --- drivers/cpufreq/amd-pstate.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-)