Qualcomm RPM sleep states
Subsystems (4)
cover Qualcomm RPM sleep states
During the review of the Qualcomm SMD RPM regulators [1], I got (offline) feedback that my implementation did not handle "sleep states". As the problem is shared between all families of Qualcomm platforms I use [2] (family A) to propose a solution (as I hope to get that merged sooner).
The "sleep states" comment boils down to certain regulators (or rpm resources in general) are used by the currently clocked/running CPU(s) and can not be disabled while we're still running. Further more, these resources are shared with peripherals in the system; e.g. LDO12 on PM8941 is used to clock the CPU and WiFi/BT PLLs as well as providing power to the display in our devices. So the suspend functionality in the regulator framework doesn't cut it.
The downstream solution to this is to expose 3 regulators per regulator resource, each specified to control the active mode, sleep mode or both modes respectively. Peripherals are directed to use the "both" regulator while the CPUs are directed to the "active only" regulator.
After reviewing this solution and looking at what it's actually achieving I here propose flagging these regulators to have "deferred disable";
* we consider the specific regulators as always-on _while running_
* hence, disable and enable affect only the sleep state
* we update both active and sleep state with all other properties
This gives us a single regulator exposed for the resource, that will be kept on with parameters as specified by the clients if it's referenced and upon loosing the last reference (disabling all consumers) it will be turned off when the CPU(s) are sleeping.
As far as I can see this should give the same behaviour as we have downstream, without the need for playing tricks with how we expose the regulators. However there's plenty of details hidden in that code, so I hope to get some feedback from the Qualcomm engineers on this.
The first patch in the series applies on top of [2], it should be squashed with 1 & 2 of [2] and is only included for completeness.
[1] https://lkml.org/lkml/2014/9/29/747
[2] https://lkml.org/lkml/2014/9/22/731
Bjorn Andersson (2):
mfd: qcom-rpm: Expose sleep state resources to clients
regulator: qcom-rpm: Implement RPM assisted disable
Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++ drivers/mfd/qcom_rpm.c | 9 +-- drivers/regulator/qcom_rpm-regulator.c | 67 +++++++++++++++----- include/linux/mfd/qcom_rpm.h | 5 +- 4 files changed, 88 insertions(+), 21 deletions(-)
Signature · 1 line
1.7.9.5
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Resources exposed from the RPM have an "active state" that is used during normal operations and a "sleep state" that is used for HW assisted sleep modes. Expose this in the api to let client drivers set the "sleep state" as well.
--- drivers/mfd/qcom_rpm.c | 9 +++++---- drivers/regulator/qcom_rpm-regulator.c | 1 + include/linux/mfd/qcom_rpm.h | 5 ++++- 3 files changed, 10 insertions(+), 5 deletions(-)
drivers/mfd/qcom_rpm.c +5 -4
@@ -67,7 +67,6 @@ struct qcom_rpm {#define RPM_ACK_SELECTOR 23#define RPM_SELECT_SIZE 7 -#define RPM_ACTIVE_STATE BIT(0)#define RPM_NOTIFICATION BIT(30)#define RPM_REJECTED BIT(31)
@@ -332,7 +331,10 @@ static const struct of_device_id qcom_rpm_of_match[] = { };MODULE_DEVICE_TABLE (of, qcom_rpm_of_match); -int qcom_rpm_write (struct qcom_rpm *rpm,int resource,u32 *buf,size_t count) +int qcom_rpm_write (struct qcom_rpm *rpm, +int state, +int resource, +u32 *buf,size_t count) {const struct qcom_rpm_resource *res;const struct qcom_rpm_data *data = rpm->data;
@@ -359,8 +361,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i)); } -writel_relaxed (RPM_ACTIVE_STATE , -RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT )); +writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));reinit_completion (&rpm->ack);regmap_write (rpm->ipc_regmap, rpm->ipc_offset,BIT (rpm->ipc_bit));
drivers/regulator/qcom_rpm-regulator.c +1 -0
@@ -198,6 +198,7 @@ static int rpm_reg_write(struct qcom_rpm_reg *vreg, vreg->val[req->word] |= value << req->shift;return qcom_rpm_write (vreg->rpm, +RPM_ACTIVE_STATE , vreg->resource, vreg->val, vreg->parts->request_len);
include/linux/mfd/qcom_rpm.h +4 -1
@@ -5,6 +5,9 @@struct qcom_rpm ; -int qcom_rpm_write (struct qcom_rpm *rpm,int resource,u32 *buf,size_t count); +#define RPM_ACTIVE_STATE 0 +#define RPM_SLEEP_STATE 1 + +int qcom_rpm_write (struct qcom_rpm *rpm,int state,int resource,u32 *buf,size_t count);#endif
Signature · 1 line
1.7.9.5
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 4 lines · Bjorn Andersson
Resources exposed from the RPM have an "active state" that is used during normal operations and a "sleep state" that is used for HW assisted sleep modes. Expose this in the api to let client drivers set the "sleep state" as well.
I assume you have users lined up which will request a sleeping state?
Quoted 32 lines
Signed-off-by Bjorn Andersson <[email protected]>--- drivers/mfd/qcom_rpm.c | 9 +++++---- drivers/regulator/qcom_rpm-regulator.c | 1 + include/linux/mfd/qcom_rpm.h | 5 ++++- 3 files changed, 10 insertions(+), 5 deletions(-)drivers/mfd/qcom_rpm.c +4
-2@@ -67,7 +67,6 @@ struct qcom_rpm {#define RPM_ACK_SELECTOR 23#define RPM_SELECT_SIZE 7 -#define RPM_ACTIVE_STATE BIT(0)#define RPM_NOTIFICATION BIT(30)#define RPM_REJECTED BIT(31)@@ -332,7 +331,10 @@ static const struct of_device_id qcom_rpm_of_match[] = { };MODULE_DEVICE_TABLE (of, qcom_rpm_of_match); -int qcom_rpm_write (struct qcom_rpm *rpm,int resource,u32 *buf,size_t count) +int qcom_rpm_write (struct qcom_rpm *rpm, +int state, +int resource, +u32 *buf,size_t count) {const struct qcom_rpm_resource *res;const struct qcom_rpm_data *data = rpm->data;
drivers/mfd/qcom_rpm.c int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) · line 364
Quoted 6 lines
drivers/mfd/qcom_rpm.c +1
-2@@ -359,8 +361,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i)); } -writel_relaxed (RPM_ACTIVE_STATE , -RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT )); +writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));
How are the state bits organised?
Quoted 29 lines
drivers/mfd/qcom_rpm.c +0
-0reinit_completion (&rpm->ack);regmap_write (rpm->ipc_regmap, rpm->ipc_offset,BIT (rpm->ipc_bit));drivers/regulator/qcom_rpm-regulator.c +1
-0@@ -198,6 +198,7 @@ static int rpm_reg_write(struct qcom_rpm_reg *vreg, vreg->val[req->word] |= value << req->shift;return qcom_rpm_write (vreg->rpm, +RPM_ACTIVE_STATE , vreg->resource, vreg->val, vreg->parts->request_len);include/linux/mfd/qcom_rpm.h +4
-1@@ -5,6 +5,9 @@struct qcom_rpm ; -int qcom_rpm_write (struct qcom_rpm *rpm,int resource,u32 *buf,size_t count); +#define RPM_ACTIVE_STATE 0 +#define RPM_SLEEP_STATE 1 + +int qcom_rpm_write (struct qcom_rpm *rpm,int state,int resource,u32 *buf,size_t count);#endif
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 9 lines · Lee Jones
Quoted 4 lines · Bjorn Andersson
Resources exposed from the RPM have an "active state" that is used during normal operations and a "sleep state" that is used for HW assisted sleep modes. Expose this in the api to let client drivers set the "sleep state" as well.
I assume you have users lined up which will request a sleeping state?
All users of this interface (regulators, clocks and bus scaling) will have to be able to specify this.
[..]
Quoted 10 lines
drivers/mfd/qcom_rpm.c int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) · line 364
Quoted 7 lines
drivers/mfd/qcom_rpm.c +1
-2@@ -359,8 +361,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i)); } -writel_relaxed (RPM_ACTIVE_STATE , -RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT )); +writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
[..]
Quoted 6 lines
include/linux/mfd/qcom_rpm.h line 9
Quoted 6 lines
include/linux/mfd/qcom_rpm.h +2
-0+#define RPM_ACTIVE_STATE 0 +#define RPM_SLEEP_STATE 1
Regards,
Bjorn
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 27 lines · Bjorn Andersson
Quoted 9 lines · Lee Jones
Quoted 4 lines · Bjorn Andersson
Resources exposed from the RPM have an "active state" that is used during normal operations and a "sleep state" that is used for HW assisted sleep modes. Expose this in the api to let client drivers set the "sleep state" as well.
I assume you have users lined up which will request a sleeping state?
All users of this interface (regulators, clocks and bus scaling) will have to be able to specify this.
[..]
Quoted 10 lines
drivers/mfd/qcom_rpm.c int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) · line 364
Quoted 7 lines
drivers/mfd/qcom_rpm.c +1
-2@@ -359,8 +361,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i)); } -writel_relaxed (RPM_ACTIVE_STATE , -RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT )); +writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
Quoted 2 lines
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
I'm just double checking that you know what that means.
BIT(0) == b01
BIT(1) == b10
It seems strange to represent a single boolean state over 2 bits.
Also, what happens if b11 or b00 occurs?
Quoted 10 lines
[..]
Quoted 6 lines
include/linux/mfd/qcom_rpm.h line 9
Quoted 6 lines
include/linux/mfd/qcom_rpm.h +2
-0+#define RPM_ACTIVE_STATE 0 +#define RPM_SLEEP_STATE 1Regards,
Bjorn
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 15 lines · Lee Jones
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines · Lee Jones
On Mon, 10 Nov 2014, Bjorn Andersson wrote:
Quoted 7 lines
Quoted 4 lines
drivers/mfd/qcom_rpm.c +1
-0+writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
Quoted 9 lines
I'm just double checking that you know what that means.
BIT(0) == b01
BIT(1) == b10It seems strange to represent a single boolean state over 2 bits.
Also, what happens if b11 or b00 occurs?
Lee is correct, it should be 0 for Active and 1 for Sleep set.
Lina
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 18 lines · Lina Iyer
Quoted 17 lines · Lee Jones
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines · Lee Jones
On Mon, 10 Nov 2014, Bjorn Andersson wrote:
Quoted 7 lines
Quoted 4 lines
drivers/mfd/qcom_rpm.c +1
-0+writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
I'm just double checking that you know what that means.
Quoted 9 lines
Quoted 7 lines
BIT(0) == b01
BIT(1) == b10It seems strange to represent a single boolean state over 2 bits.
Also, what happens if b11 or b00 occurs?
Lee is correct, it should be 0 for Active and 1 for Sleep set.
In the caf msm-3.4 tree the regulator core will call e.g. vreg_set_voltage() that will call vreg_set(), that will call msm_rpm_set(MSM_RPM_CTX_SET_0,...). That comes from the following:
enum {
MSM_RPM_CTX_SET_0,
MSM_RPM_CTX_SET_SLEEP,
MSM_RPM_CTX_SET_COUNT,
MSM_RPM_CTX_NOTIFICATION = 30,
MSM_RPM_CTX_REJECTED = 31,
};So there's your 0 and 1.
msm_rpm_set() calls msm_rpm_set_common() that calls msm_rpm_set_exclusive() that contains these two statements:
uint32_t ctx_mask = msm_rpm_get_ctx_mask(ctx); ... msm_rpm_write(MSM_RPM_PAGE_CTRL, target_ctrl(MSM_RPM_CTRL_REQ_CTX_0), ctx_mask);
And we have:
static inline uint32_t msm_rpm_get_ctx_mask(unsigned int ctx)
{
return 1UL << ctx;
}So, as far as I can see it should be BIT(state) here. But there's a lot of code and a lot of indirections here and I've been tricked by it before, so please let me know if I got something wrong on the way.
Regards,
Bjorn
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
Quoted 54 lines · Bjorn Andersson
Quoted 18 lines · Lina Iyer
Quoted 17 lines · Lee Jones
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines · Lee Jones
On Mon, 10 Nov 2014, Bjorn Andersson wrote:
Quoted 7 lines
Quoted 4 lines
drivers/mfd/qcom_rpm.c +1
-0+writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
I'm just double checking that you know what that means.
Quoted 9 lines
Quoted 7 lines
BIT(0) == b01
BIT(1) == b10It seems strange to represent a single boolean state over 2 bits.
Also, what happens if b11 or b00 occurs?
Lee is correct, it should be 0 for Active and 1 for Sleep set.
In the caf msm-3.4 tree the regulator core will call e.g.
vreg_set_voltage()that will callvreg_set(), that will callmsm_rpm_set(MSM_RPM_CTX_SET_0,...). That comes from the following:enum { MSM_RPM_CTX_SET_0, MSM_RPM_CTX_SET_SLEEP, MSM_RPM_CTX_SET_COUNT, MSM_RPM_CTX_NOTIFICATION = 30, MSM_RPM_CTX_REJECTED = 31, };So there's your 0 and 1.
msm_rpm_set()callsmsm_rpm_set_common()that callsmsm_rpm_set_exclusive()that contains these two statements:uint32_t ctx_mask = msm_rpm_get_ctx_mask(ctx); ... msm_rpm_write(MSM_RPM_PAGE_CTRL, target_ctrl(MSM_RPM_CTRL_REQ_CTX_0), ctx_mask);And we have:
Quoted 8 lines
static inline
uint32_tmsm_rpm_get_ctx_mask(unsigned int ctx){ return 1UL << ctx; }So, as far as I can see it should be BIT(state) here. But there's a lot of code and a lot of indirections here and I've been tricked by it before, so please let me know if I got something wrong on the way.
Sorry, I retract my earlier objection. This is how it is, strangely.
Quoted 3 lines
Regards,
Bjorn
RFC 1/2 mfd: qcom-rpm: Expose sleep state resources to clients
In reply to Lee JonesQuoted 32 lines · Lee Jones
Quoted 27 lines · Bjorn Andersson
Quoted 9 lines · Lee Jones
Quoted 4 lines · Bjorn Andersson
Resources exposed from the RPM have an "active state" that is used during normal operations and a "sleep state" that is used for HW assisted sleep modes. Expose this in the api to let client drivers set the "sleep state" as well.
I assume you have users lined up which will request a sleeping state?
All users of this interface (regulators, clocks and bus scaling) will have to be able to specify this.
[..]
Quoted 10 lines
drivers/mfd/qcom_rpm.c int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) · line 364
Quoted 7 lines
drivers/mfd/qcom_rpm.c +1
-2@@ -359,8 +361,7 @@ int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count) RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i)); } -writel_relaxed (RPM_ACTIVE_STATE , -RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT )); +writel_relaxed (BIT (state),RPM_CTRL_REG (rpm,RPM_REQUEST_CONTEXT ));How are the state bits organised?
Quoted 2 lines
BIT(0) is active mode, BIT(1) is sleep mode, as specified below. I could add some sanity checking here if you would like to.
Quoted 7 lines
I'm just double checking that you know what that means.
BIT(0) == b01
BIT(1) == b10It seems strange to represent a single boolean state over 2 bits.
I'm aware of this and find most parts of this solution to be strange.
Quoted 2 lines
Also, what happens if b11 or b00 occurs?
Had to pull out the firmware for the other side; it boils down to a count of trailing zeros, with 0 giving 0, that is then turned back into and enum where ACTIVE is 0 and SLEEP is 1. This is then sanity checked to be less than 2.
So the possible values and results would be:
00 => ACTIVE (seems invalid but "accidentally" accepted) 01 => ACTIVE
10 => SLEEP
11 => ACTIVE
In one of the later versions of the RPM (not applicable for any of these platforms) 2 seems to be a valid choice as well; but there it's passed as an u32 and not a bitmask.
Regards,
Bjorn
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
In reply to Bjorn AnderssonSome regulators are used to power e.g. PLLs that clocks the core we're running on, so we can't turn them off directly; but we do want them off when the core is powered down. To handle this the Qualcomm SoC provides a means to specify a "sleep state" for RPM resources that can be triggered by the hardware when the cores are brought down.
Resources of this type should be considered always-on while the CPU(s) are running and we can utilize the "sleep state" functionality to defer disabling them until the CPU(s) go to sleep, if not used by other peripherals at that time.
Other properties are kept in sync between the states, so that if the CPU(s) go to sleep with a particular regulator still enabled there will be no change.
--- Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++ drivers/regulator/qcom_rpm-regulator.c | 68 +++++++++++++++----- 2 files changed, 79 insertions(+), 17 deletions(-)
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +28 -0
@@ -112,6 +112,13 @@ of valid subnodes that can operate on these resources. Definition: select that the power supply should operate in hysteretic mode, instead of the default pwm mode +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep + Standard regulator bindings are used inside switch mode power supply subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
@@ -160,6 +167,13 @@ details. qcom,rpm-pm8921-nldo, qcom,rpm-pm8921-nldo1200 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep + Standard regulator bindings are used inside switch low-dropout regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
@@ -190,6 +204,13 @@ more details. 2740000, 2400000, 2130000, 1920000, 1750000, 1600000, 1480000, 1370000, 1280000, 1200000 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep + Standard regulator bindings are used inside negative charge pump regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
@@ -215,6 +236,13 @@ more details. QCOM_RPM_PM8921_LVS1 - QCOM_RPM_PM8921_LVS7, QCOM_RPM_PM8921_MVS +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep + = EXAMPLE #include <dt-bindings/mfd/qcom-rpm.h>
drivers/regulator/qcom_rpm-regulator.c +51 -17
197 lines not on this page · raw message
Signature · 1 line
1.7.9.5
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
Hi,
Am 10.11.2014 um 23:52 schrieb Bjorn Andersson:
Documentation/devicetree/bindings/mfd/qcom-rpm.txt of valid subnodes that can operate on these resources. · line 119
Quoted 13 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +5
-0@@ -112,6 +112,13 @@ of valid subnodes that can operate on these resources. Definition: select that the power supply should operate in hysteretic mode, instead of the default pwm mode +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM
"assistance"
Quoted 6 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +2
-0+ after going to sleep + Standard regulator bindings are used inside switch mode power supply subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt details. · line 174
Quoted 8 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +5
-0@@ -160,6 +167,13 @@ details. qcom,rpm-pm8921-nldo, qcom,rpm-pm8921-nldo1200 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM
ditto
Quoted 6 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +2
-0+ after going to sleep + Standard regulator bindings are used inside switch low-dropout regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt more details. · line 211
Quoted 8 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +5
-0@@ -190,6 +204,13 @@ more details. 2740000, 2400000, 2130000, 1920000, 1750000, 1600000, 1480000, 1370000, 1280000, 1200000 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM
ditto
Quoted 6 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +2
-0+ after going to sleep + Standard regulator bindings are used inside negative charge pump regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt more details. · line 243
Quoted 8 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +5
-0@@ -215,6 +236,13 @@ more details. QCOM_RPM_PM8921_LVS1 - QCOM_RPM_PM8921_LVS7, QCOM_RPM_PM8921_MVS +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM
ditto
Documentation/devicetree/bindings/mfd/qcom-rpm.txt more details. · line 248
Quoted 5 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +2
-0+ after going to sleep + = EXAMPLE #include <dt-bindings/mfd/qcom-rpm.h>
[snip]
Regards,
Andreas
Signature · 2 lines
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 21284 AG N?rnberg
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
Quoted 11 lines · Andreas F?rber
Hi,
Am 10.11.2014 um 23:52 schrieb Bjorn Andersson:
Quoted 8 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0@@ -112,6 +112,13 @@ of valid subnodes that can operate on these resources. Definition: select that the power supply should operate in hysteretic mode, instead of the default pwm mode
Quoted 8 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt of valid subnodes that can operate on these resources. · line 119
Quoted 5 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +5
-0+- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM"assistance"
Thanks
Regards,
Bjorn
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
In reply to Bjorn AnderssonQuoted 11 lines · Bjorn Andersson
Some regulators are used to power e.g. PLLs that clocks the core we're running on, so we can't turn them off directly; but we do want them off when the core is powered down. To handle this the Qualcomm SoC provides a means to specify a "sleep state" for RPM resources that can be triggered by the hardware when the cores are brought down.
Resources of this type should be considered always-on while the CPU(s) are running and we can utilize the "sleep state" functionality to defer disabling them until the CPU(s) go to sleep, if not used by other peripherals at that time.
Quoted 7 lines
Other properties are kept in sync between the states, so that if the CPU(s) go to sleep with a particular regulator still enabled there will be no change.
Signed-off-by Bjorn Andersson <[email protected]>--- Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++
Please place this into a seperate patch.
See: Documentation/devicetree/bindings/submitting-patches.txt
Quoted 8 lines
drivers/regulator/qcom_rpm-regulator.c | 68 +++++++++++++++----- 2 files changed, 79 insertions(+), 17 deletions(-)Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0
Documentation/devicetree/bindings/mfd/qcom-rpm.txt of valid subnodes that can operate on these resources. · line 121
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0@@ -112,6 +112,13 @@ of valid subnodes that can operate on these resources. Definition: select that the power supply should operate in hysteretic mode, instead of the default pwm mode +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +
This requires a DT Ack.
Quoted 4 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0Standard regulator bindings are used inside switch mode power supply subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt details. · line 176
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0@@ -160,6 +167,13 @@ details. qcom,rpm-pm8921-nldo, qcom,rpm-pm8921-nldo1200 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +
Eh? I think I'm seeing double.
Quoted 4 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0Standard regulator bindings are used inside switch low-dropout regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt more details. · line 213
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0@@ -190,6 +204,13 @@ more details. 2740000, 2400000, 2130000, 1920000, 1750000, 1600000, 1480000, 1370000, 1280000, 1200000 +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +
Triple?
Quoted 4 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0Standard regulator bindings are used inside negative charge pump regulator subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.
Documentation/devicetree/bindings/mfd/qcom-rpm.txt more details. · line 245
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0@@ -215,6 +236,13 @@ more details. QCOM_RPM_PM8921_LVS1 - QCOM_RPM_PM8921_LVS7, QCOM_RPM_PM8921_MVS +- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +
There must be a better way to document this.
Quoted 3 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0= EXAMPLE #include <dt-bindings/mfd/qcom-rpm.h>
[...]
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
Quoted 17 lines · Lee Jones
Quoted 15 lines · Bjorn Andersson
Some regulators are used to power e.g. PLLs that clocks the core we're running on, so we can't turn them off directly; but we do want them off when the core is powered down. To handle this the Qualcomm SoC provides a means to specify a "sleep state" for RPM resources that can be triggered by the hardware when the cores are brought down.
Resources of this type should be considered always-on while the CPU(s) are running and we can utilize the "sleep state" functionality to defer disabling them until the CPU(s) go to sleep, if not used by other peripherals at that time.
Other properties are kept in sync between the states, so that if the CPU(s) go to sleep with a particular regulator still enabled there will be no change.
Quoted 8 lines
Quoted 3 lines
Signed-off-by Bjorn Andersson <[email protected]>--- Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++Please place this into a seperate patch.
See:
Documentation/devicetree/bindings/submitting-patches.txt
Yes, I'm aware. As you probably remember we still haven't gotten an ack on the original file, so given the outcome of the discussion of the design of this I plan to fold it into the original patch and send out a new version.
Quoted 11 lines
Quoted 11 lines
drivers/regulator/qcom_rpm-regulator.c | 68 +++++++++++++++----- 2 files changed, 79 insertions(+), 17 deletions(-)Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0@@ -112,6 +112,13 @@ of valid subnodes that can operate on these resources. Definition: select that the power supply should operate in hysteretic mode, instead of the default pwm mode
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt of valid subnodes that can operate on these resources. · line 121
Quoted 7 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0+- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +This requires a DT Ack.
The entire file still requires a dt ack, but the "sleep state" will change the binding so let's see where this RFC takes us.
I would have preferred if someone gave me the "sleep state" feedback once we had this out for review, but it didn't reach the right people at Qualcomm at the time.
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +0
-0Standard regulator bindings are used inside switch mode power supply subnodes. Check Documentation/devicetree/bindings/regulator/regulator.txt for more details.@@ -160,6 +167,13 @@ details. qcom,rpm-pm8921-nldo, qcom,rpm-pm8921-nldo1200
Quoted 10 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt of valid subnodes that can operate on these resources. · line 121
Quoted 7 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt +7
-0+- qcom,rpm-assisted-disable: + Usage: optional + Value type: <empty> + Definition: select that the regulator is supplying the active CPU(s) + and can only be disabled with the assistans from the RPM + after going to sleep +Eh? I think I'm seeing double.
You're right, it would make sense to move this to a "common subnode properties" section.
Regards,
Bjorn
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
In reply to Bjorn AnderssonHello Bjorn,
Quoted 6 lines · Bjorn Andersson
Some regulators are used to power e.g. PLLs that clocks the core we're running on, so we can't turn them off directly; but we do want them off when the core is powered down. To handle this the Qualcomm SoC provides a means to specify a "sleep state" for RPM resources that can be triggered by the hardware when the cores are brought down.
The regulator core already has support to control the state of regulators when the system enters into a sleep state. Now the generic regulator DT binding has also been extended to support defining if a regulator should be "regulator-{on,off}-in-suspend". Please take a look at the topic/suspend branch [0] in the regulator tree that has the latest binding.
If that is not enough for your hardware and use case, I've been working on adding initial and suspend mode for regulators. The latest series is [1] and the needed bits for the max77802 regulator driver is [2].
It's always better to use existing functionality if possible, instead of adding custom per driver DT bindings. So it would be great if you can take a look to the mentioned patches.
Quoted 2 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++ drivers/regulator/qcom_rpm-regulator.c | 68 +++++++++++++++-----
Against which tree this patch has been developed? afaict Documentation/devicetree/bindings/mfd/qcom-rpm.txt does not exist even in the latest for-next [3] branch of the mfd tree.
Quoted 2 lines
#include <dt-bindings/mfd/qcom-rpm.h>This file doesn't exit either and the regulator driver depends on MFD_QCOM_RPM which also is not present in mainline.
By looking at the mail archives I see that you posted both the regulator and mfd driver in the same series [4] but only the regulator driver was picked by Mark so I guess Lee has to pick the mfd driver in order to allow the regulator driver to be built.
Best regards,
Javier
[0]: https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=topic/suspend
[1]: https://lkml.org/lkml/2014/11/10/325
[2]: https://lkml.org/lkml/2014/11/11/403
[3]: https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/log/?h=for-mfd-next
[4]: https://lkml.org/lkml/2014/9/22/731
RFC 2/2 regulator: qcom-rpm: Implement RPM assisted disable
Quoted 2 lines · Javier Martinez Canillas
Hello Bjorn,
Hi Javier,
Quoted 9 lines
Quoted 6 lines · Bjorn Andersson
Some regulators are used to power e.g. PLLs that clocks the core we're running on, so we can't turn them off directly; but we do want them off when the core is powered down. To handle this the Qualcomm SoC provides a means to specify a "sleep state" for RPM resources that can be triggered by the hardware when the cores are brought down.
Quoted 7 lines
The regulator core already has support to control the state of regulators when the system enters into a sleep state. Now the generic regulator DT binding has also been extended to support defining if a regulator should be "regulator-{on,off}-in-suspend". Please take a look at the topic/suspend branch [0] in the regulator tree that has the latest binding.
I was planning to utilize the suspend states functionality in the core and was looking at implementing [0] myself, so glad that's coming in place.
However, as a practical example we have LDO12 on the MSM8974 SoC that is used for powering CPU PLLs, WiFi/BT PLLs, display, camera sensor and hdmi.
In a phone it's most reasonable to expect the WiFi core keeping a vote for the regulators to be enabled during a suspend, but if you're in airplane mode (or WiFi is turned off) you want to save the power - so it's not possible to configure this statically.
Further more, the CPU vote is not tied to suspend state but rather cpuidle state. It's not unreasonable to think of a state where we're clocking out pixels to the display in Android with the CPU turned off and hence the CPU PLL vote lifted.
Quoted 5 lines
If that is not enough for your hardware and use case, I've been working on adding initial and suspend mode for regulators. The latest series is [1] and the needed bits for the max77802 regulator driver is [2].
Looks good.
Quoted 4 lines
It's always better to use existing functionality if possible, instead of adding custom per driver DT bindings. So it would be great if you can take a look to the mentioned patches.
I totally agree, but due to the dynamic nature described above I have a hard time figuring out how to make it fit; hence this RFC.
Quoted 7 lines
Quoted 2 lines
Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 28 ++++++++ drivers/regulator/qcom_rpm-regulator.c | 68 +++++++++++++++-----Against which tree this patch has been developed? afaict
Documentation/devicetree/bindings/mfd/qcom-rpm.txtdoes not exist even in the latest for-next [3] branch of the mfd tree.
v3.18-rc1 + https://lkml.org/lkml/2014/9/22/731
As I tried to explain in the cover letter, the DT bindings that I change here are not acked and this RFC is an attempt to come to a conclusion in how to design the sleep state part - which will change the bindings.
Quoted 11 lines
Quoted 2 lines
#include <dt-bindings/mfd/qcom-rpm.h>This file doesn't exit either and the regulator driver depends on
MFD_QCOM_RPMwhich also is not present in mainline.By looking at the mail archives I see that you posted both the regulator and mfd driver in the same series [4] but only the regulator driver was picked by Mark so I guess Lee has to pick the mfd driver in order to allow the regulator driver to be built.
Correct, I hope that after sorting the sleep state part out we can get some acks on things.
Thanks for your review!
Regards,
Bjorn
cover Qualcomm RPM sleep states
In reply to Bjorn AnderssonHi Bjorn,
Quoted 8 lines · Bjorn Andersson
During the review of the Qualcomm SMD RPM regulators [1], I got (offline) feedback that my implementation did not handle "sleep states". As the problem is shared between all families of Qualcomm platforms I use [2] (family A) to propose a solution (as I hope to get that merged sooner).
The "sleep states" comment boils down to certain regulators (or rpm resources in general) are used by the currently clocked/running CPU(s) and can not be disabled while we're still running.
This is the first flaw in the argument. There are certain regulators that are used to power the clocks for CPUs that can be disabled while we're running as long as the CPUs aren't using these clocks. I'll explain further down below.
Quoted 4 lines
Further more, these resources are shared
with peripherals in the system; e.g. LDO12 on PM8941 is used to clock the CPU and WiFi/BT PLLs as well as providing power to the display in our devices. So the suspend functionality in the regulator framework doesn't cut it.
Furthermore, we can trigger a transition to the sleep set during idle paths so suspend definitely won't cut it.
Quoted 19 lines
The downstream solution to this is to expose 3 regulators per regulator resource, each specified to control the active mode, sleep mode or both modes respectively. Peripherals are directed to use the "both" regulator while the CPUs are directed to the "active only" regulator.
After reviewing this solution and looking at what it's actually achieving I here propose flagging these regulators to have "deferred disable";
* we consider the specific regulators as always-on_whilerunning_
* hence, disable and enable affect only the sleep state
* we update both active and sleep state with all other propertiesThis gives us a single regulator exposed for the resource, that will be kept on with parameters as specified by the clients if it's referenced and upon loosing the last reference (disabling all consumers) it will be turned off when the CPU(s) are sleeping.
Quoted 4 lines
As far as I can see this should give the same behaviour as we have downstream, without the need for playing tricks with how we expose the regulators. However there's plenty of details hidden in that code, so I hope to get some feedback from the Qualcomm engineers on this.
First some background. The Krait CPUs are clocked by dedicated per-cpu PLLs and a global PLL. The dedicated per-cpu PLLs are supplied by a regulator (actually a handful of them, but let's just assume one for the sake of argument). When we go to suspend or idle we will turn off these dedicated per-cpu PLLs and switch the CPUs to source off the global PLL. The global PLL is also used as the lowest rate that the CPUs can run at. We'd like to not communicate with the RPM unnecessarily during idle/suspend to turn off these resources when we go idle/suspend because it takes more time and thus more power.
So the CPU really wants to only be voting for the HFPLL regulator supplies in the active set. This way, if we're not using the HFPLLs (i.e. the CPUs are all running off the global PLL), then we can disable the regulator in the active set (RPM code only caches sleep set so we won't be doing unnecessary flushes in the idle/suspend path). The clock driver will make sure to turn off the HFPLLs before we go into a sleep state that would trigger a switch from active to sleep set (commonly referred to as RPM notification). It very well could be that there are other consumers of the same regulator, but that doesn't matter to the CPU clock driver because it only cares about the active set. Now you may ask why can't the CPU clock driver disable the regulator when the HFPLL is disabled? We don't do that in this case because a) it causes more RPM communication overhead and b) we will be in atomic context when the HFPLL is disabled during idle/suspend and the regulator APIs are sleeping calls. In the non-idle/suspend path we will disable the regulator.
Also the active/sleep sets are about more than just on/off state. We may have a situation where the active set voltage (or some other attribute like current, mode, etc.) is different than the sleep set voltage. For example, the CPU is supplied by a digital logic regulator that is shared with other digital logic in the SoC (GPU, wifi, etc.). The CPU may require some high voltage, but the GPU only needs some lower voltage. The suspend/idle code relies on the fact that the GPU is voting on the active+sleep regulator while the CPU is voting on the active only regulator so that the RPM can automatically switch between high voltage and low voltage when the CPU notifies the RPM that it's gone idle (or the CPU wakes up).
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 1 line · Stephen Boyd
On 11/10/2014 02:52 PM, Bjorn Andersson wrote:
I don't seem to have been CCed on earlier messages here, or perhaps the lack of obvious relevance (like mention of regulators) caused me not to read the mails so I'm missing a *lot* of context.
Quoted 7 lines
Quoted 4 lines
Further more, these resources are shared
with peripherals in the system; e.g. LDO12 on PM8941 is used to clock the CPU and WiFi/BT PLLs as well as providing power to the display in our devices. So the suspend functionality in the regulator framework doesn't cut it.Furthermore, we can trigger a transition to the sleep set during idle paths so suspend definitely won't cut it.
Honestly I'm not really sure what support the above is talking about... the regulator API has support for setting the configuration the regulator has when we are in suspend but it doesn't have any real support for entering or leaving such a mode which appears to be what is being discussed.
Quoted 7 lines
So the CPU really wants to only be voting for the HFPLL regulator supplies in the active set. This way, if we're not using the HFPLLs (i.e. the CPUs are all running off the global PLL), then we can disable the regulator in the active set (RPM code only caches sleep set so we won't be doing unnecessary flushes in the idle/suspend path). The clock driver will make sure to turn off the HFPLLs before we go into a sleep state that would trigger a switch from active to sleep set (commonly
Quoted 8 lines
referred to as RPM notification). It very well could be that there are other consumers of the same regulator, but that doesn't matter to the CPU clock driver because it only cares about the active set. Now you may ask why can't the CPU clock driver disable the regulator when the HFPLL is disabled? We don't do that in this case because a) it causes more RPM communication overhead and b) we will be in atomic context when the HFPLL is disabled during idle/suspend and the regulator APIs are sleeping calls. In the non-idle/suspend path we will disable the regulator.
I'm afraid the above is making very little sense to me. What is "voting" and how is it different to "enabling", "notification" or "flushing"?
Quoted 3 lines
Also the active/sleep sets are about more than just on/off state. We may have a situation where the active set voltage (or some other attribute like current, mode, etc.) is different than the sleep set voltage. For
In a regulator API context if we are talking about suspend it's expected that all properties may vary in suspend.
Quoted 8 lines
example, the CPU is supplied by a digital logic regulator that is shared with other digital logic in the SoC (GPU, wifi, etc.). The CPU may require some high voltage, but the GPU only needs some lower voltage. The suspend/idle code relies on the fact that the GPU is voting on the active+sleep regulator while the CPU is voting on the active only regulator so that the RPM can automatically switch between high voltage and low voltage when the CPU notifies the RPM that it's gone idle (or the CPU wakes up).
Again I'm not following this at all, sorry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141121/baed5af0/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 5 lines · Mark Brown
Quoted 1 line · Stephen Boyd
On 11/10/2014 02:52 PM, Bjorn Andersson wrote:
I don't seem to have been CCed on earlier messages here, or perhaps the lack of obvious relevance (like mention of regulators) caused me not to read the mails so I'm missing a *lot* of context.
This mail has been lingering in my inbox for weeks just haven't gotten around to replying.
This sleep set/active set stuff has to do with more than just regulators. It applies to any resource that the RPM provides, but regulators are a primary use case so you're on Cc and it would be great if you fully understood the regulator aspect here.
Quoted 17 lines
Quoted 17 lines
So the CPU really wants to only be voting for the HFPLL regulator supplies in the active set. This way, if we're not using the HFPLLs (i.e. the CPUs are all running off the global PLL), then we can disable the regulator in the active set (RPM code only caches sleep set so we won't be doing unnecessary flushes in the idle/suspend path). The clock driver will make sure to turn off the HFPLLs before we go into a sleep state that would trigger a switch from active to sleep set (commonly referred to as RPM notification). It very well could be that there are other consumers of the same regulator, but that doesn't matter to the CPU clock driver because it only cares about the active set. Now you may ask why can't the CPU clock driver disable the regulator when the HFPLL is disabled? We don't do that in this case because a) it causes more RPM communication overhead and b) we will be in atomic context when the HFPLL is disabled during idle/suspend and the regulator APIs are sleeping calls. In the non-idle/suspend path we will disable the regulator.
Quoted 3 lines
I'm afraid the above is making very little sense to me. What is "voting" and how is it different to "enabling", "notification" or "flushing"?
The regulators are shared between multiple "masters" in the SoC. So the CPUs that are running linux only "vote" on attributes of regulators and then another processor that isn't running linux (called RPM) aggregates the request from Linux along with other "masters" like wifi, modem, etc. and then changes something for that regulator like voltage, enable/disable, etc. We also have some hardware next to our CPUs that notifies the RPM when we transition into or out of idle/suspend (it's called an SPM). This is the notification part. Flushing has to do with batching up multiple RPM sleep set requests and sending them before we enter idle/suspend.
Quoted 15 lines
Quoted 4 lines
Also the active/sleep sets are about more than just on/off state. We may have a situation where the active set voltage (or some other attribute like current, mode, etc.) is different than the sleep set voltage. For
In a regulator API context if we are talking about suspend it's expected that all properties may vary in suspend.
Quoted 8 lines
example, the CPU is supplied by a digital logic regulator that is shared with other digital logic in the SoC (GPU, wifi, etc.). The CPU may require some high voltage, but the GPU only needs some lower voltage. The suspend/idle code relies on the fact that the GPU is voting on the active+sleep regulator while the CPU is voting on the active only regulator so that the RPM can automatically switch between high voltage and low voltage when the CPU notifies the RPM that it's gone idle (or the CPU wakes up).
Quoted 1 line
Again I'm not following this at all, sorry.
Hopefully it's clear now. If not I can clarify further.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 4 lines · Stephen Boyd
This sleep set/active set stuff has to do with more than just regulators. It applies to any resource that the RPM provides, but regulators are a primary use case so you're on Cc and it would be great if you fully understood the regulator aspect here.
Step back: what is the RPM? Some kind of power management coprocessor
from the sounds of it?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141121/52c6136a/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 8 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
This sleep set/active set stuff has to do with more than just regulators. It applies to any resource that the RPM provides, but regulators are a primary use case so you're on Cc and it would be great if you fully understood the regulator aspect here.
Step back: what is the RPM? Some kind of power management coprocessor from the sounds of it?
Yes.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
cover Qualcomm RPM sleep states
In reply to Mark BrownQuoted 9 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
This sleep set/active set stuff has to do with more than just regulators. It applies to any resource that the RPM provides, but regulators are a primary use case so you're on Cc and it would be great if you fully understood the regulator aspect here.
Step back: what is the RPM? Some kind of power management coprocessor from the sounds of it?
Correct, it's a coprocessor that does the actual poking of the PMIC.
To each processor in the SoC it exposes a set of resources, e.g. regulators. Each resource is represented by two set of properties, the active and the sleep state sets. The selection between the two state sets are basically controlled by the CPU being in WFI (wait-for-interrupt) or not.
The qcom_rpm-regulator.c (and rfc for qcom_smd-regulator.c) that we have in the tree only pokes the active state and we end up leaving certain regulators as always-on; due to the fact that we're clocked off PLLs powered by them.
To further complicate matters these regulators are also used to power peripherals, so enable is not 1:1 with WFI, but rather depends on other consumers.
My proposal was to flag these regulators so that we write the active/sleep states in a way that they will be turned off on WFI, iff the regulator isn't enabled. But as Stephen pointed out, this might not be enough.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
In reply to Stephen BoydQuoted 16 lines · Stephen Boyd
Quoted 3 lines · Mark Brown
I'm afraid the above is making very little sense to me. What is "voting" and how is it different to "enabling", "notification" or "flushing"?
The regulators are shared between multiple "masters" in the SoC. So the CPUs that are running linux only "vote" on attributes of regulators and then another processor that isn't running linux (called RPM) aggregates the request from Linux along with other "masters" like wifi, modem, etc. and then changes something for that regulator like voltage, enable/disable, etc. We also have some hardware next to our CPUs that notifies the RPM when we transition into or out of idle/suspend (it's called an SPM). This is the notification part. Flushing has to do with batching up multiple RPM sleep set requests and sending them before we enter idle/suspend.
...
Quoted 1 line
Hopefully it's clear now. If not I can clarify further.
A bit, but I'm afraid I'm still at a loss as to what the problems and
debate are here. Why is this more complex than the Linux part of the
system just saying what it wants at any given time? It sounds like that
has some sort of performance issue?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141124/0f9234ad/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 15 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
Quoted 3 lines · Mark Brown
I'm afraid the above is making very little sense to me. What is "voting" and how is it different to "enabling", "notification" or "flushing"?
Quoted 10 lines
The regulators are shared between multiple "masters" in the SoC. So the CPUs that are running linux only "vote" on attributes of regulators and then another processor that isn't running linux (called RPM) aggregates the request from Linux along with other "masters" like wifi, modem, etc. and then changes something for that regulator like voltage, enable/disable, etc. We also have some hardware next to our CPUs that notifies the RPM when we transition into or out of idle/suspend (it's called an SPM). This is the notification part. Flushing has to do with batching up multiple RPM sleep set requests and sending them before we enter idle/suspend.
Quoted 7 lines
...
Quoted 1 line
Hopefully it's clear now. If not I can clarify further.
A bit, but I'm afraid I'm still at a loss as to what the problems and debate are here. Why is this more complex than the Linux part of the system just saying what it wants at any given time? It sounds like that has some sort of performance issue?
Yes, communicating with the RPM is not that fast. The sleep set could be updated hundreds of times before we go idle and actually switch from the active set to sleep set. One optimization is to buffer the sleep set requests and send them right before we switch to the sleep set. This saves on the messaging overhead for sleep sets that don't matter when we're active by moving the overhead to the idle/suspend path. The hope is to consolidate multiple sleep set requests into one request.
I think the problem that Bjorn is trying to come up with a solution for is how to represent the sleep set and active set in the kernel clock and regulator frameworks. You can think of each RPM resource as a regulator supply. Each one of those resources has an active and sleep set. In the downstream vendor kernels we make two regulators for an RPM resource. One regulator for the active set and one regulator for the active + sleep set. We call these 'active only' and 'active + sleep' regulators respectively. The RPM regulator driver aggregates the active set for both the regulators via a max() operation and sends that as a request to the RPM. The sleep set is the same as the active set for the 'active + sleep' regulator, so we just send whatever the value is that was sent down via the regulator APIs on the 'active + sleep' regulator. The only driver that really cares about the active only regulators is the CPU clock driver. Otherwise drivers are using the active + sleep regulators because their devices don't stop running when the CPU goes to idle/suspend.
Maybe another solution would be to push the problem into the regulator core and educate it about the two different sets. RPM resources would map one-to-one with a regulator and the sleep set and active sets would be selectable via the regulator_get() API or some other consumer mapping method. This would allow consumers to request whatever set they care about and consolidate the aggregation logic that's duplicated at the consumer level and the driver level into the core.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 6 lines · Stephen Boyd
Quoted 4 lines · Mark Brown
A bit, but I'm afraid I'm still at a loss as to what the problems and debate are here. Why is this more complex than the Linux part of the system just saying what it wants at any given time? It sounds like that has some sort of performance issue?
I'm still quite confused here...
Quoted 7 lines
Yes, communicating with the RPM is not that fast. The sleep set could be updated hundreds of times before we go idle and actually switch from the active set to sleep set. One optimization is to buffer the sleep set requests and send them right before we switch to the sleep set. This saves on the messaging overhead for sleep sets that don't matter when we're active by moving the overhead to the idle/suspend path. The hope is to consolidate multiple sleep set requests into one request.
Right, that's one of the obvious ones. Though I am a bit mystified as to why you're needing to change these settings so often that it's a problem - most hardware can do this but generally the configuration when idle is fairly constant so it's irrelevant for performance how often we set these things, a lot of the time it's going to boil down to I2C writes so we're talking really infrequent here. What's different about these systems that means that this stuff needs constant tweaking?
Quoted 7 lines
I think the problem that Bjorn is trying to come up with a solution for is how to represent the sleep set and active set in the kernel clock and regulator frameworks. You can think of each RPM resource as a regulator supply. Each one of those resources has an active and sleep set. In the downstream vendor kernels we make two regulators for an RPM resource. One regulator for the active set and one regulator for the active + sleep set. We call these 'active only' and 'active + sleep' regulators
That seems like it's a big abstraction problem.
Quoted 8 lines
respectively. The RPM regulator driver aggregates the active set for both the regulators via a
max()operation and sends that as a request to the RPM. The sleep set is the same as the active set for the 'active + sleep' regulator, so we just send whatever the value is that was sent down via the regulator APIs on the 'active + sleep' regulator. The only driver that really cares about the active only regulators is the CPU clock driver. Otherwise drivers are using the active + sleep regulators because their devices don't stop running when the CPU goes to idle/suspend.
Hang on a minute. What you're saying seems to be that this isn't really about suspend but actually about the normal operating configuration? That makes it a bit more understandable why one would change the settings a lot, I think what I'm hearing here is that the runtime state changes a lot for some reason and the system needs the suspend state to track this?
I can't help but think that this all sounds like the RPM isn't mapping very well onto practical systems and needs revisiting in future versions... for example with what I'm parsing out of the above an active+sleep set command or otherwise having the two modes tied together for some regulators would make the whole problem go away.
Quoted 7 lines
Maybe another solution would be to push the problem into the regulator core and educate it about the two different sets. RPM resources would map one-to-one with a regulator and the sleep set and active sets would be selectable via the
regulator_get()API or some other consumer mapping method. This would allow consumers to request whatever set they care about and consolidate the aggregation logic that's duplicated at the consumer level and the driver level into the core.
I think any duplication that's going on sounds like a consequence of
the way this is currently implemented. I think based on what I *think*
you're saying the RPM driver probably ought to be hiding this and adding
a property which makes the active and sleep sets track each other with
normal suspend mode control otherwise. That could potentially be done
in the core, though the tracking would be substantial surgery.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141125/1821dd83/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 7 lines · Mark Brown
Quoted 5 lines · Stephen Boyd
Quoted 4 lines · Mark Brown
A bit, but I'm afraid I'm still at a loss as to what the problems and debate are here. Why is this more complex than the Linux part of the system just saying what it wants at any given time? It sounds like that has some sort of performance issue?
I'm still quite confused here...
Hm... hopefully it's not getting worse.
Quoted 8 lines
Quoted 8 lines
Yes, communicating with the RPM is not that fast. The sleep set could be updated hundreds of times before we go idle and actually switch from the active set to sleep set. One optimization is to buffer the sleep set requests and send them right before we switch to the sleep set. This saves on the messaging overhead for sleep sets that don't matter when we're active by moving the overhead to the idle/suspend path. The hope is to consolidate multiple sleep set requests into one request.
Quoted 7 lines
Right, that's one of the obvious ones. Though I am a bit mystified as to why you're needing to change these settings so often that it's a problem - most hardware can do this but generally the configuration when idle is fairly constant so it's irrelevant for performance how often we set these things, a lot of the time it's going to boil down to I2C writes so we're talking really infrequent here. What's different about these systems that means that this stuff needs constant tweaking?
Yes I don't think we're constantly changing the regulators because it does boil down to I2C writes or something similar, but we could be modifying other resources that the RPM controls more often. For example, the RPM controls some clock frequencies that eventually end up being some mmio writes. We change these frequencies quite often in response to CPU frequency scaling or other DCVS algorithms.
Quoted 17 lines
Quoted 7 lines
I think the problem that Bjorn is trying to come up with a solution for is how to represent the sleep set and active set in the kernel clock and regulator frameworks. You can think of each RPM resource as a regulator supply. Each one of those resources has an active and sleep set. In the downstream vendor kernels we make two regulators for an RPM resource. One regulator for the active set and one regulator for the active + sleep set. We call these 'active only' and 'active + sleep' regulators
That seems like it's a big abstraction problem.
Quoted 8 lines
respectively. The RPM regulator driver aggregates the active set for both the regulators via a
max()operation and sends that as a request to the RPM. The sleep set is the same as the active set for the 'active + sleep' regulator, so we just send whatever the value is that was sent down via the regulator APIs on the 'active + sleep' regulator. The only driver that really cares about the active only regulators is the CPU clock driver. Otherwise drivers are using the active + sleep regulators because their devices don't stop running when the CPU goes to idle/suspend.
Quoted 6 lines
Hang on a minute. What you're saying seems to be that this isn't really about suspend but actually about the normal operating configuration? That makes it a bit more understandable why one would change the settings a lot, I think what I'm hearing here is that the runtime state changes a lot for some reason and the system needs the suspend state to track this?
It also matters for idle. Both suspend and idle can cause an RPM notification and switch to the sleep set.
Quoted 6 lines
I can't help but think that this all sounds like the RPM isn't mapping very well onto practical systems and needs revisiting in future versions... for example with what I'm parsing out of the above an active+sleep set command or otherwise having the two modes tied together for some regulators would make the whole problem go away.
We create the 'active only' regulators for consumers that actually need them. From the set of regulators on a board only a couple need this treatment. I don't see how tying the two states together via an active+sleep set command would make this problem go away for the cases I already described before, i.e. CPU wants some voltage and other IO devices want another voltage and the CPU doesn't care what the voltage is when the CPU is in idle or suspend. Having a combination active + sleep set command would be nice. The RPM already sort of supports this by allowing you to only modify the active set. If you never modify the sleep set, then the RPM just applies whatever is in the active set to the sleep set. We can probably go through and figure out what resources could get away with only using the active set so we can cut down on sleep set requests that are always the same between active and sleep set.
Quoted 7 lines
Quoted 7 lines
Maybe another solution would be to push the problem into the regulator core and educate it about the two different sets. RPM resources would map one-to-one with a regulator and the sleep set and active sets would be selectable via the
regulator_get()API or some other consumer mapping method. This would allow consumers to request whatever set they care about and consolidate the aggregation logic that's duplicated at the consumer level and the driver level into the core.
Quoted 6 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
Sorry I don't follow this part. It's about more than suspend, we also care about idle. I agree that pushing the concept of active vs. sleep into the framework is substantial.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 5 lines · Stephen Boyd
Quoted 1 line · Mark Brown
I'm still quite confused here...
Hm... hopefully it's not getting worse.
It is a bit, the story keeps changing a lot.
Quoted 8 lines
Quoted 5 lines
I can't help but think that this all sounds like the RPM isn't mapping very well onto practical systems and needs revisiting in future versions... for example with what I'm parsing out of the above an active+sleep set command or otherwise having the two modes tied together for some regulators would make the whole problem go away.
We create the 'active only' regulators for consumers that actually need them. From the set of regulators on a board only a couple need this
That's what I don't think is making any sense. Creating two regulators for the same thing seems like bad news, at best it's going to make reuse harder.
Quoted 4 lines
treatment. I don't see how tying the two states together via an active+sleep set command would make this problem go away for the cases I already described before, i.e. CPU wants some voltage and other IO devices want another voltage and the CPU doesn't care what the voltage is when the CPU is
Not paying attention to requests from a disabled consumer seems like something we should be able to do in general.
Quoted 7 lines
in idle or suspend. Having a combination active + sleep set command would be nice. The RPM already sort of supports this by allowing you to only modify the active set. If you never modify the sleep set, then the RPM just applies whatever is in the active set to the sleep set. We can probably go through and figure out what resources could get away with only using the active set so we can cut down on sleep set requests that are always the same between active and sleep set.
This version is starting to sound a lot like the consumers need to be able to do an idle to sleep transition and change their settings when they enter their sleep mode. It seems a lot like what many devices do now when they enter and exit runtime PM but with deferred application depending on other users (for regulators I'm assuming the active set would actually be a subset of the sleep set of valid voltages).
Quoted 10 lines
Quoted 6 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
Sorry I don't follow this part. It's about more than suspend, we also care about idle. I agree that pushing the concept of active vs. sleep into the framework is substantial.
That's not what it seemed like people had been talking about up until
this most recent e-mail, you'd been talking about suspend mode. The
starting point was mapping onto the suspend support the regulator has.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141126/99fea7c6/attachment-0001.sig>
cover Qualcomm RPM sleep states
Quoted 9 lines · Mark Brown
Quoted 8 lines · Stephen Boyd
Quoted 5 lines · Mark Brown
I can't help but think that this all sounds like the RPM isn't mapping very well onto practical systems and needs revisiting in future versions... for example with what I'm parsing out of the above an active+sleep set command or otherwise having the two modes tied together for some regulators would make the whole problem go away.
Quoted 5 lines
Quoted 2 lines
We create the 'active only' regulators for consumers that actually need them. From the set of regulators on a board only a couple need this
That's what I don't think is making any sense. Creating two regulators for the same thing seems like bad news, at best it's going to make reuse harder.
What reuse is harder?
Quoted 7 lines
Quoted 5 lines
treatment. I don't see how tying the two states together via an active+sleep set command would make this problem go away for the cases I already described before, i.e. CPU wants some voltage and other IO devices want another voltage and the CPU doesn't care what the voltage is when the CPU is
Not paying attention to requests from a disabled consumer seems like something we should be able to do in general.
Sounds good.
Quoted 8 lines
Quoted 8 lines
in idle or suspend. Having a combination active + sleep set command would be nice. The RPM already sort of supports this by allowing you to only modify the active set. If you never modify the sleep set, then the RPM just applies whatever is in the active set to the sleep set. We can probably go through and figure out what resources could get away with only using the active set so we can cut down on sleep set requests that are always the same between active and sleep set.
Quoted 6 lines
This version is starting to sound a lot like the consumers need to be able to do an idle to sleep transition and change their settings when they enter their sleep mode. It seems a lot like what many devices do now when they enter and exit runtime PM but with deferred application depending on other users (for regulators I'm assuming the active set would actually be a subset of the sleep set of valid voltages).
Yes, one key point is that we want the transition to be handled by the RPM co-processor. The reason being that usually we want to change settings after the last CPU transitions into their sleep mode. There's no way to do this without relying on some external co-processor to go and change the settings for us.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
Quoted 6 lines
Quoted 3 lines
Sorry I don't follow this part. It's about more than suspend, we also care about idle. I agree that pushing the concept of active vs. sleep into the framework is substantial.
That's not what it seemed like people had been talking about up until this most recent e-mail, you'd been talking about suspend mode. The starting point was mapping onto the suspend support the regulator has.
Hm? I've been saying "idle and suspend" each time. Perhaps the initial discussion about regulator suspend support threw things off.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 7 lines · Stephen Boyd
Quoted 3 lines · Mark Brown
That's what I don't think is making any sense. Creating two regulators for the same thing seems like bad news, at best it's going to make reuse harder.
What reuse is harder?
The consumer needs to understand the fact that a given supply might be represented by multiple different regulators that it sees. This means the system integration is not being abstracted away from the consumer.
Quoted 7 lines
Quoted 6 lines
This version is starting to sound a lot like the consumers need to be able to do an idle to sleep transition and change their settings when they enter their sleep mode. It seems a lot like what many devices do now when they enter and exit runtime PM but with deferred application depending on other users (for regulators I'm assuming the active set would actually be a subset of the sleep set of valid voltages).
Quoted 5 lines
Yes, one key point is that we want the transition to be handled by the RPM co-processor. The reason being that usually we want to change settings after the last CPU transitions into their sleep mode. There's no way to do this without relying on some external co-processor to go and change the settings for us.
That all seems like something that can be handled in the RPM driver, it just needs something to kick it to do the actual writes and let the consumers proceed as normal.
Quoted 6 lines
Quoted 3 lines
That's not what it seemed like people had been talking about up until this most recent e-mail, you'd been talking about suspend mode. The starting point was mapping onto the suspend support the regulator has.
Hm? I've been saying "idle and suspend" each time. Perhaps the initial discussion about regulator suspend support threw things off.
I was being asked about use of regulator suspend support, I naively
thought that this might mean the discussion was something about suspend!
It's also not terribly clear to me what the involvement of idle is here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141127/a56329be/attachment.sig>
cover Qualcomm RPM sleep states
In reply to Mark BrownQuoted 2 lines · Mark Brown
Quoted 1 line · Stephen Boyd
On 11/24/2014 10:16 AM, Mark Brown wrote:
[..]
Quoted 9 lines
Quoted 8 lines
respectively. The RPM regulator driver aggregates the active set for both the regulators via a
max()operation and sends that as a request to the RPM. The sleep set is the same as the active set for the 'active + sleep' regulator, so we just send whatever the value is that was sent down via the regulator APIs on the 'active + sleep' regulator. The only driver that really cares about the active only regulators is the CPU clock driver. Otherwise drivers are using the active + sleep regulators because their devices don't stop running when the CPU goes to idle/suspend.
Quoted 7 lines
Hang on a minute. What you're saying seems to be that this isn't really about suspend but actually about the normal operating configuration? That makes it a bit more understandable why one would change the settings a lot, I think what I'm hearing here is that the runtime state changes a lot for some reason and the system needs the suspend state to track this?
I think it's important that we clarify that we have 2 (somewhat) diffrent problem on the table here:
Problem 1:
Regulators are shared between CPU PLLs and other peripherals and when we take the cpu to idle we need a way to let go of the CPU PLL "vote" on the regulator.
In the case of the CPU being the only consumer I think we've come down to the problems being:
* we're in atomic context
* we don't want to waste the time of making sending out the request
* we have hardware support for this and we want to use it(?)
In the case of us having other active consumers (e.g. GPU, display or some peripherals) the voltage range or operating mode specified by the cpufreq driver might be suboptimal when we remove the cpu.
I assume we don't want to handle this explicitly due to the same reasons as above(?)
Problem 2:
As we're implementing a solution for problem 1 we end up with a set of writes to the sleep state that are superfluous. The codeaurora solution for this is to buffer any writes to the sleep state and right before putting the cpu into idle state there's a direct call going, that flushes the buffered writes.
This is "only" and optimization and I think the tricky parts are how to actually trigger the flush from the cpuidle driver and how to handle the smd communication in a sane way.
Quoted 6 lines
I can't help but think that this all sounds like the RPM isn't mapping very well onto practical systems and needs revisiting in future versions... for example with what I'm parsing out of the above an active+sleep set command or otherwise having the two modes tied together for some regulators would make the whole problem go away.
As Stephen answered for regulators that only have the "both state" only one write is needed.
And if I read the docs correctly there are 4 regulators that needs this special handling and 29 that works just as you normally would expect.
Quoted 8 lines
Quoted 7 lines
Maybe another solution would be to push the problem into the regulator core and educate it about the two different sets. RPM resources would map one-to-one with a regulator and the sleep set and active sets would be selectable via the
regulator_get()API or some other consumer mapping method. This would allow consumers to request whatever set they care about and consolidate the aggregation logic that's duplicated at the consumer level and the driver level into the core.
Quoted 6 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
I think we should start off by hiding this in the driver, if others come forward needing something similar we can look at how to integrate parts of it into the core.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 7 lines · Bjorn Andersson
Problem 1:
Regulators are shared between CPU PLLs and other peripherals and when we take the cpu to idle we need a way to let go of the CPU PLL "vote" on the regulator.In the case of the CPU being the only consumer I think we've come down to the problems being:
* we're in atomic context
That shouldn't be an issue, we can do something like what we've got for regmap and allow regulators to replace their mutexes with spinlocks (users and drivers will need to be careful they don't do expensive things, plus we wamy want to look at optimizing the core, but that's true anyway).
Quoted 2 lines
* we don't want to waste the time of making sending out the request
* we have hardware support for this and we want to use it(?)
I'm still hazy on what that actually means. I thought the CPU case was one of the big ones where you *did* want to act on the configuration changes?
Quoted 3 lines
In the case of us having other active consumers (e.g. GPU, display or some peripherals) the voltage range or operating mode specified by the cpufreq driver might be suboptimal when we remove the cpu.
I don't understand. Why is the other consumer asking for something it doesn't need itself?
Quoted 9 lines
Problem 2:
As we're implementing a solution for problem 1 we end up with a set of writes to the sleep state that are superfluous. The codeaurora solution for this is to buffer any writes to the sleep state and right before putting the cpu into idle state there's a direct call going, that flushes the buffered writes.This is "only" and optimization and I think the tricky parts are how to actually trigger the flush from the cpuidle driver and how to handle the smd communication in a sane way.
That seems like a very obvious thing to do.
Quoted 10 lines
Quoted 6 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
I think we should start off by hiding this in the driver, if others come forward needing something similar we can look at how to integrate parts of it into the core.
If you're hiding it in the driver make sure it's *hidden* in the driver
- exporting duplicate regulators doesn't sound like hiding to me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141127/afd860fd/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 10 lines · Mark Brown
Quoted 7 lines · Bjorn Andersson
Problem 1:
Regulators are shared between CPU PLLs and other peripherals and when we take the cpu to idle we need a way to let go of the CPU PLL "vote" on the regulator.In the case of the CPU being the only consumer I think we've come down to the problems being:
* we're in atomic context
Quoted 6 lines
That shouldn't be an issue, we can do something like what we've got for regmap and allow regulators to replace their mutexes with spinlocks (users and drivers will need to be careful they don't do expensive things, plus we wamy want to look at optimizing the core, but that's true anyway).
If I understand Stephen correctly, this is basically what would happen when we go to suspend (take the CPUs off the individual PLLs and releasing the regulators), but for going in and out of _idle_ this is too costly and we want to utilise the hardware assist for this.
Quoted 7 lines
Quoted 2 lines
* we don't want to waste the time of making sending out the request
* we have hardware support for this and we want to use it(?)I'm still hazy on what that actually means. I thought the CPU case was one of the big ones where you *did* want to act on the configuration changes?
By utilising the hardware support for switching between active and sleep state (and having the cpu only affecting the active state) we can do this asynchronously, while being in wfi.
Quoted 7 lines
Quoted 3 lines
In the case of us having other active consumers (e.g. GPU, display or some peripherals) the voltage range or operating mode specified by the cpufreq driver might be suboptimal when we remove the cpu.
I don't understand. Why is the other consumer asking for something it doesn't need itself?
I don't have the actual numbers, but we have a situation that looks like this:
gpu driver does regulator_set_voltage(.5V, 1V);
cpufreq driver does regulator_set_voltage(1V,1V);
The regulator framework will make the regulator tick at 1V
Upon going to sleep the cpuidle driver will, through the hardware support, make the regulator tick at .5V. Before returning the cpu from idle the hardware will make the regulator tick at 1V again.
[..]
Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
I think any duplication that's going on sounds like a consequence of the way this is currently implemented. I think based on what I *think* you're saying the RPM driver probably ought to be hiding this and adding a property which makes the active and sleep sets track each other with normal suspend mode control otherwise. That could potentially be done in the core, though the tracking would be substantial surgery.
Quoted 7 lines
Quoted 3 lines
I think we should start off by hiding this in the driver, if others come forward needing something similar we can look at how to integrate parts of it into the core.
If you're hiding it in the driver make sure it's *hidden* in the driver
- exporting duplicate regulators doesn't sound like hiding to me.
This is the problem at hand, we have not found any way to actually hide this in the driver as the two modes depend on the consumer and not the aggregated values we get out of the regulator framework.
Or rather, my proposal would do that, but that will not be able to handle the case when there's different voltages in the two states. It would only take care of the enable/disable case.
The exposure of multiple regulators moves the problem to the devicetree, making sure to map the consumers to the right state/regulator. But it should only be the cpu (in its various forms) that ever consume the active only regulator.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines · Mark Brown
Quoted 2 lines
* we don't want to waste the time of making sending out the request
* we have hardware support for this and we want to use it(?)
Quoted 7 lines
Quoted 3 lines
I'm still hazy on what that actually means. I thought the CPU case was one of the big ones where you *did* want to act on the configuration changes?
By utilising the hardware support for switching between active and sleep state (and having the cpu only affecting the active state) we can do this asynchronously, while being in wfi.
Hrm, right. This is starting to make sense. So what we're saying here is roughly that you want to use a separate suspend-like mode (the extra set of registers for the low power mode mode) but are entering it without actually suspending the whole system?
Quoted 11 lines
Quoted 6 lines
Quoted 3 lines
In the case of us having other active consumers (e.g. GPU, display or some peripherals) the voltage range or operating mode specified by the cpufreq driver might be suboptimal when we remove the cpu.
I don't understand. Why is the other consumer asking for something it doesn't need itself?
I don't have the actual numbers, but we have a situation that looks like this:
gpu driver doesregulator_set_voltage(.5V, 1V);
cpufreq driver doesregulator_set_voltage(1V,1V);
Quoted 5 lines
The regulator framework will make the regulator tick at 1V
Upon going to sleep the cpuidle driver will, through the hardware support, make the regulator tick at .5V. Before returning the cpu from idle the hardware will make the regulator tick at 1V again.
So the regulator framework should be fine if the CPU regulator constraint was disabled while the CPU consumer was disabled (modulo the performance issue)?
Quoted 10 lines
Quoted 2 lines
If you're hiding it in the driver make sure it's *hidden* in the driver
- exporting duplicate regulators doesn't sound like hiding to me.This is the problem at hand, we have not found any way to actually hide this in the driver as the two modes depend on the consumer and not the aggregated values we get out of the regulator framework.
Or rather, my proposal would do that, but that will not be able to handle the case when there's different voltages in the two states. It would only take care of the enable/disable case.
So, I think something along the lines of suspend mode ought to be able to handle this. What we need is a way of accelerating the entry and exit, right?
Quoted 4 lines
The exposure of multiple regulators moves the problem to the devicetree, making sure to map the consumers to the right state/regulator. But it should only be the cpu (in its various forms) that ever consume the active only regulator.
He said hopefully... :)
I think I now have a reasonable picture of what's going on but wanted to
confirm that what I'm saying above makes sense.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141128/8704411b/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 7 lines · Mark Brown
Quoted 6 lines · Bjorn Andersson
Quoted 5 lines · Mark Brown
Quoted 2 lines
* we don't want to waste the time of making sending out the request
* we have hardware support for this and we want to use it(?)I'm still hazy on what that actually means. I thought the CPU case was one of the big ones where you *did* want to act on the configuration changes?
Quoted 7 lines
Quoted 3 lines
By utilising the hardware support for switching between active and sleep state (and having the cpu only affecting the active state) we can do this asynchronously, while being in wfi.
Hrm, right. This is starting to make sense. So what we're saying here is roughly that you want to use a separate suspend-like mode (the extra set of registers for the low power mode mode) but are entering it without actually suspending the whole system?
For the idle path, yes. We also want to do the same sort of thing on the system suspend path.
Quoted 13 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
In the case of us having other active consumers (e.g. GPU, display or some peripherals) the voltage range or operating mode specified by the cpufreq driver might be suboptimal when we remove the cpu.
I don't understand. Why is the other consumer asking for something it doesn't need itself?
Quoted 7 lines
I don't have the actual numbers, but we have a situation that looks like this:
gpu driver doesregulator_set_voltage(.5V, 1V);
cpufreq driver doesregulator_set_voltage(1V,1V);
The regulator framework will make the regulator tick at 1V
Upon going to sleep the cpuidle driver will, through the hardware support, make the regulator tick at .5V. Before returning the cpu from idle the hardware will make the regulator tick at 1V again.
Quoted 3 lines
So the regulator framework should be fine if the CPU regulator constraint was disabled while the CPU consumer was disabled (modulo the performance issue)?
The CPU regulator consumer won't be disabled through any software mechanism. It will stay enabled in software and when we execute wfi the hardware will drop that consumers request because it was only requesting in the active set.
Quoted 13 lines
Quoted 9 lines
Quoted 3 lines
If you're hiding it in the driver make sure it's *hidden* in the driver
- exporting duplicate regulators doesn't sound like hiding to me.This is the problem at hand, we have not found any way to actually hide this in the driver as the two modes depend on the consumer and not the aggregated values we get out of the regulator framework. Or rather, my proposal would do that, but that will not be able to handle the case when there's different voltages in the two states. It would only take care of the enable/disable case.
So, I think something along the lines of suspend mode ought to be able to handle this. What we need is a way of accelerating the entry and exit, right?
Quoted 8 lines
Quoted 4 lines
The exposure of multiple regulators moves the problem to the devicetree, making sure to map the consumers to the right state/regulator. But it should only be the cpu (in its various forms) that ever consume the active only regulator.
He said hopefully... :)
I think I now have a reasonable picture of what's going on but wanted to confirm that what I'm saying above makes sense.
That's good. Is there any conclusion here? I'm still thinking that having multiple regulators for the same physical supply is the right thing to do.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
cover Qualcomm RPM sleep states
Quoted 2 lines · Stephen Boyd
Quoted 1 line · Mark Brown
On Thu, Nov 27, 2014 at 11:42:41AM -0800, Bjorn Andersson wrote:
[..]
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The exposure of multiple regulators moves the problem to the devicetree, making sure to map the consumers to the right state/regulator. But it should only be the cpu (in its various forms) that ever consume the active only regulator.
He said hopefully... :)
:)
Quoted 8 lines
Quoted 3 lines
I think I now have a reasonable picture of what's going on but wanted to confirm that what I'm saying above makes sense.
That's good. Is there any conclusion here? I'm still thinking that having multiple regulators for the same physical supply is the right thing to do.
As I said, the only other idea I've come up with will not cut it for the regulators that need more than enable/disable support when going to sleep.
Even if we came up with some model of exposing something similar to the suspend state, we would still need to expose it in a way that we can specify which (active/sleep/both) state in the consumer. Hence in practice we end up with exposing multiple regulators in one way or another.
I can't help thinking that this would be a problem with the static suspend settings as well; i.e. what is the static suspend state for a regulator that powers a WiFi chip? For me the answer would often be "it's enabled iff the WiFi consumer asks for it" - but maybe it's not supposed to be used for "dynamic" regulators.
Nontheless, we have reached conclusions regarding my RFC. So I'll move on and finish up a multi-regulator implementation and we can continue the discussion based on that.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 9 lines · Bjorn Andersson
Quoted 8 lines · Stephen Boyd
Quoted 2 lines · Mark Brown
I think I now have a reasonable picture of what's going on but wanted to confirm that what I'm saying above makes sense.
That's good. Is there any conclusion here? I'm still thinking that having multiple regulators for the same physical supply is the right thing to do.
I'm still not finding it easy to find this tasteful.
Quoted 7 lines
As I said, the only other idea I've come up with will not cut it for the regulators that need more than enable/disable support when going to sleep.
Even if we came up with some model of exposing something similar to the suspend state, we would still need to expose it in a way that we can specify which (active/sleep/both) state in the consumer. Hence in practice we end up with exposing multiple regulators in one way or another.
Now I'm confused again. I thought entry and exit was all done separately so it was just about saying what should happen if the device were to idle?
Quoted 5 lines
I can't help thinking that this would be a problem with the static suspend settings as well; i.e. what is the static suspend state for a regulator that powers a WiFi chip? For me the answer would often be "it's enabled iff the WiFi consumer asks for it" - but maybe it's not supposed to be used for "dynamic" regulators.
That's what should happen, we just don't currently really support doing
this configuration dynamically (practically speaking I suspect anyone
who cares just doesn't have a suspend mode for affected regulators at
the minute).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141208/29eb47f8/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 16 lines · Mark Brown
Quoted 9 lines · Bjorn Andersson
Quoted 8 lines · Stephen Boyd
Quoted 2 lines · Mark Brown
I think I now have a reasonable picture of what's going on but wanted to confirm that what I'm saying above makes sense.
That's good. Is there any conclusion here? I'm still thinking that having multiple regulators for the same physical supply is the right thing to do.
I'm still not finding it easy to find this tasteful.
Quoted 2 lines
As I said, the only other idea I've come up with will not cut it for the regulators that need more than enable/disable support when going to sleep.
Quoted 9 lines
Quoted 4 lines
Even if we came up with some model of exposing something similar to the suspend state, we would still need to expose it in a way that we can specify which (active/sleep/both) state in the consumer. Hence in practice we end up with exposing multiple regulators in one way or another.
Now I'm confused again. I thought entry and exit was all done separately so it was just about saying what should happen if the device were to idle?
But how do you actually expose that control to the consumers?
Quoted 10 lines
Quoted 5 lines
I can't help thinking that this would be a problem with the static suspend settings as well; i.e. what is the static suspend state for a regulator that powers a WiFi chip? For me the answer would often be "it's enabled iff the WiFi consumer asks for it" - but maybe it's not supposed to be used for "dynamic" regulators.
That's what should happen, we just don't currently really support doing this configuration dynamically (practically speaking I suspect anyone who cares just doesn't have a suspend mode for affected regulators at the minute).
What would such configuration look like?
In the suspend case you could introduce an api for setting parameters of the suspend state, but that means that the individual drivers needs to be written with awareness of the system state. Our proposal is to use the same api, but with a different regulator*, but how should that acquired (and expressed in dt).
In our case, as most things change the both state (or active only if no both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 7 lines · Bjorn Andersson
Quoted 3 lines · Mark Brown
Now I'm confused again. I thought entry and exit was all done separately so it was just about saying what should happen if the device were to idle?
But how do you actually expose that control to the consumers?
I think having an idle state (or possibly just using the suspend state, I'm still foggy if the two states are actually different) seems to make sense?
Quoted 11 lines
Quoted 4 lines
That's what should happen, we just don't currently really support doing this configuration dynamically (practically speaking I suspect anyone who cares just doesn't have a suspend mode for affected regulators at the minute).
What would such configuration look like?
In the suspend case you could introduce an api for setting parameters of the suspend state, but that means that the individual drivers needs to be written with awareness of the system state. Our proposal is to use the same api, but with a different regulator*, but how should that acquired (and expressed in dt).
We already have an API for the suspend state, we just don't enable its use in DT yet other than static configuration. I'm not sure I see the connection to the system state in the consumer API here - I thought this was pretty much just setting the state when the device is turned off which seems device specific?
Quoted 3 lines
In our case, as most things change the both state (or active only if no both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141209/4bc5766f/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 7 lines · Mark Brown
Quoted 5 lines · Bjorn Andersson
Quoted 3 lines · Mark Brown
Now I'm confused again. I thought entry and exit was all done separately so it was just about saying what should happen if the device were to idle?
Quoted 6 lines
Quoted 1 line
But how do you actually expose that control to the consumers?
I think having an idle state (or possibly just using the suspend state, I'm still foggy if the two states are actually different) seems to make sense?
In 8974 we have LDO12 that have the following consumers:
csi
dsi
hdmi
edp
modem pll
pronto pll (wifi/bt)
krait-clock
Each of these drivers will have a regulator object, upon which they can make requests on.
The drivers for csi to pronto does not care about the state of the cpu - we want to output pixels even if the cpu is idle - so their requests (on their regulator *) must be aggregated towards the active state as well as the idle state.
The krait-clock should not affect the idle state.
So the idle state enable property should be aggregated from csi to pronto enable properties. I.e. if they are all off, then the idle state should be off, otherwise it's on.
The switch-mode regulators also needs to aggregate voltage and mode - from a subset of the regulator objects/consumers.
Quoted 12 lines
Quoted 11 lines
Quoted 4 lines
That's what should happen, we just don't currently really support doing this configuration dynamically (practically speaking I suspect anyone who cares just doesn't have a suspend mode for affected regulators at the minute).
What would such configuration look like?
In the suspend case you could introduce an api for setting parameters of the suspend state, but that means that the individual drivers needs to be written with awareness of the system state. Our proposal is to use the same api, but with a different regulator*, but how should that acquired (and expressed in dt).
Quoted 6 lines
We already have an API for the suspend state, we just don't enable its use in DT yet other than static configuration. I'm not sure I see the connection to the system state in the consumer API here - I thought this was pretty much just setting the state when the device is turned off which seems device specific?
The suspend state (with the additional exposure in DT) allows us to specify a static configuration of the suspend state of a regulator. Here we see a need for the properties of that state to be aggregated from a subset of the regulator objects.
Quoted 5 lines
Quoted 3 lines
In our case, as most things change the both state (or active only if no both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
It's just that I, possibly incorrectly, considered that to be the outlier and the one that would be the easiest to model separately.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 15 lines · Bjorn Andersson
Quoted 3 lines · Mark Brown
I think having an idle state (or possibly just using the suspend state, I'm still foggy if the two states are actually different) seems to make sense?
In 8974 we have LDO12 that have the following consumers:
csi
dsi
hdmi
edp
modem pll
pronto pll (wifi/bt)
krait-clock
Quoted 7 lines
Each of these drivers will have a regulator object, upon which they can make requests on.
The drivers for csi to pronto does not care about the state of the cpu - we want to output pixels even if the cpu is idle - so their requests (on their regulator *) must be aggregated towards the active state as well as the idle state.
Are they actually twiddling anything except enable, and I'm not sure that this answers my question about suspend state either?
Quoted 5 lines
The krait-clock should not affect the idle state.
So the idle state enable property should be aggregated from csi to pronto enable properties. I.e. if they are all off, then the idle state should be off, otherwise it's on.
OK, so this is pointing back to the idea that we just want a way to knock out this one consumer from the constraints when it's going idle (you mention later on that other regulators need parameters other than the enable changing). Now, I seem to recall that the reason that this is a problem is that you want to use this state changing stuff to accelerate that transition. This seems like something other regulators can do which we don't really take advantage of yet except in that some regualtors remember previous settings for voltage and can switch to them quickly.
What this is *now* sounding like is that we need a way to say that this one consumer is magic and we want to essentially tell the hardware about settings with and without that consumer then the hardware will magically switch between the two when the consumer is enabled and disabled. Does that sound about right?
Quoted 2 lines
The switch-mode regulators also needs to aggregate voltage and mode - from a subset of the regulator objects/consumers.
We don't have any support at all for aggregating mode except via current estimation which is unused and I'd be surprised if that works usefully...
Quoted 10 lines
Quoted 5 lines
We already have an API for the suspend state, we just don't enable its use in DT yet other than static configuration. I'm not sure I see the connection to the system state in the consumer API here - I thought this was pretty much just setting the state when the device is turned off which seems device specific?
The suspend state (with the additional exposure in DT) allows us to specify a static configuration of the suspend state of a regulator. Here we see a need for the properties of that state to be aggregated from a subset of the regulator objects.
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
In our case, as most things change the both state (or active only if no both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
It's just that I, possibly incorrectly, considered that to be the outlier and the one that would be the easiest to model separately.
I'm sorry, I'm just not getting this at all.
Can I please ask that people stop giving partial pictures of what's going on? There's a few reasons why this is all so hard to follow - one of them is that the story is coming out in dribs and drabs so every time I feel I understand what everyone is talking about everything shifts.
I really do fear that the bodge you're using at the minute with multiple
regulators has poor abstraction and is hence too fragile - it seems like
there's too much knowledge of the system spread around different drivers
and it's all vulnerable to changes in system integration which could
potentially be made per board.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141209/498255d9/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 19 lines · Mark Brown
Quoted 17 lines · Bjorn Andersson
Quoted 3 lines · Mark Brown
I think having an idle state (or possibly just using the suspend state, I'm still foggy if the two states are actually different) seems to make sense?
In 8974 we have LDO12 that have the following consumers:
csi
dsi
hdmi
edp
modem pll
pronto pll (wifi/bt)
krait-clockEach of these drivers will have a regulator object, upon which they can make requests on.
Quoted 8 lines
Quoted 4 lines
The drivers for csi to pronto does not care about the state of the cpu - we want to output pixels even if the cpu is idle - so their requests (on their regulator *) must be aggregated towards the active state as well as the idle state.
Are they actually twiddling anything except enable, and I'm not sure that this answers my question about suspend state either?
Yes, for the switch-mode regulators there's different enable state, voltage and mode.
Quoted 6 lines
Quoted 5 lines
The krait-clock should not affect the idle state.
So the idle state enable property should be aggregated from csi to pronto enable properties. I.e. if they are all off, then the idle state should be off, otherwise it's on.
Quoted 10 lines
OK, so this is pointing back to the idea that we just want a way to knock out this one consumer from the constraints when it's going idle (you mention later on that other regulators need parameters other than the enable changing). Now, I seem to recall that the reason that this is a problem is that you want to use this state changing stuff to accelerate that transition. This seems like something other regulators can do which we don't really take advantage of yet except in that some regualtors remember previous settings for voltage and can switch to them quickly.
Correct, the problem here is twofold in my eyes;
At the point then the consumer decides that it does not need the regulator it's in atomic context. So the regulator core and driver would need to handle this. Further more the regulator driver uses a packet-based protocol over a shared memory channel, so this entire path would have to be invoked in atomic context.
We have to put the consumer into a state where we can "release" the regulator, then we need to tell the RPM to update the state of the regulator and we have to wait for the ack to come back before we can actually go idle.
Quoted 6 lines
What this is *now* sounding like is that we need a way to say that this one consumer is magic and we want to essentially tell the hardware about settings with and without that consumer then the hardware will magically switch between the two when the consumer is enabled and disabled. Does that sound about right?
I think it sounds spot on.
Quoted 7 lines
Quoted 2 lines
The switch-mode regulators also needs to aggregate voltage and mode - from a subset of the regulator objects/consumers.
We don't have any support at all for aggregating mode except via current estimation which is unused and I'd be surprised if that works usefully...
Let's treat that as a separate topic.
Quoted 6 lines
Quoted 5 lines
Quoted 5 lines
We already have an API for the suspend state, we just don't enable its use in DT yet other than static configuration. I'm not sure I see the connection to the system state in the consumer API here - I thought this was pretty much just setting the state when the device is turned off which seems device specific?
Quoted 8 lines
Quoted 4 lines
The suspend state (with the additional exposure in DT) allows us to specify a static configuration of the suspend state of a regulator. Here we see a need for the properties of that state to be aggregated from a subset of the regulator objects.
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Do you mean alter properties of a state or select between the predefined states? I've found some apis related to the latter, but not the first.
Quoted 11 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
In our case, as most things change the both state (or active only if no both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
It's just that I, possibly incorrectly, considered that to be the outlier and the one that would be the easiest to model separately.
I'm sorry, I'm just not getting this at all.
Because for the overall system the active state is the outlier here. But it probably doesn't matter for our conclusions.
Quoted 5 lines
Can I please ask that people stop giving partial pictures of what's going on? There's a few reasons why this is all so hard to follow - one of them is that the story is coming out in dribs and drabs so every time I feel I understand what everyone is talking about everything shifts.
Sorry, I've tried to describe the hardware setup but have probably not made a clear enough distinction towards the downstream implementation and solution suggestions.
Quoted 5 lines
I really do fear that the bodge you're using at the minute with multiple regulators has poor abstraction and is hence too fragile - it seems like there's too much knowledge of the system spread around different drivers and it's all vulnerable to changes in system integration which could potentially be made per board.
Just to make sure we're on the same page here, the way this is expressed in downstream is like the following:
ldo12 {
l12: l12 {
set-both-states;
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
};
l12_active: l12-active {
set-only-active-state;
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
};
};dsi {
vddio-supply = <&l12>;
};clock-krait {
hfpll-analog-supply = <&l12_active>;
};All the consumers uses the standard regulator accessor functions to operate on their respective regulator *. Further more it's only the clock driver for the core (krait) that references the active only regulators.
So the knowledge spread out would be contained to the dt bindings things together.
I can however not argue against you on the fact that the way this is exposed is suboptimal. The question is if we can figure a better way to express it - note that we do not want e.g. the dsi driver to know anything about the fact that ldo12 is special.
What I don't like with that solution is that we above have two different rdev and that we need to aggregate state between the various rdevs in the ldo12 grouping. (so that active only and both actually affect the active state).
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 9 lines · Bjorn Andersson
At the point then the consumer decides that it does not need the regulator it's in atomic context. So the regulator core and driver would need to handle this. Further more the regulator driver uses a packet-based protocol over a shared memory channel, so this entire path would have to be invoked in atomic context.
We have to put the consumer into a state where we can "release" the regulator, then we need to tell the RPM to update the state of the regulator and we have to wait for the ack to come back before we can actually go idle.
None of which sounds like such a big deal, a bigger problem seemed to be trying to suppress intermediate updates of the device state that were being done though that can comfortably be done in the driver I think.
Quoted 6 lines
Quoted 5 lines
What this is *now* sounding like is that we need a way to say that this one consumer is magic and we want to essentially tell the hardware about settings with and without that consumer then the hardware will magically switch between the two when the consumer is enabled and disabled. Does that sound about right?
Quoted 8 lines
I think it sounds spot on.
Quoted 2 lines
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Do you mean alter properties of a state or select between the predefined states? I've found some apis related to the latter, but not the first.
Hrm, indeed. Well, duplicating the existing APIs seems like a good way forwards - the logic is all the same, it's just that we don't want to apply the changes to the state used at runtime.
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
It's just that I, possibly incorrectly, considered that to be the outlier and the one that would be the easiest to model separately.
I'm sorry, I'm just not getting this at all.
Because for the overall system the active state is the outlier here. But it probably doesn't matter for our conclusions.
I'd argue that for the purpose of running Linux it's the common state...
Quoted 14 lines
Quoted 5 lines
I really do fear that the bodge you're using at the minute with multiple regulators has poor abstraction and is hence too fragile - it seems like there's too much knowledge of the system spread around different drivers and it's all vulnerable to changes in system integration which could potentially be made per board.
Just to make sure we're on the same page here, the way this is expressed in downstream is like the following:
ldo12 { l12: l12 { l12_active: l12-active {
Quoted 6 lines
All the consumers uses the standard regulator accessor functions to operate on their respective regulator *. Further more it's only the clock driver for the core (krait) that references the active only regulators.
So the knowledge spread out would be contained to the dt bindings things together.
It's not just the DT binding, it's also the regulator driver that needs to do the aggregation that's being discussed and is going to assume a particular arrangement of clients.
Quoted 3 lines
What I don't like with that solution is that we above have two different rdev and that we need to aggregate state between the various rdevs in the ldo12 grouping. (so that active only and both actually affect the active state).
Yes, that's a big part of the problem - there's things going on that the
core should know about but doesn't.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141215/23b963f3/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 2 lines · Mark Brown
On Thu, Dec 11, 2014 at 02:36:54PM -0800, Bjorn Andersson wrote:
[..]
Quoted 8 lines
Quoted 3 lines
We have to put the consumer into a state where we can "release" the regulator, then we need to tell the RPM to update the state of the regulator and we have to wait for the ack to come back before we can actually go idle.
None of which sounds like such a big deal, a bigger problem seemed to be trying to suppress intermediate updates of the device state that were being done though that can comfortably be done in the driver I think.
As I said previously, suppressing unnecessary updates is an optimization - although an important one. The key question is how we model the multiple states.
[..]
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Do you mean alter properties of a state or select between the predefined states? I've found some apis related to the latter, but not the first.
Hrm, indeed. Well, duplicating the existing APIs seems like a good way forwards - the logic is all the same, it's just that we don't want to apply the changes to the state used at runtime.
Are you suggesting that we introduce a shadow of the current API for the purpose of affecting a certain state? Can you elaborate on how that would look like and work?
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
both/sleep), the standard regulator api would have to affect the both state. Would we then have a separate api to modify the active state?
Why would we need to introduce a new API for the active state?
It's just that I, possibly incorrectly, considered that to be the outlier and the one that would be the easiest to model separately.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
I'm sorry, I'm just not getting this at all.
Because for the overall system the active state is the outlier here. But it probably doesn't matter for our conclusions.
I'd argue that for the purpose of running Linux it's the common state...
Does "running Linux" indicate that there's instructions flowing through the CPU pipeline or that the hardware overall is up and running?
Quoted 18 lines
Quoted 17 lines
Quoted 5 lines
I really do fear that the bodge you're using at the minute with multiple regulators has poor abstraction and is hence too fragile - it seems like there's too much knowledge of the system spread around different drivers and it's all vulnerable to changes in system integration which could potentially be made per board.
Just to make sure we're on the same page here, the way this is expressed in downstream is like the following:
ldo12 { l12: l12 { l12_active: l12-active {All the consumers uses the standard regulator accessor functions to operate on their respective regulator *. Further more it's only the clock driver for the core (krait) that references the active only regulators.
Quoted 7 lines
Quoted 2 lines
So the knowledge spread out would be contained to the dt bindings things together.
It's not just the DT binding, it's also the regulator driver that needs to do the aggregation that's being discussed and is going to assume a particular arrangement of clients.
The downstream implementation sports 3 rdevs per regulator and their requests are aggregated into the two states. So the regulator implementation are not aware of the individual clients, it just aggregates the 3 rdev states and programs the hardware accordingly.
Quoted 6 lines
Quoted 3 lines
What I don't like with that solution is that we above have two different rdev and that we need to aggregate state between the various rdevs in the ldo12 grouping. (so that active only and both actually affect the active state).
Yes, that's a big part of the problem - there's things going on that the core should know about but doesn't.
The way that the aggregation of these properties works there's no problems doing it in stages. But it comes with code duplication and the need of the various rdevs to share common state.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
Quoted 9 lines · Bjorn Andersson
Quoted 7 lines · Mark Brown
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Do you mean alter properties of a state or select between the predefined states? I've found some apis related to the latter, but not the first.
Quoted 7 lines
Quoted 3 lines
Hrm, indeed. Well, duplicating the existing APIs seems like a good way forwards - the logic is all the same, it's just that we don't want to apply the changes to the state used at runtime.
Are you suggesting that we introduce a shadow of the current API for the purpose of affecting a certain state? Can you elaborate on how that would look like and work?
Like the current APIs but specifying a state?
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Because for the overall system the active state is the outlier here. But it probably doesn't matter for our conclusions.
I'd argue that for the purpose of running Linux it's the common state...
Does "running Linux" indicate that there's instructions flowing through the CPU pipeline or that the hardware overall is up and running?
I think that for most practical purposes with Android (which is the interesting thing here) those are very much the same?
Quoted 8 lines
Quoted 3 lines
It's not just the DT binding, it's also the regulator driver that needs to do the aggregation that's being discussed and is going to assume a particular arrangement of clients.
The downstream implementation sports 3 rdevs per regulator and their requests are aggregated into the two states. So the regulator implementation are not aware of the individual clients, it just aggregates the 3 rdev states and programs the hardware accordingly.
To me doing that aggregation requires some knowledge.
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
What I don't like with that solution is that we above have two different rdev and that we need to aggregate state between the various rdevs in the ldo12 grouping. (so that active only and both actually affect the active state).
Yes, that's a big part of the problem - there's things going on that the core should know about but doesn't.
The way that the aggregation of these properties works there's no problems doing it in stages. But it comes with code duplication and the need of the various rdevs to share common state.
To me both the sharing of state and the code duplication are problems.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141226/4b243377/attachment.sig>
cover Qualcomm RPM sleep states
Quoted 14 lines · Mark Brown
Quoted 12 lines · Bjorn Andersson
Quoted 11 lines · Mark Brown
Quoted 5 lines · Bjorn Andersson
Quoted 2 lines
We already have a runtime API for specifying suspend state. It doesn't currently aggregate but that's a simple matter of programming.
Do you mean alter properties of a state or select between the predefined states? I've found some apis related to the latter, but not the first.
Hrm, indeed. Well, duplicating the existing APIs seems like a good way forwards - the logic is all the same, it's just that we don't want to apply the changes to the state used at runtime.
Quoted 6 lines
Quoted 3 lines
Are you suggesting that we introduce a shadow of the current API for the purpose of affecting a certain state? Can you elaborate on how that would look like and work?
Like the current APIs but specifying a state?
So then we have two choices:
1) We make the standard API affect both our states and use the special api to affect only the active state. Which doesn't seem completely unreasonable to implement as we only have one special consumer and the aggregation in the driver is trivial.
2) We make the "run queue empty" the special state and in e.g. the display driver (DSI) we have to pair every regulator api call with a call to the special version. So suddenly we have riddled drivers with "knowledge" about how cpuidle works on these platforms (but not all).
Quoted 12 lines
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Because for the overall system the active state is the outlier here. But it probably doesn't matter for our conclusions.
I'd argue that for the purpose of running Linux it's the common state...
Does "running Linux" indicate that there's instructions flowing through the CPU pipeline or that the hardware overall is up and running?
I think that for most practical purposes with Android (which is the interesting thing here) those are very much the same?
While Android is not the single usecase that we have to consider it's an excellent example of the opposite.
Take e.g. a Nexus 5, hit the power button to "wake it up" sitting in lockscreen. You will now have pm8941_ldo12 requested enabled by the display core (DSI) and the CPU PLLs. When the run queue empties out the display will be kept on but the CPU no longer need ldo12. (And it might sit there for the next 15 seconds - or whatever timeout you have)
So I don't know which one is statistically more common, but both are very common to be in.
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
It's not just the DT binding, it's also the regulator driver that needs to do the aggregation that's being discussed and is going to assume a particular arrangement of clients.
The downstream implementation sports 3 rdevs per regulator and their requests are aggregated into the two states. So the regulator implementation are not aware of the individual clients, it just aggregates the 3 rdev states and programs the hardware accordingly.
To me doing that aggregation requires some knowledge.
Yeah, but all the aggregations that we do can be done stepwise, so taking the various regulator_dev* requests and aggregating them one step futhrer (with the added knowledge about our states) is fine.
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
What I don't like with that solution is that we above have two different rdev and that we need to aggregate state between the various rdevs in the ldo12 grouping. (so that active only and both actually affect the active state).
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Yes, that's a big part of the problem - there's things going on that the core should know about but doesn't.
The way that the aggregation of these properties works there's no problems doing it in stages. But it comes with code duplication and the need of the various rdevs to share common state.
To me both the sharing of state and the code duplication are problems.
I agree, but it's enough time and power cost for us to have to have a solution.
If we implement 1) above then we would need to share the system knowledge with 1 driver (the CPU clock driver) and the actual aggregation in the regulator_dev driver would be extreemly trivial and more importantly specific.
But it's a step away from the current idea of what those states are meant to define.
I'll make some prototyping and see how it looks.
Regards,
Bjorn
Mark Brown
maintainercover Qualcomm RPM sleep states
On Mon, Dec 29, 2014 at 01:54:55PM -0800, Bjorn Andersson wrote:
Could you please fix your mailer to wrap sufficiently far before 80 characters to allow for some quoting?
Quoted 16 lines · Mark Brown
Quoted 7 lines · Bjorn Andersson
Quoted 4 lines
Are you suggesting that we introduce a shadow of the current API for the purpose of affecting a certain state? Can you elaborate on how that would look like and work?
Like the current APIs but specifying a state?
So then we have two choices:
1) We make the standard API affect both our states and use the special api to affect only the active state. Which doesn't seem completely unreasonable to implement as we only have one special consumer and the aggregation in the driver is trivial.
Quoted 4 lines
2) We make the "run queue empty" the special state and in e.g. the display driver (DSI) we have to pair every regulator api call with a call to the special version. So suddenly we have riddled drivers with "knowledge" about how cpuidle works on these platforms (but not all).
I think there's more options than that but whatever...
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
Does "running Linux" indicate that there's instructions flowing through the CPU pipeline or that the hardware overall is up and running?
I think that for most practical purposes with Android (which is the interesting thing here) those are very much the same?
While Android is not the single usecase that we have to consider it's an excellent example of the opposite.
Quoted 8 lines
Take e.g. a Nexus 5, hit the power button to "wake it up" sitting in lockscreen. You will now have
pm8941_ldo12requested enabled by the display core (DSI) and the CPU PLLs. When the run queue empties out the display will be kept on but the CPU no longer need ldo12. (And it might sit there for the next 15 seconds - or whatever timeout you have)So I don't know which one is statistically more common, but both are very common to be in.
So, a couple of things here. One is that unless the system leakages are
truly spectacular I'd expect nobody is going to notice a practical
impact from leaving the CPU PLL LDO on while the display is on given
that in an idle display state the display will tend to dominate the
system power consumption, making it all a bit academic. If you can run
the CPU without the PLL for low power but long term tasks that's more
interesting but a different problem. The other is that you're thinking
of the common state as being the one that is running most whereas I'm
thinking of it as being the one that we have to work with the most - I'd
expect that we're going to be making more changes to the state while
we're actively running than we will be adjusting the state we're going
to go into on idle.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141230/224da6da/attachment.sig>
cover Qualcomm RPM sleep states
In reply to Stephen BoydQuoted 2 lines · Stephen Boyd
Hi Bjorn,
Hi Stephen,
Thanks for taking the time to look at my proposal.
[..]
Quoted 6 lines
So the CPU really wants to only be voting for the HFPLL regulator supplies in the active set. This way, if we're not using the HFPLLs (i.e. the CPUs are all running off the global PLL), then we can disable the regulator in the active set (RPM code only caches sleep set so we won't be doing unnecessary flushes in the idle/suspend path). The clock
In the codeaurora tree the flushing is handled by a call from the Qcom PM driver straight into the regulator/resource driver that have a private list of all regulators that it goes through and pushes out requests (while in atomic context).
I'm not sure how to model this in a sane way...
Quoted 5 lines
driver will make sure to turn off the HFPLLs before we go into a sleep state that would trigger a switch from active to sleep set (commonly referred to as RPM notification). It very well could be that there are other consumers of the same regulator, but that doesn't matter to the CPU clock driver because it only cares about the active set. Now you may
Okay, so CPUFREQ or a system suspend can explicitly call regulator_disable() on our shared regulator and in CPUIDLE we will just depend on the sleep state?
Quoted 9 lines
ask why can't the CPU clock driver disable the regulator when the HFPLL is disabled? We don't do that in this case because a) it causes more RPM communication overhead and b) we will be in atomic context when the HFPLL is disabled during idle/suspend and the regulator APIs are sleeping calls. In the non-idle/suspend path we will disable the regulator.
Also the active/sleep sets are about more than just on/off state. We may have a situation where the active set voltage (or some other attribute like current, mode, etc.) is different than the sleep set voltage. For
You're using the word "may" here and the code clearly would let you do this, the part I've been struggeling with is to backtrack the consumers to figure out if this is actually done.
Quoted 8 lines
example, the CPU is supplied by a digital logic regulator that is shared with other digital logic in the SoC (GPU, wifi, etc.). The CPU may require some high voltage, but the GPU only needs some lower voltage. The suspend/idle code relies on the fact that the GPU is voting on the active+sleep regulator while the CPU is voting on the active only regulator so that the RPM can automatically switch between high voltage and low voltage when the CPU notifies the RPM that it's gone idle (or the CPU wakes up).
Okay, so when we're hitting CPUIDLE we need to lower the voltage on the regulator, should I assume that this then must be raised before the CPU starts executing again?
So what it boils down to is, like the codeaurora code shows, our regulator driver must be able to set properties on active only or both states depending on which regulator consumer makes the request.
For the reference; the codeaurora code models this as "resources" with 2 set of properties, multiple regulators are then tied to the same resource, poking the individual sets based on properties of the regulator (active or both). So in essense we get multiple regulator instances per regulator hardware.
The problem with this model is that it's circumventing the fact that the regulator framework doesn't support our use cases - and I don't like this. I think we need to sit down and discuss this more in detail, most likely with Mark.
This concludes that we cannot merge my currently proposed solution, because we will have to redesign the DT bindings.
Regards,
Bjorn
cover Qualcomm RPM sleep states
Quoted 21 lines · Bjorn Andersson
Quoted 2 lines · Stephen Boyd
Hi Bjorn,
Hi Stephen,
Thanks for taking the time to look at my proposal.
[..]
Quoted 5 lines
So the CPU really wants to only be voting for the HFPLL regulator supplies in the active set. This way, if we're not using the HFPLLs (i.e. the CPUs are all running off the global PLL), then we can disable the regulator in the active set (RPM code only caches sleep set so we won't be doing unnecessary flushes in the idle/suspend path). The clock
In the codeaurora tree the flushing is handled by a call from the Qcom PM driver straight into the regulator/resource driver that have a private list of all regulators that it goes through and pushes out requests (while in atomic context).
I'm not sure how to model this in a sane way...
Quoted 7 lines
Quoted 5 lines
driver will make sure to turn off the HFPLLs before we go into a sleep state that would trigger a switch from active to sleep set (commonly referred to as RPM notification). It very well could be that there are other consumers of the same regulator, but that doesn't matter to the CPU clock driver because it only cares about the active set. Now you may
Okay, so CPUFREQ or a system suspend can explicitly call
regulator_disable()on our shared regulator and in CPUIDLE we will just depend on the sleep state?
Yes.
Quoted 6 lines
Quoted 6 lines
ask why can't the CPU clock driver disable the regulator when the HFPLL is disabled? We don't do that in this case because a) it causes more RPM communication overhead and b) we will be in atomic context when the HFPLL is disabled during idle/suspend and the regulator APIs are sleeping calls. In the non-idle/suspend path we will disable the regulator.
Quoted 6 lines
Quoted 3 lines
Also the active/sleep sets are about more than just on/off state. We may have a situation where the active set voltage (or some other attribute like current, mode, etc.) is different than the sleep set voltage. For
You're using the word "may" here and the code clearly would let you do this, the part I've been struggeling with is to backtrack the consumers to figure out if this is actually done.
s/may//
We have this case for the example I gave, digital logic. On msm8960 it's S3, where the CPU clock driver requests some active only voltage to power the CPU itself whereas the peripheral clock driver requests an active and sleep set voltage for some peripheral like uart or i2c that will be active even when the CPU is idle or suspended.
Quoted 9 lines
Quoted 9 lines
example, the CPU is supplied by a digital logic regulator that is shared with other digital logic in the SoC (GPU, wifi, etc.). The CPU may require some high voltage, but the GPU only needs some lower voltage. The suspend/idle code relies on the fact that the GPU is voting on the active+sleep regulator while the CPU is voting on the active only regulator so that the RPM can automatically switch between high voltage and low voltage when the CPU notifies the RPM that it's gone idle (or the CPU wakes up).
Quoted 3 lines
Okay, so when we're hitting CPUIDLE we need to lower the voltage on the regulator, should I assume that this then must be raised before the CPU starts executing again?
Yes the RPM will raise the voltage when it transitions us from sleep set to active set.
Quoted 12 lines
So what it boils down to is, like the codeaurora code shows, our regulator driver must be able to set properties on active only or both states depending on which regulator consumer makes the request.
For the reference; the codeaurora code models this as "resources" with 2 set of properties, multiple regulators are then tied to the same resource, poking the individual sets based on properties of the regulator (active or both). So in essense we get multiple regulator instances per regulator hardware.
Quoted 4 lines
The problem with this model is that it's circumventing the fact that the regulator framework doesn't support our use cases - and I don't like this. I think we need to sit down and discuss this more in detail, most likely with Mark.
What exactly are we circumventing? I can only guess that we're talking about the aggregation logic?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
cover Qualcomm RPM sleep states
On Mon 24 Nov 13:19 PST 2014, Stephen Boyd wrote:
[..]
Quoted 4 lines
What exactly are we circumventing? I can only guess that we're talking about the aggregation logic?
We're circumventing the fact that the regulator core doesn't have knowledge about our multiple presented views of the same resource.
As the downstream driver shows, if we just implement the right pieces it should work, i.e. give us the correct end result, but it will not be future proof nor pretty. That's why I think we need to discuss how to solve it either in the regulator driver or in the framework.
And based on your feedback, it looks like we would have to do something about this in the framework.
Regards,
Bjorn
cover Qualcomm RPM sleep states
Quoted 8 lines · Bjorn Andersson
On Mon 24 Nov 13:19 PST 2014, Stephen Boyd wrote:
[..]
Quoted 3 lines
What exactly are we circumventing? I can only guess that we're talking about the aggregation logic?
We're circumventing the fact that the regulator core doesn't have knowledge about our multiple presented views of the same resource.
Sorry I don't follow here. Why would the regulator framework care about the different views of a resource? Each regulator we make for the different views will reflect the request made by Linux for a particular set of that RPM resource. So all consumers who are using the S3 active + sleep set regulator will aggregate into one request. Likewise, all the consumers for the S3 active set regulator will aggregate into another request. The only thing that isn't visible is the aggregation between the active only and active + sleep regulators, but that can be determined by doing a max() of the regulators. Even then, if we consider that there are other masters also requesting voltages or enable/disable for these resources we quickly discover there are other things the regulator core doesn't have knowledge about, like what the actual voltage is or if the regulator is really off vs. Linux requesting for it to be off.
Quoted 8 lines
As the downstream driver shows, if we just implement the right pieces it should work, i.e. give us the correct end result, but it will not be future proof nor pretty. That's why I think we need to discuss how to solve it either in the regulator driver or in the framework.
And based on your feedback, it looks like we would have to do something about this in the framework.
I don't see any problems with making multiple regulators for one RPM resource that represent the active set or active + sleep set. Everything could be handled in the RPM regulator driver by looking for the other regulators that are acting on the same RPM resource and aggregating. Maybe you can elaborate on what you think isn't future proof nor pretty about this design?
As a thought experiment, what if there really was two physical independent controllable regulators and a pin from the CPU to the PMIC toggled a mux to select between the two. Such a pin would only be asserted when the CPU turned off. Would you still want to expose this as one regulator to the kernel given that only one supply goes to the CPU at any given time?
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
cover Qualcomm RPM sleep states
Quoted 10 lines · Stephen Boyd
Quoted 8 lines · Bjorn Andersson
On Mon 24 Nov 13:19 PST 2014, Stephen Boyd wrote:
[..]
Quoted 3 lines
What exactly are we circumventing? I can only guess that we're talking about the aggregation logic?
We're circumventing the fact that the regulator core doesn't have knowledge about our multiple presented views of the same resource.
Quoted 9 lines
Sorry I don't follow here. Why would the regulator framework care about the different views of a resource? Each regulator we make for the different views will reflect the request made by Linux for a particular set of that RPM resource. So all consumers who are using the S3 active + sleep set regulator will aggregate into one request. Likewise, all the consumers for the S3 active set regulator will aggregate into another request. The only thing that isn't visible is the aggregation between the active only and active + sleep regulators, but that can be determined by doing a
max()of the regulators. Even then, if we consider
My concern was the outcome of this snippet:
regulator_disable(ldo1_active);regulator_enable(ldo1_both);regulator_is_enable(ldo1_active);
But after reviewing it again, if one treat it in any other way then 'both' and 'active' being separate on a regulator driver level the end result will not be sane.
So you're right, if we're to expose X number of regulators per resource they have to be separate devices.
Quoted 6 lines
that there are other masters also requesting voltages or enable/disable for these resources we quickly discover there are other things the regulator core doesn't have knowledge about, like what the actual voltage is or if the regulator is really off vs. Linux requesting for it to be off.
As you say, we're quite a bit down the rabbit hole already by not even being able to query the hardware for its current state.
Quoted 9 lines
Quoted 8 lines
As the downstream driver shows, if we just implement the right pieces it should work, i.e. give us the correct end result, but it will not be future proof nor pretty. That's why I think we need to discuss how to solve it either in the regulator driver or in the framework.
And based on your feedback, it looks like we would have to do something about this in the framework.
Quoted 7 lines
I don't see any problems with making multiple regulators for one RPM resource that represent the active set or active + sleep set. Everything could be handled in the RPM regulator driver by looking for the other regulators that are acting on the same RPM resource and aggregating. Maybe you can elaborate on what you think isn't future proof nor pretty about this design?
If the regulators are considered completely separate, then the regulator framework would not notice. I was, incorrectly, assuming that some state was shared between them.
The "not pretty" part comes from the regulator driver (or a common parent entity) needing to know what other regulators share a resource and thereby aggregating the requests. An aggregation that does look a lot like the one already done in the regulator core.
Quoted 7 lines
As a thought experiment, what if there really was two physical independent controllable regulators and a pin from the CPU to the PMIC toggled a mux to select between the two. Such a pin would only be asserted when the CPU turned off. Would you still want to expose this as one regulator to the kernel given that only one supply goes to the CPU at any given time?
I think we would have to expose this as two different regulators to the non-cpu consumers, as that looks like the only way we can affect the "both state".
A possible way around that would be to have the individual regulators exposed and then provide a regulator with both specified as supply. The regulator core would aggregate "both" and individual consumer requests to the individual regulators - without the need of the regulator devices needing to know about each other.
Something like:
s3a: pm8921_s3_active {
compatible = "pm8921-smps";
...
};s3s: pm8921_s3_sleep {
compatible = "pm8921-smps";
sleep; /* <- make requests affect the sleep state only */
};pm8921_s3 {
compatible = "dual-supply-regulator";
active-supply = <&s3a>;
sleep-supply = <&s3s>;
};But maybe that's just too crazy...
Regards,
Bjorn