[PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[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 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]{ },
+};
+
+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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[Ulf Hansson]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[Geert Uytterhoeven]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[Daniel Lezcano]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 5/6] rockchip/soc/drivers: Add DTPM description for rk3399

[Daniel Lezcano]

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?


-- 
<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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel