Re: [PATCH v4 2/4] soc: qcom: icc-bwmon: Add bandwidth monitoring driver

Georgi Djakov
2022-06-06T16:36:11Z

Hi Krzysztof,

Thanks for working on this!

On 1.06.22 13:11, Krzysztof Kozlowski wrote:
> Bandwidth monitoring (BWMON) sits between various subsytems like CPU,
> GPU, Last Level caches and memory subsystem. The BWMON can be
> configured to monitor the data throuhput between memory and other
> subsytems. The throughput is measured within specified sampling window
> and is used to vote for corresponding interconnect bandwidth.
>
> Current implementation brings support for BWMON v4, used for example on
> SDM845 to measure bandwidth between CPU (gladiator_noc) and Last Level
> Cache (memnoc). Usage of this BWMON allows to remove fixed bandwidth
> votes from cpufreq (CPU nodes) thus achieve high memory throughput even
> with lower CPU frequencies.

I am curious if you ran any tests - e.g set the CPU to some fixed
frequency and run memory throughput benchmarks with/without this
driver? Could you share any data?

> Co-developed-by: Thara Gopinath <thara.gopinath@linaro.org>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> MAINTAINERS | 7 +
> drivers/soc/qcom/Kconfig | 15 ++
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/icc-bwmon.c | 421 +++++++++++++++++++++++++++++++++++
> 4 files changed, 444 insertions(+)
> create mode 100644 drivers/soc/qcom/icc-bwmon.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6157e706ed02..bc123f706256 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16376,6 +16376,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/i2c/i2c-qcom-cci.txt
> F: drivers/i2c/busses/i2c-qcom-cci.c
>
> +QUALCOMM INTERCONNECT BWMON DRIVER
> +M: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> +L: linux-arm-msm@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/interconnect/qcom,sdm845-cpu-bwmon.yaml
> +F: drivers/soc/qcom/icc-bwmon.c
> +
> QUALCOMM IOMMU
> M: Rob Clark <robdclark@gmail.com>
> L: iommu@lists.linux-foundation.org
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index e718b8735444..35c5192dcfc7 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -228,4 +228,19 @@ config QCOM_APR
> application processor and QDSP6. APR is
> used by audio driver to configure QDSP6
> ASM, ADM and AFE modules.
> +
> +config QCOM_ICC_BWMON
> + tristate "QCOM Interconnect Bandwidth Monitor driver"
> + depends on ARCH_QCOM || COMPILE_TEST
> + select PM_OPP
> + help
> + Sets up driver monitoring bandwidth on various interconnects and
> + based on that voting for interconnect bandwidth, adjusting their
> + speed to current demand.
> + Current implementation brings support for BWMON v4, used for example
> + on SDM845 to measure bandwidth between CPU (gladiator_noc) and Last
> + Level Cache (memnoc). Usage of this BWMON allows to remove fixed
> + bandwidth votes from cpufreq (CPU nodes) thus achieve high memory
> + throughput even with lower CPU frequencies.
> +
> endmenu
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 70d5de69fd7b..d66604aff2b0 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
> obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
> obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
> obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) += kryo-l2-accessors.o
> +obj-$(CONFIG_QCOM_ICC_BWMON) += icc-bwmon.o
> diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
> new file mode 100644
> index 000000000000..1eed075545db
> --- /dev/null
> +++ b/drivers/soc/qcom/icc-bwmon.c
> @@ -0,0 +1,421 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2021-2022 Linaro Ltd
> + * Author: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, based on
> + * previous work of Thara Gopinath and msm-4.9 downstream sources.
> + */
> +#include <linux/interconnect.h>

Is this used?

> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/sizes.h>

Ditto.

Thanks,
Georgi