| Message ID | 20211218130014.4037640-6-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Hi Daniel, Rob, On Sat, 18 Dec 2021 at 14:00, 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 and 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 if it has the energy information. > > The hierarchy uses the GPU devfreq with the panfrost driver, and this > one could be loaded as a module. If the hierarchy is created before > the panfrost driver is loaded, it will fail. For this reason the > Kconfig option depends on the panfrost Kconfig's option. If this one > is compiled as a module, automatically the dtpm hierarchy code will be > a module also. Module loading ordering will fix this chicken-egg > problem. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/soc/rockchip/Kconfig | 8 +++++ > drivers/soc/rockchip/Makefile | 1 + > drivers/soc/rockchip/dtpm.c | 56 +++++++++++++++++++++++++++++++++++ > 3 files changed, 65 insertions(+) > create mode 100644 drivers/soc/rockchip/dtpm.c > > diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig > index 25eb2c1e31bb..a88fe6d3064a 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 > + 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..77edc565c110 > --- /dev/null > +++ b/drivers/soc/rockchip/dtpm.c > @@ -0,0 +1,56 @@ > +// 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" }, > + [1]{ .name = "package", > + .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 = "rockchip,rk3399-mali", > + .type = DTPM_NODE_DT, > + .parent = &rk3399_hierarchy[1] }, > + [9]{ }, > +}; I will not object to this, as in the end this seems like what we need to do, unless we can describe things through generic DT bindings for DTPM. Right? Although, if the above is correct, I need to stress that I am kind of worried that this doesn't really scale. We would need to copy lots of information from the DTS files into platform specific c-files, to be able to describe the DTPM hierarchy. > + > +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); > +} > +late_initcall(rockchip_dtpm_init); > + > +MODULE_DESCRIPTION("Rockchip DTPM driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:dtpm"); > +MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@kernel.org"); > -- > 2.25.1 > Kind regards Uffe
On Fri, Dec 31, 2021 at 2:58 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > On Sat, 18 Dec 2021 at 14:00, 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 and 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 if it has the energy information. > > > > The hierarchy uses the GPU devfreq with the panfrost driver, and this > > one could be loaded as a module. If the hierarchy is created before > > the panfrost driver is loaded, it will fail. For this reason the > > Kconfig option depends on the panfrost Kconfig's option. If this one > > is compiled as a module, automatically the dtpm hierarchy code will be > > a module also. Module loading ordering will fix this chicken-egg > > problem. > > > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > --- /dev/null > > +++ b/drivers/soc/rockchip/dtpm.c > > @@ -0,0 +1,56 @@ > > +// 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" }, > > + [1]{ .name = "package", > > + .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 = "rockchip,rk3399-mali", > > + .type = DTPM_NODE_DT, > > + .parent = &rk3399_hierarchy[1] }, > > + [9]{ }, > > +}; > > I will not object to this, as in the end this seems like what we need > to do, unless we can describe things through generic DT bindings for > DTPM. Right? > > Although, if the above is correct, I need to stress that I am kind of > worried that this doesn't really scale. We would need to copy lots of > information from the DTS files into platform specific c-files, to be > able to describe the DTPM hierarchy. The description in rk3399_hierarchy[] looks fairly similar to a power-domains hierarchy, like we have in e.g. the various drivers/soc/renesas/r8*-sysc.c files. One big difference is that the latter do not hardcode the node paths in the driver, but use power domain indices, referenced from DT in power-domains properties. Perhaps a similar approach can be used for DTPM? Does DTPM differ a lot from PM Domains? If not, perhaps no new properties are needed, and power-domains/#power-domain-cells can be used as is? 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
Hi Geert, thanks for your feedback On 04/01/2022 10:29, Geert Uytterhoeven wrote: > On Fri, Dec 31, 2021 at 2:58 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On Sat, 18 Dec 2021 at 14:00, 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 and 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 if it has the energy information. >>> >>> The hierarchy uses the GPU devfreq with the panfrost driver, and this >>> one could be loaded as a module. If the hierarchy is created before >>> the panfrost driver is loaded, it will fail. For this reason the >>> Kconfig option depends on the panfrost Kconfig's option. If this one >>> is compiled as a module, automatically the dtpm hierarchy code will be >>> a module also. Module loading ordering will fix this chicken-egg >>> problem. >>> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > >>> --- /dev/null >>> +++ b/drivers/soc/rockchip/dtpm.c >>> @@ -0,0 +1,56 @@ >>> +// 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" }, >>> + [1]{ .name = "package", >>> + .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 = "rockchip,rk3399-mali", >>> + .type = DTPM_NODE_DT, >>> + .parent = &rk3399_hierarchy[1] }, >>> + [9]{ }, >>> +}; >> >> I will not object to this, as in the end this seems like what we need >> to do, unless we can describe things through generic DT bindings for >> DTPM. Right? >> >> Although, if the above is correct, I need to stress that I am kind of >> worried that this doesn't really scale. We would need to copy lots of >> information from the DTS files into platform specific c-files, to be >> able to describe the DTPM hierarchy. > > The description in rk3399_hierarchy[] looks fairly similar to a > power-domains hierarchy, like we have in e.g. the various > drivers/soc/renesas/r8*-sysc.c files. One big difference is that the > latter do not hardcode the node paths in the driver, but use power > domain indices, referenced from DT in power-domains properties. > > Perhaps a similar approach can be used for DTPM? > Does DTPM differ a lot from PM Domains? Yes they differ. A DTPM node is a powerzone, a place where we can get and set the power. That is the reason why initially a separate binding was proposed. > If not, perhaps no new > properties are needed, and power-domains/#power-domain-cells can be > used as is?
On 31/12/2021 14:57, Ulf Hansson wrote: [ ... ] >> +static struct dtpm_node __initdata rk3399_hierarchy[] = { >> + [0]{ .name = "rk3399" }, >> + [1]{ .name = "package", >> + .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 = "rockchip,rk3399-mali", >> + .type = DTPM_NODE_DT, >> + .parent = &rk3399_hierarchy[1] }, >> + [9]{ }, >> +}; > > I will not object to this, as in the end this seems like what we need > to do, unless we can describe things through generic DT bindings for > DTPM. Right? Yes, as asked by Rob, we should try to describe in the kernel first. > Although, if the above is correct, I need to stress that I am kind of > worried that this doesn't really scale. We would need to copy lots of > information from the DTS files into platform specific c-files, to be > able to describe the DTPM hierarchy. TBH I don't think it is a lot and it is a __initdata. At least we should begin with something and see later how to consolidate if it is needed, no?
diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig index 25eb2c1e31bb..a88fe6d3064a 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 + 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..77edc565c110 --- /dev/null +++ b/drivers/soc/rockchip/dtpm.c @@ -0,0 +1,56 @@ +// 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" }, + [1]{ .name = "package", + .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 = "rockchip,rk3399-mali", + .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); +} +late_initcall(rockchip_dtpm_init); + +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 and 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 if it has the energy information. The hierarchy uses the GPU devfreq with the panfrost driver, and this one could be loaded as a module. If the hierarchy is created before the panfrost driver is loaded, it will fail. For this reason the Kconfig option depends on the panfrost Kconfig's option. If this one is compiled as a module, automatically the dtpm hierarchy code will be a module also. Module loading ordering will fix this chicken-egg problem. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/soc/rockchip/Kconfig | 8 +++++ drivers/soc/rockchip/Makefile | 1 + drivers/soc/rockchip/dtpm.c | 56 +++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/soc/rockchip/dtpm.c