From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
Date: Thu, 21 Mar 2013 15:28:52 -0700 [thread overview]
Message-ID: <20130321222852.834.43745@quantum> (raw)
In-Reply-To: <CAPDyKFq4cPqVAfjbG7wZFS2B4PEt5ev_AkwBNi3LjL4OgxJ=tA@mail.gmail.com>
Quoting Ulf Hansson (2013-03-18 03:36:29)
> On 15 March 2013 20:38, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 03/15/2013 06:33 AM, Ulf Hansson wrote:
> >> On 15 March 2013 13:06, Bill Huang <bilhuang@nvidia.com> wrote:
> >>> On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote:
> > ...
> >>>> Some prerequisites; I think am in favor of using the clk API to
> >>>> trigger DVFS changes and then I agree on that clk_prepare|unprepare
> >>>> needs to be possible to track from a DVFS perspective. clk_set_rate is
> >>>> not enough.
> >>>>
> >>>> So if we decide to do the above (using the clk API to trigger DVFS
> >>>> changes), I believe we should discuss two possible solutions;
> >>>> - clk notifiers or..
> >>>> - dvfs clock type.
> >>>>
> >>>> I am trying to make up my mind of what I think is the best solution.
> >>>> Have you considered "dvfs clock type"?
> >>>> I put some comments about this for "[PATCH 2/5] clk: notifier handler
> >>>> for dynamic voltage scaling" recently as well.
> >>>>
> >>>> What could the advantages/disadvantages be between the two options?
> >>>
> >>> I personally prefer clk notifiers since that's easy and all the existing
> >>> device drivers don't need to be modified, a new clock or API might be
> >>> more thoroughly considered (and hence maybe more graceful) but that
> >>> means we need more time to cook and many drivers need to plug into that
> >>> API when it comes out, a lot of test/verification or maybe chaos
> >>> follows, I'm not sure will that be a little overkill.
> >>
> >> I guess you did not fully got what I meant with "dvfs clock type". It
> >> will not affect the clock API. But instead the dvfs is handled by
> >> implementing a specific clk hw type. So the same thing is accomplished
> >> as with clk notifiers, no changes should be needed to device drivers.
> >>
> >> The difference is only that no notifiers will be needed, and all the
> >> dvfs stuff will be handled in the clk hw instead. It will mean that we
> >> will bundle dvfs stuff into the clock drivers, instead of separating
> >> the code outside the clock drivers. But, on the other hand no
> >> notifiers will be needed.
> >
> > The advantage here is that I assume that a notifier would continually
> > have to check whether the clock being modified was one that the DVFS
> > notifier cared about. By integrating the CVFS logic into the clk_hw
> > itself, it'll only ever get executed for clocks that really care about
> > DVFS. Presumably, the code that implements the clk_hw could also use
> > some common DVFS library as part of the implementation, and still share
> > code. Or perhaps, what about putting DVFS "ops" into a clk_hw alongside
> > any other existing ops, and having the clock core call them whenever
> > appropriate?
>
> Thanks for your comment Stephen.
>
> I agree to your reflections as well. It will probably be a more
> optimized solution going this direction and we don't have to add more
> "clk notifier code" to the clk API, which I guess is good.
> It would be interesting to get some input from some of the maintainers
> to this discussion as well, let's see.
>
I do not like the dvfs clock type at all. For the set of DVFS problems
that we are trying to solve, voltage scaling is more of a function of a
device's requirements than of a clock. Put another way, we don't scale
voltage because a clock runs at a rate, we scale voltage because a
device runs at a rate. That is why the clock rate change notifiers were
interesting for dvfs: the device scales the voltage in response to a
clock frequency change.
However it seems the rate-change notifiers are a bit messy for dvfs.
As such I'm currently hacking on a new rfc to introduce a separate dvfs
api. Based on recent discussions and some face-to-face feedback I think
that kicking off DVFS transitions from the clock framework (which looks
very enticing at first glance) is coming at the problem from the wrong
direction. A new api that builds on top of clocks, regulators and opps
is straight-forward, requires no weird cross-layer calls and is
generally cleaner. The downside is that driver authors will have to
choose between using clk_set_rate or magic_dvfs_set_rate apis. That
dilemma is entirely analogous to the runtime pm versus
clk_enable/clk_disable dilemma and hopefully won't be a big hurdle to
acceptance.
I'll post in the v2 thread as well, specifically addressing the
devfreq/clk_set_rate idea.
Regards,
Mike
> Kind regards
> Ulf Hansson
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: Ulf Hansson <ulf.hansson@linaro.org>,
Stephen Warren <swarren@wwwdotorg.org>
Cc: "linaro-dev@lists.linaro.org" <linaro-dev@lists.linaro.org>,
"patches@linaro.org" <patches@linaro.org>,
Peter De Schrijver <pdeschrijver@nvidia.com>,
Bill Huang <bilhuang@nvidia.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Russell King - ARM Linux <linux@arm.linux.org.uk>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
Date: Thu, 21 Mar 2013 15:28:52 -0700 [thread overview]
Message-ID: <20130321222852.834.43745@quantum> (raw)
In-Reply-To: <CAPDyKFq4cPqVAfjbG7wZFS2B4PEt5ev_AkwBNi3LjL4OgxJ=tA@mail.gmail.com>
Quoting Ulf Hansson (2013-03-18 03:36:29)
> On 15 March 2013 20:38, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 03/15/2013 06:33 AM, Ulf Hansson wrote:
> >> On 15 March 2013 13:06, Bill Huang <bilhuang@nvidia.com> wrote:
> >>> On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote:
> > ...
> >>>> Some prerequisites; I think am in favor of using the clk API to
> >>>> trigger DVFS changes and then I agree on that clk_prepare|unprepare
> >>>> needs to be possible to track from a DVFS perspective. clk_set_rate is
> >>>> not enough.
> >>>>
> >>>> So if we decide to do the above (using the clk API to trigger DVFS
> >>>> changes), I believe we should discuss two possible solutions;
> >>>> - clk notifiers or..
> >>>> - dvfs clock type.
> >>>>
> >>>> I am trying to make up my mind of what I think is the best solution.
> >>>> Have you considered "dvfs clock type"?
> >>>> I put some comments about this for "[PATCH 2/5] clk: notifier handler
> >>>> for dynamic voltage scaling" recently as well.
> >>>>
> >>>> What could the advantages/disadvantages be between the two options?
> >>>
> >>> I personally prefer clk notifiers since that's easy and all the existing
> >>> device drivers don't need to be modified, a new clock or API might be
> >>> more thoroughly considered (and hence maybe more graceful) but that
> >>> means we need more time to cook and many drivers need to plug into that
> >>> API when it comes out, a lot of test/verification or maybe chaos
> >>> follows, I'm not sure will that be a little overkill.
> >>
> >> I guess you did not fully got what I meant with "dvfs clock type". It
> >> will not affect the clock API. But instead the dvfs is handled by
> >> implementing a specific clk hw type. So the same thing is accomplished
> >> as with clk notifiers, no changes should be needed to device drivers.
> >>
> >> The difference is only that no notifiers will be needed, and all the
> >> dvfs stuff will be handled in the clk hw instead. It will mean that we
> >> will bundle dvfs stuff into the clock drivers, instead of separating
> >> the code outside the clock drivers. But, on the other hand no
> >> notifiers will be needed.
> >
> > The advantage here is that I assume that a notifier would continually
> > have to check whether the clock being modified was one that the DVFS
> > notifier cared about. By integrating the CVFS logic into the clk_hw
> > itself, it'll only ever get executed for clocks that really care about
> > DVFS. Presumably, the code that implements the clk_hw could also use
> > some common DVFS library as part of the implementation, and still share
> > code. Or perhaps, what about putting DVFS "ops" into a clk_hw alongside
> > any other existing ops, and having the clock core call them whenever
> > appropriate?
>
> Thanks for your comment Stephen.
>
> I agree to your reflections as well. It will probably be a more
> optimized solution going this direction and we don't have to add more
> "clk notifier code" to the clk API, which I guess is good.
> It would be interesting to get some input from some of the maintainers
> to this discussion as well, let's see.
>
I do not like the dvfs clock type at all. For the set of DVFS problems
that we are trying to solve, voltage scaling is more of a function of a
device's requirements than of a clock. Put another way, we don't scale
voltage because a clock runs at a rate, we scale voltage because a
device runs at a rate. That is why the clock rate change notifiers were
interesting for dvfs: the device scales the voltage in response to a
clock frequency change.
However it seems the rate-change notifiers are a bit messy for dvfs.
As such I'm currently hacking on a new rfc to introduce a separate dvfs
api. Based on recent discussions and some face-to-face feedback I think
that kicking off DVFS transitions from the clock framework (which looks
very enticing at first glance) is coming at the problem from the wrong
direction. A new api that builds on top of clocks, regulators and opps
is straight-forward, requires no weird cross-layer calls and is
generally cleaner. The downside is that driver authors will have to
choose between using clk_set_rate or magic_dvfs_set_rate apis. That
dilemma is entirely analogous to the runtime pm versus
clk_enable/clk_disable dilemma and hopefully won't be a big hurdle to
acceptance.
I'll post in the v2 thread as well, specifically addressing the
devfreq/clk_set_rate idea.
Regards,
Mike
> Kind regards
> Ulf Hansson
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
next prev parent reply other threads:[~2013-03-21 22:28 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-12 12:37 [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare Bill Huang
2013-03-12 12:37 ` Bill Huang
2013-03-12 13:40 ` Russell King - ARM Linux
2013-03-12 13:40 ` Russell King - ARM Linux
2013-03-13 1:47 ` Bill Huang
2013-03-13 1:47 ` Bill Huang
2013-03-13 4:42 ` Stephen Warren
2013-03-13 4:42 ` Stephen Warren
2013-03-13 5:08 ` Bill Huang
2013-03-13 5:08 ` Bill Huang
2013-03-13 5:24 ` Stephen Warren
2013-03-13 5:24 ` Stephen Warren
2013-03-13 5:40 ` Bill Huang
2013-03-13 5:40 ` Bill Huang
2013-03-13 18:10 ` Stephen Warren
2013-03-13 18:10 ` Stephen Warren
2013-03-14 2:15 ` Bill Huang
2013-03-14 2:15 ` Bill Huang
2013-03-14 9:21 ` Peter De Schrijver
2013-03-14 9:21 ` Peter De Schrijver
2013-03-14 9:28 ` Bill Huang
2013-03-14 9:28 ` Bill Huang
2013-03-14 17:54 ` Stephen Warren
2013-03-14 17:54 ` Stephen Warren
2013-03-15 1:20 ` Bill Huang
2013-03-15 1:20 ` Bill Huang
2013-03-15 5:22 ` Stephen Warren
2013-03-15 5:22 ` Stephen Warren
2013-03-15 5:48 ` Bill Huang
2013-03-15 5:48 ` Bill Huang
2013-03-15 9:39 ` Peter De Schrijver
2013-03-15 9:39 ` Peter De Schrijver
2013-03-15 10:08 ` Ulf Hansson
2013-03-15 10:08 ` Ulf Hansson
2013-03-15 12:06 ` Bill Huang
2013-03-15 12:06 ` Bill Huang
2013-03-15 12:33 ` Ulf Hansson
2013-03-15 12:33 ` Ulf Hansson
2013-03-15 19:38 ` Stephen Warren
2013-03-15 19:38 ` Stephen Warren
2013-03-16 1:54 ` Bill Huang
2013-03-16 1:54 ` Bill Huang
2013-03-18 10:36 ` Ulf Hansson
2013-03-18 10:36 ` Ulf Hansson
2013-03-21 22:28 ` Mike Turquette [this message]
2013-03-21 22:28 ` Mike Turquette
2013-03-16 2:23 ` Bill Huang
2013-03-16 2:23 ` Bill Huang
2013-03-15 17:12 ` Russell King - ARM Linux
2013-03-15 17:12 ` Russell King - ARM Linux
2013-03-15 17:09 ` Russell King - ARM Linux
2013-03-15 17:09 ` Russell King - ARM Linux
2013-03-16 2:25 ` Bill Huang
2013-03-16 2:25 ` Bill Huang
2013-03-15 16:59 ` Russell King - ARM Linux
2013-03-15 16:59 ` Russell King - ARM Linux
2013-03-15 16:57 ` Russell King - ARM Linux
2013-03-15 16:57 ` Russell King - ARM Linux
2013-03-15 18:44 ` Nicolas Pitre
2013-03-15 18:44 ` Nicolas Pitre
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=20130321222852.834.43745@quantum \
--to=mturquette@linaro.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.