linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@baylibre.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Rob Herring <robh@kernel.org>, Rafael Wysocki <rjw@rjwysocki.net>,
	linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Lina Iyer <lina.iyer@linaro.org>,
	devicetree@vger.kernel.org, Stephen Boyd <sboyd@codeaurora.org>,
	Nayak Rajendra <rnayak@codeaurora.org>
Subject: Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
Date: Tue, 22 Nov 2016 10:12:28 -0800	[thread overview]
Message-ID: <m260nfz8mr.fsf@baylibre.com> (raw)
In-Reply-To: <20161122031717.GE10014@vireshk-i7> (Viresh Kumar's message of "Tue, 22 Nov 2016 08:47:17 +0530")

Viresh Kumar <viresh.kumar@linaro.org> writes:

> On 21-11-16, 09:07, Rob Herring wrote:
>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>> > Some platforms have the capability to configure the performance state of
>> > their Power Domains. The performance levels are represented by positive
>> > integer values, a lower value represents lower performance state.
>> > 
>> > The power-domains until now were only concentrating on the idle state
>> > management of the device and this needs to change in order to reuse the
>> > infrastructure of power domains for active state management.
>> > 
>> > This patch introduces a new optional property for the consumers of the
>> > power-domains: domain-performance-state.
>> > 
>> > If the consumers don't need the capability of switching to different
>> > domain performance states at runtime, then they can simply define their
>> > required domain performance state in their node directly. Otherwise the
>> > consumers can define their requirements with help of other
>> > infrastructure, for example the OPP table.
>> > 
>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> > ---
>> >  Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> > 
>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> > index e1650364b296..db42eacf8b5c 100644
>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>> >   - power-domains : A phandle and PM domain specifier as defined by bindings of
>> >                     the power controller specified by phandle.
>> >  
>> > +Optional properties:
>> > +- domain-performance-state: A positive integer value representing the minimum
>> > +  performance level (of the parent domain) required by the consumer for its
>> > +  working. The integer value '1' represents the lowest performance level and the
>> > +  highest value represents the highest performance level.
>> 
>> How does one come up with the range of values?
>
> Why would we need a range here? The value here represents the minimum 'state'
> and the assumption is that everything above that level would be fine. So the
> range is automatically: domain-performance-state -> MAX.
>
>> It seems like you are 
>> just making up numbers. Couldn't the domain performance level be an OPP 
>> in the sense that it is a collection of clock frequencies and voltage 
>> settings?
>
> The clock is going to be handled by the device itself (at least for the case we
> have today) and the performance-state lies with the power-domain which is
> configured separately. If the performance level includes both clk and voltage,
> then why would we need to show the clock rates in the DT ? Wouldn't a
> performance level be enough in such cases?

I think the question is: what does the performance-level of a domain
actually mean?  Or, what are the units?

Depending on the SoC, there's probably a few things this could mean.  It
might mean is that an underlying bus/interconnect can be configured to
guarantee a specific bandwidth or throughput.  That in turn might mean
that that bus/interconnect might have to be set at a specific
frequency/voltage.

In your case, IIUC, you're just passing some magic value to some
firmware running on a micro-controller, but under the hood that uC is
probably configuring a frequency/voltage someplace.

So, if we're going to have a generic DT binding for this, it needs to be
something that's useful on platforms that are not using magic numbers
managed by a uC as well.

Kevin

  reply	other threads:[~2016-11-22 18:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-18  9:23 [PATCH 0/2] PM / Domains / OPP: Introduce domain-performance-state binding Viresh Kumar
2016-11-18  9:23 ` [PATCH 1/2] PM / Domains: " Viresh Kumar
2016-11-21 15:07   ` Rob Herring
2016-11-22  3:17     ` Viresh Kumar
2016-11-22 18:12       ` Kevin Hilman [this message]
2016-11-22 18:34         ` Vincent Guittot
2016-11-23  3:22           ` Viresh Kumar
2016-11-23 15:51           ` Kevin Hilman
2016-11-23 15:55             ` Vincent Guittot
2016-11-23 22:30               ` Kevin Hilman
2016-11-24  2:03                 ` Stephen Boyd
2016-11-24  4:40                   ` Viresh Kumar
2016-11-28 18:27                     ` Stephen Boyd
2016-11-29  6:57                       ` Viresh Kumar
2016-11-30  1:08                         ` Stephen Boyd
2016-12-02 10:47                           ` Viresh Kumar
2016-11-18  9:23 ` [PATCH 2/2] PM / OPP: Introduce domain-performance-state binding to OPP nodes Viresh Kumar

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=m260nfz8mr.fsf@baylibre.com \
    --to=khilman@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lina.iyer@linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=rnayak@codeaurora.org \
    --cc=robh@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.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).