On 03/14/2014 10:07 AM, Viresh Kumar wrote:
On 14 March 2014 20:40, Dirk Brandewie <dirk.brande...@gmail.com> wrote:
Are you proposing adding cpufreq_generic_suspend() to the core I can not
find
it in the mainline code.
Its already there in linux-next. I am suggesting to reuse that
infrastructure with
some necessary modification to support both suspend and hotplug.
Suspend and hotplug are two very different things and if we start
crossing those wires bad things are going to happen IMHO.
In "normal" operation using the suspend path to do this work could
work in principal but doesn't handle the case where the user does
echo 0 | sudo tee /sys/devices/system/cpu/cpuX/online
Trying force hotplug and suspend into a common mechanism would
lead to a bunch of special case code or a significant rework of the
core code IMHO.
Over that I don't think Dirk's solution is going to work if we twist
the systems a bit.
Could you explain what "twist the systems a bit" means?
The one I explained in the below paragraph.
For example, Dirk probably wanted to set P-STATE of every core to MIN
when it goes down. But his solution probably doesn't do that right now.
No, I wanted to set the core that was being off-lined to min P state.
Sorry, probably my words were a bit confusing. I meant exactly what
you just wrote. Core going down will set its freq to min.
As exit() is called only when the policy expires or all the CPUs of that
policy
are down. Suppose only one CPU out of 4 goes down from a policy, then
pstate driver would never know that happened. And that core wouldn't go
to min state.
My patch does not change the semantics of exit() or when it is called. For
intel_pstate their is one cpu per policy so I moved all the cleanup to
I didn't knew that its guaranteed by pstate driver. I thought it would still be
hardware dependent as some cores might share clock line.
This is guaranteed by the hardware. Each core has its own MSR for P state
request. Any coordination that is required between cores to select the
package P state is handled by the hardware.
exit_prepare() if there were work I needed to do at CPU_POST_DEAD I would
have
continued to export the *optional* exit callback.
The callback name exit_prepare() was probably unfortunate and might be
causing
some confusion. I will be changing the name per Rafael's feedback.
Don't know.. There is another problem here that exit_prepare() would be called
for each CPU whereas exit() would be called for each policy.
Granted but I don't see this as a problem in this case there is a 1:1
relationship. If a driver chooses to use the *optional* exit_prepare() callback
and knows that there is a many:1 relationship between the policy and CPUs
then it would have to deal with it.
And I strongly feel that we shouldn't give another callback here but instead
just set core to a specific freq as mentioned by driver with some other field.
I think we have two solutions here:
- If its just about setting core a particular freq when it goes down, I
think it
looks a generic enough problem and so better fix core for that. Probably
with
help of flags field/suspend-freq (maybe renamed) and without calling
drivers
exit() at all..
ATM the only thing that needs to be done in this case is to allow
intel_pstate
to set the P state on the core when it is going done. My solution from the
cores point of view is more generic, it allows any driver that needs to do
work
during CPU_DOWN_PREPARE to do it without adding any new logic to the core.
Yeah, do we really need to give that freedom right now? Would be better
to get this into core as that would be more generic and people looking to set
core to some freq at shutdown wouldn't be replicating that code.
IMHO yes and it would be hard to be more generic, if your platform needs to
do architecture specific during the PREPARE phase of cpu hotplug use this
callback or not.
BTW now that you have added a path where the cpufreq_suspend() could fail
it return a value and be checked in dpm_suspend() instead of printing an
error and just continuing.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/