Linux-PM Archive mirror
 help / color / mirror / Atom feed
From: Mario Limonciello <mario.limonciello@amd.com>
To: Perry Yuan <perry.yuan@amd.com>,
	rafael.j.wysocki@intel.com, viresh.kumar@linaro.org,
	Ray.Huang@amd.com, gautham.shenoy@amd.com,
	Borislav.Petkov@amd.com
Cc: Alexander.Deucher@amd.com, Xinmei.Huang@amd.com,
	Xiaojian.Du@amd.com, Li.Meng@amd.com, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/11] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS
Date: Tue, 7 May 2024 09:55:32 -0500	[thread overview]
Message-ID: <66bfb2f2-074c-4d13-a3d7-56c6558f7010@amd.com> (raw)
In-Reply-To: <0ec1b203bc83c1acdaf1c5a2f3e51031b4374da2.1715065568.git.perry.yuan@amd.com>

On 5/7/2024 02:15, Perry Yuan wrote:
> If CPPC feature is supported by the CPU however the CPUID flag bit is not
> set by SBIOS, the `amd_pstate` will be failed to load while system
> booting.
> So adding one more debug message to inform user to check the SBIOS setting,
> The change also can help maintainers to debug why amd_pstate driver failed
> to be loaded at system booting if the processor support CPPC.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218686
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index cb766c061c86..e94b55a7bb59 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1698,6 +1698,17 @@ static bool amd_cppc_supported(void)
>   		return false;
>   	}
>   
> +	/*
> +	 * If the CPPC flag is disabled in the BIOS for processors that support MSR-based CPPC
> +	 * the AMD Pstate driver may not function correctly.
> +	 */
> +	if ((boot_cpu_data.x86 >= 0x19) && (boot_cpu_data.x86_model >= 0x40) &&
> +			!cpu_feature_enabled(X86_FEATURE_CPPC)) {

I don't think this if statement is correct.  The intent behind is is 
family 0x19 but models 0x40 to 0x4f AFAICT and then family 0x1a all 
models right?  This message will miss any models that are 0x00 through 
0x39 in family 0x1a.

I'll give you two ideas:

What do you think about instead doing the inverse like this:

If (boot_cpu_data.x86 == 0x19) && (boot_cpu_data.x86_model < 0x40)
	return true;

if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
	pr_debug_once();
	return false;
}

return true;

Another idea can be to look at X86_FEATURE_ZEN# to decide what to do. 
For example all Zen4 and Zen5 should have the architectural MSR.

if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
	if (cpu_feature_enabled(X86_FEATURE_ZEN5) {
		// do stuff
	} else if (cpu_feature_enabled(X86_FEATURE_ZEN4) {
		// do stuff
	} else if (cpu_feature_enabled(X86_FEATURE_ZEN3) {
		// do stuff
	}
}

> +		pr_debug_once("The CPPC feature is supported but disabled by the BIOS. "
> +						"Please enable it if your BIOS has the CPPC option.\n");
> +		return false;
> +	}
> +
>   	return true;
>   }
>   


  reply	other threads:[~2024-05-07 14:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-07  7:15 [PATCH 00/11] AMD Pstate Driver Fixes and Improvements Perry Yuan
2024-05-07  7:15 ` [PATCH 01/11] cpufreq: amd-pstate: optimiza the initial frequency values verification Perry Yuan
2024-05-07 14:44   ` Mario Limonciello
2024-05-07 21:02   ` kernel test robot
2024-05-07  7:15 ` [PATCH 02/11] cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported Perry Yuan
2024-05-07 14:45   ` Mario Limonciello
2024-05-07  7:15 ` [PATCH 03/11] cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Perry Yuan
2024-05-07 14:55   ` Mario Limonciello [this message]
2024-05-07  7:15 ` [PATCH 04/11] Documentation: PM: amd-pstate: introducing recommended reboot requirement during driver switch Perry Yuan
2024-05-07  7:15 ` [PATCH 05/11] Documentation: PM: amd-pstate: add debugging section for driver loading failure Perry Yuan
2024-05-07 15:01   ` Mario Limonciello
2024-05-07  7:15 ` [PATCH 06/11] Documentation: PM: amd-pstate: add guide mode to the Operation mode Perry Yuan
2024-05-07 15:02   ` Mario Limonciello
2024-05-07  7:15 ` [PATCH 07/11] cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() Perry Yuan
2024-05-07 15:03   ` Mario Limonciello
2024-05-07  7:15 ` [PATCH 08/11] x86/cpufeatures: Add feature bits for AMD heterogeneous processor Perry Yuan
2024-05-07  7:15 ` [PATCH 09/11] cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization Perry Yuan
2024-05-07 15:19   ` Mario Limonciello
2024-05-09 16:36     ` Yuan, Perry
2024-05-07 18:14   ` kernel test robot
2024-05-07 19:40   ` kernel test robot
2024-05-07  7:15 ` [PATCH 10/11] cpufreq: amd-pstate: fix the highest frequency issue which limit performance Perry Yuan
2024-05-07 15:23   ` Mario Limonciello
2024-05-07  7:15 ` [PATCH 11/11] cpufreq: amd-pstate: automatically load pstate driver by default Perry Yuan
2024-05-07 14:41   ` Mario Limonciello
2024-05-08 15:20     ` Oleksandr Natalenko
2024-05-08 15:24       ` 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=66bfb2f2-074c-4d13-a3d7-56c6558f7010@amd.com \
    --to=mario.limonciello@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Borislav.Petkov@amd.com \
    --cc=Li.Meng@amd.com \
    --cc=Ray.Huang@amd.com \
    --cc=Xiaojian.Du@amd.com \
    --cc=Xinmei.Huang@amd.com \
    --cc=gautham.shenoy@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=perry.yuan@amd.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).