mbox series

[v4,0/4] soc/arm64: qcom: Add initial version of bwmon

Message ID 20220601101140.170504-1-krzysztof.kozlowski@linaro.org
Headers show
Series soc/arm64: qcom: Add initial version of bwmon | expand

Message

Krzysztof Kozlowski June 1, 2022, 10:11 a.m. UTC
Hi,

Changes since v3
================
1. Patch #2 (bwmon): remove unused irq_enable (kbuild robot);
   split bwmon_clear() into clearing counters and interrupts, so bwmon_start()
   does not clear the counters twice.

Changes since v2
================
1. Spent a lot of time on benchmarking and learning the BWMON behavior.
2. Drop PM/OPP patch - applied.
3. Patch #1: drop opp-avg-kBps.
4. Patch #2: Add several comments explaining pieces of code and BWMON, extend
   commit msg with measurements, extend help message, add new #defines to document
   some magic values, reorder bwmon clear/disable/enable operations to match
   downstream source and document this with comments, fix unit count from 1 MB
   to 65 kB.
5. Patch #4: drop opp-avg-kBps.
6. Add accumulated Rb tags.

Changes since v1
================
1. Add defconfig change.
2. Fix missing semicolon in MODULE_AUTHOR.
3. Add original downstream (msm-4.9 tree) copyrights to the driver.

Description
===========
BWMON is a data bandwidth monitor providing throughput/bandwidth over certain
interconnect links in a SoC.  It might be used to gather current bus usage and
vote for interconnect bandwidth, thus adjusting the bus speed based on actual
usage.

The work is built on top of Thara Gopinath's patches with several cleanups,
changes and simplifications.

Best regards,
Krzysztof

Krzysztof Kozlowski (4):
  dt-bindings: interconnect: qcom,sdm845-cpu-bwmon: add BWMON device
  soc: qcom: icc-bwmon: Add bandwidth monitoring driver
  arm64: defconfig: enable Qualcomm Bandwidth Monitor
  arm64: dts: qcom: sdm845: Add CPU BWMON

 .../interconnect/qcom,sdm845-cpu-bwmon.yaml   |  97 ++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  54 +++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/soc/qcom/Kconfig                      |  15 +
 drivers/soc/qcom/Makefile                     |   1 +
 drivers/soc/qcom/icc-bwmon.c                  | 421 ++++++++++++++++++
 7 files changed, 596 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
 create mode 100644 drivers/soc/qcom/icc-bwmon.c

Comments

Bjorn Andersson June 6, 2022, 9:11 p.m. UTC | #1
On Wed 01 Jun 03:11 PDT 2022, Krzysztof Kozlowski wrote:

> Add bindings for the Qualcomm Bandwidth Monitor device providing
> performance data on interconnects.  The bindings describe only BWMON
> version 4, e.g. the instance on SDM845 between CPU and Last Level Cache
> Controller.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Georgi Djakov <djakov@kernel.org>
> ---
>  .../interconnect/qcom,sdm845-cpu-bwmon.yaml   | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> new file mode 100644
> index 000000000000..8c82e06ee432
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,sdm845-cpu-bwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Interconnect Bandwidth Monitor
> +
> +maintainers:
> +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> +
> +description:
> +  Bandwidth Monitor measures current throughput on buses between various NoC
> +  fabrics and provides information when it crosses configured thresholds.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sdm845-cpu-bwmon       # BWMON v4

It seems the thing that's called bwmon v4 is compatible with a number of
different platforms, should we add a generic compatible to the binding
as well, to avoid having to update the implementation for each SoC?

(I.e. "qcom,sdm845-cpu-bwmon", "qcom,bwmon-v4")

Regards,
Bjorn

> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: ddr
> +      - const: l3c
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +  opp-table: true
> +
> +  reg:
> +    # Currently described BWMON v4 and v5 use one register address space.
> +    # BWMON v2 uses two register spaces - not yet described.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - interconnects
> +  - interconnect-names
> +  - interrupts
> +  - operating-points-v2
> +  - opp-table
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interconnect/qcom,osm-l3.h>
> +    #include <dt-bindings/interconnect/qcom,sdm845.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    pmu@1436400 {
> +        compatible = "qcom,sdm845-cpu-bwmon";
> +        reg = <0x01436400 0x600>;
> +
> +        interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        interconnects = <&gladiator_noc MASTER_APPSS_PROC 3 &mem_noc SLAVE_EBI1 3>,
> +                        <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
> +        interconnect-names = "ddr", "l3c";
> +
> +        operating-points-v2 = <&cpu_bwmon_opp_table>;
> +
> +        cpu_bwmon_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-0 {
> +                opp-peak-kBps = <800000 4800000>;
> +            };
> +            opp-1 {
> +                opp-peak-kBps = <1804000 9216000>;
> +            };
> +            opp-2 {
> +                opp-peak-kBps = <2188000 11980800>;
> +            };
> +            opp-3 {
> +                opp-peak-kBps = <3072000 15052800>;
> +            };
> +            opp-4 {
> +                opp-peak-kBps = <4068000 19353600>;
> +            };
> +            opp-5 {
> +                opp-peak-kBps = <5412000 20889600>;
> +            };
> +            opp-6 {
> +                opp-peak-kBps = <6220000 22425600>;
> +            };
> +            opp-7 {
> +                opp-peak-kBps = <7216000 25497600>;
> +            };
> +        };
> +    };
> -- 
> 2.34.1
>
Bjorn Andersson June 26, 2022, 3:19 a.m. UTC | #2
On Wed 22 Jun 06:58 CDT 2022, Rajendra Nayak wrote:

> 
> 
> On 6/7/2022 12:20 PM, Krzysztof Kozlowski wrote:
> > On 06/06/2022 23:11, Bjorn Andersson wrote:
> > > On Wed 01 Jun 03:11 PDT 2022, Krzysztof Kozlowski wrote:
> > > 
> > > > Add bindings for the Qualcomm Bandwidth Monitor device providing
> > > > performance data on interconnects.  The bindings describe only BWMON
> > > > version 4, e.g. the instance on SDM845 between CPU and Last Level Cache
> > > > Controller.
> > > > 
> > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Acked-by: Georgi Djakov <djakov@kernel.org>
> > > > ---
> > > >   .../interconnect/qcom,sdm845-cpu-bwmon.yaml   | 97 +++++++++++++++++++
> > > >   1 file changed, 97 insertions(+)
> > > >   create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > new file mode 100644
> > > > index 000000000000..8c82e06ee432
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> > > > @@ -0,0 +1,97 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/interconnect/qcom,sdm845-cpu-bwmon.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Qualcomm Interconnect Bandwidth Monitor
> > > > +
> > > > +maintainers:
> > > > +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > +
> > > > +description:
> > > > +  Bandwidth Monitor measures current throughput on buses between various NoC
> > > > +  fabrics and provides information when it crosses configured thresholds.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - qcom,sdm845-cpu-bwmon       # BWMON v4
> > > 
> > > It seems the thing that's called bwmon v4 is compatible with a number of
> > > different platforms, should we add a generic compatible to the binding
> > > as well, to avoid having to update the implementation for each SoC?
> > > 
> > > (I.e. "qcom,sdm845-cpu-bwmon", "qcom,bwmon-v4")
> 
> it seems pretty useful to have the "qcom,bwmon-v4" and "qcom,bwmon-v5"
> compatibles, I tried these patches on a sc7280 device which has a bwmon4
> between the cpu and caches (and also has a bwmon5 between the caches and DDR)
> and the driver works with zero changes.
> 

But does the '4' and '5' has a relation to the hardware? Or is just the
4th and 5th register layout supported by the downstream driver?

Regards,
Bjorn
Rajendra Nayak June 28, 2022, 10:43 a.m. UTC | #3
On 6/26/2022 8:49 AM, Bjorn Andersson wrote:
> On Wed 22 Jun 06:58 CDT 2022, Rajendra Nayak wrote:
> 
>>
>>
>> On 6/7/2022 12:20 PM, Krzysztof Kozlowski wrote:
>>> On 06/06/2022 23:11, Bjorn Andersson wrote:
>>>> On Wed 01 Jun 03:11 PDT 2022, Krzysztof Kozlowski wrote:
>>>>
>>>>> Add bindings for the Qualcomm Bandwidth Monitor device providing
>>>>> performance data on interconnects.  The bindings describe only BWMON
>>>>> version 4, e.g. the instance on SDM845 between CPU and Last Level Cache
>>>>> Controller.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>> Acked-by: Georgi Djakov <djakov@kernel.org>
>>>>> ---
>>>>>    .../interconnect/qcom,sdm845-cpu-bwmon.yaml   | 97 +++++++++++++++++++
>>>>>    1 file changed, 97 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..8c82e06ee432
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
>>>>> @@ -0,0 +1,97 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/interconnect/qcom,sdm845-cpu-bwmon.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm Interconnect Bandwidth Monitor
>>>>> +
>>>>> +maintainers:
>>>>> +  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> +
>>>>> +description:
>>>>> +  Bandwidth Monitor measures current throughput on buses between various NoC
>>>>> +  fabrics and provides information when it crosses configured thresholds.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - qcom,sdm845-cpu-bwmon       # BWMON v4
>>>>
>>>> It seems the thing that's called bwmon v4 is compatible with a number of
>>>> different platforms, should we add a generic compatible to the binding
>>>> as well, to avoid having to update the implementation for each SoC?
>>>>
>>>> (I.e. "qcom,sdm845-cpu-bwmon", "qcom,bwmon-v4")
>>
>> it seems pretty useful to have the "qcom,bwmon-v4" and "qcom,bwmon-v5"
>> compatibles, I tried these patches on a sc7280 device which has a bwmon4
>> between the cpu and caches (and also has a bwmon5 between the caches and DDR)
>> and the driver works with zero changes.
>>
> 
> But does the '4' and '5' has a relation to the hardware? Or is just the
> 4th and 5th register layout supported by the downstream driver?

Right, it was just based on the downstream driver register layouts, i could not
find these numbers in HW specs anywhere, but that said I do see 2 instances of
these, one of them called the LAGG bwmon which is the one between the LLCC and DDR
and is documented as part of the LLCC specs. I'll try and dig somemore into the
documentation to see how we could define compatibles to match hw revs.

> 
> Regards,
> Bjorn