Messages in this thread | | | Subject | Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399 | From | Daniel Lezcano <> | Date | Wed, 5 Jan 2022 10:21:58 +0100 |
| |
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?
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |