[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<CYYPR12MB8655E346702A34FAD22300B89CE32@CYYPR12MB8655.namprd12.prod.outlook.com>
Date: Tue, 14 May 2024 04:54:31 +0000
From: "Yuan, Perry" <Perry.Yuan@....com>
To: "Ugwekar, Dhananjay" <Dhananjay.Ugwekar@....com>, "Limonciello, Mario"
<Mario.Limonciello@....com>, "Shenoy, Gautham Ranjal"
<gautham.shenoy@....com>, "Huang, Ray" <Ray.Huang@....com>, "Petkov,
Borislav" <Borislav.Petkov@....com>
CC: "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>, "Deucher,
Alexander" <Alexander.Deucher@....com>, "Huang, Shimmer"
<Shimmer.Huang@....com>, "Du, Xiaojian" <Xiaojian.Du@....com>, "Meng, Li
(Jassmine)" <Li.Meng@....com>, "linux-pm@...r.kernel.org"
<linux-pm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate
driver by default
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Dhananjay
> -----Original Message-----
> From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@....com>
> Sent: Monday, May 13, 2024 5:37 PM
> To: Yuan, Perry <Perry.Yuan@....com>; Limonciello, Mario
> <Mario.Limonciello@....com>; Shenoy, Gautham Ranjal
> <gautham.shenoy@....com>; Huang, Ray <Ray.Huang@....com>; Petkov,
> Borislav <Borislav.Petkov@....com>
> Cc: rafael.j.wysocki@...el.com; Deucher, Alexander
> <Alexander.Deucher@....com>; Huang, Shimmer
> <Shimmer.Huang@....com>; Du, Xiaojian <Xiaojian.Du@....com>; Meng,
> Li (Jassmine) <Li.Meng@....com>; linux-pm@...r.kernel.org; linux-
> kernel@...r.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@...teo.net>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218705
> > Signed-off-by: Perry Yuan <perry.yuan@....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;
> > }
> >
Powered by blists - more mailing lists