On 08/14/2014 09:22 PM, Lina Iyer wrote:
On Thu, Aug 14, 2014 at 06:11:43PM +0200, Daniel Lezcano wrote:
On 08/13/2014 04:16 PM, Lina Iyer wrote:
On Wed, Aug 13, 2014 at 01:18:01PM +0200, Daniel Lezcano wrote:
On 08/12/2014 09:43 PM, Lina Iyer wrote:
Add interface layer to abstract and handle hardware specific
functionality for executing various cpu low power modes in QCOM
chipsets.
Signed-off-by: Venkat Devarasetty <vdevaras@xxxxxxxxxxxxxx>
Signed-off-by: Mahesh Sivasubramanian <msivasub@xxxxxxxxxxxxxx>
Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx>
---
[ ... ]
+static bool msm_pm_retention(bool from_idle)
+{
+ int ret = 0;
+
+ ret = msm_spm_set_low_power_mode(MSM_SPM_MODE_RETENTION, false);
+ WARN_ON(ret);
+
+ msm_arch_idle();
+
+ ret = msm_spm_set_low_power_mode(MSM_SPM_MODE_CLOCK_GATING,
false);
+ WARN_ON(ret);
Why do you need to set the clock gating mode each time you exit the
retention mode ?
So if the SPM did not reset to clockgating, we would not do retention
when we intended to do clockgating. Btw, we dont set clockgating
everytime we do clockgating, helps reduce the latency in doing WFI.
Thanks for the explanation in the other email. So IIUC, the SCM keeps
the last state configuration and we have to set it back to clock
gating, right ?
Correct.
I don't think it is up to this function to do this but the clock
gating function.
Also, this function prototype looks a bit weird. Just for the sake of
using callbacks.
And finally, the WARN_ON is not desirable here, except if the goal is
to flood the terminal :)
Was debating the use of it myself. Will remove it.
What not using first simple functions ?
void qcom_do_idle(void)
{
myfirmware_call(MSM_SPM_MODE_CLOCK_GATING);
wfi();
}
void qcom_cpu_retention(void)
{
myfirmware_call(MSM_SPM_MODE_RETENTION);
dsb();
wfi();
}
void qcom_cpu_powerdown(int flags)
{
scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
}
and then you build on top of that the cpuidle driver.
Okay. Will do this
The patchset adds all the features in one shot and for someone not
used with the platform it is really hard to follow all the code.
I suggest you write a simple cpuidle driver based on the DT Lorenzo
patches bringing the clock gating, then another patchset with the
retention mode, etc ...
>>
Do you agree with this approach ?
+ return true;
+}
+
+static int msm_pm_collapse(unsigned long from_idle)
+{
+ enum msm_pm_l2_scm_flag flag = MSM_SCM_L2_ON;
+
+ /**
+ * Single core processors need to have L2
+ * flushed when powering down the core.
+ * Notify SCM to flush secure L2 lines.
+ */
+ if (num_possible_cpus() == 1)
+ flag = MSM_SCM_L2_OFF;
I am wondering if this shouldn't be handle by a mcpm driver.
Cc nico.
Well, possibly, sorry, not sure what features of the mcpm driver you
think I need here?
Please correct me if I am wrong. IIUC, this function is checking the
number of the cpus of the cluster in order to flush the L2 cache
because the SCM will power down the cluster if it is the last one,
right ?
Nope. Some QCOM variants which have a single CPU, cannot be powered down
without flushing the caches. Warm boot of the cpu resets the L2
logic as well. The cluster core is lot more complex than this :)
Ok, probably we can discuss this later when we reach this state in the
incremental implementation.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
