@@ -111,15 +111,11 @@
static struct pcc_cpu *pcc_cpu_info;
-static struct cpufreq_frequency_table pcc_freq_table[] = {
- {0x1, 0},
- {0x2, 0},
- {0, CPUFREQ_TABLE_END},
-};
-
static int pcc_cpufreq_verify(struct cpufreq_policy *policy)
{
- return cpufreq_frequency_table_verify(policy, pcc_freq_table);
+ cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
+ policy->cpuinfo.max_freq);
+ return 0;
}
static inline void pcc_cmd(void)
@@ -534,10 +530,6 @@
goto pcch_free;
}
- pcc_freq_table[0].frequency =
- ioread32(&pcch_hdr->minimum_frequency) * 1000;
- pcc_freq_table[1].frequency = ioread32(&pcch_hdr->nominal) * 1000;
-
printk(KERN_DEBUG "pcc-cpufreq: (v%s) driver loaded with frequency"
" limits: %d MHz, %d MHz\n", PCC_VERSION,
ioread32(&pcch_hdr->minimum_frequency),
@@ -576,12 +568,6 @@
dprintk("init: policy->max is %d, policy->min is %d\n",
policy->max, policy->min);
- result = cpufreq_frequency_table_cpuinfo(policy, pcc_freq_table);
- if (result)
- goto free;
-
- cpufreq_frequency_table_get_attr(pcc_freq_table, policy->cpu);
-
return 0;
free:
pcc_clear_mapping();
@@ -592,15 +578,9 @@
static int pcc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
{
- cpufreq_frequency_table_put_attr(policy->cpu);
return 0;
}
-static struct freq_attr *pcc_cpufreq_attr[] = {
- &cpufreq_freq_attr_scaling_available_freqs,
- NULL,
-};
-
static struct cpufreq_driver pcc_cpufreq_driver = {
.flags = CPUFREQ_CONST_LOOPS,
.get = pcc_get_freq,
@@ -610,7 +590,6 @@
.exit = pcc_cpufreq_cpu_exit,
.name = "pcc-cpufreq",
.owner = THIS_MODULE,
- .attr = pcc_cpufreq_attr,
};
static int __init pcc_cpufreq_init(void)
@@ -152,11 +152,10 @@
2.1 scaling_available_frequencies:
----------------------------------
-scaling_available_frequencies indicates the minimum and maximum speed
-the CPU can take as advertised by the BIOS. No intermediate frequencies are
-listed because the BIOS will try to achieve any intermediate frequency
-requested by the governor. An intermediate frequency does not have to be
-strictly associated with a P-state.
+scaling_available_frequencies is not created in /sys. No intermediate
+frequencies need to be listed because the BIOS will try to achieve any
+frequency, within limits, requested by the governor. A frequency does not have
+to be strictly associated with a P-state.
2.2 cpuinfo_transition_latency:
-------------------------------
@@ -203,7 +202,7 @@
3. Caveats:
-----------
-Currently, the "cpufreq_stats" module in its present form cannot be loaded and
-expected to work with the PCC driver. A patch to cpufreq_stats will be
-submitted to fix this.
+The "cpufreq_stats" module in its present form cannot be loaded and
+expected to work with the PCC driver. Since the "cpufreq_stats" module
+provides information wrt each P-state, it is not applicable to the PCC driver.
>> +2.2 cpuinfo_transition_latency:
>> +-------------------------------
>> +The cpuinfo_transition_latency field is 0. The PCC
>specification does
>> +not include a field to expose this value currently.
>
>Uh, bad specification... So does it work properly with ondemand and/or
>conservative (which read out this field, and if latency=0 use a minimum
>value)?
>
Yes, it works fine. Both "ondemand" and "conservative" sanitize the
latency value, and set it to a default value which is good.
>> +config X86_PCC_CPUFREQ
>> + tristate "Processor Clocking Control interface driver"
>> + select CPU_FREQ_TABLE
>Uh, see above.
>
@@ -12,7 +12,6 @@
config X86_PCC_CPUFREQ
tristate "Processor Clocking Control interface driver"
- select CPU_FREQ_TABLE
depends on ACPI && ACPI_PROCESSOR
help
This driver adds support for the PCC interface.
>> +static int pcc_cpufreq_verify(struct cpufreq_policy *policy)
>> +{
>> + return cpufreq_frequency_table_verify(policy, pcc_freq_table);
>> +}
>
>Well, AFAICS, this limits the whole interface to two values:
>min or max. So
>let's allow for any intermediate value:
>
>static int pcc_cpufreq_verify(struct cpufreq_policy *policy)
>{
> cpufreq_verify_within_limits(policy,
>policy->cpuinfo.min_freq, policy->cpuinfo.max_freq);
>}
>
Took this suggestion, (see the first "diff" changeset above), especially
since I cannot use the freq-table helper function anymore.
>but well... later on, you check this
>
>> + if (target_freq <=
>(ioread32(&pcch_hdr->minimum_frequency) * 1000)) {
>> + target_freq =
>ioread32(&pcch_hdr->minimum_frequency) * 1000;
>> + dprintk("target: target_freq for cpu %d was
>below limit, "
>> + "converted it to %d\n", cpu, target_freq);
>> + }
>
>why not do this in the _verify() step? Does pcch_hdr->minimum_frequency
>even change "on the fly"?
pcch_hdr->minimum_frequency does not change "on the fly". Also, there is no
need for those IO accesses:
@@ -212,12 +212,18 @@
pcc_cpu_data = per_cpu_ptr(pcc_cpu_info, cpu);
- if (target_freq <= (ioread32(&pcch_hdr->minimum_frequency) * 1000)) {
- target_freq = ioread32(&pcch_hdr->minimum_frequency) * 1000;
+ if (target_freq < policy->min) {
+ target_freq = policy->min;
dprintk("target: target_freq for cpu %d was below limit, "
"converted it to %d\n", cpu, target_freq);
}
+ if (target_freq > policy->max) {
+ target_freq = policy->max;
+ dprintk("target: target_freq for cpu %d was above limit, "
+ "converted it to %d\n", cpu, target_freq);
+ }
+
dprintk("target: CPU %d should go to target freq: %d "
"(virtual) input_offset is 0x%x\n",
cpu, target_freq,