| Message ID | 1533171465-25508-2-git-send-email-skannan@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v3,1/2] PM / devfreq: Generic CPU frequency to device frequency mapping governor | expand |
>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 <skannan@codeaurora.org> > --- > 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 <skannan@codeaurora.org> >> --- >> 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 <skannan@codeaurora.org> > --- > 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 <skannan@codeaurora.org> >> --- >> 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 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 <skannan@codeaurora.org> > --- > 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");
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");
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 <skannan@codeaurora.org> --- 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