2022-07-07 17:03:10

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

Add acpi function check in case ACPI is not enabled, that will cause
pstate driver failed to call cppc acpi to change perf or update epp
value for shared memory solution processors.

When CPPC is invalid, warning log will be needed to be printed to tell
user what is wrong.

Signed-off-by: Perry Yuan <[email protected]>
---
drivers/acpi/cppc_acpi.c | 3 +++
drivers/cpufreq/amd-pstate.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 6ff1901d7d43..17d67e3ededf 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
struct cpc_desc *cpc_ptr;
int cpu;

+ if (acpi_disabled)
+ return false;
+
for_each_present_cpu(cpu) {
cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
if (!cpc_ptr)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index b54b3b559993..6d81a9a94dde 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -684,7 +684,7 @@ static int __init amd_pstate_init(void)
return -ENODEV;

if (!acpi_cpc_valid()) {
- pr_debug("the _CPC object is not present in SBIOS\n");
+ pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
return -ENODEV;
}

--
2.25.1


2022-07-07 19:48:09

by Nathan Fontenot

[permalink] [raw]
Subject: Re: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

On 7/7/22 12:01, Perry Yuan wrote:
> Add acpi function check in case ACPI is not enabled, that will cause
> pstate driver failed to call cppc acpi to change perf or update epp
> value for shared memory solution processors.
>
> When CPPC is invalid, warning log will be needed to be printed to tell
> user what is wrong.
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> drivers/acpi/cppc_acpi.c | 3 +++
> drivers/cpufreq/amd-pstate.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 6ff1901d7d43..17d67e3ededf 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
> struct cpc_desc *cpc_ptr;
> int cpu;
>
> + if (acpi_disabled)
> + return false> +

This seems ok, the only other places I find that call acpi_cpc_valid() also check
for acpi_disabled.

If the acpi_disabled check is added to acpi_cpc_valid() the other calling sites should
be updated to remove their check for acpi_disabled.

-Nathan

> for_each_present_cpu(cpu) {
> cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
> if (!cpc_ptr)
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index b54b3b559993..6d81a9a94dde 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -684,7 +684,7 @@ static int __init amd_pstate_init(void)
> return -ENODEV;
>
> if (!acpi_cpc_valid()) {
> - pr_debug("the _CPC object is not present in SBIOS\n");
> + pr_warn_once("the _CPC object is not present in SBIOS or ACPI disabled\n");
> return -ENODEV;
> }
>

2022-07-08 12:21:52

by Yuan, Perry

[permalink] [raw]
Subject: RE: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check

[AMD Official Use Only - General]

Hi Nathan.

> -----Original Message-----
> From: Fontenot, Nathan <[email protected]>
> Sent: Friday, July 8, 2022 3:46 AM
> To: Yuan, Perry <[email protected]>; [email protected];
> [email protected]; Huang, Ray <[email protected]>; Rafael J.
> Wysocki <[email protected]>; Len Brown <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: Sharma, Deepak <[email protected]>; Limonciello, Mario
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
> Huang, Shimmer <[email protected]>; Du, Xiaojian
> <[email protected]>; Meng, Li (Jassmine) <[email protected]>
> Subject: Re: [PATCH 11/12] cpufreq: amd-pstate: add ACPI disabled check
>
> On 7/7/22 12:01, Perry Yuan wrote:
> > Add acpi function check in case ACPI is not enabled, that will cause
> > pstate driver failed to call cppc acpi to change perf or update epp
> > value for shared memory solution processors.
> >
> > When CPPC is invalid, warning log will be needed to be printed to tell
> > user what is wrong.
> >
> > Signed-off-by: Perry Yuan <[email protected]>
> > ---
> > drivers/acpi/cppc_acpi.c | 3 +++
> > drivers/cpufreq/amd-pstate.c | 2 +-
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index
> > 6ff1901d7d43..17d67e3ededf 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -424,6 +424,9 @@ bool acpi_cpc_valid(void)
> > struct cpc_desc *cpc_ptr;
> > int cpu;
> >
> > + if (acpi_disabled)
> > + return false> +
>
> This seems ok, the only other places I find that call acpi_cpc_valid() also
> check for acpi_disabled.
>
> If the acpi_disabled check is added to acpi_cpc_valid() the other calling sites
> should be updated to remove their check for acpi_disabled.
>
> -Nathan

You are correct. It needs to remove the same check code from other driver file.
Will add this feedback into V2 patch.

Perry.

>
> > for_each_present_cpu(cpu) {
> > cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
> > if (!cpc_ptr)
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index b54b3b559993..6d81a9a94dde
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -684,7 +684,7 @@ static int __init amd_pstate_init(void)
> > return -ENODEV;
> >
> > if (!acpi_cpc_valid()) {
> > - pr_debug("the _CPC object is not present in SBIOS\n");
> > + pr_warn_once("the _CPC object is not present in SBIOS or
> ACPI
> > +disabled\n");
> > return -ENODEV;
> > }
> >