Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1195820 > unrolled thread

Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs Lee Jones <lee.jones@linaro.org> - 2015-07-30 10:50 +0200
    Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs Stephen Boyd <sboyd@codeaurora.org> - 2015-07-31 18:40 +0200
      Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs Viresh Kumar <viresh.kumar@linaro.org> - 2015-08-01 13:40 +0200
      Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs Viresh Kumar <viresh.kumar@linaro.org> - 2015-08-03 05:50 +0200

#1195820 — Re: [PATCH v4 2/2] dt: power: st: Provide bindings for ST's OPPs

On Wed, 29 Jul 2015, Stephen Boyd wrote:

> On 07/29, Lee Jones wrote:
> > On Tue, 28 Jul 2015, Stephen Boyd wrote:
> > 
> > > On 07/28, Viresh Kumar wrote:
> > > > Cc'ing few people (whom I cc'd last time as well :)).
> > > > 
> > > > On 27-07-15, 16:20, Lee Jones wrote:
> > > > > + - opp-hz		: CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > > > > + - st,avs		: List of available voltages [uV] indexed by process code
> > > > > + - st,cuts		: Cut version this OPP is suitable for [0xFF means ALL]
> > > > > + - st,substrate		: Substrate version this OPP is suitable for [0xFF means ALL]
> [...]
> > > > > +cpu0-opp-list {
> > > > > +	compatible	= "operating-points-v2-sti";
> > > > > +	st,syscfg	= <&syscfg [major_offset]>;
> > > > > +	st,syscfg-eng   = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > > > > +
> > > > > +	opp0 {
> > > > > +		opp-hz		= <1200000000>;
> > > > > +		st,avs		= <1110 1150 1100 1080 1040 1020 980 930>;
> > > > > +		st,substrate	= <0xff>;
> > > > > +		st,cuts		= <0xff>;
> > > > > +	};
> > > > > +	opp1 {
> > > > > +		opp-hz		= <1500000000>;
> > > > > +		st,avs		= <1200 1200 1200 1200 1170 1140 1100 1070>;
> > > > > +		st,substrate	= <0xff>;
> > > > > +		st,cuts		= <0x2>;
> > > > > +	};
> > > > > +};
> > > > 
> > > > I don't see more problems here, unless we can move some of this to the
> > > > generic bindings.
> > > > 
> > > > @Rob/Stephen: Please respond before it is late :)
> > > 
> > > It's interesting to have vendor specific properties like avs,
> > > cuts, and substrate. That could replace our planned usage of the
> > > opp-names property where we encode similar information (speed
> > > bin, revision, etc.) into a string that we look for.
> > > 
> > > So I wonder why the avs/cut/substrate information can't be
> > > encoded into the opp name? That would make these properties
> > > obsolete, given that all they're used for is to pick out the
> > > correct OPP?
> > 
> > You could hack the substrate and cut version into a string, but that's
> > exactly what it would be, a hack.  I'm struggling how you would do the
> > same for 'st,avs', which is an array of u32s.
> > 
> 
> 
> (I don't understand the st,avs property to begin with, so feel
> free to ignore the rest of this mail.)
> 
> For qcom platforms we have the pvs bin and speed bin, and
> sometimes a revision number. Each one of these properties
> corresponds to a different set of OPPs (opp table). So we might
> have speed1-pvs2-v0 for speed bin 1, pvs bin 2 and version 0. We
> fill out an opp table for this and then point the opps property
> at the table and have a corresponding opp-name "speed1-pvs2-v0"
> in the "consumer" node.
> 
> 	operating-points-v2 = <&speed1_pvs2_v0>;
> 	operating-points-names = "speed1-pvs2-v0";
> 
> We have quite a few of these tables because the values are
> always different. If the values were the same then we could use
> the same table with different names I suppose, but we're not
> doing that.
> 
> From a quick read of the st properties (that I admit I don't
> understand), it looks like we're trying to compress the OPP
> tables by listing all the voltages that could be used for a
> particular frequency depending on which avs is present on the
> device? And then limiting the frequency voltage pairs depending
> on which cut and substrate is present?
> 
> So we'd probably have to expand out the tables to be unique per
> avs/cut/substrate parameter. Something like:
> 
> avs0-cut2 {
> 	compatible = "operating-points-v2";
> 
> 	opp0 {
> 		opp-hz = <1200000000>;
> 		opp-microvolt = <1100>;
> 	};
> 
> 	opp1 {
> 		opp-hz = <1500000000>;
> 		opp-microvolt = <1200>;
> 	};
> };
> 
> avs1-cut2 {
> 	compatible = "operating-points-v2";
> 
> 	opp0 {
> 		opp-hz = <1200000000>;
> 		opp-microvolt = <1150>;
> 	};
> 
> 	opp1 {
> 		opp-hz = <1500000000>;
> 		opp-microvolt = <1200>;
> 	};
> };
> 
> And then another copy of these for the devices without cuts != 2
> where the top frequency is gone?

There is nothing stopping us from representing the data in this way.
On the plus side, it would mean that we wouldn't need any vendor
specific properties.  However, far outweighing the positives are the
fact that, even in our very simple example provided, where we only
have 2 frequencies, differ between only 1 cut and support all
substrates, we would still need 16 OPP tables.  When any one of those
variables increase (and they will), we would then have a large number
of permutations and subsequently and unruly amount of OPP tables.

(... and we haven't even provided the body-biasing information yet.)

The way we've chosen to represent our circumstance is the least
intrusive and the most succinct way we can think of.  Which IMHO
outweighs the fact that we have to introduce a couple of vendor
properties by a country mile.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1197178

On 07/30, Rob Herring wrote:
> On Thu, Jul 30, 2015 at 3:46 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >
> > There is nothing stopping us from representing the data in this way.
> > On the plus side, it would mean that we wouldn't need any vendor
> > specific properties.  However, far outweighing the positives are the
> > fact that, even in our very simple example provided, where we only
> > have 2 frequencies, differ between only 1 cut and support all
> > substrates, we would still need 16 OPP tables.  When any one of those
> > variables increase (and they will), we would then have a large number
> > of permutations and subsequently and unruly amount of OPP tables.
> >
> > (... and we haven't even provided the body-biasing information yet.)
> >
> > The way we've chosen to represent our circumstance is the least
> > intrusive and the most succinct way we can think of.  Which IMHO
> > outweighs the fact that we have to introduce a couple of vendor
> > properties by a country mile.
> 
> Regardless of which is better or worse, you are both doing essentially
> the same thing. You are selecting OPPs based on some Si parameters. We
> should not really be doing this 2 different ways. I'd be fine with 2
> ways if it was 2 for all SOCs, but right now it is 2 for 2 SOCs.
> Really, I'd like to see most if not all the selection or fixup of OPPs
> be done in the bootloader and the kernel just deals with the correct
> OPP table.
> 
> Where are you storing the data that gets selected to fill into these
> properties? Is it just look-up tables or is there some kind of
> algorithm to generate the values? If the former, then DT is as good a
> data struct as any to store the tables. There is a lot of duplication
> though with only a single property varying in each set. If you both
> have that problem, then perhaps we can come up with a generic way to
> list all possible values more concisely.

For qcom platforms, the frequency is almost always constant.
There may be some tables where we have a couple higher
frequencies than others because the speed bin is different.
Otherwise the voltage/current is changing based on the silicon
characteristics. So the biggest duplication is the frequency
property.

As far as I know there isn't any algorithm to generate the
voltage values. It's all hand tuned tables based on the silicon
characterization, so we're left to store these tables in DT and
pick the right one at runtime. With regards to the table
explosion, on qcom platforms we haven't worried that we have ~40
tables, but I'm not opposed to expressing it in a smaller set of
nodes, tables, etc. if that's what's desired.

Do we need vendor specific properties for that though? Or do we
need some sort of extended frequency/voltage properties that are
arrays of values that we can index into based on some silicon
characteristics? I like the name based approach because it's
simple. Use this OPP table because it's called
x-y-z-characteristics and be done. Cramming the tables into less
lines may save us some typing and dtb space, but I'm not sure
what else it does.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1197979

On 31-07-15, 09:37, Stephen Boyd wrote:
> Do we need vendor specific properties for that though?

Sorry Lee :), but this is exactly why I wanted this thread to exist. We must and
should do this in a generic enough way.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1198524

On 31-07-15, 09:37, Stephen Boyd wrote:
> For qcom platforms, the frequency is almost always constant.
> There may be some tables where we have a couple higher
> frequencies than others because the speed bin is different.
> Otherwise the voltage/current is changing based on the silicon
> characteristics. So the biggest duplication is the frequency
> property.
> 
> As far as I know there isn't any algorithm to generate the
> voltage values. It's all hand tuned tables based on the silicon
> characterization, so we're left to store these tables in DT and
> pick the right one at runtime. With regards to the table
> explosion, on qcom platforms we haven't worried that we have ~40
> tables, but I'm not opposed to expressing it in a smaller set of
> nodes, tables, etc. if that's what's desired.
> 
> Do we need vendor specific properties for that though? Or do we
> need some sort of extended frequency/voltage properties that are
> arrays of values that we can index into based on some silicon
> characteristics? I like the name based approach because it's
> simple. Use this OPP table because it's called
> x-y-z-characteristics and be done. Cramming the tables into less
> lines may save us some typing and dtb space, but I'm not sure
> what else it does.

What about something like this:

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 0cb44dc21f97..bad7a8299b9c 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -74,6 +74,8 @@ This describes the OPPs belonging to a device. This node can have following
   reference an OPP.
 
 Optional properties:
+- opp-cuts: One or more strings, describing the versions of hardware the OPPs
+  can support.
 - opp-shared: Indicates that device nodes using this OPP Table Node's phandle
   switch their DVFS state together, i.e. they share clock/voltage/current lines.
   Missing property means devices have independent clock/voltage/current lines,
@@ -100,6 +102,9 @@ properties.
   Entries for multiple regulators must be present in the same order as
   regulators are specified in device's DT node.
 
+  If used with 'opp-cuts', then the number of entries present here must match
+  the number of strings present in 'opp-cuts'.
+
 - opp-microamp: The maximum current drawn by the device in microamperes
   considering system specific parameters (such as transients, process, aging,
   maximum operating temperature range etc.) as necessary. This may be used to

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web