Linux-PM Archive mirror
 help / color / mirror / Atom feed
From: "Yuan, Perry" <Perry.Yuan@amd.com>
To: "Ugwekar, Dhananjay" <Dhananjay.Ugwekar@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" <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-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default
Date: Tue, 14 May 2024 04:54:31 +0000	[thread overview]
Message-ID: <CYYPR12MB8655E346702A34FAD22300B89CE32@CYYPR12MB8655.namprd12.prod.outlook.com> (raw)
In-Reply-To: <c3275383-9b3c-41b2-a65f-2df50fefafb5@amd.com>

[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;
> >     }
> >


      reply	other threads:[~2024-05-14  4:54 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13  2:07 [PATCH v2 00/10] AMD Pstate Driver Fixes and Improvements Perry Yuan
2024-05-13  2:07 ` [PATCH v2 01/10] cpufreq: amd-pstate: optimize the initial frequency values verification Perry Yuan
2024-05-14  4:50   ` Dhananjay Ugwekar
2024-05-28  5:29   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 02/10] cpufreq: amd-pstate: remove unused variable nominal_freq Perry Yuan
2024-05-17  9:03   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 03/10] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
2024-06-06 14:26   ` Gautham R. Shenoy
2024-05-13  2:07 ` [PATCH v2 04/10] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
2024-05-13 17:21   ` kernel test robot
2024-05-13  2:07 ` [PATCH v2 05/10] Documentation: PM: amd-pstate: add debugging section for driver loading failure Perry Yuan
2024-05-13  2:07 ` [PATCH v2 06/10] Documentation: PM: amd-pstate: add guided mode to the Operation mode Perry Yuan
2024-05-13  2:07 ` [PATCH v2 07/10] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() Perry Yuan
2024-05-13  2:07 ` [PATCH v2 08/10] x86/cpufeatures: Add feature bits for AMD heterogeneous processor Perry Yuan
2024-05-13  2:07 ` [PATCH v2 09/10] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization Perry Yuan
2024-05-13  2:07 ` [PATCH v2 10/10] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
2024-05-13  9:36   ` Dhananjay Ugwekar
2024-05-14  4:54     ` Yuan, Perry [this message]

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=CYYPR12MB8655E346702A34FAD22300B89CE32@CYYPR12MB8655.namprd12.prod.outlook.com \
    --to=perry.yuan@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Borislav.Petkov@amd.com \
    --cc=Dhananjay.Ugwekar@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=rafael.j.wysocki@intel.com \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).