LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Sibi Sankar <sibis@codeaurora.org>
To: Viresh Kumar <viresh.kumar@linaro.org>,
	Rajendra Nayak <rnayak@codeaurora.org>
Cc: Georgi Djakov <georgi.djakov@linaro.org>,
	vireshk@kernel.org, sboyd@kernel.org, nm@ti.com,
	robh+dt@kernel.org, mark.rutland@arm.com, rjw@rjwysocki.net,
	jcrouse@codeaurora.org, vincent.guittot@linaro.org,
	bjorn.andersson@linaro.org, amit.kucheria@linaro.org,
	seansw@qti.qualcomm.com, daidavid1@codeaurora.org,
	evgreen@chromium.org, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v2 1/5] dt-bindings: opp: Introduce bandwidth-MBps bindings
Date: Wed, 24 Apr 2019 14:30:11 +0530
Message-ID: <dd6f055c-e7df-13e8-72aa-2973d3c664d0@codeaurora.org> (raw)
In-Reply-To: <20190424064942.v5g6jr5l3xy5z3xv@vireshk-i7>

Hey Viresh,

On 4/24/19 12:19 PM, Viresh Kumar wrote:
> On 24-04-19, 12:16, Rajendra Nayak wrote:
>>
>>
>> On 4/23/2019 6:58 PM, Georgi Djakov wrote:
>>> In addition to frequency and voltage, some devices may have bandwidth
>>> requirements for their interconnect throughput - for example a CPU
>>> or GPU may also need to increase or decrease their bandwidth to DDR
>>> memory based on the current operating performance point.
>>>
>>> Extend the OPP tables with additional property to describe the bandwidth
>>> needs of a device. The average and peak bandwidth values depend on the
>>> hardware and its properties.
>>>
>>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>>> ---
>>>    Documentation/devicetree/bindings/opp/opp.txt | 38 +++++++++++++++++++
>>>    .../devicetree/bindings/property-units.txt    |  4 ++
>>>    2 files changed, 42 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>> index 76b6c79604a5..830f0206aea7 100644
>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>> @@ -132,6 +132,9 @@ Optional properties:
>>>    - opp-level: A value representing the performance level of the device,
>>>      expressed as a 32-bit integer.
>>> +- bandwidth-MBps: The interconnect bandwidth is specified with an array containing
>>> +  the two integer values for average and peak bandwidth in megabytes per second.
>>> +
>>>    - clock-latency-ns: Specifies the maximum possible transition latency (in
>>>      nanoseconds) for switching to this OPP from any other OPP.
>>> @@ -546,3 +549,38 @@ Example 6: opp-microvolt-<name>, opp-microamp-<name>:
>>>    		};
>>>    	};
>>>    };
>>> +
>>> +Example 7: bandwidth-MBps:
>>> +Average and peak bandwidth values for the interconnects between CPU and DDR
>>> +memory and also between CPU and L3 are defined per each OPP. Bandwidth of both
>>> +interconnects is scaled together with CPU frequency.
>>> +
>>> +/ {
>>> +	cpus {
>>> +		CPU0: cpu@0 {
>>> +			compatible = "arm,cortex-a53", "arm,armv8";
>>> +			...
>>> +			operating-points-v2 = <&cpu_opp_table>;
>>> +			/* path between CPU and DDR memory and CPU and L3 */
>>> +			interconnects = <&noc MASTER_CPU &noc SLAVE_DDR>,
>>> +					<&noc MASTER_CPU &noc SLAVE_L3>;
>>> +		};
>>> +	};
>>> +
>>> +	cpu_opp_table: cpu_opp_table {
>>> +		compatible = "operating-points-v2";
>>> +		opp-shared;
>>> +
>>> +		opp-200000000 {
>>> +			opp-hz = /bits/ 64 <200000000>;
>>> +			/* CPU<->DDR bandwidth: 457 MB/s average, 1525 MB/s peak */
>>> +			 * CPU<->L3 bandwidth: 914 MB/s average, 3050 MB/s peak */
>>> +			bandwidth-MBps = <457 1525>, <914 3050>;
>>
>> Should this also have a bandwidth-MBps-name perhaps? Without that I guess we assume
>> the order in which we specify the interconnects is the same as the order here?
> 
> Right, so I suggested not to add the -name property and to rely on the
> order. Though I missed that he hasn't mentioned the order thing here.

by skipping names, aren't we forced to specify all the specified paths
bandwidths for each opp even if it is redundant? i.e if the first/second
icc path doesn't have to change across a few opps but if the other path
does need to change this scheme would force it to be included and will
try to set the first/second path again.


e.g: Here the first path does not have to change across these two opps
but have to specified nonetheless since we omit names.

+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			bandwidth-MBps = <457 1525>, <914 3050>;
+		};
+		opp-1400000000 {
+			opp-hz = /bits/ 64 <1400000000>;
+			bandwidth-MBps = <457 1525>, <1828 6102>;
+		};


> 
> @Georgi: Please mention above in the binding that the order is same as
> interconnects binding.
> 

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

  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-23 13:28 [PATCH v2 0/5] Introduce OPP bandwidth bindings Georgi Djakov
2019-04-23 13:28 ` [PATCH v2 1/5] dt-bindings: opp: Introduce bandwidth-MBps bindings Georgi Djakov
2019-04-24  5:33   ` Viresh Kumar
2019-04-24  6:46   ` Rajendra Nayak
2019-04-24  6:49     ` Viresh Kumar
2019-04-24  9:00       ` Sibi Sankar [this message]
2019-04-24  9:05         ` Viresh Kumar
2019-04-25  4:24         ` Bjorn Andersson
2019-04-24  8:44   ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 2/5] interconnect: Add of_icc_get_by_index() helper function Georgi Djakov
2019-05-07 11:59   ` Sibi Sankar
2019-06-27  5:56     ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 3/5] OPP: Add support for parsing the interconnect bandwidth Georgi Djakov
2019-04-24  5:52   ` Viresh Kumar
2019-06-27  6:27     ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 4/5] OPP: Update the bandwidth on OPP frequency changes Georgi Djakov
2019-04-24  5:55   ` Viresh Kumar
2019-04-24 10:05   ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 5/5] cpufreq: dt: Add support for interconnect bandwidth scaling Georgi Djakov
2019-06-01  2:12 ` [PATCH v2 0/5] Introduce OPP bandwidth bindings Saravana Kannan
2019-06-03 15:56   ` Jordan Crouse
2019-06-03 19:12     ` Saravana Kannan

Reply instructions:

You may reply publically 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=dd6f055c-e7df-13e8-72aa-2973d3c664d0@codeaurora.org \
    --to=sibis@codeaurora.org \
    --cc=amit.kucheria@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daidavid1@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=evgreen@chromium.org \
    --cc=georgi.djakov@linaro.org \
    --cc=jcrouse@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=rnayak@codeaurora.org \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=seansw@qti.qualcomm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=vireshk@kernel.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox