ACPI: update user_policy.max when _PPC updated

PATCH In review Joe Jin 2 participants 7 messages Jun 5, 2013 – Jun 7, 2013 ACPI

lore

Share & tools

lore

Joe Jin

author

2013-06-05 00:52 UTC #0 lore raw

PATCH 1/1 ACPI: update user_policy.max when _PPC updated

When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 1 line

1.8.1.4

Rafael J. Wysocki

2013-06-05 20:31 UTC #1 lore raw

Quoted 2 lines · Joe Jin

When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.

Why should the user setting be always related to the current maximum available frequency? What if the user sets the limit for power capping purposes?

Rafael

Quoted 44 lines

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Joe Jin

author

2013-06-06 00:27 UTC #2 lore raw

Quoted 6 lines · Rafael J. Wysocki

Quoted 2 lines · Joe Jin
When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.
Why should the user setting be always related to the current maximum available frequency? What if the user sets the limit for power capping purposes?

cpufreq_update_policy() get policy->max from user_policy.max:

1782 int cpufreq_update_policy(unsigned int cpu)
1783 {
[...]

1800         policy.min = data->user_policy.min;
1801         policy.max = data->user_policy.max;
1802         policy.policy = data->user_policy.policy;
1803         policy.governor = data->user_policy.governor;

[...]
1819 ret = __cpufreq_set_policy(data, &policy);
[...]

/sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max and user_policy->max, when update it, so I think _PPC changes also need to update these two?

Thanks,
Joe

Quoted 48 lines

Rafael

Quoted 44 lines

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 4 lines

Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing

Rafael J. Wysocki

2013-06-06 10:57 UTC #3 lore raw

Quoted 17 lines · Joe Jin

Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Joe Jin
When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.
Why should the user setting be always related to the current maximum available frequency? What if the user sets the limit for power capping purposes?
cpufreq_update_policy() get policy->max from user_policy.max:
1782 int cpufreq_update_policy(unsigned int cpu)
1783 {
[...]
1800         policy.min = data->user_policy.min;
1801         policy.max = data->user_policy.max;
1802         policy.policy = data->user_policy.policy;
1803         policy.governor = data->user_policy.governor;

Quoted 7 lines

[...]
1819 ret = __cpufreq_set_policy(data, &policy);
[...]
/sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max and user_policy->max, when update it, so I think _PPC changes also need to update these two?

Yes, if policy.max happens to be greater that the maximum available frequency, then (and only then) it probably should be updated. It should never be bumped up, though.

Thanks,
Rafael

Quoted 47 lines

Quoted 44 lines

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Joe Jin

author

2013-06-06 13:29 UTC #4 lore raw

Quoted 22 lines · Rafael J. Wysocki

Quoted 21 lines · Joe Jin
Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Joe Jin
When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.
Why should the user setting be always related to the current maximum available frequency? What if the user sets the limit for power capping purposes?
cpufreq_update_policy() get policy->max from user_policy.max:
1782 int cpufreq_update_policy(unsigned int cpu)
1783 {
[...]
1800         policy.min = data->user_policy.min;
1801         policy.max = data->user_policy.max;
1802         policy.policy = data->user_policy.policy;
1803         policy.governor = data->user_policy.governor;
[...]
1819 ret = __cpufreq_set_policy(data, &policy);
[...]

Quoted 7 lines

Quoted 3 lines
/sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max and user_policy->max, when update it, so I think _PPC changes also need to update these two?
Yes, if policy.max happens to be greater that the maximum available frequency, then (and only then) it probably should be updated. It should never be bumped up, though.

Does this means if I enabled hardcap before kernel boot up, and later system brought up and I disabled hardcap, I has to enlarge the max frequency manually?

Thanks,
Joe

Quoted 52 lines

Thanks,
Rafael

Quoted 47 lines

Quoted 44 lines

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 4 lines

Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing

Rafael J. Wysocki

2013-06-06 19:45 UTC #5 lore raw

Quoted 27 lines · Joe Jin

Quoted 26 lines · Rafael J. Wysocki
Quoted 24 lines · Joe Jin
Quoted 6 lines · Rafael J. Wysocki
Quoted 2 lines · Joe Jin
When _PPC changed dynamically the user_policy.max will not be updated, this prevent CPU run on the highest frequency.
Why should the user setting be always related to the current maximum available frequency? What if the user sets the limit for power capping purposes?
cpufreq_update_policy() get policy->max from user_policy.max:
1782 int cpufreq_update_policy(unsigned int cpu)
1783 {
[...]
1800         policy.min = data->user_policy.min;
1801         policy.max = data->user_policy.max;
1802         policy.policy = data->user_policy.policy;
1803         policy.governor = data->user_policy.governor;
[...]
1819 ret = __cpufreq_set_policy(data, &policy);
[...]
/sys/devices/system/cpu/cpu$/cpufreq/scaling_max_freq using policy->max and user_policy->max, when update it, so I think _PPC changes also need to update these two?

Quoted 6 lines

Quoted 3 lines
Yes, if policy.max happens to be greater that the maximum available frequency, then (and only then) it probably should be updated. It should never be bumped up, though.
Does this means if I enabled hardcap before kernel boot up, and later system brought up and I disabled hardcap, I has to enlarge the max frequency manually?

Do you mean you set a limit in the BIOS setup and the kernel changed that limit on boot?

Rafael

Quoted 50 lines

Quoted 47 lines

Quoted 44 lines

Signed-off-by Joe Jin <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Viresh Kumar <[email protected]>

---
 drivers/acpi/processor_perflib.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/acpi/processor_perflib.c +16 -1

index e854582..e01aa7d 100644
@@ -180,6 +180,7 @@ static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 {
 	int ret;
+	unsigned int saved = (unsigned int)pr->performance_platform_limit;
 
 	if (ignore_ppc) {
 		/*

@@ -204,8 +205,22 @@ int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 	}
 	if (ret < 0)
 		return (ret);
-	else
+	else {
+		unsigned int ppc = (unsigned int)pr->performance_platform_limit;
+
+		if (saved != ppc) {
+			struct cpufreq_policy *policy;
+
+			policy = cpufreq_cpu_get(pr->id);
+			if (likely(policy))
+				policy->user_policy.max =
+					pr->performance->states[ppc].
+					core_frequency * 1000;
+			cpufreq_cpu_put(policy);
+		}
+
 		return cpufreq_update_policy(pr->id);
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Joe Jin

author

2013-06-07 01:48 UTC #6 lore raw

Quoted 2 lines · Rafael J. Wysocki

Do you mean you set a limit in the BIOS setup and the kernel changed that limit on boot?

Sorry for the confusing.

The issue is when we disable hardcap before kernel boot up, after kernel bring up, any changes of _PPC will update scaling_max_freq properly.

If we enable hardcap before kernel boot up, after kernel bring up, even we disable it, scaling_max_freq does not be updated to max frequency, the max frequency just up to the value when bring up.

Review related codes I found the limit came from user_policy.max, means when we set user_policy.max to 1000MHZ when boot up, then any changes of _PPC could not enlarge the scaling_max_freq, I think this is not as expected? please advise.

Thanks,
Joe