This patch makes sense to me. Just several comments.
> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Mark Langsdorf
> Sent: Tuesday, March 30, 2010 2:28 AM
> To: xen-devel@xxxxxxxxxxxxxxxxxxx; guanqun.lu@xxxxxxxxx
> Subject: [Xen-devel] [PATCH] 1/3 Refactor Xen support for Intel Turbo boost
>
> # HG changeset patch
> # User mark.langsdorf@xxxxxxx
> # Date 1269540083 18000
> # Node ID 6d42054cb1947841ab34f04c1172971068487922
> # Parent 18f4db5f72d756dda30e28d5872d91d35fdf9f0c
> Refactor the existing code that supports the Intel Turbo feature to
> move all the driver specific bits in the cpufreq driver. Create
> a tri-state interface for the Turbo feature that can distinguish
> amongst enabled Turbo, disabled Turbo, and processors that don't
> support Turbo at all.
>
> Changing the code this way is necessary because the similar AMD
> feature Boost has a similar external interface but a very
> different internal implementation.
>
> Signed-off-by: Mark Langsdorf <mark.langsdorf@xxxxxxx>
>
> diff -r 18f4db5f72d7 -r 6d42054cb194 tools/misc/xenpm.c
> --- a/tools/misc/xenpm.c Thu Mar 25 10:01:05 2010 +0000
> +++ b/tools/misc/xenpm.c Thu Mar 25 13:01:23 2010 -0500
> @@ -529,8 +529,13 @@
> p_cpufreq->u.ondemand.sampling_rate);
> printf(" up_threshold : %u\n",
> p_cpufreq->u.ondemand.up_threshold);
> - printf(" turbo mode : %s\n",
> - p_cpufreq->u.ondemand.turbo_enabled ? "enabled" : "disabled");
> + if (p_cpufreq->u.ondemand.turbo_enabled != 0) {
> + printf(" turbo mode : ");
> + if (p_cpufreq->u.ondemand.turbo_enabled == 1)
> + printf("enabled\n");
> + else
> + printf("disabled\n");
> + }
> }
Since the "turbo_enabled" is changed from governor specific flag to global
cpufreq flag, this code should also be changed accordingly. E.g.
- move "turbo_enabled" from "struct xen_ondemand" to "struct
xen_get_cpufreq_para"
- display turbo_enabled as cpufreq parameter, not dbs governor parameter.
>
> printf("scaling_avail_freq :");
> diff -r 18f4db5f72d7 -r 6d42054cb194 xen/arch/x86/acpi/cpufreq/cpufreq.c
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c Thu Mar 25 10:01:05 2010
> +0000
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c Thu Mar 25 13:01:23
> 2010 -0500
> @@ -410,6 +410,10 @@
> return -ENODEV;
> }
>
> + if (policy->turbo == -1)
> + if (target_freq > policy->cpuinfo.second_max_freq)
> + target_freq = policy->cpuinfo.second_max_freq;
> +
> perf = data->acpi_data;
> result = cpufreq_frequency_table_target(policy,
> data->freq_table,
> @@ -610,12 +614,19 @@
> break;
> }
>
> - /* Check for APERF/MPERF support in hardware */
> + /* Check for APERF/MPERF support in hardware
> + * also check for boost support */
> if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 6) {
> unsigned int ecx;
> + unsigned int eax;
> ecx = cpuid_ecx(6);
> if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY)
> acpi_cpufreq_driver.getavg = get_measured_perf;
> + eax = cpuid_eax(6);
> + if ( eax & 0x2 ) {
> + policy->turbo = 1;
> + printk(XENLOG_INFO "Turbo Mode detected and enabled!\n");
> + }
> }
>
> /*
> diff -r 18f4db5f72d7 -r 6d42054cb194
> xen/drivers/cpufreq/cpufreq_ondemand.c
> --- a/xen/drivers/cpufreq/cpufreq_ondemand.c Thu Mar 25 10:01:05
> 2010 +0000
> +++ b/xen/drivers/cpufreq/cpufreq_ondemand.c Thu Mar 25 13:01:23
> 2010 -0500
> @@ -58,9 +58,6 @@
>
> static struct timer dbs_timer[NR_CPUS];
>
> -/* Turbo Mode */
> -static int turbo_detected = 0;
> -
> int write_ondemand_sampling_rate(unsigned int sampling_rate)
> {
> if ( (sampling_rate > MAX_SAMPLING_RATE / MICROSECS(1)) ||
> @@ -111,10 +108,6 @@
>
> policy = this_dbs_info->cur_policy;
> max = policy->max;
> - if (turbo_detected && !this_dbs_info->turbo_enabled) {
> - if (max > policy->cpuinfo.second_max_freq)
> - max = policy->cpuinfo.second_max_freq;
> - }
>
> if (unlikely(policy->resume)) {
> __cpufreq_driver_target(policy, max,CPUFREQ_RELATION_H);
> @@ -275,7 +268,6 @@
> } else
> dbs_tuners_ins.sampling_rate = usr_sampling_rate;
> }
> - this_dbs_info->turbo_enabled = 1;
> dbs_timer_init(this_dbs_info);
>
> break;
> @@ -352,13 +344,6 @@
>
> static int __init cpufreq_gov_dbs_init(void)
> {
> -#ifdef CONFIG_X86
> - unsigned int eax = cpuid_eax(6);
> - if ( eax & 0x2 ) {
> - turbo_detected = 1;
> - printk(XENLOG_INFO "Turbo Mode detected!\n");
> - }
> -#endif
> return cpufreq_register_governor(&cpufreq_gov_dbs);
> }
> __initcall(cpufreq_gov_dbs_init);
> @@ -406,16 +391,27 @@
>
> void cpufreq_dbs_enable_turbo(int cpuid)
> {
> - per_cpu(cpu_dbs_info, cpuid).turbo_enabled = 1;
> + struct cpufreq_policy *policy;
> +
> + policy = per_cpu(cpu_dbs_info, cpuid).cur_policy;
> + if (policy->turbo != 0)
> + policy->turbo = 1;
> }
>
> void cpufreq_dbs_disable_turbo(int cpuid)
> {
> - per_cpu(cpu_dbs_info, cpuid).turbo_enabled = 0;
> + struct cpufreq_policy *policy;
> +
> + policy = per_cpu(cpu_dbs_info, cpuid).cur_policy;
> + if (policy->turbo != 0)
> + policy->turbo = -1;
> }
>
> unsigned int cpufreq_dbs_get_turbo_status(int cpuid)
> {
> - return turbo_detected && per_cpu(cpu_dbs_info, cpuid).turbo_enabled;
> + struct cpufreq_policy *policy;
> +
> + policy = per_cpu(cpu_dbs_info, cpuid).cur_policy;
> + return policy->turbo;
> }
Since "turbo_enabled " is cpufreq policy flag now, better to move these routine
to xen/drivers/cpufreq/utility.c, and change name to something like
cpufreq_enable_turbo...
>
> diff -r 18f4db5f72d7 -r 6d42054cb194 xen/include/acpi/cpufreq/cpufreq.h
> --- a/xen/include/acpi/cpufreq/cpufreq.h Thu Mar 25 10:01:05 2010
> +0000
> +++ b/xen/include/acpi/cpufreq/cpufreq.h Thu Mar 25 13:01:23 2010 -0500
> @@ -55,6 +55,8 @@
>
> unsigned int resume; /* flag for cpufreq 1st run
> * S3 wakeup, hotplug cpu, etc */
> + int turbo; /* tristate flag: 0 for unsupported
> + * -1 for disable, 1 for enabled */
> };
> extern struct cpufreq_policy *cpufreq_cpu_policy[NR_CPUS];
Better to define readable constant like:
#define CPUFREQ_TURBO_DISABLED -1
#define CPUFREQ_TURBO_UNSUPPORTED 0
#define CPUFREQ_TURBO_ENABLED 1
instead of using magic number -1,0,1 in the above codes.
Best Regards
Ke
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home