Does it need a hardware change for using CPPC?
+static int cppc_cpufreq_target(struct cpufreq_policy *policy,
unsigned int target_freq,
unsigned int relation)
+{
- unsigned int cpu = policy->cpu;
- struct cpc_desc *current_cpu_cpc = per_cpu(cpc_desc, cpu);
- struct cpufreq_freqs freqs;
- u16 status;
- freqs.old = policy->cur;
- freqs.new = target_freq;
- cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
- /* Set CPU Perf thresholds and current desired perf value. */
- acpi_write(policy->max, ¤t_cpu_cpc->pcc_regs[MAX_PERF]);
- acpi_write(policy->min, ¤t_cpu_cpc->pcc_regs[MIN_PERF]);
- acpi_write(target_freq, ¤t_cpu_cpc->pcc_regs[DESIRED_PERF]);
- status = send_pcc_cmd(CMD_WRITE, 0, PCC_SUBSPACE_IDX,
comm_base_addr);
- if (status & CMD_COMPLETE) {
pr_debug("Failed to set target CPU perf for CPU:%d,
status:%d\n",
cpu, status);
return -EINVAL;
- }
- return 0;
+}
-> Don't need cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE)?
- /* Base address returned from PCC subspace desc needs to ioremap'd.
* Used by the client to send/recv data from platform.
*/
- comm_base_addr = ioremap_nocache(pcc_comm_base_addr, len);
- if (comm_base_addr) {
->Should be if (!comm_base_addr) ?
pr_err("Could not map PCC communicate channel\n");
ret = -ENOMEM;
goto out_err;
- }
Thanks Best Regards.
Hi Jonghwan,
On 7 May 2014 20:42, Jonghwan Choi jhbird.choi@samsung.com wrote:
Does it need a hardware change for using CPPC?
CPPC requires support in firmware (e.g. in the BMC). The performance registers can be implemented as memory mapped counters or dedicated registers.
+static int cppc_cpufreq_target(struct cpufreq_policy *policy,
unsigned int target_freq,
unsigned int relation)
+{
unsigned int cpu = policy->cpu;
struct cpc_desc *current_cpu_cpc = per_cpu(cpc_desc, cpu);
struct cpufreq_freqs freqs;
u16 status;
freqs.old = policy->cur;
freqs.new = target_freq;
cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
/* Set CPU Perf thresholds and current desired perf value. */
acpi_write(policy->max, ¤t_cpu_cpc->pcc_regs[MAX_PERF]);
acpi_write(policy->min, ¤t_cpu_cpc->pcc_regs[MIN_PERF]);
acpi_write(target_freq, ¤t_cpu_cpc->pcc_regs[DESIRED_PERF]);
status = send_pcc_cmd(CMD_WRITE, 0, PCC_SUBSPACE_IDX,
comm_base_addr);
if (status & CMD_COMPLETE) {
pr_debug("Failed to set target CPU perf for CPU:%d,
status:%d\n",
cpu, status);
return -EINVAL;
}
return 0;
+}
-> Don't need cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE)?
Yes will need some sort of post notify transition here.
/* Base address returned from PCC subspace desc needs to ioremap'd.
* Used by the client to send/recv data from platform.
*/
comm_base_addr = ioremap_nocache(pcc_comm_base_addr, len);
if (comm_base_addr) {
->Should be if (!comm_base_addr) ?
D'oh! Thanks for catching this.
Like I mentioned in the cover letter of this series, the CPPC patch is only compile tested. I'm trying to verify it at runtime, but I've put it out there early so that anyone who already has platform support for CPPC can help us immensely in giving it a try. Come to think of it, I should have probably mentioned this again in the subject of this patch. Will do it in the next revision. :)
Thanks for your quick review!
Cheers, Ashwin
On Wed, May 7, 2014 at 9:14 PM, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
Like I mentioned in the cover letter of this series, the CPPC patch is only compile tested. I'm trying to verify it at runtime, but I've put it out there early so that anyone who already has platform support for CPPC can help us immensely in giving it a try. Come to think of it, I should have probably mentioned this again in the subject of this patch. Will do it in the next revision. :)
I have some questions.
1. When os request the changing frequent to firmware, can the request be changed? (for example. if os request P0 state, can firmware change P0 into P2, p3 considering thermal or other condition)?
2. Can firmware change the P-state itself without OS’s request? -If it is possible, I think that cpufreq governor also should be changed. -And can firmware nofity OS the change then?
Thanks
Best Regards.
Hello,
On 11 May 2014 07:46, jonghwan Choi jhbird.choi@gmail.com wrote:
On Wed, May 7, 2014 at 9:14 PM, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
Like I mentioned in the cover letter of this series, the CPPC patch is only compile tested. I'm trying to verify it at runtime, but I've put it out there early so that anyone who already has platform support for CPPC can help us immensely in giving it a try. Come to think of it, I should have probably mentioned this again in the subject of this patch. Will do it in the next revision. :)
I have some questions.
- When os request the changing frequent to firmware, can the request
be changed? (for example. if os request P0 state, can firmware change P0 into P2, p3 considering thermal or other condition)?
With CPPC there are no "P" states, so I'm assuming you're referring to it just for conveying your point. So, when the OSPM sends a request, it sends three parameters (for now) . min (Min perf register), max (Max perf register) and desired (Desired perf register). The platform can choose to deliver (via Delivered counter register) back CPU performance within this range. The value chosen by the platform will be dependent on what information it has available. e.g. thermal, other performance counters etc.
- Can firmware change the P-state itself without OS’s request?
Yes.
-If it is possible, I think that cpufreq governor also should be changed.
In the short term, we'd want to avoid changes to the CPUfreq governor as much as possible and for this case, I think dont we need to make any. It should just be a matter of refreshing the values seen by the OSPM on a notify event. In the long term, CPUFreq governors (if there is one anymore :) ) could be made aware of the other knobs that CPPC exports. (See the _CPC table in the ACPI Spec) to make more smarter decisions on CPU performance.
-And can firmware nofity OS the change then?
There is an ACPI event notification proposal in progress that notifies the OS that the delivered performance has changed. This event should trigger the refreshing of values seen by the CPUFreq governors.
Cheers, Ashwin