linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: bjorn.andersson@sonymobile.com (Bjorn Andersson)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 0/2] Qualcomm RPM sleep states
Date: Mon, 29 Dec 2014 13:54:55 -0800	[thread overview]
Message-ID: <20141229215454.GA25436@sonymobile.com> (raw)
In-Reply-To: <20141226170912.GI17800@sirena.org.uk>

On Fri 26 Dec 09:09 PST 2014, Mark Brown wrote:

> On Mon, Dec 15, 2014 at 10:05:54PM -0800, Bjorn Andersson wrote:
> > On Mon, Dec 15, 2014 at 10:04 AM, Mark Brown <broonie@kernel.org> wrote:
> > > On Thu, Dec 11, 2014 at 02:36:54PM -0800, Bjorn Andersson wrote:
> 
> > >> > 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?
> 
> 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).

> > >> 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.

> > > 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.

> > >> 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.

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

  reply	other threads:[~2014-12-29 21:54 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-10 22:52 [RFC 0/2] Qualcomm RPM sleep states Bjorn Andersson
2014-11-10 22:52 ` [RFC 1/2] mfd: qcom-rpm: Expose sleep state resources to clients Bjorn Andersson
2014-11-11 12:04   ` Lee Jones
2014-11-11 18:33     ` Bjorn Andersson
2014-11-12  9:52       ` Lee Jones
2014-11-12 14:45         ` Lina Iyer
2014-11-12 19:23           ` Bjorn Andersson
2014-11-19 18:06             ` Lina Iyer
2014-11-12 19:55         ` Bjorn Andersson
2014-11-10 22:52 ` [RFC 2/2] regulator: qcom-rpm: Implement RPM assisted disable Bjorn Andersson
2014-11-11  9:11   ` Andreas Färber
2014-11-11 18:34     ` Bjorn Andersson
2014-11-11 11:59   ` Lee Jones
2014-11-11 18:39     ` Bjorn Andersson
2014-11-11 14:21   ` Javier Martinez Canillas
2014-11-11 19:23     ` Bjorn Andersson
2014-11-21 23:10 ` [RFC 0/2] Qualcomm RPM sleep states Stephen Boyd
2014-11-21 23:27   ` Mark Brown
2014-11-21 23:43     ` Stephen Boyd
2014-11-21 23:54       ` Mark Brown
2014-11-22  0:03         ` Stephen Boyd
2014-11-22  0:16         ` Bjorn Andersson
2014-11-24 18:16       ` Mark Brown
2014-11-24 21:19         ` Stephen Boyd
2014-11-25 20:44           ` Mark Brown
2014-11-26  1:02             ` Stephen Boyd
2014-11-26 13:40               ` Mark Brown
2014-11-27  1:51                 ` Stephen Boyd
2014-11-27 18:56                   ` Mark Brown
2014-11-26 23:34             ` Bjorn Andersson
2014-11-27 19:02               ` Mark Brown
2014-11-27 19:42                 ` Bjorn Andersson
2014-11-28 20:16                   ` Mark Brown
2014-12-04 21:15                     ` Stephen Boyd
2014-12-08 18:06                       ` Bjorn Andersson
2014-12-08 19:39                         ` Mark Brown
2014-12-08 20:55                           ` Bjorn Andersson
2014-12-09 18:16                             ` Mark Brown
2014-12-09 19:25                               ` Bjorn Andersson
2014-12-09 20:28                                 ` Mark Brown
2014-12-11 22:36                                   ` Bjorn Andersson
2014-12-15 18:04                                     ` Mark Brown
2014-12-16  6:05                                       ` Bjorn Andersson
2014-12-26 17:09                                         ` Mark Brown
2014-12-29 21:54                                           ` Bjorn Andersson [this message]
2014-12-30 16:43                                             ` Mark Brown
2014-11-24 17:02   ` Bjorn Andersson
2014-11-24 21:19     ` Stephen Boyd
2014-11-24 21:59       ` Bjorn Andersson
2014-11-25  0:02         ` Stephen Boyd
2014-11-26 22:49           ` Bjorn Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141229215454.GA25436@sonymobile.com \
    --to=bjorn.andersson@sonymobile.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).