RFC Qualcomm RPM sleep states
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest0 of 2 patches 0 repliesNew
Patches in v1 No applies recorded yet
None of the 2 patches the cover letter announces are in this archive.
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [RFC 0/2] 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