[PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Pierre Gondois]

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 <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
---
 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..9eca43d1d941 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_SPACE;
+
 	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_SPACE;
 	}
 
 	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..03465db16b68 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_SPACE		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

[PATCH v2 2/5] ACPI: bus: Set CPPC _OSC bits for all and when CPPC_LIB is supported

[Pierre Gondois]

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 <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
---
 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 9eca43d1d941..1fc24f4fbcb4 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_SPACE;
@@ -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

[PATCH v2 3/5] ACPI: CPPC: Assume no transition latency if no PCCT

[Pierre Gondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

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 <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
---
 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..840223c12540 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_SYSTEM_MEMORY(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_SYSTEM_IO(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_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
+		return 0;
+	else if (!CPC_IN_PCC(desired_reg))
 		return CPUFREQ_ETERNAL;
 
 	if (pcc_ss_id < 0)
-- 
2.25.1

[PATCH v2 4/5] cpufreq: CPPC: Enable fast_switch

[Pierre Gondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

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 <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/acpi/cppc_acpi.c       | 18 ++++++++++++++++++
 drivers/cpufreq/cppc_cpufreq.c | 24 ++++++++++++++++++++++++
 include/acpi/cppc_acpi.h       |  5 +++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 840223c12540..3b299b28a8af 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -434,6 +434,24 @@ 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_SYSTEM_MEMORY(desired_reg) &&
+				!CPC_IN_SYSTEM_IO(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

[PATCH v2 5/5] cpufreq: CPPC: Enable dvfs_possible_from_any_cpu

[Pierre Gondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

The communication mean of the _CPC desired performance can be
PCC, System Memory, System IO, or Functional Fixed Hardware (FFH).

PCC, SystemMemory and SystemIo address spaces are available from any
CPU. Thus, dvfs_possible_from_any_cpu should be enabled in such case.
For FFH, let the FFH implementation do smp_call_function_*() calls.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cppc_cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 000a0c610c30..ad1535fbf389 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -558,6 +558,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	}
 
 	policy->fast_switch_possible = cppc_allow_fast_switch();
+	policy->dvfs_possible_from_any_cpu = true;
 
 	/*
 	 * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost
-- 
2.25.1

Re: [PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Viresh Kumar]

On 18-05-22, 11:08, Pierre Gondois wrote:
> 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 <pierre.gondois@arm.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/acpi/bus.c       | 18 ++++++++++++++++++
>  drivers/acpi/cppc_acpi.c |  9 +++++++++
>  include/linux/acpi.h     |  2 ++
>  3 files changed, 29 insertions(+)

It would be much useful to get a cover letter for this series with
what you have changed since the previous version. It is almost
impossible to find that out otherwise.

-- 
viresh

Re: [PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Pierre Gondois]

On 5/18/22 11:42, Viresh Kumar wrote:
> On 18-05-22, 11:08, Pierre Gondois wrote:
>> 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 <pierre.gondois@arm.com>
>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/acpi/bus.c       | 18 ++++++++++++++++++
>>   drivers/acpi/cppc_acpi.c |  9 +++++++++
>>   include/linux/acpi.h     |  2 ++
>>   3 files changed, 29 insertions(+)
> 
> It would be much useful to get a cover letter for this series with
> what you have changed since the previous version. It is almost
> impossible to find that out otherwise.
> 

Yes indeed, sorry for that, the changelog is:
v2:
[1/5] ACPI: CPPC: Check _OSC for flexible address space
- Renamed OSC_SB_CPC_FLEXIBLE_ADR_SP to OSC_SB_CPC_FLEXIBLE_ADR_SPACE
[3/5] ACPI: CPPC: Assume no transition latency if no PCCT
and
[4/5] cpufreq: CPPC: Enable fast_switch
- Renamed CPC_IN_SIO to CPC_IN_SYSTEM_IO
- Renamed CPC_IN_SM to CPC_IN_SYSTEM_MEMORY

or do you prefer to have the serie re-submitted ?

Regards,
Pierre

Re: [PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Viresh Kumar]

On 18-05-22, 12:34, Pierre Gondois wrote:
> 
> 
> On 5/18/22 11:42, Viresh Kumar wrote:
> > On 18-05-22, 11:08, Pierre Gondois wrote:
> > > 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 <pierre.gondois@arm.com>
> > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > >   drivers/acpi/bus.c       | 18 ++++++++++++++++++
> > >   drivers/acpi/cppc_acpi.c |  9 +++++++++
> > >   include/linux/acpi.h     |  2 ++
> > >   3 files changed, 29 insertions(+)
> > 
> > It would be much useful to get a cover letter for this series with
> > what you have changed since the previous version. It is almost
> > impossible to find that out otherwise.
> > 
> 
> Yes indeed, sorry for that, the changelog is:
> v2:
> [1/5] ACPI: CPPC: Check _OSC for flexible address space
> - Renamed OSC_SB_CPC_FLEXIBLE_ADR_SP to OSC_SB_CPC_FLEXIBLE_ADR_SPACE
> [3/5] ACPI: CPPC: Assume no transition latency if no PCCT
> and
> [4/5] cpufreq: CPPC: Enable fast_switch
> - Renamed CPC_IN_SIO to CPC_IN_SYSTEM_IO
> - Renamed CPC_IN_SM to CPC_IN_SYSTEM_MEMORY
> 
> or do you prefer to have the serie re-submitted ?

No it is fine for now :)

-- 
viresh

Re: [PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Rafael J. Wysocki]

On Wed, May 18, 2022 at 11:09 AM Pierre Gondois <pierre.gondois@arm.com> wrote:
>
> 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 <pierre.gondois@arm.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  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..9eca43d1d941 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_SPACE;
> +
>         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_SPACE;
>         }
>
>         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..03465db16b68 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_SPACE          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
> --

Applied as 5.19 material along with the rest of the series, thanks!

[Devel] Re: [PATCH v2 1/5] ACPI: CPPC: Check _OSC for flexible address space

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 5729 bytes --]

On Wed, May 18, 2022 at 11:09 AM Pierre Gondois <pierre.gondois(a)arm.com> wrote:
>
> 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 <pierre.gondois(a)arm.com>
> Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com>
> ---
>  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..9eca43d1d941 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_SPACE;
> +
>         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_SPACE;
>         }
>
>         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..03465db16b68 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_SPACE          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
> --

Applied as 5.19 material along with the rest of the series, thanks!