[PATCH 0/5] arm64: sunxi: h616: Enable Mali GPU

[PATCH 0/5] arm64: sunxi: h616: Enable Mali GPU

[Andre Przywara]

The Allwinner H616/H618/H313/H700 SoCs contain a Mali G32 MP2 GPU. This
IP is from the Bifrost family and is already supported by the panfrost
driver, so enabling support for 3D graphics on this SoC is rather
straight-forward.
However Allwinner added some bits in the PRCM block, that control the
power domain for the GPU - on top of its power *supply*.

This series enables the Mali GPU on those SoCs, by first introducing a
power domain driver for that SoC (patch 1/5: DT binding, patch 2/5:
the actual driver). For the Mali GPU to work we literally need to flip a
single bit (the BSP does this in the bootloader), and this full featured
power domain driver is admittedly a bit over the top for that purpose.
However it seems to be the right thing to do architecturally, and while
at it I added the other power domains (for analogue, PLLs, and the
management core), even though we won't use them in Linux and they would
be always on. I have a simpler version of the driver which just covers
this single bit controlling the GPU, please let me know if you prefer
that.
Please also note that this supersedes an RFC patch I sent a year ago,
which included this power domain in the R-CCU driver:
https://lore.kernel.org/linux-sunxi/20240225160616.15001-1-andre.przywara@arm.com/T/#u

The rest of the patches enable the Mali GPU on the DT side: patch 3/5
adds the compatible string to the Mali DT binding, while patch 4/5 adds
the purely SoC specific DT nodes, for both the power domain and the Mali
GPU. The final patch 5/5 then enables the GPU on all existing H616-family
boards.

There seems to be an existing problem with powering up the GPU, after it
has been turned off by the kernel. Chances are this is a problem with the
proper power-up (or power-down) sequence, where clock gates, reset lines
and the power domain must be asserted in a specific order.
A workaround used so far downstream is to keep the power domain enabled,
by ignoring the power-off request. Observing any assumed "proper" sequence
is a bit more tricky, since there is no Allwinner specific glue driver
or anything, so things would need be changed in the generic panfrost
code, where they have the potential of breaking other Mali users.
I would be interested in hearing opinions about this.

Cheers,
Andre

Andre Przywara (5):
  dt-bindings: power: Add Allwinner H6/H616 PRCM PPU
  pmdomain: sunxi: add H6 PRCM PPU driver
  dt-bindings: gpu: mali-bifrost: Add Allwinner H616 compatible
  arm64: dts: allwinner: h616: Add Mali GPU node
  arm64: dts: allwinner: h616: enable Mali GPU for all boards

 .../bindings/gpu/arm,mali-bifrost.yaml        |   1 +
 .../power/allwinner,sun50i-h6-prcm-ppu.yaml   |  42 ++++
 .../dts/allwinner/sun50i-h313-tanix-tx1.dts   |   5 +
 .../sun50i-h616-bigtreetech-cb1.dtsi          |   5 +
 .../allwinner/sun50i-h616-orangepi-zero.dtsi  |   4 +
 .../allwinner/sun50i-h616-orangepi-zero2.dts  |   4 +
 .../dts/allwinner/sun50i-h616-x96-mate.dts    |   5 +
 .../arm64/boot/dts/allwinner/sun50i-h616.dtsi |  21 ++
 .../sun50i-h618-longan-module-3h.dtsi         |   5 +
 .../allwinner/sun50i-h618-orangepi-zero2w.dts |   5 +
 .../allwinner/sun50i-h618-orangepi-zero3.dts  |   4 +
 .../sun50i-h618-transpeed-8k618-t.dts         |   5 +
 .../sun50i-h618-yuzukihd-chameleon.dts        |   5 +
 .../sun50i-h700-anbernic-rg35xx-2024.dts      |   5 +
 drivers/pmdomain/sunxi/Kconfig                |  10 +
 drivers/pmdomain/sunxi/Makefile               |   1 +
 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c   | 191 ++++++++++++++++++
 17 files changed, 318 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml
 create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c

-- 
2.46.3

[PATCH 1/5] dt-bindings: power: Add Allwinner H6/H616 PRCM PPU

[Andre Przywara]

The Allwinner H6 and some later SoCs contain some bits in the PRCM (Power
Reset Clock Management) block that control some power domains.
Those power domains include the one for the GPU, the PLLs and some
analogue circuits.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../power/allwinner,sun50i-h6-prcm-ppu.yaml   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml

diff --git a/Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml b/Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml
new file mode 100644
index 0000000000000..7eaff9baf7268
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/allwinner,sun50i-h6-prcm-ppu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner SoCs PRCM power domain controller
+
+maintainers:
+  - Andre Przywara <andre.przywara@arm.com>
+
+description:
+  The Allwinner Power Reset Clock Management (PRCM) unit contains bits to
+  control a few power domains.
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun50i-h6-prcm-ppu
+      - allwinner,sun50i-h616-prcm-ppu
+      - allwinner,sun55i-a523-prcm-ppu
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#power-domain-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    prcm_ppu: power-controller@7010210 {
+        compatible = "allwinner,sun50i-h616-prcm-ppu";
+        reg = <0x07010250 0x10>;
+        #power-domain-cells = <1>;
+    };
-- 
2.46.3

[PATCH 2/5] pmdomain: sunxi: add H6 PRCM PPU driver

[Andre Przywara]

The Allwinner Power Reset Clock Management (RPCM) block contains a few
bits that control some power domains. The most prominent one is the one
for the Mali GPU. On the Allwinner H6 this domain is enabled at reset, so
we didn't care about it so far, but the H616 defaults to it being disabled.

Add a power domain driver for those bits. Some BSP code snippets and
some spare documentation describe three bits, slightly different between
the H6 and H616, so add three power domains for each SoC, connected to
their compatible string.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pmdomain/sunxi/Kconfig              |  10 +
 drivers/pmdomain/sunxi/Makefile             |   1 +
 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c | 191 ++++++++++++++++++++
 3 files changed, 202 insertions(+)
 create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c

diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
index 17781bf8d86d7..43eecb3ea9819 100644
--- a/drivers/pmdomain/sunxi/Kconfig
+++ b/drivers/pmdomain/sunxi/Kconfig
@@ -8,3 +8,13 @@ config SUN20I_PPU
 	help
 	  Say y to enable the PPU power domain driver. This saves power
 	  when certain peripherals, such as the video engine, are idle.
+
+config SUN50I_H6_PRCM_PPU
+	tristate "Allwinner H6 PRCM power domain driver"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	depends on PM
+	select PM_GENERIC_DOMAINS
+	help
+	  Say y to enable the Allwinner H6/H616 PRCM power domain driver.
+	  This is required to enable the Mali GPU in the H616 SoC, it is
+	  optional for the H6.
diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
index ec1d7a2fb21db..c1343e1237599 100644
--- a/drivers/pmdomain/sunxi/Makefile
+++ b/drivers/pmdomain/sunxi/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_SUN20I_PPU)		+= sun20i-ppu.o
+obj-$(CONFIG_SUN50I_H6_PRCM_PPU)	+= sun50i-h6-prcm-ppu.o
diff --git a/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
new file mode 100644
index 0000000000000..1c6b0c78b222d
--- /dev/null
+++ b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) Arm Ltd. 2024
+ *
+ * Allwinner H6/H616 PRCM power domain driver.
+ * This covers a few registers inside the PRCM (Power Reset Clock Management)
+ * block that control some power rails, most prominently for the Mali GPU.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/reset.h>
+
+/*
+ * The PRCM block covers multiple devices, starting with some clocks,
+ * then followed by the power rails.
+ * The clocks are covered by a different driver, so this driver's MMIO range
+ * starts later in the PRCM MMIO frame, not at the beginning of it.
+ * To keep the register offsets consistent with other PRCM documentation,
+ * express the registers relative to the beginning of the whole PRCM, and
+ * subtract the PPU offset this driver is bound to.
+ */
+#define PD_H6_PPU_OFFSET		0x250
+#define PD_H6_VDD_SYS_REG		0x250
+#define PD_H616_ANA_VDD_GATE			BIT(4)
+#define PD_H6_CPUS_VDD_GATE			BIT(3)
+#define PD_H6_AVCC_VDD_GATE			BIT(2)
+#define PD_H6_GPU_REG			0x254
+#define PD_H6_GPU_GATE				BIT(0)
+
+struct sun50i_h6_ppu_pd {
+	struct generic_pm_domain	genpd;
+	void __iomem			*reg;
+	u32				gate_mask;
+	bool				negated;
+};
+
+#define FLAG_PPU_ALWAYS_ON	BIT(0)
+#define FLAG_PPU_NEGATED	BIT(1)
+
+struct sun50i_h6_ppu_desc {
+	const char *name;
+	u32 offset;
+	u32 mask;
+	unsigned int flags;
+};
+
+struct sun50i_h6_ppu_desc sun50i_h6_ppus[] = {
+	{ "AVCC", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE },
+	{ "CPUS", PD_H6_VDD_SYS_REG, PD_H6_CPUS_VDD_GATE },
+	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE },
+	{}
+};
+
+struct sun50i_h6_ppu_desc sun50i_h616_ppus[] = {
+	{ "PLL", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE,
+		FLAG_PPU_ALWAYS_ON | FLAG_PPU_NEGATED },
+	{ "ANA", PD_H6_VDD_SYS_REG, PD_H616_ANA_VDD_GATE, FLAG_PPU_ALWAYS_ON },
+	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE, FLAG_PPU_NEGATED },
+	{}
+};
+#define to_sun50i_h6_ppu_pd(_genpd) \
+	container_of(_genpd, struct sun50i_h6_ppu_pd, genpd)
+
+static bool sun50i_h6_ppu_power_status(const struct sun50i_h6_ppu_pd *pd)
+{
+	bool bit = readl(pd->reg) & pd->gate_mask;
+
+	return bit ^ pd->negated;
+}
+
+static int sun50i_h6_ppu_pd_set_power(const struct sun50i_h6_ppu_pd *pd,
+				      bool set_bit)
+{
+	u32 reg = readl(pd->reg);
+
+	if (set_bit)
+		writel(reg | pd->gate_mask, pd->reg);
+	else
+		writel(reg & ~pd->gate_mask, pd->reg);
+
+	return 0;
+}
+
+static int sun50i_h6_ppu_pd_power_on(struct generic_pm_domain *genpd)
+{
+	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
+
+	return sun50i_h6_ppu_pd_set_power(pd, !pd->negated);
+}
+
+static int sun50i_h6_ppu_pd_power_off(struct generic_pm_domain *genpd)
+{
+	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
+
+	return sun50i_h6_ppu_pd_set_power(pd, pd->negated);
+}
+
+static int sun50i_h6_ppu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct genpd_onecell_data *ppu;
+	struct sun50i_h6_ppu_pd *pds;
+	const struct sun50i_h6_ppu_desc *desc;
+	void __iomem *base;
+	int ret, i, count;
+
+	desc = of_device_get_match_data(dev);
+	if (!desc)
+		return -EINVAL;
+
+	for (count = 0; desc[count].name; count++)
+		;
+
+	pds = devm_kcalloc(dev, count, sizeof(*pds), GFP_KERNEL);
+	if (!pds)
+		return -ENOMEM;
+
+	ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
+	if (!ppu)
+		return -ENOMEM;
+
+	ppu->num_domains = count;
+	ppu->domains = devm_kcalloc(dev, count, sizeof(*ppu->domains),
+				    GFP_KERNEL);
+	if (!ppu->domains)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ppu);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	for (i = 0; i < count; i++) {
+		struct sun50i_h6_ppu_pd *pd = &pds[i];
+
+		pd->genpd.name		= desc[i].name;
+		pd->genpd.power_off	= sun50i_h6_ppu_pd_power_off;
+		pd->genpd.power_on	= sun50i_h6_ppu_pd_power_on;
+		if (desc[i].flags & FLAG_PPU_ALWAYS_ON)
+			pd->genpd.flags = GENPD_FLAG_ALWAYS_ON;
+		pd->negated		= !!(desc[i].flags & FLAG_PPU_NEGATED);
+		pd->reg			= base + desc[i].offset - PD_H6_PPU_OFFSET;
+		pd->gate_mask		= desc[i].mask;
+
+		ret = pm_genpd_init(&pd->genpd, NULL,
+				    !sun50i_h6_ppu_power_status(pd));
+		if (ret) {
+			dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);
+			return ret;
+		}
+		ppu->domains[i] = &pd->genpd;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
+	if (ret)
+		dev_warn(dev, "Failed to add provider: %d\n", ret);
+
+	return 0;
+}
+
+static const struct of_device_id sun50i_h6_ppu_of_match[] = {
+	{ .compatible	= "allwinner,sun50i-h6-prcm-ppu",
+	  .data		= &sun50i_h6_ppus },
+	{ .compatible	= "allwinner,sun50i-h616-prcm-ppu",
+	  .data		= &sun50i_h616_ppus },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sun50i_h6_ppu_of_match);
+
+static struct platform_driver sun50i_h6_ppu_driver = {
+	.probe	= sun50i_h6_ppu_probe,
+	.driver	= {
+		.name			= "sun50i-h6-prcm-ppu",
+		.of_match_table		= sun50i_h6_ppu_of_match,
+		/* Power domains cannot be removed while they are in use. */
+		.suppress_bind_attrs	= true,
+	},
+};
+module_platform_driver(sun50i_h6_ppu_driver);
+
+MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
+MODULE_DESCRIPTION("Allwinner H6 PRCM power domain driver");
+MODULE_LICENSE("GPL");
-- 
2.46.3

[PATCH 3/5] dt-bindings: gpu: mali-bifrost: Add Allwinner H616 compatible

[Andre Przywara]

The Allwinner H616 SoC has a Mali-G31 MP2 GPU, which is of the Mali
Bifrost family.
Add the SoC specific compatible string and pair it with the bifrost
fallback compatible.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
index 735c7f06c24e6..439d5c59daa28 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
@@ -17,6 +17,7 @@ properties:
     oneOf:
       - items:
           - enum:
+              - allwinner,sun50i-h616-mali
               - amlogic,meson-g12a-mali
               - mediatek,mt8183-mali
               - mediatek,mt8183b-mali
-- 
2.46.3

[PATCH 4/5] arm64: dts: allwinner: h616: Add Mali GPU node

[Andre Przywara]

The Allwinner H616 SoC contains a Mali-G31 MP2 GPU, which is of the Mali
Bifrost family. There is a power domain specifically for that GPU, which
needs to be enabled to make use of the it.

Add the DT nodes for those two devices, and link them together through
the "power-domains" property.
Any board wishing to use the GPU would need to enable the GPU node and
specify the "mali-supply" regulator.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
index cdce3dcb8ec02..ceedae9e399b6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
@@ -150,6 +150,21 @@ soc {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x40000000>;
 
+		gpu: gpu@1800000 {
+			compatible = "allwinner,sun50i-h616-mali",
+				     "arm,mali-bifrost";
+			reg = <0x1800000 0x40000>;
+			interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "job", "mmu", "gpu";
+			clocks = <&ccu CLK_GPU0>, <&ccu CLK_BUS_GPU>;
+			clock-names = "core", "bus";
+			power-domains = <&prcm_ppu 2>;
+			resets = <&ccu RST_BUS_GPU>;
+			status = "disabled";
+		};
+
 		crypto: crypto@1904000 {
 			compatible = "allwinner,sun50i-h616-crypto";
 			reg = <0x01904000 0x800>;
@@ -874,6 +889,12 @@ r_ccu: clock@7010000 {
 			#reset-cells = <1>;
 		};
 
+		prcm_ppu: power-controller@7010250 {
+			compatible = "allwinner,sun50i-h616-prcm-ppu";
+			reg = <0x07010250 0x10>;
+			#power-domain-cells = <1>;
+		};
+
 		nmi_intc: interrupt-controller@7010320 {
 			compatible = "allwinner,sun50i-h616-nmi",
 				     "allwinner,sun9i-a80-nmi";
-- 
2.46.3

[PATCH 5/5] arm64: dts: allwinner: h616: enable Mali GPU for all boards

[Andre Przywara]

All Allwinner H616/H618 SoCs contain a Mali G31 MP2 GPU.

Enable the DT nodes for that GPU, and specify the regulator providing
power to the VDD_GPU pins of the package. The rest of the DT node is set
by the SoC, so is not board specific.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h313-tanix-tx1.dts      | 5 +++++
 .../boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi      | 5 +++++
 arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero.dtsi | 4 ++++
 arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts | 4 ++++
 arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts       | 5 +++++
 .../boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi     | 5 +++++
 .../arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts | 5 +++++
 arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts | 4 ++++
 .../boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts     | 5 +++++
 .../boot/dts/allwinner/sun50i-h618-yuzukihd-chameleon.dts    | 5 +++++
 .../boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts  | 5 +++++
 11 files changed, 52 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h313-tanix-tx1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h313-tanix-tx1.dts
index 17e6aef67aaf9..7906b79c03898 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h313-tanix-tx1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h313-tanix-tx1.dts
@@ -79,6 +79,11 @@ &ehci0 {
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &ir {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
index d12b01c5f41b6..bebfeb2a337a3 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
@@ -67,6 +67,11 @@ &cpu0 {
 	cpu-supply = <&reg_dcdc2>;
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_dldo1>;
 	/* Card detection pin is not connected */
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero.dtsi
index 908fa3b847a66..a8644fb52b04e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero.dtsi
@@ -77,6 +77,10 @@ &emac0 {
 	status = "okay";
 };
 
+&gpu {
+	status = "okay";
+};
+
 &mdio0 {
 	ext_rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts
index a360d8567f955..f2e3300e078a9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-orangepi-zero2.dts
@@ -24,6 +24,10 @@ &emac0 {
 	phy-supply = <&reg_dcdce>;
 };
 
+&gpu {
+	mali-supply = <&reg_dcdcc>;
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_dcdce>;
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts
index 968960ebf1d18..085f3e4e8eaa8 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-x96-mate.dts
@@ -50,6 +50,11 @@ &ehci2 {
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&reg_dcdcc>;
+	status = "okay";
+};
+
 &ir {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
index e92d150aaf1c1..3f416d129b727 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
@@ -10,6 +10,11 @@ &cpu0 {
 	cpu-supply = <&reg_dcdc2>;
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &mmc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc2_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts
index a0fe7a9afb77c..b340bbcb710de 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero2w.dts
@@ -69,6 +69,11 @@ &ehci1 {
 
 /* USB 2 & 3 are on the FPC connector (or the exansion board) */
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &mmc0 {
 	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;	/* PF6 */
 	bus-width = <4>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts
index e1cd7572a14ce..c51d4d9120dee 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-orangepi-zero3.dts
@@ -27,6 +27,10 @@ &ext_rgmii_phy {
 	motorcomm,clk-out-frequency-hz = <125000000>;
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+};
+
 &mmc0 {
 	/*
 	 * The schematic shows the card detect pin wired up to PF6, via an
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
index f828ca1ce51ef..efe0faa252f5e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
@@ -69,6 +69,11 @@ &ehci1 {
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &ir {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-yuzukihd-chameleon.dts b/arch/arm64/boot/dts/allwinner/sun50i-h618-yuzukihd-chameleon.dts
index 747242becbb6e..d6768455404ee 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h618-yuzukihd-chameleon.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-yuzukihd-chameleon.dts
@@ -68,6 +68,11 @@ &ehci3 {
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc1>;
+	status = "okay";
+};
+
 &mmc0 {
 	bus-width = <4>;
 	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;	/* PF6 */
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
index a231abf1684ad..4a7f382e0e722 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
@@ -191,6 +191,11 @@ &ehci0 {
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&reg_dcdc2>;
+	status = "okay";
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_cldo3>;
 	disable-wp;
-- 
2.46.3

Re: [PATCH 2/5] pmdomain: sunxi: add H6 PRCM PPU driver

[Jernej Škrabec]

Dne petek, 21. februar 2025 ob 01:57:59 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> The Allwinner Power Reset Clock Management (RPCM) block contains a few
> bits that control some power domains. The most prominent one is the one
> for the Mali GPU. On the Allwinner H6 this domain is enabled at reset, so
> we didn't care about it so far, but the H616 defaults to it being disabled.
> 
> Add a power domain driver for those bits. Some BSP code snippets and
> some spare documentation describe three bits, slightly different between
> the H6 and H616, so add three power domains for each SoC, connected to
> their compatible string.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pmdomain/sunxi/Kconfig              |  10 +
>  drivers/pmdomain/sunxi/Makefile             |   1 +
>  drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c | 191 ++++++++++++++++++++
>  3 files changed, 202 insertions(+)
>  create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> 
> diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> index 17781bf8d86d7..43eecb3ea9819 100644
> --- a/drivers/pmdomain/sunxi/Kconfig
> +++ b/drivers/pmdomain/sunxi/Kconfig
> @@ -8,3 +8,13 @@ config SUN20I_PPU
>  	help
>  	  Say y to enable the PPU power domain driver. This saves power
>  	  when certain peripherals, such as the video engine, are idle.
> +
> +config SUN50I_H6_PRCM_PPU
> +	tristate "Allwinner H6 PRCM power domain driver"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	depends on PM
> +	select PM_GENERIC_DOMAINS
> +	help
> +	  Say y to enable the Allwinner H6/H616 PRCM power domain driver.
> +	  This is required to enable the Mali GPU in the H616 SoC, it is
> +	  optional for the H6.
> diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> index ec1d7a2fb21db..c1343e1237599 100644
> --- a/drivers/pmdomain/sunxi/Makefile
> +++ b/drivers/pmdomain/sunxi/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_SUN20I_PPU)		+= sun20i-ppu.o
> +obj-$(CONFIG_SUN50I_H6_PRCM_PPU)	+= sun50i-h6-prcm-ppu.o
> diff --git a/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> new file mode 100644
> index 0000000000000..1c6b0c78b222d
> --- /dev/null
> +++ b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) Arm Ltd. 2024
> + *
> + * Allwinner H6/H616 PRCM power domain driver.
> + * This covers a few registers inside the PRCM (Power Reset Clock Management)
> + * block that control some power rails, most prominently for the Mali GPU.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/reset.h>
> +
> +/*
> + * The PRCM block covers multiple devices, starting with some clocks,
> + * then followed by the power rails.
> + * The clocks are covered by a different driver, so this driver's MMIO range
> + * starts later in the PRCM MMIO frame, not at the beginning of it.
> + * To keep the register offsets consistent with other PRCM documentation,
> + * express the registers relative to the beginning of the whole PRCM, and
> + * subtract the PPU offset this driver is bound to.
> + */
> +#define PD_H6_PPU_OFFSET		0x250
> +#define PD_H6_VDD_SYS_REG		0x250
> +#define PD_H616_ANA_VDD_GATE			BIT(4)
> +#define PD_H6_CPUS_VDD_GATE			BIT(3)
> +#define PD_H6_AVCC_VDD_GATE			BIT(2)
> +#define PD_H6_GPU_REG			0x254
> +#define PD_H6_GPU_GATE				BIT(0)
> +
> +struct sun50i_h6_ppu_pd {
> +	struct generic_pm_domain	genpd;
> +	void __iomem			*reg;
> +	u32				gate_mask;
> +	bool				negated;
> +};
> +
> +#define FLAG_PPU_ALWAYS_ON	BIT(0)
> +#define FLAG_PPU_NEGATED	BIT(1)
> +
> +struct sun50i_h6_ppu_desc {
> +	const char *name;
> +	u32 offset;
> +	u32 mask;
> +	unsigned int flags;
> +};
> +
> +struct sun50i_h6_ppu_desc sun50i_h6_ppus[] = {
> +	{ "AVCC", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE },
> +	{ "CPUS", PD_H6_VDD_SYS_REG, PD_H6_CPUS_VDD_GATE },
> +	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE },
> +	{}
> +};
> +
> +struct sun50i_h6_ppu_desc sun50i_h616_ppus[] = {
> +	{ "PLL", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE,
> +		FLAG_PPU_ALWAYS_ON | FLAG_PPU_NEGATED },
> +	{ "ANA", PD_H6_VDD_SYS_REG, PD_H616_ANA_VDD_GATE, FLAG_PPU_ALWAYS_ON },
> +	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE, FLAG_PPU_NEGATED },
> +	{}
> +};
> +#define to_sun50i_h6_ppu_pd(_genpd) \
> +	container_of(_genpd, struct sun50i_h6_ppu_pd, genpd)
> +
> +static bool sun50i_h6_ppu_power_status(const struct sun50i_h6_ppu_pd *pd)
> +{
> +	bool bit = readl(pd->reg) & pd->gate_mask;
> +
> +	return bit ^ pd->negated;
> +}
> +
> +static int sun50i_h6_ppu_pd_set_power(const struct sun50i_h6_ppu_pd *pd,
> +				      bool set_bit)
> +{
> +	u32 reg = readl(pd->reg);
> +
> +	if (set_bit)
> +		writel(reg | pd->gate_mask, pd->reg);
> +	else
> +		writel(reg & ~pd->gate_mask, pd->reg);
> +
> +	return 0;
> +}
> +
> +static int sun50i_h6_ppu_pd_power_on(struct generic_pm_domain *genpd)
> +{
> +	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> +
> +	return sun50i_h6_ppu_pd_set_power(pd, !pd->negated);
> +}
> +
> +static int sun50i_h6_ppu_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> +
> +	return sun50i_h6_ppu_pd_set_power(pd, pd->negated);
> +}
> +
> +static int sun50i_h6_ppu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct genpd_onecell_data *ppu;
> +	struct sun50i_h6_ppu_pd *pds;
> +	const struct sun50i_h6_ppu_desc *desc;
> +	void __iomem *base;
> +	int ret, i, count;
> +
> +	desc = of_device_get_match_data(dev);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	for (count = 0; desc[count].name; count++)
> +		;
> +
> +	pds = devm_kcalloc(dev, count, sizeof(*pds), GFP_KERNEL);
> +	if (!pds)
> +		return -ENOMEM;
> +
> +	ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> +	if (!ppu)
> +		return -ENOMEM;
> +
> +	ppu->num_domains = count;
> +	ppu->domains = devm_kcalloc(dev, count, sizeof(*ppu->domains),
> +				    GFP_KERNEL);
> +	if (!ppu->domains)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, ppu);
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	for (i = 0; i < count; i++) {
> +		struct sun50i_h6_ppu_pd *pd = &pds[i];
> +
> +		pd->genpd.name		= desc[i].name;
> +		pd->genpd.power_off	= sun50i_h6_ppu_pd_power_off;
> +		pd->genpd.power_on	= sun50i_h6_ppu_pd_power_on;
> +		if (desc[i].flags & FLAG_PPU_ALWAYS_ON)
> +			pd->genpd.flags = GENPD_FLAG_ALWAYS_ON;
> +		pd->negated		= !!(desc[i].flags & FLAG_PPU_NEGATED);
> +		pd->reg			= base + desc[i].offset - PD_H6_PPU_OFFSET;
> +		pd->gate_mask		= desc[i].mask;
> +
> +		ret = pm_genpd_init(&pd->genpd, NULL,
> +				    !sun50i_h6_ppu_power_status(pd));
> +		if (ret) {
> +			dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);

I suppose you want to replace "GPU" with desc[i].name.

Otherwise it looks good to me, but I'd like to hear a comment from genpd maintainers.

Best regards,
Jernej

> +			return ret;
> +		}
> +		ppu->domains[i] = &pd->genpd;
> +	}
> +
> +	ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
> +	if (ret)
> +		dev_warn(dev, "Failed to add provider: %d\n", ret);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun50i_h6_ppu_of_match[] = {
> +	{ .compatible	= "allwinner,sun50i-h6-prcm-ppu",
> +	  .data		= &sun50i_h6_ppus },
> +	{ .compatible	= "allwinner,sun50i-h616-prcm-ppu",
> +	  .data		= &sun50i_h616_ppus },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sun50i_h6_ppu_of_match);
> +
> +static struct platform_driver sun50i_h6_ppu_driver = {
> +	.probe	= sun50i_h6_ppu_probe,
> +	.driver	= {
> +		.name			= "sun50i-h6-prcm-ppu",
> +		.of_match_table		= sun50i_h6_ppu_of_match,
> +		/* Power domains cannot be removed while they are in use. */
> +		.suppress_bind_attrs	= true,
> +	},
> +};
> +module_platform_driver(sun50i_h6_ppu_driver);
> +
> +MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
> +MODULE_DESCRIPTION("Allwinner H6 PRCM power domain driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 3/5] dt-bindings: gpu: mali-bifrost: Add Allwinner H616 compatible

[Jernej Škrabec]

Dne petek, 21. februar 2025 ob 01:58:00 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> The Allwinner H616 SoC has a Mali-G31 MP2 GPU, which is of the Mali
> Bifrost family.
> Add the SoC specific compatible string and pair it with the bifrost
> fallback compatible.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 4/5] arm64: dts: allwinner: h616: Add Mali GPU node

[Jernej Škrabec]

Dne petek, 21. februar 2025 ob 01:58:01 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> The Allwinner H616 SoC contains a Mali-G31 MP2 GPU, which is of the Mali
> Bifrost family. There is a power domain specifically for that GPU, which
> needs to be enabled to make use of the it.
> 
> Add the DT nodes for those two devices, and link them together through
> the "power-domains" property.
> Any board wishing to use the GPU would need to enable the GPU node and
> specify the "mali-supply" regulator.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 5/5] arm64: dts: allwinner: h616: enable Mali GPU for all boards

[Jernej Škrabec]

Dne petek, 21. februar 2025 ob 01:58:02 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> All Allwinner H616/H618 SoCs contain a Mali G31 MP2 GPU.
> 
> Enable the DT nodes for that GPU, and specify the regulator providing
> power to the VDD_GPU pins of the package. The rest of the DT node is set
> by the SoC, so is not board specific.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 1/5] dt-bindings: power: Add Allwinner H6/H616 PRCM PPU

[Rob Herring (Arm)]

On Fri, 21 Feb 2025 00:57:58 +0000, Andre Przywara wrote:
> The Allwinner H6 and some later SoCs contain some bits in the PRCM (Power
> Reset Clock Management) block that control some power domains.
> Those power domains include the one for the GPU, the PLLs and some
> analogue circuits.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../power/allwinner,sun50i-h6-prcm-ppu.yaml   | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 3/5] dt-bindings: gpu: mali-bifrost: Add Allwinner H616 compatible

[Rob Herring (Arm)]

On Fri, 21 Feb 2025 00:58:00 +0000, Andre Przywara wrote:
> The Allwinner H616 SoC has a Mali-G31 MP2 GPU, which is of the Mali
> Bifrost family.
> Add the SoC specific compatible string and pair it with the bifrost
> fallback compatible.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Applied, thanks!

Re: [PATCH 2/5] pmdomain: sunxi: add H6 PRCM PPU driver

[Ulf Hansson]

On Fri, 21 Feb 2025 at 02:00, Andre Przywara <andre.przywara@arm.com> wrote:
>
> The Allwinner Power Reset Clock Management (RPCM) block contains a few
> bits that control some power domains. The most prominent one is the one
> for the Mali GPU. On the Allwinner H6 this domain is enabled at reset, so
> we didn't care about it so far, but the H616 defaults to it being disabled.
>
> Add a power domain driver for those bits. Some BSP code snippets and
> some spare documentation describe three bits, slightly different between
> the H6 and H616, so add three power domains for each SoC, connected to
> their compatible string.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pmdomain/sunxi/Kconfig              |  10 +
>  drivers/pmdomain/sunxi/Makefile             |   1 +
>  drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c | 191 ++++++++++++++++++++
>  3 files changed, 202 insertions(+)
>  create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
>
> diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> index 17781bf8d86d7..43eecb3ea9819 100644
> --- a/drivers/pmdomain/sunxi/Kconfig
> +++ b/drivers/pmdomain/sunxi/Kconfig
> @@ -8,3 +8,13 @@ config SUN20I_PPU
>         help
>           Say y to enable the PPU power domain driver. This saves power
>           when certain peripherals, such as the video engine, are idle.
> +
> +config SUN50I_H6_PRCM_PPU
> +       tristate "Allwinner H6 PRCM power domain driver"
> +       depends on ARCH_SUNXI || COMPILE_TEST
> +       depends on PM
> +       select PM_GENERIC_DOMAINS
> +       help
> +         Say y to enable the Allwinner H6/H616 PRCM power domain driver.
> +         This is required to enable the Mali GPU in the H616 SoC, it is
> +         optional for the H6.
> diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> index ec1d7a2fb21db..c1343e1237599 100644
> --- a/drivers/pmdomain/sunxi/Makefile
> +++ b/drivers/pmdomain/sunxi/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_SUN20I_PPU)               += sun20i-ppu.o
> +obj-$(CONFIG_SUN50I_H6_PRCM_PPU)       += sun50i-h6-prcm-ppu.o
> diff --git a/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> new file mode 100644
> index 0000000000000..1c6b0c78b222d
> --- /dev/null
> +++ b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) Arm Ltd. 2024
> + *
> + * Allwinner H6/H616 PRCM power domain driver.
> + * This covers a few registers inside the PRCM (Power Reset Clock Management)
> + * block that control some power rails, most prominently for the Mali GPU.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/reset.h>
> +
> +/*
> + * The PRCM block covers multiple devices, starting with some clocks,
> + * then followed by the power rails.
> + * The clocks are covered by a different driver, so this driver's MMIO range
> + * starts later in the PRCM MMIO frame, not at the beginning of it.
> + * To keep the register offsets consistent with other PRCM documentation,
> + * express the registers relative to the beginning of the whole PRCM, and
> + * subtract the PPU offset this driver is bound to.
> + */
> +#define PD_H6_PPU_OFFSET               0x250
> +#define PD_H6_VDD_SYS_REG              0x250
> +#define PD_H616_ANA_VDD_GATE                   BIT(4)
> +#define PD_H6_CPUS_VDD_GATE                    BIT(3)
> +#define PD_H6_AVCC_VDD_GATE                    BIT(2)
> +#define PD_H6_GPU_REG                  0x254
> +#define PD_H6_GPU_GATE                         BIT(0)
> +
> +struct sun50i_h6_ppu_pd {
> +       struct generic_pm_domain        genpd;
> +       void __iomem                    *reg;
> +       u32                             gate_mask;
> +       bool                            negated;
> +};
> +
> +#define FLAG_PPU_ALWAYS_ON     BIT(0)
> +#define FLAG_PPU_NEGATED       BIT(1)
> +
> +struct sun50i_h6_ppu_desc {
> +       const char *name;
> +       u32 offset;
> +       u32 mask;
> +       unsigned int flags;
> +};
> +
> +struct sun50i_h6_ppu_desc sun50i_h6_ppus[] = {
> +       { "AVCC", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE },
> +       { "CPUS", PD_H6_VDD_SYS_REG, PD_H6_CPUS_VDD_GATE },
> +       { "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE },
> +       {}
> +};

I suggest we define a new struct (perhaps sun50_domain_data!?) that
has a struct sun50i_h6_ppu_desc* member along with a size_t member to
keep the number of the domains, set by using ARRAY_SIZE().

In this way we don't need the additional NULL domain at the end - and
we can also avoid looping through the array during probe  to count
them.

> +
> +struct sun50i_h6_ppu_desc sun50i_h616_ppus[] = {
> +       { "PLL", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE,
> +               FLAG_PPU_ALWAYS_ON | FLAG_PPU_NEGATED },
> +       { "ANA", PD_H6_VDD_SYS_REG, PD_H616_ANA_VDD_GATE, FLAG_PPU_ALWAYS_ON },
> +       { "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE, FLAG_PPU_NEGATED },
> +       {}
> +};
> +#define to_sun50i_h6_ppu_pd(_genpd) \
> +       container_of(_genpd, struct sun50i_h6_ppu_pd, genpd)
> +
> +static bool sun50i_h6_ppu_power_status(const struct sun50i_h6_ppu_pd *pd)
> +{
> +       bool bit = readl(pd->reg) & pd->gate_mask;
> +
> +       return bit ^ pd->negated;
> +}
> +
> +static int sun50i_h6_ppu_pd_set_power(const struct sun50i_h6_ppu_pd *pd,
> +                                     bool set_bit)
> +{
> +       u32 reg = readl(pd->reg);
> +
> +       if (set_bit)
> +               writel(reg | pd->gate_mask, pd->reg);
> +       else
> +               writel(reg & ~pd->gate_mask, pd->reg);
> +
> +       return 0;
> +}
> +
> +static int sun50i_h6_ppu_pd_power_on(struct generic_pm_domain *genpd)
> +{
> +       const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> +
> +       return sun50i_h6_ppu_pd_set_power(pd, !pd->negated);
> +}
> +
> +static int sun50i_h6_ppu_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +       const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> +
> +       return sun50i_h6_ppu_pd_set_power(pd, pd->negated);
> +}
> +
> +static int sun50i_h6_ppu_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct genpd_onecell_data *ppu;
> +       struct sun50i_h6_ppu_pd *pds;
> +       const struct sun50i_h6_ppu_desc *desc;
> +       void __iomem *base;
> +       int ret, i, count;
> +
> +       desc = of_device_get_match_data(dev);
> +       if (!desc)
> +               return -EINVAL;
> +
> +       for (count = 0; desc[count].name; count++)
> +               ;
> +
> +       pds = devm_kcalloc(dev, count, sizeof(*pds), GFP_KERNEL);
> +       if (!pds)
> +               return -ENOMEM;
> +
> +       ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> +       if (!ppu)
> +               return -ENOMEM;
> +
> +       ppu->num_domains = count;
> +       ppu->domains = devm_kcalloc(dev, count, sizeof(*ppu->domains),
> +                                   GFP_KERNEL);
> +       if (!ppu->domains)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, ppu);
> +
> +       base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       for (i = 0; i < count; i++) {
> +               struct sun50i_h6_ppu_pd *pd = &pds[i];
> +
> +               pd->genpd.name          = desc[i].name;
> +               pd->genpd.power_off     = sun50i_h6_ppu_pd_power_off;
> +               pd->genpd.power_on      = sun50i_h6_ppu_pd_power_on;
> +               if (desc[i].flags & FLAG_PPU_ALWAYS_ON)
> +                       pd->genpd.flags = GENPD_FLAG_ALWAYS_ON;
> +               pd->negated             = !!(desc[i].flags & FLAG_PPU_NEGATED);
> +               pd->reg                 = base + desc[i].offset - PD_H6_PPU_OFFSET;
> +               pd->gate_mask           = desc[i].mask;
> +
> +               ret = pm_genpd_init(&pd->genpd, NULL,
> +                                   !sun50i_h6_ppu_power_status(pd));
> +               if (ret) {
> +                       dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);
> +                       return ret;
> +               }
> +               ppu->domains[i] = &pd->genpd;
> +       }
> +
> +       ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
> +       if (ret)
> +               dev_warn(dev, "Failed to add provider: %d\n", ret);

If of_genpd_add_provider_onecell() fails, shouldn't we remove the
genpds in the error path?

> +
> +       return 0;
> +}
> +
> +static const struct of_device_id sun50i_h6_ppu_of_match[] = {
> +       { .compatible   = "allwinner,sun50i-h6-prcm-ppu",
> +         .data         = &sun50i_h6_ppus },
> +       { .compatible   = "allwinner,sun50i-h616-prcm-ppu",
> +         .data         = &sun50i_h616_ppus },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, sun50i_h6_ppu_of_match);
> +
> +static struct platform_driver sun50i_h6_ppu_driver = {
> +       .probe  = sun50i_h6_ppu_probe,
> +       .driver = {
> +               .name                   = "sun50i-h6-prcm-ppu",
> +               .of_match_table         = sun50i_h6_ppu_of_match,
> +               /* Power domains cannot be removed while they are in use. */
> +               .suppress_bind_attrs    = true,
> +       },
> +};
> +module_platform_driver(sun50i_h6_ppu_driver);
> +
> +MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
> +MODULE_DESCRIPTION("Allwinner H6 PRCM power domain driver");
> +MODULE_LICENSE("GPL");
> --
> 2.46.3
>

Kind regards
Uffe

Re: [PATCH 0/5] arm64: sunxi: h616: Enable Mali GPU

[Philippe Simons]

Tested this on a RG35XX-H (H700),

launching glmark2-es2-drm completely hangs the board.

No kernel oops or error messages.

Philippe

On 21/02/2025 01:57, Andre Przywara wrote:
> The Allwinner H616/H618/H313/H700 SoCs contain a Mali G32 MP2 GPU. This
> IP is from the Bifrost family and is already supported by the panfrost
> driver, so enabling support for 3D graphics on this SoC is rather
> straight-forward.
> However Allwinner added some bits in the PRCM block, that control the
> power domain for the GPU - on top of its power *supply*.
>
> This series enables the Mali GPU on those SoCs, by first introducing a
> power domain driver for that SoC (patch 1/5: DT binding, patch 2/5:
> the actual driver). For the Mali GPU to work we literally need to flip a
> single bit (the BSP does this in the bootloader), and this full featured
> power domain driver is admittedly a bit over the top for that purpose.
> However it seems to be the right thing to do architecturally, and while
> at it I added the other power domains (for analogue, PLLs, and the
> management core), even though we won't use them in Linux and they would
> be always on. I have a simpler version of the driver which just covers
> this single bit controlling the GPU, please let me know if you prefer
> that.
> Please also note that this supersedes an RFC patch I sent a year ago,
> which included this power domain in the R-CCU driver:
> https://lore.kernel.org/linux-sunxi/20240225160616.15001-1-andre.przywara@arm.com/T/#u
>
> The rest of the patches enable the Mali GPU on the DT side: patch 3/5
> adds the compatible string to the Mali DT binding, while patch 4/5 adds
> the purely SoC specific DT nodes, for both the power domain and the Mali
> GPU. The final patch 5/5 then enables the GPU on all existing H616-family
> boards.
>
> There seems to be an existing problem with powering up the GPU, after it
> has been turned off by the kernel. Chances are this is a problem with the
> proper power-up (or power-down) sequence, where clock gates, reset lines
> and the power domain must be asserted in a specific order.
> A workaround used so far downstream is to keep the power domain enabled,
> by ignoring the power-off request. Observing any assumed "proper" sequence
> is a bit more tricky, since there is no Allwinner specific glue driver
> or anything, so things would need be changed in the generic panfrost
> code, where they have the potential of breaking other Mali users.
> I would be interested in hearing opinions about this.
>
> Cheers,
> Andre
>
> Andre Przywara (5):
>    dt-bindings: power: Add Allwinner H6/H616 PRCM PPU
>    pmdomain: sunxi: add H6 PRCM PPU driver
>    dt-bindings: gpu: mali-bifrost: Add Allwinner H616 compatible
>    arm64: dts: allwinner: h616: Add Mali GPU node
>    arm64: dts: allwinner: h616: enable Mali GPU for all boards
>
>   .../bindings/gpu/arm,mali-bifrost.yaml        |   1 +
>   .../power/allwinner,sun50i-h6-prcm-ppu.yaml   |  42 ++++
>   .../dts/allwinner/sun50i-h313-tanix-tx1.dts   |   5 +
>   .../sun50i-h616-bigtreetech-cb1.dtsi          |   5 +
>   .../allwinner/sun50i-h616-orangepi-zero.dtsi  |   4 +
>   .../allwinner/sun50i-h616-orangepi-zero2.dts  |   4 +
>   .../dts/allwinner/sun50i-h616-x96-mate.dts    |   5 +
>   .../arm64/boot/dts/allwinner/sun50i-h616.dtsi |  21 ++
>   .../sun50i-h618-longan-module-3h.dtsi         |   5 +
>   .../allwinner/sun50i-h618-orangepi-zero2w.dts |   5 +
>   .../allwinner/sun50i-h618-orangepi-zero3.dts  |   4 +
>   .../sun50i-h618-transpeed-8k618-t.dts         |   5 +
>   .../sun50i-h618-yuzukihd-chameleon.dts        |   5 +
>   .../sun50i-h700-anbernic-rg35xx-2024.dts      |   5 +
>   drivers/pmdomain/sunxi/Kconfig                |  10 +
>   drivers/pmdomain/sunxi/Makefile               |   1 +
>   drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c   | 191 ++++++++++++++++++
>   17 files changed, 318 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/power/allwinner,sun50i-h6-prcm-ppu.yaml
>   create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
>

Re: [PATCH 2/5] pmdomain: sunxi: add H6 PRCM PPU driver

[Andre Przywara]

On Fri, 21 Feb 2025 19:10:33 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

> Dne petek, 21. februar 2025 ob 01:57:59 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > The Allwinner Power Reset Clock Management (RPCM) block contains a few
> > bits that control some power domains. The most prominent one is the one
> > for the Mali GPU. On the Allwinner H6 this domain is enabled at reset, so
> > we didn't care about it so far, but the H616 defaults to it being disabled.
> > 
> > Add a power domain driver for those bits. Some BSP code snippets and
> > some spare documentation describe three bits, slightly different between
> > the H6 and H616, so add three power domains for each SoC, connected to
> > their compatible string.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/pmdomain/sunxi/Kconfig              |  10 +
> >  drivers/pmdomain/sunxi/Makefile             |   1 +
> >  drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c | 191 ++++++++++++++++++++
> >  3 files changed, 202 insertions(+)
> >  create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> > 
> > diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> > index 17781bf8d86d7..43eecb3ea9819 100644
> > --- a/drivers/pmdomain/sunxi/Kconfig
> > +++ b/drivers/pmdomain/sunxi/Kconfig
> > @@ -8,3 +8,13 @@ config SUN20I_PPU
> >  	help
> >  	  Say y to enable the PPU power domain driver. This saves power
> >  	  when certain peripherals, such as the video engine, are idle.
> > +
> > +config SUN50I_H6_PRCM_PPU
> > +	tristate "Allwinner H6 PRCM power domain driver"
> > +	depends on ARCH_SUNXI || COMPILE_TEST
> > +	depends on PM
> > +	select PM_GENERIC_DOMAINS
> > +	help
> > +	  Say y to enable the Allwinner H6/H616 PRCM power domain driver.
> > +	  This is required to enable the Mali GPU in the H616 SoC, it is
> > +	  optional for the H6.
> > diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> > index ec1d7a2fb21db..c1343e1237599 100644
> > --- a/drivers/pmdomain/sunxi/Makefile
> > +++ b/drivers/pmdomain/sunxi/Makefile
> > @@ -1,2 +1,3 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-$(CONFIG_SUN20I_PPU)		+= sun20i-ppu.o
> > +obj-$(CONFIG_SUN50I_H6_PRCM_PPU)	+= sun50i-h6-prcm-ppu.o
> > diff --git a/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> > new file mode 100644
> > index 0000000000000..1c6b0c78b222d
> > --- /dev/null
> > +++ b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) Arm Ltd. 2024
> > + *
> > + * Allwinner H6/H616 PRCM power domain driver.
> > + * This covers a few registers inside the PRCM (Power Reset Clock Management)
> > + * block that control some power rails, most prominently for the Mali GPU.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/reset.h>
> > +
> > +/*
> > + * The PRCM block covers multiple devices, starting with some clocks,
> > + * then followed by the power rails.
> > + * The clocks are covered by a different driver, so this driver's MMIO range
> > + * starts later in the PRCM MMIO frame, not at the beginning of it.
> > + * To keep the register offsets consistent with other PRCM documentation,
> > + * express the registers relative to the beginning of the whole PRCM, and
> > + * subtract the PPU offset this driver is bound to.
> > + */
> > +#define PD_H6_PPU_OFFSET		0x250
> > +#define PD_H6_VDD_SYS_REG		0x250
> > +#define PD_H616_ANA_VDD_GATE			BIT(4)
> > +#define PD_H6_CPUS_VDD_GATE			BIT(3)
> > +#define PD_H6_AVCC_VDD_GATE			BIT(2)
> > +#define PD_H6_GPU_REG			0x254
> > +#define PD_H6_GPU_GATE				BIT(0)
> > +
> > +struct sun50i_h6_ppu_pd {
> > +	struct generic_pm_domain	genpd;
> > +	void __iomem			*reg;
> > +	u32				gate_mask;
> > +	bool				negated;
> > +};
> > +
> > +#define FLAG_PPU_ALWAYS_ON	BIT(0)
> > +#define FLAG_PPU_NEGATED	BIT(1)
> > +
> > +struct sun50i_h6_ppu_desc {
> > +	const char *name;
> > +	u32 offset;
> > +	u32 mask;
> > +	unsigned int flags;
> > +};
> > +
> > +struct sun50i_h6_ppu_desc sun50i_h6_ppus[] = {
> > +	{ "AVCC", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE },
> > +	{ "CPUS", PD_H6_VDD_SYS_REG, PD_H6_CPUS_VDD_GATE },
> > +	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE },
> > +	{}
> > +};
> > +
> > +struct sun50i_h6_ppu_desc sun50i_h616_ppus[] = {
> > +	{ "PLL", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE,
> > +		FLAG_PPU_ALWAYS_ON | FLAG_PPU_NEGATED },
> > +	{ "ANA", PD_H6_VDD_SYS_REG, PD_H616_ANA_VDD_GATE, FLAG_PPU_ALWAYS_ON },
> > +	{ "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE, FLAG_PPU_NEGATED },
> > +	{}
> > +};
> > +#define to_sun50i_h6_ppu_pd(_genpd) \
> > +	container_of(_genpd, struct sun50i_h6_ppu_pd, genpd)
> > +
> > +static bool sun50i_h6_ppu_power_status(const struct sun50i_h6_ppu_pd *pd)
> > +{
> > +	bool bit = readl(pd->reg) & pd->gate_mask;
> > +
> > +	return bit ^ pd->negated;
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_set_power(const struct sun50i_h6_ppu_pd *pd,
> > +				      bool set_bit)
> > +{
> > +	u32 reg = readl(pd->reg);
> > +
> > +	if (set_bit)
> > +		writel(reg | pd->gate_mask, pd->reg);
> > +	else
> > +		writel(reg & ~pd->gate_mask, pd->reg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_power_on(struct generic_pm_domain *genpd)
> > +{
> > +	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> > +
> > +	return sun50i_h6_ppu_pd_set_power(pd, !pd->negated);
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_power_off(struct generic_pm_domain *genpd)
> > +{
> > +	const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> > +
> > +	return sun50i_h6_ppu_pd_set_power(pd, pd->negated);
> > +}
> > +
> > +static int sun50i_h6_ppu_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct genpd_onecell_data *ppu;
> > +	struct sun50i_h6_ppu_pd *pds;
> > +	const struct sun50i_h6_ppu_desc *desc;
> > +	void __iomem *base;
> > +	int ret, i, count;
> > +
> > +	desc = of_device_get_match_data(dev);
> > +	if (!desc)
> > +		return -EINVAL;
> > +
> > +	for (count = 0; desc[count].name; count++)
> > +		;
> > +
> > +	pds = devm_kcalloc(dev, count, sizeof(*pds), GFP_KERNEL);
> > +	if (!pds)
> > +		return -ENOMEM;
> > +
> > +	ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> > +	if (!ppu)
> > +		return -ENOMEM;
> > +
> > +	ppu->num_domains = count;
> > +	ppu->domains = devm_kcalloc(dev, count, sizeof(*ppu->domains),
> > +				    GFP_KERNEL);
> > +	if (!ppu->domains)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, ppu);
> > +
> > +	base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	for (i = 0; i < count; i++) {
> > +		struct sun50i_h6_ppu_pd *pd = &pds[i];
> > +
> > +		pd->genpd.name		= desc[i].name;
> > +		pd->genpd.power_off	= sun50i_h6_ppu_pd_power_off;
> > +		pd->genpd.power_on	= sun50i_h6_ppu_pd_power_on;
> > +		if (desc[i].flags & FLAG_PPU_ALWAYS_ON)
> > +			pd->genpd.flags = GENPD_FLAG_ALWAYS_ON;
> > +		pd->negated		= !!(desc[i].flags & FLAG_PPU_NEGATED);
> > +		pd->reg			= base + desc[i].offset - PD_H6_PPU_OFFSET;
> > +		pd->gate_mask		= desc[i].mask;
> > +
> > +		ret = pm_genpd_init(&pd->genpd, NULL,
> > +				    !sun50i_h6_ppu_power_status(pd));
> > +		if (ret) {
> > +			dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);  
> 
> I suppose you want to replace "GPU" with desc[i].name.

Ah yeah, good point, fixed that.

> Otherwise it looks good to me, but I'd like to hear a comment from genpd maintainers.

Thanks!

Cheers,
Andre

> Best regards,
> Jernej
> 
> > +			return ret;
> > +		}
> > +		ppu->domains[i] = &pd->genpd;
> > +	}
> > +
> > +	ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
> > +	if (ret)
> > +		dev_warn(dev, "Failed to add provider: %d\n", ret);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id sun50i_h6_ppu_of_match[] = {
> > +	{ .compatible	= "allwinner,sun50i-h6-prcm-ppu",
> > +	  .data		= &sun50i_h6_ppus },
> > +	{ .compatible	= "allwinner,sun50i-h616-prcm-ppu",
> > +	  .data		= &sun50i_h616_ppus },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, sun50i_h6_ppu_of_match);
> > +
> > +static struct platform_driver sun50i_h6_ppu_driver = {
> > +	.probe	= sun50i_h6_ppu_probe,
> > +	.driver	= {
> > +		.name			= "sun50i-h6-prcm-ppu",
> > +		.of_match_table		= sun50i_h6_ppu_of_match,
> > +		/* Power domains cannot be removed while they are in use. */
> > +		.suppress_bind_attrs	= true,
> > +	},
> > +};
> > +module_platform_driver(sun50i_h6_ppu_driver);
> > +
> > +MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
> > +MODULE_DESCRIPTION("Allwinner H6 PRCM power domain driver");
> > +MODULE_LICENSE("GPL");
> >   
> 
> 
> 
> 

Re: [PATCH 2/5] pmdomain: sunxi: add H6 PRCM PPU driver

[Andre Przywara]

On Thu, 27 Feb 2025 23:43:29 +0100
Ulf Hansson <ulf.hansson@linaro.org> wrote:

Hi Ulf,

sorry for the delay, I actually changed to code according to your comments
back then already, just now find time to come back to this.

> On Fri, 21 Feb 2025 at 02:00, Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The Allwinner Power Reset Clock Management (RPCM) block contains a few
> > bits that control some power domains. The most prominent one is the one
> > for the Mali GPU. On the Allwinner H6 this domain is enabled at reset, so
> > we didn't care about it so far, but the H616 defaults to it being disabled.
> >
> > Add a power domain driver for those bits. Some BSP code snippets and
> > some spare documentation describe three bits, slightly different between
> > the H6 and H616, so add three power domains for each SoC, connected to
> > their compatible string.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  drivers/pmdomain/sunxi/Kconfig              |  10 +
> >  drivers/pmdomain/sunxi/Makefile             |   1 +
> >  drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c | 191 ++++++++++++++++++++
> >  3 files changed, 202 insertions(+)
> >  create mode 100644 drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> >
> > diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> > index 17781bf8d86d7..43eecb3ea9819 100644
> > --- a/drivers/pmdomain/sunxi/Kconfig
> > +++ b/drivers/pmdomain/sunxi/Kconfig
> > @@ -8,3 +8,13 @@ config SUN20I_PPU
> >         help
> >           Say y to enable the PPU power domain driver. This saves power
> >           when certain peripherals, such as the video engine, are idle.
> > +
> > +config SUN50I_H6_PRCM_PPU
> > +       tristate "Allwinner H6 PRCM power domain driver"
> > +       depends on ARCH_SUNXI || COMPILE_TEST
> > +       depends on PM
> > +       select PM_GENERIC_DOMAINS
> > +       help
> > +         Say y to enable the Allwinner H6/H616 PRCM power domain driver.
> > +         This is required to enable the Mali GPU in the H616 SoC, it is
> > +         optional for the H6.
> > diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> > index ec1d7a2fb21db..c1343e1237599 100644
> > --- a/drivers/pmdomain/sunxi/Makefile
> > +++ b/drivers/pmdomain/sunxi/Makefile
> > @@ -1,2 +1,3 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-$(CONFIG_SUN20I_PPU)               += sun20i-ppu.o
> > +obj-$(CONFIG_SUN50I_H6_PRCM_PPU)       += sun50i-h6-prcm-ppu.o
> > diff --git a/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> > new file mode 100644
> > index 0000000000000..1c6b0c78b222d
> > --- /dev/null
> > +++ b/drivers/pmdomain/sunxi/sun50i-h6-prcm-ppu.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) Arm Ltd. 2024
> > + *
> > + * Allwinner H6/H616 PRCM power domain driver.
> > + * This covers a few registers inside the PRCM (Power Reset Clock Management)
> > + * block that control some power rails, most prominently for the Mali GPU.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/reset.h>
> > +
> > +/*
> > + * The PRCM block covers multiple devices, starting with some clocks,
> > + * then followed by the power rails.
> > + * The clocks are covered by a different driver, so this driver's MMIO range
> > + * starts later in the PRCM MMIO frame, not at the beginning of it.
> > + * To keep the register offsets consistent with other PRCM documentation,
> > + * express the registers relative to the beginning of the whole PRCM, and
> > + * subtract the PPU offset this driver is bound to.
> > + */
> > +#define PD_H6_PPU_OFFSET               0x250
> > +#define PD_H6_VDD_SYS_REG              0x250
> > +#define PD_H616_ANA_VDD_GATE                   BIT(4)
> > +#define PD_H6_CPUS_VDD_GATE                    BIT(3)
> > +#define PD_H6_AVCC_VDD_GATE                    BIT(2)
> > +#define PD_H6_GPU_REG                  0x254
> > +#define PD_H6_GPU_GATE                         BIT(0)
> > +
> > +struct sun50i_h6_ppu_pd {
> > +       struct generic_pm_domain        genpd;
> > +       void __iomem                    *reg;
> > +       u32                             gate_mask;
> > +       bool                            negated;
> > +};
> > +
> > +#define FLAG_PPU_ALWAYS_ON     BIT(0)
> > +#define FLAG_PPU_NEGATED       BIT(1)
> > +
> > +struct sun50i_h6_ppu_desc {
> > +       const char *name;
> > +       u32 offset;
> > +       u32 mask;
> > +       unsigned int flags;
> > +};
> > +
> > +struct sun50i_h6_ppu_desc sun50i_h6_ppus[] = {
> > +       { "AVCC", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE },
> > +       { "CPUS", PD_H6_VDD_SYS_REG, PD_H6_CPUS_VDD_GATE },
> > +       { "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE },
> > +       {}
> > +};  
> 
> I suggest we define a new struct (perhaps sun50_domain_data!?) that
> has a struct sun50i_h6_ppu_desc* member along with a size_t member to
> keep the number of the domains, set by using ARRAY_SIZE().
> 
> In this way we don't need the additional NULL domain at the end - and
> we can also avoid looping through the array during probe  to count
> them.

OK, I implemented it that way, indeed removes the need for the extra
iteration of all entries.

> > +
> > +struct sun50i_h6_ppu_desc sun50i_h616_ppus[] = {
> > +       { "PLL", PD_H6_VDD_SYS_REG, PD_H6_AVCC_VDD_GATE,
> > +               FLAG_PPU_ALWAYS_ON | FLAG_PPU_NEGATED },
> > +       { "ANA", PD_H6_VDD_SYS_REG, PD_H616_ANA_VDD_GATE, FLAG_PPU_ALWAYS_ON },
> > +       { "GPU", PD_H6_GPU_REG, PD_H6_GPU_GATE, FLAG_PPU_NEGATED },
> > +       {}
> > +};
> > +#define to_sun50i_h6_ppu_pd(_genpd) \
> > +       container_of(_genpd, struct sun50i_h6_ppu_pd, genpd)
> > +
> > +static bool sun50i_h6_ppu_power_status(const struct sun50i_h6_ppu_pd *pd)
> > +{
> > +       bool bit = readl(pd->reg) & pd->gate_mask;
> > +
> > +       return bit ^ pd->negated;
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_set_power(const struct sun50i_h6_ppu_pd *pd,
> > +                                     bool set_bit)
> > +{
> > +       u32 reg = readl(pd->reg);
> > +
> > +       if (set_bit)
> > +               writel(reg | pd->gate_mask, pd->reg);
> > +       else
> > +               writel(reg & ~pd->gate_mask, pd->reg);
> > +
> > +       return 0;
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_power_on(struct generic_pm_domain *genpd)
> > +{
> > +       const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> > +
> > +       return sun50i_h6_ppu_pd_set_power(pd, !pd->negated);
> > +}
> > +
> > +static int sun50i_h6_ppu_pd_power_off(struct generic_pm_domain *genpd)
> > +{
> > +       const struct sun50i_h6_ppu_pd *pd = to_sun50i_h6_ppu_pd(genpd);
> > +
> > +       return sun50i_h6_ppu_pd_set_power(pd, pd->negated);
> > +}
> > +
> > +static int sun50i_h6_ppu_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct genpd_onecell_data *ppu;
> > +       struct sun50i_h6_ppu_pd *pds;
> > +       const struct sun50i_h6_ppu_desc *desc;
> > +       void __iomem *base;
> > +       int ret, i, count;
> > +
> > +       desc = of_device_get_match_data(dev);
> > +       if (!desc)
> > +               return -EINVAL;
> > +
> > +       for (count = 0; desc[count].name; count++)
> > +               ;
> > +
> > +       pds = devm_kcalloc(dev, count, sizeof(*pds), GFP_KERNEL);
> > +       if (!pds)
> > +               return -ENOMEM;
> > +
> > +       ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> > +       if (!ppu)
> > +               return -ENOMEM;
> > +
> > +       ppu->num_domains = count;
> > +       ppu->domains = devm_kcalloc(dev, count, sizeof(*ppu->domains),
> > +                                   GFP_KERNEL);
> > +       if (!ppu->domains)
> > +               return -ENOMEM;
> > +
> > +       platform_set_drvdata(pdev, ppu);
> > +
> > +       base = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(base))
> > +               return PTR_ERR(base);
> > +
> > +       for (i = 0; i < count; i++) {
> > +               struct sun50i_h6_ppu_pd *pd = &pds[i];
> > +
> > +               pd->genpd.name          = desc[i].name;
> > +               pd->genpd.power_off     = sun50i_h6_ppu_pd_power_off;
> > +               pd->genpd.power_on      = sun50i_h6_ppu_pd_power_on;
> > +               if (desc[i].flags & FLAG_PPU_ALWAYS_ON)
> > +                       pd->genpd.flags = GENPD_FLAG_ALWAYS_ON;
> > +               pd->negated             = !!(desc[i].flags & FLAG_PPU_NEGATED);
> > +               pd->reg                 = base + desc[i].offset - PD_H6_PPU_OFFSET;
> > +               pd->gate_mask           = desc[i].mask;
> > +
> > +               ret = pm_genpd_init(&pd->genpd, NULL,
> > +                                   !sun50i_h6_ppu_power_status(pd));
> > +               if (ret) {
> > +                       dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);
> > +                       return ret;
> > +               }
> > +               ppu->domains[i] = &pd->genpd;
> > +       }
> > +
> > +       ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
> > +       if (ret)
> > +               dev_warn(dev, "Failed to add provider: %d\n", ret);  
> 
> If of_genpd_add_provider_onecell() fails, shouldn't we remove the
> genpds in the error path?

Ah, good point, I was put off by the name "init" in pm_genpd_init(), but I
see that it does an allocation in there, so would need to be paired with
the remove on the error path.
I also added a goto to deal with a failed init in the middle of the loop
above, so that it cleans up the already registered genpds.
I tested this lightly by faking an error return for the 3rd
pm_genpd_init() call, and seeing the other genpds being cleaned up.

So thanks for your comments and the heads up, v2 incoming.

Cheers,
Andre

> > +
> > +       return 0;
> > +}
> > +
> > +static const struct of_device_id sun50i_h6_ppu_of_match[] = {
> > +       { .compatible   = "allwinner,sun50i-h6-prcm-ppu",
> > +         .data         = &sun50i_h6_ppus },
> > +       { .compatible   = "allwinner,sun50i-h616-prcm-ppu",
> > +         .data         = &sun50i_h616_ppus },
> > +       { }
> > +};
> > +MODULE_DEVICE_TABLE(of, sun50i_h6_ppu_of_match);
> > +
> > +static struct platform_driver sun50i_h6_ppu_driver = {
> > +       .probe  = sun50i_h6_ppu_probe,
> > +       .driver = {
> > +               .name                   = "sun50i-h6-prcm-ppu",
> > +               .of_match_table         = sun50i_h6_ppu_of_match,
> > +               /* Power domains cannot be removed while they are in use. */
> > +               .suppress_bind_attrs    = true,
> > +       },
> > +};
> > +module_platform_driver(sun50i_h6_ppu_driver);
> > +
> > +MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
> > +MODULE_DESCRIPTION("Allwinner H6 PRCM power domain driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.46.3
> >  
> 
> Kind regards
> Uffe