2019-07-03 01:11:17

by Saravana Kannan

[permalink] [raw]
Subject: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

Interconnects often quantify their performance points in terms of
bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
allow specifying Bandwidth OPP tables in DT.

opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
tables.

opp-avg-KBps is an optional property that can be used in Bandwidth OPP
tables.

Signed-off-by: Saravana Kannan <[email protected]>
---
Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 76b6c79604a5..c869e87caa2a 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -83,9 +83,14 @@ properties.

Required properties:
- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
- required property for all device nodes but devices like power domains. The
- power domain nodes must have another (implementation dependent) property which
- uniquely identifies the OPP nodes.
+ required property for all device nodes but for devices like power domains or
+ bandwidth opp tables. The power domain nodes must have another (implementation
+ dependent) property which uniquely identifies the OPP nodes. The interconnect
+ opps are required to have the opp-peak-bw property.
+
+- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
+ big-endian integer. This is a required property for all devices that don't
+ have opp-hz. For example, bandwidth OPP tables for interconnect paths.

Optional properties:
- opp-microvolt: voltage in micro Volts.
@@ -132,6 +137,10 @@ Optional properties:
- opp-level: A value representing the performance level of the device,
expressed as a 32-bit integer.

+- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
+ 32-bit big-endian integer. This property is only meaningful in OPP tables
+ where opp-peak-KBps is present.
+
- clock-latency-ns: Specifies the maximum possible transition latency (in
nanoseconds) for switching to this OPP from any other OPP.

--
2.22.0.410.gd8fdbe21b5-goog


2019-07-16 17:26:21

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

Hey Saravana,

https://patchwork.kernel.org/patch/10850815/
There was already a discussion ^^ on how bandwidth bindings were to be
named.

On 7/3/19 6:40 AM, Saravana Kannan wrote:
> Interconnects often quantify their performance points in terms of
> bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> allow specifying Bandwidth OPP tables in DT.
>
> opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> tables.
>
> opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> tables.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 76b6c79604a5..c869e87caa2a 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -83,9 +83,14 @@ properties.
>
> Required properties:
> - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> - required property for all device nodes but devices like power domains. The
> - power domain nodes must have another (implementation dependent) property which
> - uniquely identifies the OPP nodes.
> + required property for all device nodes but for devices like power domains or
> + bandwidth opp tables. The power domain nodes must have another (implementation
> + dependent) property which uniquely identifies the OPP nodes. The interconnect
> + opps are required to have the opp-peak-bw property.
> +
> +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
> + big-endian integer. This is a required property for all devices that don't
> + have opp-hz. For example, bandwidth OPP tables for interconnect paths.
>
> Optional properties:
> - opp-microvolt: voltage in micro Volts.
> @@ -132,6 +137,10 @@ Optional properties:
> - opp-level: A value representing the performance level of the device,
> expressed as a 32-bit integer.
>
> +- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
> + 32-bit big-endian integer. This property is only meaningful in OPP tables
> + where opp-peak-KBps is present.
> +
> - clock-latency-ns: Specifies the maximum possible transition latency (in
> nanoseconds) for switching to this OPP from any other OPP.
>
>

--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-07-16 19:00:19

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <[email protected]> wrote:
>
> Hey Saravana,
>
> https://patchwork.kernel.org/patch/10850815/
> There was already a discussion ^^ on how bandwidth bindings were to be
> named.

Yes, I'm aware of that series. That series is trying to define a BW
mapping for an existing frequency OPP table. This patch is NOT about
adding a mapping to an existing table. This patch is about adding the
notion of BW OPP tables where BW is the "key" instead of "frequency".

So let's not mixed up these two series.

-Saravana

> On 7/3/19 6:40 AM, Saravana Kannan wrote:
> > Interconnects often quantify their performance points in terms of
> > bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> > allow specifying Bandwidth OPP tables in DT.
> >
> > opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> > tables.
> >
> > opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> > tables.
> >
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 76b6c79604a5..c869e87caa2a 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -83,9 +83,14 @@ properties.
> >
> > Required properties:
> > - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> > - required property for all device nodes but devices like power domains. The
> > - power domain nodes must have another (implementation dependent) property which
> > - uniquely identifies the OPP nodes.
> > + required property for all device nodes but for devices like power domains or
> > + bandwidth opp tables. The power domain nodes must have another (implementation
> > + dependent) property which uniquely identifies the OPP nodes. The interconnect
> > + opps are required to have the opp-peak-bw property.
> > +
> > +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
> > + big-endian integer. This is a required property for all devices that don't
> > + have opp-hz. For example, bandwidth OPP tables for interconnect paths.
> >
> > Optional properties:
> > - opp-microvolt: voltage in micro Volts.
> > @@ -132,6 +137,10 @@ Optional properties:
> > - opp-level: A value representing the performance level of the device,
> > expressed as a 32-bit integer.
> >
> > +- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
> > + 32-bit big-endian integer. This property is only meaningful in OPP tables
> > + where opp-peak-KBps is present.
> > +
> > - clock-latency-ns: Specifies the maximum possible transition latency (in
> > nanoseconds) for switching to this OPP from any other OPP.
> >
> >
>
> --
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

2019-07-17 07:55:30

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On 02-07-19, 18:10, Saravana Kannan wrote:
> Interconnects often quantify their performance points in terms of
> bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> allow specifying Bandwidth OPP tables in DT.
>
> opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> tables.
>
> opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> tables.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 76b6c79604a5..c869e87caa2a 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -83,9 +83,14 @@ properties.
>
> Required properties:
> - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> - required property for all device nodes but devices like power domains. The
> - power domain nodes must have another (implementation dependent) property which
> - uniquely identifies the OPP nodes.
> + required property for all device nodes but for devices like power domains or
> + bandwidth opp tables. The power domain nodes must have another (implementation
> + dependent) property which uniquely identifies the OPP nodes. The interconnect
> + opps are required to have the opp-peak-bw property.

??

> +
> +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
> + big-endian integer. This is a required property for all devices that don't
> + have opp-hz. For example, bandwidth OPP tables for interconnect paths.
>
> Optional properties:
> - opp-microvolt: voltage in micro Volts.
> @@ -132,6 +137,10 @@ Optional properties:
> - opp-level: A value representing the performance level of the device,
> expressed as a 32-bit integer.
>
> +- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
> + 32-bit big-endian integer. This property is only meaningful in OPP tables
> + where opp-peak-KBps is present.
> +
> - clock-latency-ns: Specifies the maximum possible transition latency (in
> nanoseconds) for switching to this OPP from any other OPP.
>
> --
> 2.22.0.410.gd8fdbe21b5-goog

--
viresh

2019-07-17 20:31:32

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Wed, Jul 17, 2019 at 12:54 AM Viresh Kumar <[email protected]> wrote:
>
> On 02-07-19, 18:10, Saravana Kannan wrote:
> > Interconnects often quantify their performance points in terms of
> > bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> > allow specifying Bandwidth OPP tables in DT.
> >
> > opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> > tables.
> >
> > opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> > tables.
> >
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 76b6c79604a5..c869e87caa2a 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -83,9 +83,14 @@ properties.
> >
> > Required properties:
> > - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> > - required property for all device nodes but devices like power domains. The
> > - power domain nodes must have another (implementation dependent) property which
> > - uniquely identifies the OPP nodes.
> > + required property for all device nodes but for devices like power domains or
> > + bandwidth opp tables. The power domain nodes must have another (implementation
> > + dependent) property which uniquely identifies the OPP nodes. The interconnect
> > + opps are required to have the opp-peak-bw property.
>
> ??

Sorry, what's the question? Was this an accidental email?

-Saravana

>
> > +
> > +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
> > + big-endian integer. This is a required property for all devices that don't
> > + have opp-hz. For example, bandwidth OPP tables for interconnect paths.
> >
> > Optional properties:
> > - opp-microvolt: voltage in micro Volts.
> > @@ -132,6 +137,10 @@ Optional properties:
> > - opp-level: A value representing the performance level of the device,
> > expressed as a 32-bit integer.
> >
> > +- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
> > + 32-bit big-endian integer. This property is only meaningful in OPP tables
> > + where opp-peak-KBps is present.
> > +
> > - clock-latency-ns: Specifies the maximum possible transition latency (in
> > nanoseconds) for switching to this OPP from any other OPP.
> >
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
>
> --
> viresh
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>

2019-07-18 04:36:45

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On 17-07-19, 13:29, Saravana Kannan wrote:
> On Wed, Jul 17, 2019 at 12:54 AM Viresh Kumar <[email protected]> wrote:
> >
> > On 02-07-19, 18:10, Saravana Kannan wrote:
> > > Interconnects often quantify their performance points in terms of
> > > bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> > > allow specifying Bandwidth OPP tables in DT.
> > >
> > > opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> > > tables.
> > >
> > > opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> > > tables.
> > >
> > > Signed-off-by: Saravana Kannan <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> > > 1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > > index 76b6c79604a5..c869e87caa2a 100644
> > > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > > @@ -83,9 +83,14 @@ properties.
> > >
> > > Required properties:
> > > - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> > > - required property for all device nodes but devices like power domains. The
> > > - power domain nodes must have another (implementation dependent) property which
> > > - uniquely identifies the OPP nodes.
> > > + required property for all device nodes but for devices like power domains or
> > > + bandwidth opp tables. The power domain nodes must have another (implementation
> > > + dependent) property which uniquely identifies the OPP nodes. The interconnect
> > > + opps are required to have the opp-peak-bw property.
> >
> > ??
>
> Sorry, what's the question? Was this an accidental email?

Too much smartness is too bad sometimes, sorry about that :)

I placed the ?? right below "opp-peak-bw", there is no property like
that. You failed to update it :)

--
viresh

2019-07-18 17:27:31

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Wed, Jul 17, 2019 at 9:36 PM Viresh Kumar <[email protected]> wrote:
>
> On 17-07-19, 13:29, Saravana Kannan wrote:
> > On Wed, Jul 17, 2019 at 12:54 AM Viresh Kumar <[email protected]> wrote:
> > >
> > > On 02-07-19, 18:10, Saravana Kannan wrote:
> > > > Interconnects often quantify their performance points in terms of
> > > > bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> > > > allow specifying Bandwidth OPP tables in DT.
> > > >
> > > > opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> > > > tables.
> > > >
> > > > opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> > > > tables.
> > > >
> > > > Signed-off-by: Saravana Kannan <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> > > > 1 file changed, 12 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > > > index 76b6c79604a5..c869e87caa2a 100644
> > > > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > > > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > > > @@ -83,9 +83,14 @@ properties.
> > > >
> > > > Required properties:
> > > > - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> > > > - required property for all device nodes but devices like power domains. The
> > > > - power domain nodes must have another (implementation dependent) property which
> > > > - uniquely identifies the OPP nodes.
> > > > + required property for all device nodes but for devices like power domains or
> > > > + bandwidth opp tables. The power domain nodes must have another (implementation
> > > > + dependent) property which uniquely identifies the OPP nodes. The interconnect
> > > > + opps are required to have the opp-peak-bw property.
> > >
> > > ??
> >
> > Sorry, what's the question? Was this an accidental email?
>
> Too much smartness is too bad sometimes, sorry about that :)
>
> I placed the ?? right below "opp-peak-bw", there is no property like
> that. You failed to update it :)

Ah, "typo". I'll fix it.

-Saravana

2019-07-23 10:48:26

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Tue, Jul 16, 2019 at 11:58:08AM -0700, Saravana Kannan wrote:
> On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <[email protected]> wrote:
> >
> > Hey Saravana,
> >
> > https://patchwork.kernel.org/patch/10850815/
> > There was already a discussion ^^ on how bandwidth bindings were to be
> > named.
>
> Yes, I'm aware of that series. That series is trying to define a BW
> mapping for an existing frequency OPP table. This patch is NOT about
> adding a mapping to an existing table. This patch is about adding the
> notion of BW OPP tables where BW is the "key" instead of "frequency".
>
> So let's not mixed up these two series.

Maybe different reasons, but in the end we'd end up with 2 bandwidth
properties. We need to sort out how they'd overlap/coexist.

The same comment in that series about defining a standard unit suffix
also applies to this one.

Rob

2019-07-23 10:49:06

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Mon, Jul 22, 2019 at 4:35 PM Rob Herring <[email protected]> wrote:
>
> On Tue, Jul 16, 2019 at 11:58:08AM -0700, Saravana Kannan wrote:
> > On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <[email protected]> wrote:
> > >
> > > Hey Saravana,
> > >
> > > https://patchwork.kernel.org/patch/10850815/
> > > There was already a discussion ^^ on how bandwidth bindings were to be
> > > named.
> >
> > Yes, I'm aware of that series. That series is trying to define a BW
> > mapping for an existing frequency OPP table. This patch is NOT about
> > adding a mapping to an existing table. This patch is about adding the
> > notion of BW OPP tables where BW is the "key" instead of "frequency".
> >
> > So let's not mixed up these two series.
>
> Maybe different reasons, but in the end we'd end up with 2 bandwidth
> properties. We need to sort out how they'd overlap/coexist.

Oh, I totally agree! My point is that the other mapping isn't the
right approach because it doesn't handle a whole swath of use cases.
The one I'm proposing can act as a super set of the other (as in, can
handle that use case too).

> The same comment in that series about defining a standard unit suffix
> also applies to this one.

I thought I read that whole series and I don't remember reading about
the unit suffix. But I'll take a closer look. I've chosen to keep the
DT units at least as "high of a resolution" as what the APIs accept
today. The APIs take KB/s. So I make sure DT can capture KB/s
differences. If we all agree that KB/s is "too accurate" then I think
we should change everything to MB/s.

-Saravana

2019-07-24 00:13:37

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Mon, Jul 22, 2019 at 5:41 PM Saravana Kannan <[email protected]> wrote:
>
> On Mon, Jul 22, 2019 at 4:35 PM Rob Herring <[email protected]> wrote:
> >
> > On Tue, Jul 16, 2019 at 11:58:08AM -0700, Saravana Kannan wrote:
> > > On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <[email protected]> wrote:
> > > >
> > > > Hey Saravana,
> > > >
> > > > https://patchwork.kernel.org/patch/10850815/
> > > > There was already a discussion ^^ on how bandwidth bindings were to be
> > > > named.
> > >
> > > Yes, I'm aware of that series. That series is trying to define a BW
> > > mapping for an existing frequency OPP table. This patch is NOT about
> > > adding a mapping to an existing table. This patch is about adding the
> > > notion of BW OPP tables where BW is the "key" instead of "frequency".
> > >
> > > So let's not mixed up these two series.
> >
> > Maybe different reasons, but in the end we'd end up with 2 bandwidth
> > properties. We need to sort out how they'd overlap/coexist.
>
> Oh, I totally agree! My point is that the other mapping isn't the
> right approach because it doesn't handle a whole swath of use cases.
> The one I'm proposing can act as a super set of the other (as in, can
> handle that use case too).
>
> > The same comment in that series about defining a standard unit suffix
> > also applies to this one.
>
> I thought I read that whole series and I don't remember reading about
> the unit suffix. But I'll take a closer look. I've chosen to keep the
> DT units at least as "high of a resolution" as what the APIs accept
> today. The APIs take KB/s. So I make sure DT can capture KB/s
> differences. If we all agree that KB/s is "too accurate" then I think
> we should change everything to MB/s.

Either one is fine with me, but trying to align to what the OS picked
doesn't work. What does BSD use for example? More important is
aligning across DT properties so we don't have folks picking whatever
random unit they like. We generally try to go with the smallest units
that will have enough (32-bit) range for everyone, so that's probably
KB/s here.

Rob

2019-07-24 02:36:20

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Tue, Jul 23, 2019 at 7:27 AM Rob Herring <[email protected]> wrote:
>
> On Mon, Jul 22, 2019 at 5:41 PM Saravana Kannan <[email protected]> wrote:
> >
> > On Mon, Jul 22, 2019 at 4:35 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Tue, Jul 16, 2019 at 11:58:08AM -0700, Saravana Kannan wrote:
> > > > On Tue, Jul 16, 2019 at 10:25 AM Sibi Sankar <[email protected]> wrote:
> > > > >
> > > > > Hey Saravana,
> > > > >
> > > > > https://patchwork.kernel.org/patch/10850815/
> > > > > There was already a discussion ^^ on how bandwidth bindings were to be
> > > > > named.
> > > >
> > > > Yes, I'm aware of that series. That series is trying to define a BW
> > > > mapping for an existing frequency OPP table. This patch is NOT about
> > > > adding a mapping to an existing table. This patch is about adding the
> > > > notion of BW OPP tables where BW is the "key" instead of "frequency".
> > > >
> > > > So let's not mixed up these two series.
> > >
> > > Maybe different reasons, but in the end we'd end up with 2 bandwidth
> > > properties. We need to sort out how they'd overlap/coexist.
> >
> > Oh, I totally agree! My point is that the other mapping isn't the
> > right approach because it doesn't handle a whole swath of use cases.
> > The one I'm proposing can act as a super set of the other (as in, can
> > handle that use case too).
> >
> > > The same comment in that series about defining a standard unit suffix
> > > also applies to this one.
> >
> > I thought I read that whole series and I don't remember reading about
> > the unit suffix. But I'll take a closer look. I've chosen to keep the
> > DT units at least as "high of a resolution" as what the APIs accept
> > today. The APIs take KB/s. So I make sure DT can capture KB/s
> > differences. If we all agree that KB/s is "too accurate" then I think
> > we should change everything to MB/s.
>
> Either one is fine with me, but trying to align to what the OS picked
> doesn't work. What does BSD use for example? More important is
> aligning across DT properties so we don't have folks picking whatever
> random unit they like. We generally try to go with the smallest units
> that will have enough (32-bit) range for everyone, so that's probably
> KB/s here.

Yeah, that makes sense.

-Saravana

2019-07-26 19:27:11

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

Hi Saravana,

On 7/3/19 04:10, Saravana Kannan wrote:
> Interconnects often quantify their performance points in terms of
> bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> allow specifying Bandwidth OPP tables in DT.
>
> opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> tables.
>
> opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> tables.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 76b6c79604a5..c869e87caa2a 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -83,9 +83,14 @@ properties.
>
> Required properties:
> - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> - required property for all device nodes but devices like power domains. The
> - power domain nodes must have another (implementation dependent) property which
> - uniquely identifies the OPP nodes.
> + required property for all device nodes but for devices like power domains or
> + bandwidth opp tables. The power domain nodes must have another (implementation
> + dependent) property which uniquely identifies the OPP nodes. The interconnect
> + opps are required to have the opp-peak-bw property.
> +
> +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit

As Rob already mentioned, KBps should be documented. See [1].

Thanks,
Georgi

[1] https://lore.kernel.org/lkml/[email protected]/

2019-07-26 22:46:50

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings

On Fri, Jul 26, 2019 at 9:24 AM Georgi Djakov <[email protected]> wrote:
>
> Hi Saravana,
>
> On 7/3/19 04:10, Saravana Kannan wrote:
> > Interconnects often quantify their performance points in terms of
> > bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> > allow specifying Bandwidth OPP tables in DT.
> >
> > opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> > tables.
> >
> > opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> > tables.
> >
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 76b6c79604a5..c869e87caa2a 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -83,9 +83,14 @@ properties.
> >
> > Required properties:
> > - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> > - required property for all device nodes but devices like power domains. The
> > - power domain nodes must have another (implementation dependent) property which
> > - uniquely identifies the OPP nodes.
> > + required property for all device nodes but for devices like power domains or
> > + bandwidth opp tables. The power domain nodes must have another (implementation
> > + dependent) property which uniquely identifies the OPP nodes. The interconnect
> > + opps are required to have the opp-peak-bw property.
> > +
> > +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
>
> As Rob already mentioned, KBps should be documented. See [1].
>

Will do. Thanks for the pointer.

-Saravana