| Message ID | 20220125171809.1273269-6-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | powercap/drivers/dtpm: Create the dtpm hierarchy | expand |
Hi Heiko, do you have comments on this patch? On 25/01/2022 18:18, Daniel Lezcano wrote: > The DTPM framework does support now the hierarchy description. > > The platform specific code can call the hierarchy creation function > with an array of struct dtpm_node pointing to their parent. > > This patch provides a description of the big / Little CPUs and the > GPU and tie them together under a virtual 'package' name. Only rk3399 is > described now. > > The description could be extended in the future with the memory > controller with devfreq. > > The description is always a module and it describes the soft > dependencies. The userspace has to load the softdeps module in the > right order. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/soc/rockchip/Kconfig | 8 +++++ > drivers/soc/rockchip/Makefile | 1 + > drivers/soc/rockchip/dtpm.c | 59 +++++++++++++++++++++++++++++++++++ > 3 files changed, 68 insertions(+) > create mode 100644 drivers/soc/rockchip/dtpm.c > > diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig > index 25eb2c1e31bb..6dc017f02431 100644 > --- a/drivers/soc/rockchip/Kconfig > +++ b/drivers/soc/rockchip/Kconfig > @@ -34,4 +34,12 @@ config ROCKCHIP_PM_DOMAINS > > If unsure, say N. > > +config ROCKCHIP_DTPM > + tristate "Rockchip DTPM hierarchy" > + depends on DTPM && DRM_PANFROST && m > + help > + Describe the hierarchy for the Dynamic Thermal Power > + Management tree on this platform. That will create all the > + power capping capable devices. > + > endif > diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile > index 875032f7344e..05f31a4e743c 100644 > --- a/drivers/soc/rockchip/Makefile > +++ b/drivers/soc/rockchip/Makefile > @@ -5,3 +5,4 @@ > obj-$(CONFIG_ROCKCHIP_GRF) += grf.o > obj-$(CONFIG_ROCKCHIP_IODOMAIN) += io-domain.o > obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o > +obj-$(CONFIG_ROCKCHIP_DTPM) += dtpm.o > diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c > new file mode 100644 > index 000000000000..0b73a9cba954 > --- /dev/null > +++ b/drivers/soc/rockchip/dtpm.c > @@ -0,0 +1,59 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2021 Linaro Limited > + * > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org> > + * > + * DTPM hierarchy description > + */ > +#include <linux/dtpm.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > + > +static struct dtpm_node __initdata rk3399_hierarchy[] = { > + [0]{ .name = "rk3399", > + .type = DTPM_NODE_VIRTUAL }, > + [1]{ .name = "package", > + .type = DTPM_NODE_VIRTUAL, > + .parent = &rk3399_hierarchy[0] }, > + [2]{ .name = "/cpus/cpu@0", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [3]{ .name = "/cpus/cpu@1", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [4]{ .name = "/cpus/cpu@2", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [5]{ .name = "/cpus/cpu@3", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [6]{ .name = "/cpus/cpu@100", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [7]{ .name = "/cpus/cpu@101", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [8]{ .name = "/gpu@ff9a0000", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [9]{ }, > +}; > + > +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { > + { .compatible = "rockchip,rk3399", .data = rk3399_hierarchy }, > + {}, > +}; > + > +static int __init rockchip_dtpm_init(void) > +{ > + return dtpm_create_hierarchy(rockchip_dtpm_match_table); > +} > +module_init(rockchip_dtpm_init); > + > +MODULE_SOFTDEP("pre: panfrost cpufreq-dt"); > +MODULE_DESCRIPTION("Rockchip DTPM driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:dtpm"); > +MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@kernel.org"); >
Hi Daniel, On Tue, Jan 25, 2022 at 6:18 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > The DTPM framework does support now the hierarchy description. > > The platform specific code can call the hierarchy creation function > with an array of struct dtpm_node pointing to their parent. > > This patch provides a description of the big / Little CPUs and the > GPU and tie them together under a virtual 'package' name. Only rk3399 is > described now. > > The description could be extended in the future with the memory > controller with devfreq. > > The description is always a module and it describes the soft > dependencies. The userspace has to load the softdeps module in the > right order. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Thanks for your patch! > --- Can you please insert a changelog here, especially if you don't CC all parties on the cover letter? Yes, I can get it from lore, but it's easier for the audience if it's included here. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 26/01/2022 10:40, Geert Uytterhoeven wrote: > Hi Daniel, > > On Tue, Jan 25, 2022 at 6:18 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> The DTPM framework does support now the hierarchy description. >> >> The platform specific code can call the hierarchy creation function >> with an array of struct dtpm_node pointing to their parent. >> >> This patch provides a description of the big / Little CPUs and the >> GPU and tie them together under a virtual 'package' name. Only rk3399 is >> described now. >> >> The description could be extended in the future with the memory >> controller with devfreq. >> >> The description is always a module and it describes the soft >> dependencies. The userspace has to load the softdeps module in the >> right order. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > Thanks for your patch! > >> --- > > Can you please insert a changelog here, especially if you don't CC all > parties on the cover letter? Ah yes, will do in the future > Yes, I can get it from lore, but it's easier for the audience if it's included > here. Changelog: V7: - No changes V6: - Made rk3399 always as a module and added module softdeps V5: - Module creation
Am Dienstag, 25. Januar 2022, 18:18:09 CET schrieb Daniel Lezcano: > The DTPM framework does support now the hierarchy description. > > The platform specific code can call the hierarchy creation function > with an array of struct dtpm_node pointing to their parent. > > This patch provides a description of the big / Little CPUs and the > GPU and tie them together under a virtual 'package' name. Only rk3399 is > described now. > > The description could be extended in the future with the memory > controller with devfreq. > > The description is always a module and it describes the soft > dependencies. The userspace has to load the softdeps module in the > right order. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/soc/rockchip/Kconfig | 8 +++++ > drivers/soc/rockchip/Makefile | 1 + > drivers/soc/rockchip/dtpm.c | 59 +++++++++++++++++++++++++++++++++++ > 3 files changed, 68 insertions(+) > create mode 100644 drivers/soc/rockchip/dtpm.c > > diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig > index 25eb2c1e31bb..6dc017f02431 100644 > --- a/drivers/soc/rockchip/Kconfig > +++ b/drivers/soc/rockchip/Kconfig > @@ -34,4 +34,12 @@ config ROCKCHIP_PM_DOMAINS > > If unsure, say N. > > +config ROCKCHIP_DTPM > + tristate "Rockchip DTPM hierarchy" > + depends on DTPM && DRM_PANFROST && m > + help > + Describe the hierarchy for the Dynamic Thermal Power > + Management tree on this platform. That will create all the > + power capping capable devices. > + > endif > diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile > index 875032f7344e..05f31a4e743c 100644 > --- a/drivers/soc/rockchip/Makefile > +++ b/drivers/soc/rockchip/Makefile > @@ -5,3 +5,4 @@ > obj-$(CONFIG_ROCKCHIP_GRF) += grf.o > obj-$(CONFIG_ROCKCHIP_IODOMAIN) += io-domain.o > obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o > +obj-$(CONFIG_ROCKCHIP_DTPM) += dtpm.o > diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c > new file mode 100644 > index 000000000000..0b73a9cba954 > --- /dev/null > +++ b/drivers/soc/rockchip/dtpm.c > @@ -0,0 +1,59 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2021 Linaro Limited > + * > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org> > + * > + * DTPM hierarchy description > + */ > +#include <linux/dtpm.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > + > +static struct dtpm_node __initdata rk3399_hierarchy[] = { The driver is tristate so buildable as module but uses __initdata. As it depends on panfrost (which also can be a module) you probably want a "__initdata_or_module" here . > + [0]{ .name = "rk3399", > + .type = DTPM_NODE_VIRTUAL }, > + [1]{ .name = "package", > + .type = DTPM_NODE_VIRTUAL, > + .parent = &rk3399_hierarchy[0] }, > + [2]{ .name = "/cpus/cpu@0", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [3]{ .name = "/cpus/cpu@1", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [4]{ .name = "/cpus/cpu@2", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [5]{ .name = "/cpus/cpu@3", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [6]{ .name = "/cpus/cpu@100", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [7]{ .name = "/cpus/cpu@101", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [8]{ .name = "/gpu@ff9a0000", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [9]{ }, hmm, do we want a "/* sentinel */" inside the empty last entry? I think that is pretty common to denote the "this one is the last entry" of a dynamic list ;-) > +}; > + > +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { > + { .compatible = "rockchip,rk3399", .data = rk3399_hierarchy }, > + {}, > +}; > + > +static int __init rockchip_dtpm_init(void) > +{ > + return dtpm_create_hierarchy(rockchip_dtpm_match_table); > +} > +module_init(rockchip_dtpm_init); Just for my understanding what happens on driver unload? Thanks Heiko > + > +MODULE_SOFTDEP("pre: panfrost cpufreq-dt"); > +MODULE_DESCRIPTION("Rockchip DTPM driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:dtpm"); > +MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@kernel.org"); >
Hi Heiko, thanks for your comments On 28/01/2022 11:19, Heiko Stübner wrote: > Am Dienstag, 25. Januar 2022, 18:18:09 CET schrieb Daniel Lezcano: >> The DTPM framework does support now the hierarchy description. >> >> The platform specific code can call the hierarchy creation function >> with an array of struct dtpm_node pointing to their parent. >> >> This patch provides a description of the big / Little CPUs and the >> GPU and tie them together under a virtual 'package' name. Only rk3399 is >> described now. >> >> The description could be extended in the future with the memory >> controller with devfreq. >> >> The description is always a module and it describes the soft >> dependencies. The userspace has to load the softdeps module in the >> right order. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> [ ... ] >> +static struct dtpm_node __initdata rk3399_hierarchy[] = { > > The driver is tristate so buildable as module but uses __initdata. > As it depends on panfrost (which also can be a module) you > probably want a "__initdata_or_module" here . Well, actually the dependency is wrong. It should be: depends on DTPM && m It will be compiled always as a module. Referring to the Documentation/kernel-hacking/hacking.rst "After boot, the kernel frees up a special section; functions marked with ``__init`` and data structures marked with ``__initdata`` are dropped after boot is complete: similarly modules discard this memory after initialization." So after the module is loaded and the hierarchy is created, nothing will stay in memory (except the future module exit function) >> + [0]{ .name = "rk3399", >> + .type = DTPM_NODE_VIRTUAL }, >> + [1]{ .name = "package", >> + .type = DTPM_NODE_VIRTUAL, >> + .parent = &rk3399_hierarchy[0] }, >> + [2]{ .name = "/cpus/cpu@0", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [3]{ .name = "/cpus/cpu@1", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [4]{ .name = "/cpus/cpu@2", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [5]{ .name = "/cpus/cpu@3", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [6]{ .name = "/cpus/cpu@100", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [7]{ .name = "/cpus/cpu@101", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [8]{ .name = "/gpu@ff9a0000", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [9]{ }, > > hmm, do we want a "/* sentinel */" inside the empty last entry? > I think that is pretty common to denote the "this one is the last entry" > of a dynamic list ;-) Sure >> +}; >> + >> +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { >> + { .compatible = "rockchip,rk3399", .data = rk3399_hierarchy }, >> + {}, >> +}; >> + >> +static int __init rockchip_dtpm_init(void) >> +{ >> + return dtpm_create_hierarchy(rockchip_dtpm_match_table); >> +} >> +module_init(rockchip_dtpm_init); > > Just for my understanding what happens on driver unload? ATM it is not possible to unload it. A second series with the hierarchy destruction will follow once this series is merged. The module unloading will be added here.
Am Dienstag, 25. Januar 2022, 18:18:09 CET schrieb Daniel Lezcano: > The DTPM framework does support now the hierarchy description. > > The platform specific code can call the hierarchy creation function > with an array of struct dtpm_node pointing to their parent. > > This patch provides a description of the big / Little CPUs and the > GPU and tie them together under a virtual 'package' name. Only rk3399 is > described now. > > The description could be extended in the future with the memory > controller with devfreq. > > The description is always a module and it describes the soft > dependencies. The userspace has to load the softdeps module in the > right order. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Reviewed-by; Heiko Stuebner <heiko@sntech.de>
diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig index 25eb2c1e31bb..6dc017f02431 100644 --- a/drivers/soc/rockchip/Kconfig +++ b/drivers/soc/rockchip/Kconfig @@ -34,4 +34,12 @@ config ROCKCHIP_PM_DOMAINS If unsure, say N. +config ROCKCHIP_DTPM + tristate "Rockchip DTPM hierarchy" + depends on DTPM && DRM_PANFROST && m + help + Describe the hierarchy for the Dynamic Thermal Power + Management tree on this platform. That will create all the + power capping capable devices. + endif diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile index 875032f7344e..05f31a4e743c 100644 --- a/drivers/soc/rockchip/Makefile +++ b/drivers/soc/rockchip/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_ROCKCHIP_GRF) += grf.o obj-$(CONFIG_ROCKCHIP_IODOMAIN) += io-domain.o obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o +obj-$(CONFIG_ROCKCHIP_DTPM) += dtpm.o diff --git a/drivers/soc/rockchip/dtpm.c b/drivers/soc/rockchip/dtpm.c new file mode 100644 index 000000000000..0b73a9cba954 --- /dev/null +++ b/drivers/soc/rockchip/dtpm.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright 2021 Linaro Limited + * + * Author: Daniel Lezcano <daniel.lezcano@linaro.org> + * + * DTPM hierarchy description + */ +#include <linux/dtpm.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> + +static struct dtpm_node __initdata rk3399_hierarchy[] = { + [0]{ .name = "rk3399", + .type = DTPM_NODE_VIRTUAL }, + [1]{ .name = "package", + .type = DTPM_NODE_VIRTUAL, + .parent = &rk3399_hierarchy[0] }, + [2]{ .name = "/cpus/cpu@0", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [3]{ .name = "/cpus/cpu@1", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [4]{ .name = "/cpus/cpu@2", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [5]{ .name = "/cpus/cpu@3", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [6]{ .name = "/cpus/cpu@100", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [7]{ .name = "/cpus/cpu@101", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [8]{ .name = "/gpu@ff9a0000", + .type = DTPM_NODE_DT, + .parent = &rk3399_hierarchy[1] }, + [9]{ }, +}; + +static struct of_device_id __initdata rockchip_dtpm_match_table[] = { + { .compatible = "rockchip,rk3399", .data = rk3399_hierarchy }, + {}, +}; + +static int __init rockchip_dtpm_init(void) +{ + return dtpm_create_hierarchy(rockchip_dtpm_match_table); +} +module_init(rockchip_dtpm_init); + +MODULE_SOFTDEP("pre: panfrost cpufreq-dt"); +MODULE_DESCRIPTION("Rockchip DTPM driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:dtpm"); +MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@kernel.org");
The DTPM framework does support now the hierarchy description. The platform specific code can call the hierarchy creation function with an array of struct dtpm_node pointing to their parent. This patch provides a description of the big / Little CPUs and the GPU and tie them together under a virtual 'package' name. Only rk3399 is described now. The description could be extended in the future with the memory controller with devfreq. The description is always a module and it describes the soft dependencies. The userspace has to load the softdeps module in the right order. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/soc/rockchip/Kconfig | 8 +++++ drivers/soc/rockchip/Makefile | 1 + drivers/soc/rockchip/dtpm.c | 59 +++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 drivers/soc/rockchip/dtpm.c