Re: [PATCH 1/4] dt-bindings: opp: Introduce opp-bw-MBs bindings

Georgi Djakov
2019-04-09T14:36:24Z

Hi Viresh,

On 3/14/19 08:23, Viresh Kumar wrote:
> On 13-03-19, 11:00, 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 | 45 +++++++++++++++++++
>> 1 file changed, 45 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>> index 76b6c79604a5..fa598264615f 100644
>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>> @@ -129,6 +129,9 @@ Optional properties:
>> - opp-microamp-<name>: Named opp-microamp property. Similar to
>> opp-microvolt-<name> property, but for microamp instead.
>>
>> +- opp-bw-MBs: The interconnect bandwidth is specified with an array containing
>> + the two integer values for average and peak bandwidth in megabytes per second.
>> +
>> - opp-level: A value representing the performance level of the device,
>> expressed as a 32-bit integer.
>>
>> @@ -546,3 +549,45 @@ Example 6: opp-microvolt-<name>, opp-microamp-<name>:
>> };
>> };
>> };
>> +
>> +Example 7: opp-bw-MBs:
>> +(example: average and peak bandwidth values are defined for each OPP and the
>> +interconnect between CPU and DDR memory is scaled together with CPU frequency)
>> +
>> +/ {
>> + cpus {
>> + CPU0: cpu@0 {
>> + compatible = "arm,cortex-a53", "arm,armv8";
>> + ...
>> + operating-points-v2 = <&cpu_opp_table>;
>> + /* path between the CPU and DDR memory */
>> + interconnects = <&rpm_bimc MASTER_AMPSS_M0
>> + &rpm_bimc SLAVE_EBI_CH0>;
>
> Can we have multiple paths for a device ?

I suppose that this is also a possible scenario. Will propose something
to handle multiple paths too.

>> + };
>> + };
>> +
>> + cpu_opp_table: cpu_opp_table {
>> + compatible = "operating-points-v2";
>> + opp-shared;
>> +
>> + opp-200000000 {
>> + opp-hz = /bits/ 64 <200000000>;
>> + /* 457 MB/s average and 1525 MB/s peak bandwidth */
>> + opp-bw-MBs = <457 1525>;
>
> In that case fixing this to just 2 entries in the array is incorrect
> and we should take care of that in the bindings here.

We can encode the path name into the property (when there are multiple
paths). We already have opp-microamp-<name> and opp-microamp-<name>, so
we can follow the same practice.

For example:

CPU0: cpu@0 {
compatible = "arm,cortex-a53", "arm,armv8";
...
operating-points-v2 = <&cpu_opp_table>;
/* path between the CPU and DDR and path between CPU and L3 */
interconnects = <&bimc MASTER_AMPSS_M0 &bimc SLAVE_EBI_CH0>,
<&bimc MASTER_AMPSS_M0 &bimc SLAVE_L3>;
interconnect-names "cpu-mem", "cpu-l3";
};

cpu_opp_table: cpu_opp_table {
compatible = "operating-points-v2";
opp-shared;

opp-200000000 {
opp-hz = /bits/ 64 <200000000>;
/* 457 MB/s average, 1525 MB/s peak bandwidth to DDR */
opp-bw-MBps-cpu-mem = <457 1525>;
/* 914 MB/s average, 3050 MB/s peak bandwidth to L3 */
opp-bw-MBps-cpu-l3 = <914 3050>;
};
};

Thanks,
Georgi