| Message ID | 20220707170116.216912-1-Perry.Yuan@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | AMD Pstate Enhancement And Issue Fixs | expand |
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 <Perry.Yuan@amd.com> > --- > 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; > } >
[AMD Official Use Only - General] Hi Nathan. > -----Original Message----- > From: Fontenot, Nathan <Nathan.Fontenot@amd.com> > Sent: Friday, July 8, 2022 3:46 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; > viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Rafael J. > Wysocki <rafael@kernel.org>; Len Brown <lenb@kernel.org>; linux- > acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > pm@vger.kernel.org > Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; > Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian > <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com> > 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 <Perry.Yuan@amd.com> > > --- > > 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; > > } > >
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; }
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 <Perry.Yuan@amd.com> --- drivers/acpi/cppc_acpi.c | 3 +++ drivers/cpufreq/amd-pstate.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)