This driver registers itself as a devfreq device that allows devfreq
governors to make bandwidth votes for an interconnect path. This allows
applying various policies for different interconnect paths using devfreq
governors.
Example uses:
* Use the devfreq performance governor to set the CPU to DDR interconnect
path for maximum performance.
* Use the devfreq performance governor to set the GPU to DDR interconnect
path for maximum performance.
* Use the CPU frequency to device frequency mapping governor to scale the
DDR frequency based on the needs of the CPUs' current frequency.
Signed-off-by: Saravana Kannan <[email protected]>
---
Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
drivers/devfreq/Kconfig | 13 +++
drivers/devfreq/Makefile | 1 +
drivers/devfreq/devfreq_icbw.c | 116 +++++++++++++++++++++
4 files changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
create mode 100644 drivers/devfreq/devfreq_icbw.c
diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt b/Documentation/devicetree/bindings/devfreq/icbw.txt
new file mode 100644
index 0000000..36cf045
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
@@ -0,0 +1,21 @@
+Interconnect bandwidth device
+
+icbw is a device that represents an interconnect path that connects two
+devices. This device is typically used to vote for BW requirements between
+two devices. Eg: CPU to DDR, GPU to DDR, etc
+
+Required properties:
+- compatible: Must be "devfreq-icbw"
+- interconnects: Pairs of phandles and interconnect provider specifier
+ to denote the edge source and destination ports of
+ the interconnect path. See also:
+ Documentation/devicetree/bindings/interconnect/interconnect.txt
+- interconnect-names: Must have one entry with the name "path".
+
+Example:
+
+ qcom,cpubw {
+ compatible = "devfreq-icbw";
+ interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>;
+ interconnect-names = "path";
+ };
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 3d9ae68..590370e 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -121,6 +121,19 @@ config ARM_RK3399_DMC_DEVFREQ
It sets the frequency for the memory controller and reads the usage counts
from hardware.
+config DEVFREQ_ICBW
+ bool "DEVFREQ device for making bandwidth votes on interconnect paths"
+ select DEVFREQ_GOV_PERFORMANCE
+ select DEVFREQ_GOV_POWERSAVE
+ select DEVFREQ_GOV_USERSPACE
+ default n
+ help
+ Different devfreq governors use this devfreq device to make
+ bandwidth votes for interconnect paths between different devices
+ (Eg: CPU to DDR, GPU to DDR, etc). This driver provides a generic
+ interface so that the devfreq governors can be shared across SoCs
+ and architectures.
+
source "drivers/devfreq/event/Kconfig"
endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index e9dc3e9..b302c5cf 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DEVFREQ_GOV_CPUFREQ_MAP) += governor_cpufreq_map.o
obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
+obj-$(CONFIG_DEVFREQ_ICBW) += devfreq_icbw.o
# DEVFREQ Event Drivers
obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
diff --git a/drivers/devfreq/devfreq_icbw.c b/drivers/devfreq/devfreq_icbw.c
new file mode 100644
index 0000000..231fb21
--- /dev/null
+++ b/drivers/devfreq/devfreq_icbw.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights reserved.
+ */
+
+#define pr_fmt(fmt) "icbw: " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/devfreq.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/interconnect.h>
+
+struct dev_data {
+ struct icc_path *path;
+ u32 cur_ab;
+ u32 cur_pb;
+ unsigned long gov_ab;
+ struct devfreq *df;
+ struct devfreq_dev_profile dp;
+};
+
+static int icbw_target(struct device *dev, unsigned long *freq, u32 flags)
+{
+ struct dev_data *d = dev_get_drvdata(dev);
+ int ret;
+ u32 new_pb = *freq, new_ab = d->gov_ab;
+
+ if (d->cur_pb == new_pb && d->cur_ab == new_ab)
+ return 0;
+
+ dev_dbg(dev, "BW KBps: AB: %u PB: %u\n", new_ab, new_pb);
+
+ ret = icc_set(d->path, new_ab, new_pb);
+ if (ret) {
+ dev_err(dev, "bandwidth request failed (%d)\n", ret);
+ } else {
+ d->cur_pb = new_pb;
+ d->cur_ab = new_ab;
+ }
+
+ return ret;
+}
+
+static int icbw_get_dev_status(struct device *dev,
+ struct devfreq_dev_status *stat)
+{
+ struct dev_data *d = dev_get_drvdata(dev);
+
+ stat->private_data = &d->gov_ab;
+ return 0;
+}
+
+static int devfreq_icbw_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dev_data *d;
+ struct devfreq_dev_profile *p;
+
+ d = devm_kzalloc(dev, sizeof(*d), GFP_KERNEL);
+ if (!d)
+ return -ENOMEM;
+ dev_set_drvdata(dev, d);
+
+ p = &d->dp;
+ p->polling_ms = 50;
+ p->target = icbw_target;
+ p->get_dev_status = icbw_get_dev_status;
+
+ d->path = of_icc_get(dev, "path");
+ if (IS_ERR(d->path)) {
+ dev_err(dev, "Unable to register interconnect path\n");
+ return -ENODEV;
+ }
+
+ d->df = devfreq_add_device(dev, p, "performance", NULL);
+ if (IS_ERR(d->df)) {
+ icc_put(d->path);
+ return PTR_ERR(d->df);
+ }
+
+ return 0;
+}
+
+static int devfreq_icbw_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dev_data *d = dev_get_drvdata(dev);
+
+ devfreq_remove_device(d->df);
+ icc_put(d->path);
+ return 0;
+}
+
+static const struct of_device_id icbw_match_table[] = {
+ { .compatible = "devfreq-icbw" },
+ {}
+};
+
+static struct platform_driver icbw_driver = {
+ .probe = devfreq_icbw_probe,
+ .remove = devfreq_icbw_remove,
+ .driver = {
+ .name = "devfreq-icbw",
+ .of_match_table = icbw_match_table,
+ },
+};
+
+module_platform_driver(icbw_driver);
+MODULE_DESCRIPTION("Interconnect bandwidth voting driver");
+MODULE_LICENSE("GPL v2");
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
>This driver registers itself as a devfreq device that allows devfreq
>governors to make bandwidth votes for an interconnect path. This allows
>applying various policies for different interconnect paths using devfreq
>governors.
>
First of all, the name, "devfreq_icbw", is not appropriate for a
devfreq device driver. It confuses; it looks like a part of the
framework itself.
>diff --git a/drivers/devfreq/devfreq_icbw.c b/drivers/devfreq/devfreq_icbw.c
>new file mode 100644
>index 0000000..231fb21
>--- /dev/null
>+++ b/drivers/devfreq/devfreq_icbw.c
>@@ -0,0 +1,116 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights reserved.
>+ */
>+
>+#define pr_fmt(fmt) "icbw: " fmt
>+
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/init.h>
>+#include <linux/err.h>
>+#include <linux/errno.h>
>+#include <linux/mutex.h>
>+#include <linux/devfreq.h>
>+#include <linux/platform_device.h>
>+#include <linux/of.h>
>+#include <linux/interconnect.h>
Where can I find this file?
>+
>+struct dev_data {
>+ struct icc_path *path;
>+ u32 cur_ab;
>+ u32 cur_pb;
>+ unsigned long gov_ab;
>+ struct devfreq *df;
>+ struct devfreq_dev_profile dp;
>+};
>+
>+static int icbw_target(struct device *dev, unsigned long *freq, u32 flags)
>+{
>+ struct dev_data *d = dev_get_drvdata(dev);
>+ int ret;
>+ u32 new_pb = *freq, new_ab = d->gov_ab;
>+
>+ if (d->cur_pb == new_pb && d->cur_ab == new_ab)
>+ return 0;
>+
>+ dev_dbg(dev, "BW KBps: AB: %u PB: %u\n", new_ab, new_pb);
>+
>+ ret = icc_set(d->path, new_ab, new_pb);
I'm not sure if icc_set is available.
Cheers,
MyungJoo
Hi MyungJoo,
On 08/02/2018 01:13 PM, MyungJoo Ham wrote:
>> This driver registers itself as a devfreq device that allows devfreq
>> governors to make bandwidth votes for an interconnect path. This allows
>> applying various policies for different interconnect paths using devfreq
>> governors.
>>
>
> First of all, the name, "devfreq_icbw", is not appropriate for a
> devfreq device driver. It confuses; it looks like a part of the
> framework itself.
>
>> diff --git a/drivers/devfreq/devfreq_icbw.c b/drivers/devfreq/devfreq_icbw.c
>> new file mode 100644
>> index 0000000..231fb21
>> --- /dev/null
>> +++ b/drivers/devfreq/devfreq_icbw.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#define pr_fmt(fmt) "icbw: " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/mutex.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +#include <linux/interconnect.h>
>
> Where can I find this file?
It is part of the On-chip interconnect API, which is currently under
review and is available here: https://lkml.org/lkml/2018/7/31/647
Thanks,
Georgi
On 2018-08-02 03:13, MyungJoo Ham wrote:
>> This driver registers itself as a devfreq device that allows devfreq
>> governors to make bandwidth votes for an interconnect path. This
>> allows
>> applying various policies for different interconnect paths using
>> devfreq
>> governors.
>>
>
> First of all, the name, "devfreq_icbw", is not appropriate for a
> devfreq device driver. It confuses; it looks like a part of the
> framework itself.
>
>> diff --git a/drivers/devfreq/devfreq_icbw.c
>> b/drivers/devfreq/devfreq_icbw.c
>> new file mode 100644
>> index 0000000..231fb21
>> --- /dev/null
>> +++ b/drivers/devfreq/devfreq_icbw.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights
>> reserved.
>> + */
>> +
>> +#define pr_fmt(fmt) "icbw: " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/mutex.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +#include <linux/interconnect.h>
>
> Where can I find this file?
Sorry, meant to mention this in the email specific portion of the commit
text. This is on top of the interconnect framework series that Georgi
has been working on. linux-pm should have those patches.
>> +
>> +struct dev_data {
>> + struct icc_path *path;
>> + u32 cur_ab;
>> + u32 cur_pb;
>> + unsigned long gov_ab;
>> + struct devfreq *df;
>> + struct devfreq_dev_profile dp;
>> +};
>> +
>> +static int icbw_target(struct device *dev, unsigned long *freq, u32
>> flags)
>> +{
>> + struct dev_data *d = dev_get_drvdata(dev);
>> + int ret;
>> + u32 new_pb = *freq, new_ab = d->gov_ab;
>> +
>> + if (d->cur_pb == new_pb && d->cur_ab == new_ab)
>> + return 0;
>> +
>> + dev_dbg(dev, "BW KBps: AB: %u PB: %u\n", new_ab, new_pb);
>> +
>> + ret = icc_set(d->path, new_ab, new_pb);
>
> I'm not sure if icc_set is available.
Yeah, it's available on that patch series.
-Saravana
On Wed, Aug 01, 2018 at 05:57:42PM -0700, Saravana Kannan wrote:
> This driver registers itself as a devfreq device that allows devfreq
> governors to make bandwidth votes for an interconnect path. This allows
> applying various policies for different interconnect paths using devfreq
> governors.
>
> Example uses:
> * Use the devfreq performance governor to set the CPU to DDR interconnect
> path for maximum performance.
> * Use the devfreq performance governor to set the GPU to DDR interconnect
> path for maximum performance.
> * Use the CPU frequency to device frequency mapping governor to scale the
> DDR frequency based on the needs of the CPUs' current frequency.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
Please make bindings separate a patch.
> drivers/devfreq/Kconfig | 13 +++
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/devfreq_icbw.c | 116 +++++++++++++++++++++
> 4 files changed, 151 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
> create mode 100644 drivers/devfreq/devfreq_icbw.c
>
> diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt b/Documentation/devicetree/bindings/devfreq/icbw.txt
> new file mode 100644
> index 0000000..36cf045
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
> @@ -0,0 +1,21 @@
> +Interconnect bandwidth device
> +
> +icbw is a device that represents an interconnect path that connects two
> +devices. This device is typically used to vote for BW requirements between
> +two devices. Eg: CPU to DDR, GPU to DDR, etc
I'm pretty sure this doesn't represent a h/w device. This usage doesn't
encourage me to accept the interconnects binding either.
> +
> +Required properties:
> +- compatible: Must be "devfreq-icbw"
> +- interconnects: Pairs of phandles and interconnect provider specifier
> + to denote the edge source and destination ports of
> + the interconnect path. See also:
> + Documentation/devicetree/bindings/interconnect/interconnect.txt
> +- interconnect-names: Must have one entry with the name "path".
That's pretty useless...
> +
> +Example:
> +
> + qcom,cpubw {
Someone in QCom please broadcast to stop using qcom,foo for node names.
It is amazing how consistent you all are. If only folks were as
consistent in reading
Documentation/devicetree/bindings/submitting-patches.txt.
Rob
On 2018-08-07 09:51, Rob Herring wrote:
> On Wed, Aug 01, 2018 at 05:57:42PM -0700, Saravana Kannan wrote:
>> This driver registers itself as a devfreq device that allows devfreq
>> governors to make bandwidth votes for an interconnect path. This
>> allows
>> applying various policies for different interconnect paths using
>> devfreq
>> governors.
>>
>> Example uses:
>> * Use the devfreq performance governor to set the CPU to DDR
>> interconnect
>> path for maximum performance.
>> * Use the devfreq performance governor to set the GPU to DDR
>> interconnect
>> path for maximum performance.
>> * Use the CPU frequency to device frequency mapping governor to scale
>> the
>> DDR frequency based on the needs of the CPUs' current frequency.
>>
>> Signed-off-by: Saravana Kannan <[email protected]>
>> ---
>> Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
>
> Please make bindings separate a patch.
Yeah, I was aware of that. I just wanted to give some context in the v1
of this patch (I wasn't expecting this to merge as is).
>> drivers/devfreq/Kconfig | 13 +++
>> drivers/devfreq/Makefile | 1 +
>> drivers/devfreq/devfreq_icbw.c | 116
>> +++++++++++++++++++++
>> 4 files changed, 151 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
>> create mode 100644 drivers/devfreq/devfreq_icbw.c
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt
>> b/Documentation/devicetree/bindings/devfreq/icbw.txt
>> new file mode 100644
>> index 0000000..36cf045
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
>> @@ -0,0 +1,21 @@
>> +Interconnect bandwidth device
>> +
>> +icbw is a device that represents an interconnect path that connects
>> two
>> +devices. This device is typically used to vote for BW requirements
>> between
>> +two devices. Eg: CPU to DDR, GPU to DDR, etc
>
> I'm pretty sure this doesn't represent a h/w device. This usage doesn't
> encourage me to accept the interconnects binding either.
Hasn't the DT rules moved past "only HW devices" in DT? Logical devices
are still allowed in Linux DT bindings?
Having said that, this is explicitly representing a real HW path and the
ability to control its performance.
>> +
>> +Required properties:
>> +- compatible: Must be "devfreq-icbw"
>> +- interconnects: Pairs of phandles and interconnect provider
>> specifier
>> + to denote the edge source and destination ports of
>> + the interconnect path. See also:
>> + Documentation/devicetree/bindings/interconnect/interconnect.txt
>> +- interconnect-names: Must have one entry with the name "path".
>
> That's pretty useless...
True. But the current DT binding for interconnect consumer bindings
needs a interconnect name to use the of_* API. I'm open to switching to
an index based API if one is provided.
>> +
>> +Example:
>> +
>> + qcom,cpubw {
>
> Someone in QCom please broadcast to stop using qcom,foo for node names.
> It is amazing how consistent you all are. If only folks were as
> consistent in reading
> Documentation/devicetree/bindings/submitting-patches.txt.
Sorry :(
Thanks,
Saravana
Hi Saravana,
On 08/02/2018 03:57 AM, Saravana Kannan wrote:
> This driver registers itself as a devfreq device that allows devfreq
> governors to make bandwidth votes for an interconnect path. This allows
> applying various policies for different interconnect paths using devfreq
> governors.
>
> Example uses:
> * Use the devfreq performance governor to set the CPU to DDR interconnect
> path for maximum performance.
> * Use the devfreq performance governor to set the GPU to DDR interconnect
> path for maximum performance.
> * Use the CPU frequency to device frequency mapping governor to scale the
> DDR frequency based on the needs of the CPUs' current frequency.
Usually CPUs and GPUs have dedicated cpufreq/devfreq drivers and i was
wondering if the interconnect support could be put into these drivers
directly?
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
> drivers/devfreq/Kconfig | 13 +++
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/devfreq_icbw.c | 116 +++++++++++++++++++++
> 4 files changed, 151 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
> create mode 100644 drivers/devfreq/devfreq_icbw.c
>
> diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt b/Documentation/devicetree/bindings/devfreq/icbw.txt
> new file mode 100644
> index 0000000..36cf045
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
> @@ -0,0 +1,21 @@
> +Interconnect bandwidth device
> +
> +icbw is a device that represents an interconnect path that connects two
> +devices. This device is typically used to vote for BW requirements between
> +two devices. Eg: CPU to DDR, GPU to DDR, etc
> +
> +Required properties:
> +- compatible: Must be "devfreq-icbw"
> +- interconnects: Pairs of phandles and interconnect provider specifier
> + to denote the edge source and destination ports of
> + the interconnect path. See also:
> + Documentation/devicetree/bindings/interconnect/interconnect.txt
> +- interconnect-names: Must have one entry with the name "path".
> +
> +Example:
> +
> + qcom,cpubw {
> + compatible = "devfreq-icbw";
> + interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>;
> + interconnect-names = "path";
interconnect-names is optional when there is only a single path.
> + };
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 3d9ae68..590370e 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -121,6 +121,19 @@ config ARM_RK3399_DMC_DEVFREQ
> It sets the frequency for the memory controller and reads the usage counts
> from hardware.
>
> +config DEVFREQ_ICBW
> + bool "DEVFREQ device for making bandwidth votes on interconnect paths"
Can this be a module?
> + select DEVFREQ_GOV_PERFORMANCE
> + select DEVFREQ_GOV_POWERSAVE
> + select DEVFREQ_GOV_USERSPACE
> + default n
There's no need to specify this default. It is 'n' by default anyway.
Also maybe you want to add something like:
depends on INTERCONNECT=y
Thanks,
Georgi
> + help
> + Different devfreq governors use this devfreq device to make
> + bandwidth votes for interconnect paths between different devices
> + (Eg: CPU to DDR, GPU to DDR, etc). This driver provides a generic
> + interface so that the devfreq governors can be shared across SoCs
> + and architectures.
> +
> source "drivers/devfreq/event/Kconfig"
Hi Georgi,
This driver uses of_icc_get which is very likely to fail if it probes
before the interconnect provider. Would it be possible for icc_get to
return/differentiate both -EPROBE_DEFER and other errors to prevent the
driver to continually probe defer if the path doesn't actually exist
or just error out if it probes before the interconnect provider.
On 2018-08-23 18:30, Georgi Djakov wrote:
> Hi Saravana,
>
> On 08/02/2018 03:57 AM, Saravana Kannan wrote:
>> This driver registers itself as a devfreq device that allows devfreq
>> governors to make bandwidth votes for an interconnect path. This
>> allows
>> applying various policies for different interconnect paths using
>> devfreq
>> governors.
>>
>> Example uses:
>> * Use the devfreq performance governor to set the CPU to DDR
>> interconnect
>> path for maximum performance.
>> * Use the devfreq performance governor to set the GPU to DDR
>> interconnect
>> path for maximum performance.
>> * Use the CPU frequency to device frequency mapping governor to scale
>> the
>> DDR frequency based on the needs of the CPUs' current frequency.
>
> Usually CPUs and GPUs have dedicated cpufreq/devfreq drivers and i was
> wondering if the interconnect support could be put into these drivers
> directly?
>
>>
>> Signed-off-by: Saravana Kannan <[email protected]>
>> ---
>> Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
>> drivers/devfreq/Kconfig | 13 +++
>> drivers/devfreq/Makefile | 1 +
>> drivers/devfreq/devfreq_icbw.c | 116
>> +++++++++++++++++++++
>> 4 files changed, 151 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
>> create mode 100644 drivers/devfreq/devfreq_icbw.c
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt
>> b/Documentation/devicetree/bindings/devfreq/icbw.txt
>> new file mode 100644
>> index 0000000..36cf045
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
>> @@ -0,0 +1,21 @@
>> +Interconnect bandwidth device
>> +
>> +icbw is a device that represents an interconnect path that connects
>> two
>> +devices. This device is typically used to vote for BW requirements
>> between
>> +two devices. Eg: CPU to DDR, GPU to DDR, etc
>> +
>> +Required properties:
>> +- compatible: Must be "devfreq-icbw"
>> +- interconnects: Pairs of phandles and interconnect provider
>> specifier
>> + to denote the edge source and destination ports of
>> + the interconnect path. See also:
>> + Documentation/devicetree/bindings/interconnect/interconnect.txt
>> +- interconnect-names: Must have one entry with the name "path".
>> +
>> +Example:
>> +
>> + qcom,cpubw {
>> + compatible = "devfreq-icbw";
>> + interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>;
>> + interconnect-names = "path";
>
> interconnect-names is optional when there is only a single path.
>
>> + };
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 3d9ae68..590370e 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -121,6 +121,19 @@ config ARM_RK3399_DMC_DEVFREQ
>> It sets the frequency for the memory controller and reads
>> the usage counts
>> from hardware.
>>
>> +config DEVFREQ_ICBW
>> + bool "DEVFREQ device for making bandwidth votes on interconnect
>> paths"
>
> Can this be a module?
>
>> + select DEVFREQ_GOV_PERFORMANCE
>> + select DEVFREQ_GOV_POWERSAVE
>> + select DEVFREQ_GOV_USERSPACE
>> + default n
>
> There's no need to specify this default. It is 'n' by default anyway.
> Also maybe you want to add something like:
> depends on INTERCONNECT=y
>
> Thanks,
> Georgi
>
>> + help
>> + Different devfreq governors use this devfreq device to make
>> + bandwidth votes for interconnect paths between different devices
>> + (Eg: CPU to DDR, GPU to DDR, etc). This driver provides a generic
>> + interface so that the devfreq governors can be shared across SoCs
>> + and architectures.
>> +
>> source "drivers/devfreq/event/Kconfig"
--
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
Hi Saravana,
On 2018-08-02 06:27, Saravana Kannan wrote:
> This driver registers itself as a devfreq device that allows devfreq
> governors to make bandwidth votes for an interconnect path. This allows
> applying various policies for different interconnect paths using
> devfreq
> governors.
>
> Example uses:
> * Use the devfreq performance governor to set the CPU to DDR
> interconnect
> path for maximum performance.
> * Use the devfreq performance governor to set the GPU to DDR
> interconnect
> path for maximum performance.
> * Use the CPU frequency to device frequency mapping governor to scale
> the
> DDR frequency based on the needs of the CPUs' current frequency.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++
> drivers/devfreq/Kconfig | 13 +++
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/devfreq_icbw.c | 116
> +++++++++++++++++++++
> 4 files changed, 151 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
> create mode 100644 drivers/devfreq/devfreq_icbw.c
>
> diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt
> b/Documentation/devicetree/bindings/devfreq/icbw.txt
> new file mode 100644
> index 0000000..36cf045
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
> @@ -0,0 +1,21 @@
> +Interconnect bandwidth device
> +
> +icbw is a device that represents an interconnect path that connects
> two
> +devices. This device is typically used to vote for BW requirements
> between
> +two devices. Eg: CPU to DDR, GPU to DDR, etc
> +
> +Required properties:
> +- compatible: Must be "devfreq-icbw"
> +- interconnects: Pairs of phandles and interconnect provider specifier
> + to denote the edge source and destination ports of
> + the interconnect path. See also:
> + Documentation/devicetree/bindings/interconnect/interconnect.txt
> +- interconnect-names: Must have one entry with the name "path".
> +
> +Example:
> +
> + qcom,cpubw {
> + compatible = "devfreq-icbw";
> + interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>;
> + interconnect-names = "path";
> + };
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 3d9ae68..590370e 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -121,6 +121,19 @@ config ARM_RK3399_DMC_DEVFREQ
> It sets the frequency for the memory controller and reads
> the usage counts
> from hardware.
>
> +config DEVFREQ_ICBW
> + bool "DEVFREQ device for making bandwidth votes on interconnect
> paths"
> + select DEVFREQ_GOV_PERFORMANCE
> + select DEVFREQ_GOV_POWERSAVE
> + select DEVFREQ_GOV_USERSPACE
> + default n
> + help
> + Different devfreq governors use this devfreq device to make
> + bandwidth votes for interconnect paths between different devices
> + (Eg: CPU to DDR, GPU to DDR, etc). This driver provides a generic
> + interface so that the devfreq governors can be shared across SoCs
> + and architectures.
> +
> source "drivers/devfreq/event/Kconfig"
>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index e9dc3e9..b302c5cf 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_DEVFREQ_GOV_CPUFREQ_MAP) +=
> governor_cpufreq_map.o
> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
> +obj-$(CONFIG_DEVFREQ_ICBW) += devfreq_icbw.o
>
> # DEVFREQ Event Drivers
> obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
> diff --git a/drivers/devfreq/devfreq_icbw.c
> b/drivers/devfreq/devfreq_icbw.c
> new file mode 100644
> index 0000000..231fb21
> --- /dev/null
> +++ b/drivers/devfreq/devfreq_icbw.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights
> reserved.
> + */
> +
> +#define pr_fmt(fmt) "icbw: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/mutex.h>
> +#include <linux/devfreq.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/interconnect.h>
> +
> +struct dev_data {
> + struct icc_path *path;
> + u32 cur_ab;
> + u32 cur_pb;
> + unsigned long gov_ab;
> + struct devfreq *df;
> + struct devfreq_dev_profile dp;
> +};
> +
> +static int icbw_target(struct device *dev, unsigned long *freq, u32
> flags)
> +{
> + struct dev_data *d = dev_get_drvdata(dev);
> + int ret;
> + u32 new_pb = *freq, new_ab = d->gov_ab;
> +
> + if (d->cur_pb == new_pb && d->cur_ab == new_ab)
> + return 0;
> +
> + dev_dbg(dev, "BW KBps: AB: %u PB: %u\n", new_ab, new_pb);
> +
> + ret = icc_set(d->path, new_ab, new_pb);
> + if (ret) {
> + dev_err(dev, "bandwidth request failed (%d)\n", ret);
> + } else {
> + d->cur_pb = new_pb;
> + d->cur_ab = new_ab;
> + }
> +
> + return ret;
> +}
> +
> +static int icbw_get_dev_status(struct device *dev,
> + struct devfreq_dev_status *stat)
> +{
> + struct dev_data *d = dev_get_drvdata(dev);
> +
> + stat->private_data = &d->gov_ab;
> + return 0;
> +}
> +
> +static int devfreq_icbw_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dev_data *d;
> + struct devfreq_dev_profile *p;
> +
> + d = devm_kzalloc(dev, sizeof(*d), GFP_KERNEL);
> + if (!d)
> + return -ENOMEM;
> + dev_set_drvdata(dev, d);
> +
> + p = &d->dp;
> + p->polling_ms = 50;
> + p->target = icbw_target;
> + p->get_dev_status = icbw_get_dev_status;
> +
> + d->path = of_icc_get(dev, "path");
> + if (IS_ERR(d->path)) {
> + dev_err(dev, "Unable to register interconnect path\n");
> + return -ENODEV;
> + }
The probe fails if the provider is not registered yet. Worked around it
using -EPROBE_DEFER
but this should probably come from of_icc_get itself.
> +
> + d->df = devfreq_add_device(dev, p, "performance", NULL);
> + if (IS_ERR(d->df)) {
> + icc_put(d->path);
> + return PTR_ERR(d->df);
> + }
devfreq_add_device will fail with -EINVAL for set_table_freq,
find_available_min_freq and
find_available_max_freq. If you end up working your way around it, I
still ended up getting
multiple errors like "Couldn't update frequency transition information"
after probing.
> +
> + return 0;
> +}
> +
> +static int devfreq_icbw_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dev_data *d = dev_get_drvdata(dev);
> +
> + devfreq_remove_device(d->df);
> + icc_put(d->path);
> + return 0;
> +}
> +
> +static const struct of_device_id icbw_match_table[] = {
> + { .compatible = "devfreq-icbw" },
> + {}
> +};
> +
> +static struct platform_driver icbw_driver = {
> + .probe = devfreq_icbw_probe,
> + .remove = devfreq_icbw_remove,
> + .driver = {
> + .name = "devfreq-icbw",
> + .of_match_table = icbw_match_table,
> + },
> +};
> +
> +module_platform_driver(icbw_driver);
> +MODULE_DESCRIPTION("Interconnect bandwidth voting driver");
> +MODULE_LICENSE("GPL v2");
--
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.