From: Jeremy Linton <jeremy.linton at arm.com>
To: devel@acpica.org
Subject: [Devel] Re: [PATCH v3 1/2] ACPI: CPPC: Disable FIE if registers in PCC regions
Date: Wed, 24 Aug 2022 11:14:00 -0500 [thread overview]
Message-ID: <f08d1b2e-9367-6a47-b0e4-81797afa0961@arm.com> (raw)
In-Reply-To: 20220824061337.dtgfmhgossyhzjpy@vireshk-i7
[-- Attachment #1: Type: text/plain, Size: 5029 bytes --]
Hi,
On 8/24/22 01:13, Viresh Kumar wrote:
> On 18-08-22, 16:16, Jeremy Linton wrote:
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> +bool cppc_perf_ctrs_in_pcc(void)
>> +{
>> + int cpu;
>> +
>> + for_each_present_cpu(cpu) {
>> + struct cpc_register_resource *ref_perf_reg;
>> + struct cpc_desc *cpc_desc;
>> +
>> + cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>> +
>> + if (CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) ||
>> + CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) ||
>> + CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]))
>> + return true;
>> +
>> +
>> + ref_perf_reg = &cpc_desc->cpc_regs[REFERENCE_PERF];
>> +
>> + /*
>> + * If reference perf register is not supported then we should
>> + * use the nominal perf value
>> + */
>> + if (!CPC_SUPPORTED(ref_perf_reg))
>> + ref_perf_reg = &cpc_desc->cpc_regs[NOMINAL_PERF];
>> +
>> + if (CPC_IN_PCC(ref_perf_reg))
>> + return true;
>> + }
>
> Add a blank line here please.
Sure,
>
>> + return false;
>> +}
>> +EXPORT_SYMBOL_GPL(cppc_perf_ctrs_in_pcc);
>> +
>> /**
>> * cppc_get_perf_ctrs - Read a CPU's performance feedback counters.
>> * @cpunum: CPU from which to read counters.
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 24eaf0ec344d..32fcb0bf74a4 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -63,7 +63,15 @@ static struct cppc_workaround_oem_info wa_info[] = {
>>
>> static struct cpufreq_driver cppc_cpufreq_driver;
>>
>> +static enum {
>> + FIE_UNSET = -1,
>> + FIE_ENABLED,
>> + FIE_DISABLED
>> +} fie_disabled = FIE_UNSET;
>> +
>> #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>> +module_param(fie_disabled, int, 0444);
>> +MODULE_PARM_DESC(fie_disabled, "Disable Frequency Invariance Engine (FIE)");
>>
>> /* Frequency invariance support */
>> struct cppc_freq_invariance {
>> @@ -158,7 +166,7 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
>> struct cppc_freq_invariance *cppc_fi;
>> int cpu, ret;
>>
>> - if (cppc_cpufreq_driver.get == hisi_cppc_cpufreq_get_rate)
>> + if (fie_disabled)
>> return;
>>
>> for_each_cpu(cpu, policy->cpus) {
>> @@ -199,7 +207,7 @@ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy)
>> struct cppc_freq_invariance *cppc_fi;
>> int cpu;
>>
>> - if (cppc_cpufreq_driver.get == hisi_cppc_cpufreq_get_rate)
>> + if (fie_disabled)
>> return;
>>
>> /* policy->cpus will be empty here, use related_cpus instead */
>> @@ -229,7 +237,21 @@ static void __init cppc_freq_invariance_init(void)
>> };
>> int ret;
>>
>> - if (cppc_cpufreq_driver.get == hisi_cppc_cpufreq_get_rate)
>> + switch (fie_disabled) {
>> + /* honor user request */
>> + case FIE_DISABLED:
>> + case FIE_ENABLED:
>> + break;
>> + case FIE_UNSET:
>> + default:
>> + fie_disabled = FIE_ENABLED;
>> + if (cppc_perf_ctrs_in_pcc()) {
>> + pr_info("FIE not enabled on systems with registers in PCC\n");
>> + fie_disabled = FIE_DISABLED;
>> + }
>> + break;
>> + }
>
> here too.
Sure,
>
>> + if (fie_disabled)
>> return;
>>
>> kworker_fie = kthread_create_worker(0, "cppc_fie");
>> @@ -247,7 +269,7 @@ static void __init cppc_freq_invariance_init(void)
>>
>> static void cppc_freq_invariance_exit(void)
>> {
>> - if (cppc_cpufreq_driver.get == hisi_cppc_cpufreq_get_rate)
>> + if (fie_disabled)
>> return;
>>
>> kthread_destroy_worker(kworker_fie);
>> @@ -936,6 +958,7 @@ static void cppc_check_hisi_workaround(void)
>> wa_info[i].oem_revision == tbl->oem_revision) {
>> /* Overwrite the get() callback */
>> cppc_cpufreq_driver.get = hisi_cppc_cpufreq_get_rate;
>> + fie_disabled = FIE_DISABLED;
>> break;
>> }
>> }
>> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
>> index f73d357ecdf5..c5614444031f 100644
>> --- a/include/acpi/cppc_acpi.h
>> +++ b/include/acpi/cppc_acpi.h
>> @@ -140,6 +140,7 @@ extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
>> extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
>> extern int cppc_set_enable(int cpu, bool enable);
>> extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
>> +extern bool cppc_perf_ctrs_in_pcc(void);
>> extern bool acpi_cpc_valid(void);
>> extern bool cppc_allow_fast_switch(void);
>> extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
>> @@ -173,6 +174,10 @@ static inline int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps)
>> {
>> return -ENOTSUPP;
>> }
>> +static inline bool cppc_perf_ctrs_in_pcc(void)
>> +{
>> + return false;
>> +}
>> static inline bool acpi_cpc_valid(void)
>> {
>> return false;
>
> Acked-by: Viresh Kumar <viresh.kumar(a)linaro.org>
>
Thanks for looking at this.
next reply other threads:[~2022-08-24 16:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-24 16:14 Jeremy Linton [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-08-24 16:11 [Devel] Re: [PATCH v3 1/2] ACPI: CPPC: Disable FIE if registers in PCC regions Jeremy Linton
2022-08-24 13:22 Rafael J. Wysocki
2022-08-23 18:46 Jeremy Linton
2022-08-23 17:10 Rafael J. Wysocki
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=f08d1b2e-9367-6a47-b0e4-81797afa0961@arm.com \
--to=devel@acpica.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).