ACPI 6.2 Section 6.2.11.2 'Platform-Wide OSPM Capabilities':
Starting with ACPI Specification 6.2, all _CPC registers can be in
PCC, System Memory, System IO, or Functional Fixed Hardware address
spaces. OSPM support for this more flexible register space scheme is
indicated by the “Flexible Address Space for CPPC Registers” _OSC bit
Otherwise (cf ACPI 6.1, s8.4.7.1.1.X), _CPC registers must be in:
- PCC or Functional Fixed Hardware address space if defined
- SystemMemory address space (NULL register) if not defined
Add the corresponding _OSC bit and check it when parsing _CPC objects.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/acpi/bus.c | 18 ++++++++++++++++++
drivers/acpi/cppc_acpi.c | 9 +++++++++
include/linux/acpi.h | 2 ++
3 files changed, 29 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 3e58b613a2c4..a5d08de5d1e9 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -278,6 +278,20 @@ bool osc_sb_apei_support_acked;
bool osc_pc_lpi_support_confirmed;
EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
+/*
+ * ACPI 6.2 Section 6.2.11.2 'Platform-Wide OSPM Capabilities':
+ * Starting with ACPI Specification 6.2, all _CPC registers can be in
+ * PCC, System Memory, System IO, or Functional Fixed Hardware address
+ * spaces. OSPM support for this more flexible register space scheme is
+ * indicated by the “Flexible Address Space for CPPC Registers” _OSC bit.
+ *
+ * Otherwise (cf ACPI 6.1, s8.4.7.1.1.X), _CPC registers must be in:
+ * - PCC or Functional Fixed Hardware address space if defined
+ * - SystemMemory address space (NULL register) if not defined
+ */
+bool osc_cpc_flexible_adr_space_confirmed;
+EXPORT_SYMBOL_GPL(osc_cpc_flexible_adr_space_confirmed);
+
/*
* ACPI 6.4 Operating System Capabilities for USB.
*/
@@ -321,6 +335,8 @@ static void acpi_bus_osc_negotiate_platform_control(void)
}
#endif
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_FLEXIBLE_ADR_SP;
+
if (IS_ENABLED(CONFIG_SCHED_MC_PRIO))
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_DIVERSE_HIGH_SUPPORT;
@@ -366,6 +382,8 @@ static void acpi_bus_osc_negotiate_platform_control(void)
capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT;
osc_sb_native_usb4_support_confirmed =
capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT;
+ osc_cpc_flexible_adr_space_confirmed =
+ capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_CPC_FLEXIBLE_ADR_SP;
}
kfree(context.ret.pointer);
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index bc1454789a06..6f09fe011544 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -736,6 +736,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
if (gas_t->address) {
void __iomem *addr;
+ if (!osc_cpc_flexible_adr_space_confirmed) {
+ pr_debug("Flexible address space capability not supported\n");
+ goto out_free;
+ }
+
addr = ioremap(gas_t->address, gas_t->bit_width/8);
if (!addr)
goto out_free;
@@ -758,6 +763,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
gas_t->address);
goto out_free;
}
+ if (!osc_cpc_flexible_adr_space_confirmed) {
+ pr_debug("Flexible address space capability not supported\n");
+ goto out_free;
+ }
} else {
if (gas_t->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE || !cpc_ffh_supported()) {
/* Support only PCC, SystemMemory, SystemIO, and FFH type regs. */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d7136d13aa44..977d74d0465b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -574,6 +574,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
#define OSC_SB_OSLPI_SUPPORT 0x00000100
#define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT 0x00001000
#define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x00002000
+#define OSC_SB_CPC_FLEXIBLE_ADR_SP 0x00004000
#define OSC_SB_NATIVE_USB4_SUPPORT 0x00040000
#define OSC_SB_PRM_SUPPORT 0x00200000
@@ -581,6 +582,7 @@ extern bool osc_sb_apei_support_acked;
extern bool osc_pc_lpi_support_confirmed;
extern bool osc_sb_native_usb4_support_confirmed;
extern bool osc_sb_cppc_not_supported;
+extern bool osc_cpc_flexible_adr_space_confirmed;
/* USB4 Capabilities */
#define OSC_USB_USB3_TUNNELING 0x00000001
--
2.25.1
From: Pierre Gondois <[email protected]>
The communication mean of the _CPC desired performance can be
PCC, System Memory, System IO, or Functional Fixed Hardware.
commit b7898fda5bc7 ("cpufreq: Support for fast frequency switching")
fast_switching is 'for switching CPU frequencies from interrupt
context'.
Writes to SystemMemory and SystemIo are fast and suitable this.
This is not the case for PCC and might not be the case for FFH.
Enable fast_switching for the cppc_cpufreq driver in above cases.
Add cppc_allow_fast_switch() to check the desired performance
register address space and set fast_switching accordingly.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/acpi/cppc_acpi.c | 17 +++++++++++++++++
drivers/cpufreq/cppc_cpufreq.c | 24 ++++++++++++++++++++++++
include/acpi/cppc_acpi.h | 5 +++++
3 files changed, 46 insertions(+)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index cc932ec1b613..995745d2fa6c 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -434,6 +434,23 @@ bool acpi_cpc_valid(void)
}
EXPORT_SYMBOL_GPL(acpi_cpc_valid);
+bool cppc_allow_fast_switch(void)
+{
+ struct cpc_register_resource *desired_reg;
+ struct cpc_desc *cpc_ptr;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ cpc_ptr = per_cpu(cpc_desc_ptr, cpu);
+ desired_reg = &cpc_ptr->cpc_regs[DESIRED_PERF];
+ if (!CPC_IN_SM(desired_reg) && !CPC_IN_SIO(desired_reg))
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(cppc_allow_fast_switch);
+
/**
* acpi_get_psd_map - Map the CPUs in the freq domain of a given cpu
* @cpu: Find all CPUs that share a domain with cpu.
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 82d370ae6a4a..000a0c610c30 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -389,6 +389,27 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
return ret;
}
+static unsigned int cppc_cpufreq_fast_switch(struct cpufreq_policy *policy,
+ unsigned int target_freq)
+{
+ struct cppc_cpudata *cpu_data = policy->driver_data;
+ unsigned int cpu = policy->cpu;
+ u32 desired_perf;
+ int ret;
+
+ desired_perf = cppc_cpufreq_khz_to_perf(cpu_data, target_freq);
+ cpu_data->perf_ctrls.desired_perf = desired_perf;
+ ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
+
+ if (ret) {
+ pr_debug("Failed to set target on CPU:%d. ret:%d\n",
+ cpu, ret);
+ return 0;
+ }
+
+ return target_freq;
+}
+
static int cppc_verify_policy(struct cpufreq_policy_data *policy)
{
cpufreq_verify_within_cpu_limits(policy);
@@ -536,6 +557,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
goto out;
}
+ policy->fast_switch_possible = cppc_allow_fast_switch();
+
/*
* If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost
* is supported.
@@ -681,6 +704,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
.verify = cppc_verify_policy,
.target = cppc_cpufreq_set_target,
.get = cppc_cpufreq_get_rate,
+ .fast_switch = cppc_cpufreq_fast_switch,
.init = cppc_cpufreq_cpu_init,
.exit = cppc_cpufreq_cpu_exit,
.set_boost = cppc_cpufreq_set_boost,
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 92b7ea8d8f5e..c6108581d97d 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -141,6 +141,7 @@ 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 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);
extern unsigned int cppc_get_transition_latency(int cpu);
extern bool cpc_ffh_supported(void);
@@ -175,6 +176,10 @@ static inline bool acpi_cpc_valid(void)
{
return false;
}
+static inline bool cppc_allow_fast_switch(void)
+{
+ return false;
+}
static inline unsigned int cppc_get_transition_latency(int cpu)
{
return CPUFREQ_ETERNAL;
--
2.25.1
From: Pierre Gondois <[email protected]>
The transition_delay_us (struct cpufreq_policy) is currently defined
as:
Preferred average time interval between consecutive invocations of
the driver to set the frequency for this policy. To be set by the
scaling driver (0, which is the default, means no preference).
The transition_latency represents the amount of time necessary for a
CPU to change its frequency.
A PCCT table advertises mutliple values:
- pcc_nominal: Expected latency to process a command, in microseconds
- pcc_mpar: The maximum number of periodic requests that the subspace
channel can support, reported in commands per minute. 0 indicates no
limitation.
- pcc_mrtt: The minimum amount of time that OSPM must wait after the
completion of a command before issuing the next command,
in microseconds.
cppc_get_transition_latency() allows to get the max of them.
commit d4f3388afd48 ("cpufreq / CPPC: Set platform specific
transition_delay_us") allows to select transition_delay_us based on
the platform, and fallbacks to cppc_get_transition_latency()
otherwise.
If _CPC objects are not using PCC channels (no PPCT table), the
transition_delay_us is set to CPUFREQ_ETERNAL, leading to really long
periods between frequency updates (~4s).
If the desired_reg, where performance requests are written, is in
SystemMemory or SystemIo ACPI address space, there is no delay
in requests. So return 0 instead of CPUFREQ_ETERNAL, leading to
transition_delay_us being set to LATENCY_MULTIPLIER us (1000 us).
This patch also adds two macros to check the address spaces.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/acpi/cppc_acpi.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 6f09fe011544..cc932ec1b613 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -100,6 +100,16 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
(cpc)->cpc_entry.reg.space_id == \
ACPI_ADR_SPACE_PLATFORM_COMM)
+/* Check if a CPC register is in SystemMemory */
+#define CPC_IN_SM(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
+ (cpc)->cpc_entry.reg.space_id == \
+ ACPI_ADR_SPACE_SYSTEM_MEMORY)
+
+/* Check if a CPC register is in SystemIo */
+#define CPC_IN_SIO(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
+ (cpc)->cpc_entry.reg.space_id == \
+ ACPI_ADR_SPACE_SYSTEM_IO)
+
/* Evaluates to True if reg is a NULL register descriptor */
#define IS_NULL_REG(reg) ((reg)->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && \
(reg)->address == 0 && \
@@ -1456,6 +1466,9 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
* transition latency for performance change requests. The closest we have
* is the timing information from the PCCT tables which provides the info
* on the number and frequency of PCC commands the platform can handle.
+ *
+ * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
+ * then assume there is no latency.
*/
unsigned int cppc_get_transition_latency(int cpu_num)
{
@@ -1481,7 +1494,9 @@ unsigned int cppc_get_transition_latency(int cpu_num)
return CPUFREQ_ETERNAL;
desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
- if (!CPC_IN_PCC(desired_reg))
+ if (CPC_IN_SM(desired_reg) || CPC_IN_SIO(desired_reg))
+ return 0;
+ else if (!CPC_IN_PCC(desired_reg))
return CPUFREQ_ETERNAL;
if (pcc_ss_id < 0)
--
2.25.1
The _OSC method allows the OS and firmware to communicate about
supported features/capabitlities. It also allows the OS to take
control of some features.
In ACPI 6.4, s6.2.11.2 Platform-Wide OSPM Capabilities, the CPPC
(resp. v2) bit should be set by the OS if it 'supports controlling
processor performance via the interfaces described in the _CPC
object'.
The OS supports CPPC and parses the _CPC object only if
CONFIG_ACPI_CPPC_LIB is set. Replace the x86 specific
boot_cpu_has(X86_FEATURE_HWP) dynamic check with an arch
generic CONFIG_ACPI_CPPC_LIB build-time check.
Note:
CONFIG_X86_INTEL_PSTATE selects CONFIG_ACPI_CPPC_LIB.
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/acpi/bus.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a5d08de5d1e9..4fd0ea779441 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -329,10 +329,11 @@ static void acpi_bus_osc_negotiate_platform_control(void)
#endif
#ifdef CONFIG_X86
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT;
- if (boot_cpu_has(X86_FEATURE_HWP)) {
- capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
- capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
- }
+#endif
+
+#ifdef CONFIG_ACPI_CPPC_LIB
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
#endif
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_FLEXIBLE_ADR_SP;
@@ -357,10 +358,9 @@ static void acpi_bus_osc_negotiate_platform_control(void)
return;
}
-#ifdef CONFIG_X86
- if (boot_cpu_has(X86_FEATURE_HWP))
- osc_sb_cppc_not_supported = !(capbuf_ret[OSC_SUPPORT_DWORD] &
- (OSC_SB_CPC_SUPPORT | OSC_SB_CPCV2_SUPPORT));
+#ifdef CONFIG_ACPI_CPPC_LIB
+ osc_sb_cppc_not_supported = !(capbuf_ret[OSC_SUPPORT_DWORD] &
+ (OSC_SB_CPC_SUPPORT | OSC_SB_CPCV2_SUPPORT));
#endif
/*
--
2.25.1
On Wed, May 11, 2022 at 03:45:56PM +0200, Pierre Gondois wrote:
> The _OSC method allows the OS and firmware to communicate about
> supported features/capabitlities. It also allows the OS to take
> control of some features.
>
> In ACPI 6.4, s6.2.11.2 Platform-Wide OSPM Capabilities, the CPPC
> (resp. v2) bit should be set by the OS if it 'supports controlling
> processor performance via the interfaces described in the _CPC
> object'.
>
> The OS supports CPPC and parses the _CPC object only if
> CONFIG_ACPI_CPPC_LIB is set. Replace the x86 specific
> boot_cpu_has(X86_FEATURE_HWP) dynamic check with an arch
> generic CONFIG_ACPI_CPPC_LIB build-time check.
>
> Note:
> CONFIG_X86_INTEL_PSTATE selects CONFIG_ACPI_CPPC_LIB.
While this is work as per the spec, by not sure what kind of ACPI firmware are
in the wild. So be prepared to relax/constrain to original feature check
for x86, unfortunate but may be needed.
Anyways,
Reviewed-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
On Wed, May 11, 2022 at 4:30 PM Sudeep Holla <[email protected]> wrote:
>
> On Wed, May 11, 2022 at 03:45:57PM +0200, Pierre Gondois wrote:
> > From: Pierre Gondois <[email protected]>
> >
> > The transition_delay_us (struct cpufreq_policy) is currently defined
> > as:
> > Preferred average time interval between consecutive invocations of
> > the driver to set the frequency for this policy. To be set by the
> > scaling driver (0, which is the default, means no preference).
> > The transition_latency represents the amount of time necessary for a
> > CPU to change its frequency.
> >
> > A PCCT table advertises mutliple values:
> > - pcc_nominal: Expected latency to process a command, in microseconds
> > - pcc_mpar: The maximum number of periodic requests that the subspace
> > channel can support, reported in commands per minute. 0 indicates no
> > limitation.
> > - pcc_mrtt: The minimum amount of time that OSPM must wait after the
> > completion of a command before issuing the next command,
> > in microseconds.
> > cppc_get_transition_latency() allows to get the max of them.
> >
> > commit d4f3388afd48 ("cpufreq / CPPC: Set platform specific
> > transition_delay_us") allows to select transition_delay_us based on
> > the platform, and fallbacks to cppc_get_transition_latency()
> > otherwise.
> >
> > If _CPC objects are not using PCC channels (no PPCT table), the
> > transition_delay_us is set to CPUFREQ_ETERNAL, leading to really long
> > periods between frequency updates (~4s).
> >
> > If the desired_reg, where performance requests are written, is in
> > SystemMemory or SystemIo ACPI address space, there is no delay
> > in requests. So return 0 instead of CPUFREQ_ETERNAL, leading to
> > transition_delay_us being set to LATENCY_MULTIPLIER us (1000 us).
> >
> > This patch also adds two macros to check the address spaces.
> >
> > Signed-off-by: Pierre Gondois <[email protected]>
> > ---
> > drivers/acpi/cppc_acpi.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> > index 6f09fe011544..cc932ec1b613 100644
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -100,6 +100,16 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
> > (cpc)->cpc_entry.reg.space_id == \
> > ACPI_ADR_SPACE_PLATFORM_COMM)
> >
> > +/* Check if a CPC register is in SystemMemory */
> > +#define CPC_IN_SM(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
> > + (cpc)->cpc_entry.reg.space_id == \
> > + ACPI_ADR_SPACE_SYSTEM_MEMORY)
> > +
>
> Again my taste or preference: s/SM/SYS_MEM or SYSTEM_MEM
SYSTEM_MEMORY even.
>
> > +/* Check if a CPC register is in SystemIo */
> > +#define CPC_IN_SIO(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
> > + (cpc)->cpc_entry.reg.space_id == \
> > + ACPI_ADR_SPACE_SYSTEM_IO)
> > +
>
> Ditto, s/SM/SYS_IO or SYSTEM_IO
>
> I need not refer back to the macro when reading the code. SM/SIO is too
> short and makes it hard to infer from the name in general.
Right.
> > /* Evaluates to True if reg is a NULL register descriptor */
> > #define IS_NULL_REG(reg) ((reg)->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && \
> > (reg)->address == 0 && \
> > @@ -1456,6 +1466,9 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
> > * transition latency for performance change requests. The closest we have
> > * is the timing information from the PCCT tables which provides the info
> > * on the number and frequency of PCC commands the platform can handle.
> > + *
> > + * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
> > + * then assume there is no latency.
> > */
> > unsigned int cppc_get_transition_latency(int cpu_num)
> > {
> > @@ -1481,7 +1494,9 @@ unsigned int cppc_get_transition_latency(int cpu_num)
> > return CPUFREQ_ETERNAL;
> >
> > desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> > - if (!CPC_IN_PCC(desired_reg))
> > + if (CPC_IN_SM(desired_reg) || CPC_IN_SIO(desired_reg))
> > + return 0;
> > + else if (!CPC_IN_PCC(desired_reg))
> > return CPUFREQ_ETERNAL;
On Wed, May 11, 2022 at 03:45:57PM +0200, Pierre Gondois wrote:
> From: Pierre Gondois <[email protected]>
>
> The transition_delay_us (struct cpufreq_policy) is currently defined
> as:
> Preferred average time interval between consecutive invocations of
> the driver to set the frequency for this policy. To be set by the
> scaling driver (0, which is the default, means no preference).
> The transition_latency represents the amount of time necessary for a
> CPU to change its frequency.
>
> A PCCT table advertises mutliple values:
> - pcc_nominal: Expected latency to process a command, in microseconds
> - pcc_mpar: The maximum number of periodic requests that the subspace
> channel can support, reported in commands per minute. 0 indicates no
> limitation.
> - pcc_mrtt: The minimum amount of time that OSPM must wait after the
> completion of a command before issuing the next command,
> in microseconds.
> cppc_get_transition_latency() allows to get the max of them.
>
> commit d4f3388afd48 ("cpufreq / CPPC: Set platform specific
> transition_delay_us") allows to select transition_delay_us based on
> the platform, and fallbacks to cppc_get_transition_latency()
> otherwise.
>
> If _CPC objects are not using PCC channels (no PPCT table), the
> transition_delay_us is set to CPUFREQ_ETERNAL, leading to really long
> periods between frequency updates (~4s).
>
> If the desired_reg, where performance requests are written, is in
> SystemMemory or SystemIo ACPI address space, there is no delay
> in requests. So return 0 instead of CPUFREQ_ETERNAL, leading to
> transition_delay_us being set to LATENCY_MULTIPLIER us (1000 us).
>
> This patch also adds two macros to check the address spaces.
>
> Signed-off-by: Pierre Gondois <[email protected]>
> ---
> drivers/acpi/cppc_acpi.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 6f09fe011544..cc932ec1b613 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -100,6 +100,16 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
> (cpc)->cpc_entry.reg.space_id == \
> ACPI_ADR_SPACE_PLATFORM_COMM)
>
> +/* Check if a CPC register is in SystemMemory */
> +#define CPC_IN_SM(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
> + (cpc)->cpc_entry.reg.space_id == \
> + ACPI_ADR_SPACE_SYSTEM_MEMORY)
> +
Again my taste or preference: s/SM/SYS_MEM or SYSTEM_MEM
> +/* Check if a CPC register is in SystemIo */
> +#define CPC_IN_SIO(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \
> + (cpc)->cpc_entry.reg.space_id == \
> + ACPI_ADR_SPACE_SYSTEM_IO)
> +
Ditto, s/SM/SYS_IO or SYSTEM_IO
I need not refer back to the macro when reading the code. SM/SIO is too
short and makes it hard to infer from the name in general.
> /* Evaluates to True if reg is a NULL register descriptor */
> #define IS_NULL_REG(reg) ((reg)->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && \
> (reg)->address == 0 && \
> @@ -1456,6 +1466,9 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
> * transition latency for performance change requests. The closest we have
> * is the timing information from the PCCT tables which provides the info
> * on the number and frequency of PCC commands the platform can handle.
> + *
> + * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
> + * then assume there is no latency.
> */
> unsigned int cppc_get_transition_latency(int cpu_num)
> {
> @@ -1481,7 +1494,9 @@ unsigned int cppc_get_transition_latency(int cpu_num)
> return CPUFREQ_ETERNAL;
>
> desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> - if (!CPC_IN_PCC(desired_reg))
> + if (CPC_IN_SM(desired_reg) || CPC_IN_SIO(desired_reg))
> + return 0;
> + else if (!CPC_IN_PCC(desired_reg))
> return CPUFREQ_ETERNAL;
Apart from the above,
Reviewed-by: Sudeep Holla <[email protected]>
--
Regards,
Sudeep
On 11-05-22, 15:45, Pierre Gondois wrote:
> From: Pierre Gondois <[email protected]>
>
> The communication mean of the _CPC desired performance can be
> PCC, System Memory, System IO, or Functional Fixed Hardware.
>
> commit b7898fda5bc7 ("cpufreq: Support for fast frequency switching")
> fast_switching is 'for switching CPU frequencies from interrupt
> context'.
> Writes to SystemMemory and SystemIo are fast and suitable this.
> This is not the case for PCC and might not be the case for FFH.
>
> Enable fast_switching for the cppc_cpufreq driver in above cases.
>
> Add cppc_allow_fast_switch() to check the desired performance
> register address space and set fast_switching accordingly.
>
> Signed-off-by: Pierre Gondois <[email protected]>
> ---
> drivers/acpi/cppc_acpi.c | 17 +++++++++++++++++
> drivers/cpufreq/cppc_cpufreq.c | 24 ++++++++++++++++++++++++
> include/acpi/cppc_acpi.h | 5 +++++
> 3 files changed, 46 insertions(+)
Acked-by: Viresh Kumar <[email protected]>
--
viresh