[PATCH 0/9] arm64: dts: imx95: Add support for Mali G310 GPU

[PATCH 0/9] arm64: dts: imx95: Add support for Mali G310 GPU

[Marek Vasut]

The instance of the GPU populated in i.MX95 is the G310.
Add support for the GPUMIX reset via simple-reset driver,
add reset and multiple power domains support into panthor
GPU driver, add iMX95 GPU support into panthor driver and
describe the iMX95 GPU in imx95.dtsi DT.

Marek Vasut (9):
  dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU
    reset
  reset: simple: Add support for Freescale i.MX95 GPU reset
  dt-bindings: gpu: mali-valhall-csf: Document optional reset
  drm/panthor: Implement optional reset
  drm/panthor: Implement support for multiple power domains
  drm/panthor: Reset GPU after L2 cache power off
  dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support
  drm/panthor: Add i.MX95 support
  arm64: dts: imx95: Describe Mali G310 GPU

 .../bindings/gpu/arm,mali-valhall-csf.yaml    |  4 +
 .../reset/fsl,imx95-gpu-blk-ctrl.yaml         | 49 ++++++++++++
 arch/arm64/boot/dts/freescale/imx95.dtsi      | 62 +++++++++++++++
 drivers/gpu/drm/panthor/Kconfig               |  1 +
 drivers/gpu/drm/panthor/panthor_device.c      | 79 +++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_device.h      |  8 ++
 drivers/gpu/drm/panthor/panthor_drv.c         |  1 +
 drivers/gpu/drm/panthor/panthor_gpu.c         | 12 +--
 drivers/reset/reset-simple.c                  |  8 ++
 9 files changed, 219 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml

---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org

-- 
2.47.2

[PATCH 1/9] dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU reset

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 does require
release from reset by writing into a single GPUMIX block controller
GPURESET register bit 0. Document support for this reset register.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 .../reset/fsl,imx95-gpu-blk-ctrl.yaml         | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml b/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
new file mode 100644
index 0000000000000..dc701bd556c0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/fsl,imx95-gpu-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX95 GPU Block Controller
+
+maintainers:
+  - Marek Vasut <marex@denx.de>
+
+description: |
+  This reset controller is a block of ad-hoc debug registers, one of
+  which is a single-bit GPU reset.
+
+properties:
+  compatible:
+    - const: fsl,imx95-gpu-blk-ctrl
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    reset-controller@4d810000 {
+        compatible = "fsl,imx95-gpu-blk-ctrl";
+        reg = <0x0 0x4d810000 0x0 0xc>;
+        clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
+        power-domains = <&scmi_devpd IMX95_PD_GPU>;
+        #reset-cells = <1>;
+    };
-- 
2.47.2

[PATCH 2/9] reset: simple: Add support for Freescale i.MX95 GPU reset

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 does require
release from reset by writing into a single GPUMIX block controller
GPURESET register bit 0. Implement support for this reset register.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/reset/reset-simple.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 2760678398308..1415a941fd6eb 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -133,9 +133,17 @@ static const struct reset_simple_devdata reset_simple_active_low = {
 	.status_active_low = true,
 };
 
+static const struct reset_simple_devdata reset_simple_fsl_imx95_gpu_blk_ctrl = {
+	.reg_offset = 0x8,
+	.active_low = true,
+	.status_active_low = true,
+};
+
 static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "altr,stratix10-rst-mgr",
 		.data = &reset_simple_socfpga },
+	{ .compatible = "fsl,imx95-gpu-blk-ctrl",
+		.data = &reset_simple_fsl_imx95_gpu_blk_ctrl },
 	{ .compatible = "st,stm32-rcc", },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
-- 
2.47.2

[PATCH 3/9] dt-bindings: gpu: mali-valhall-csf: Document optional reset

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 does require
release from reset by writing into a single GPUMIX block controller
GPURESET register bit 0. Document support for one optional reset.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml          | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
index a5b4e00217587..0efa06822a543 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
@@ -61,6 +61,9 @@ properties:
     minItems: 1
     maxItems: 5
 
+  resets:
+    maxItems: 1
+
   sram-supply: true
 
   "#cooling-cells":
-- 
2.47.2

[PATCH 4/9] drm/panthor: Implement optional reset

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 does require
release from reset by writing into a single GPUMIX block controller
GPURESET register bit 0. Implement support for one optional reset.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/panthor/Kconfig          |  1 +
 drivers/gpu/drm/panthor/panthor_device.c | 23 +++++++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_device.h |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
index 55b40ad07f3b0..ab62bd6a0750f 100644
--- a/drivers/gpu/drm/panthor/Kconfig
+++ b/drivers/gpu/drm/panthor/Kconfig
@@ -14,6 +14,7 @@ config DRM_PANTHOR
 	select IOMMU_IO_PGTABLE_LPAE
 	select IOMMU_SUPPORT
 	select PM_DEVFREQ
+	select RESET_SIMPLE if SOC_IMX9
 	help
 	  DRM driver for ARM Mali CSF-based GPUs.
 
diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index a9da1d1eeb707..51ee9cae94504 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev)
 	return 0;
 }
 
+static int panthor_reset_init(struct panthor_device *ptdev)
+{
+	ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL);
+	if (IS_ERR(ptdev->resets))
+		return dev_err_probe(ptdev->base.dev,
+				     PTR_ERR(ptdev->resets),
+				     "get reset failed");
+
+	return 0;
+}
+
 void panthor_device_unplug(struct panthor_device *ptdev)
 {
 	/* This function can be called from two different path: the reset work
@@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
+	ret = panthor_reset_init(ptdev);
+	if (ret)
+		return ret;
+
 	ret = panthor_devfreq_init(ptdev);
 	if (ret)
 		return ret;
@@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev)
 	if (ret)
 		goto err_disable_stacks_clk;
 
+	ret = reset_control_deassert(ptdev->resets);
+	if (ret)
+		goto err_disable_coregroup_clk;
+
 	panthor_devfreq_resume(ptdev);
 
 	if (panthor_device_is_initialized(ptdev) &&
@@ -512,6 +531,9 @@ int panthor_device_resume(struct device *dev)
 
 err_suspend_devfreq:
 	panthor_devfreq_suspend(ptdev);
+	reset_control_assert(ptdev->resets);
+
+err_disable_coregroup_clk:
 	clk_disable_unprepare(ptdev->clks.coregroup);
 
 err_disable_stacks_clk:
@@ -563,6 +585,7 @@ int panthor_device_suspend(struct device *dev)
 
 	panthor_devfreq_suspend(ptdev);
 
+	reset_control_assert(ptdev->resets);
 	clk_disable_unprepare(ptdev->clks.coregroup);
 	clk_disable_unprepare(ptdev->clks.stacks);
 	clk_disable_unprepare(ptdev->clks.core);
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index da6574021664b..fea3a05778e2e 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -111,6 +111,9 @@ struct panthor_device {
 		struct clk *coregroup;
 	} clks;
 
+	/** @resets: GPU reset. */
+	struct reset_control *resets;
+
 	/** @coherent: True if the CPU/GPU are memory coherent. */
 	bool coherent;
 
-- 
2.47.2

[PATCH 5/9] drm/panthor: Implement support for multiple power domains

[Marek Vasut]

The driver code power domain binding to driver instances only works
for single power domain, in case there are multiple power domains,
it is necessary to explicitly attach via dev_pm_domain_attach*().
As DT bindings list support for up to 5 power domains, add support
for attaching them all. This is useful on Freescale i.MX95 which
does have two power domains.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/panthor/panthor_device.c | 56 ++++++++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_device.h |  5 +++
 2 files changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 51ee9cae94504..4348b7e917b64 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -75,6 +75,58 @@ static int panthor_reset_init(struct panthor_device *ptdev)
 	return 0;
 }
 
+/* Generic power domain handling code, see drivers/gpu/drm/tiny/simpledrm.c */
+static void panthor_detach_genpd(void *res)
+{
+	struct panthor_device *ptdev = res;
+	int i;
+
+	if (ptdev->pwr_dom_count <= 1)
+		return;
+
+	for (i = ptdev->pwr_dom_count - 1; i >= 0; i--)
+		dev_pm_domain_detach(ptdev->pwr_dom_devs[i], true);
+}
+
+static int panthor_genpd_init(struct panthor_device *ptdev)
+{
+	struct device *dev = ptdev->base.dev;
+	int i, ret;
+
+	ptdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node, "power-domains",
+							  "#power-domain-cells");
+	/*
+	 * Single power-domain devices are handled by driver core nothing to do
+	 * here. The same for device nodes without "power-domains" property.
+	 */
+	if (ptdev->pwr_dom_count <= 1)
+		return 0;
+
+	if (ptdev->pwr_dom_count > ARRAY_SIZE(ptdev->pwr_dom_devs)) {
+		drm_warn(&ptdev->base, "Too many power domains (%d) for this device\n",
+			 ptdev->pwr_dom_count);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ptdev->pwr_dom_count; i++) {
+		ptdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i);
+		if (!IS_ERR(ptdev->pwr_dom_devs[i]))
+			continue;
+
+		ret = PTR_ERR(ptdev->pwr_dom_devs[i]);
+		if (ret != -EPROBE_DEFER) {
+			drm_warn(&ptdev->base, "pm_domain_attach_by_id(%u) failed: %d\n", i, ret);
+			continue;
+		}
+
+		/* Missing dependency, try again. */
+		panthor_detach_genpd(ptdev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(dev, panthor_detach_genpd, ptdev);
+}
+
 void panthor_device_unplug(struct panthor_device *ptdev)
 {
 	/* This function can be called from two different path: the reset work
@@ -232,6 +284,10 @@ int panthor_device_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
+	ret = panthor_genpd_init(ptdev);
+	if (ret)
+		return ret;
+
 	ret = panthor_devfreq_init(ptdev);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index fea3a05778e2e..7fb65447253e9 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -114,6 +114,11 @@ struct panthor_device {
 	/** @resets: GPU reset. */
 	struct reset_control *resets;
 
+	/** @pwr_dom_count: Power domain count */
+	int pwr_dom_count;
+	/** @pwr_dom_dev: Power domain devices */
+	struct device *pwr_dom_devs[5];
+
 	/** @coherent: True if the CPU/GPU are memory coherent. */
 	bool coherent;
 
-- 
2.47.2

[PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Marek Vasut]

This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
from runtime PM suspend. Without this, if only the L2 is powered down
on RPM entry, the GPU gets stuck and does not indicate the firmware is
booted after RPM resume.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/panthor/panthor_gpu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index 671049020afaa..0f07ef7d9aea7 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -470,11 +470,12 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
  */
 void panthor_gpu_suspend(struct panthor_device *ptdev)
 {
-	/* On a fast reset, simply power down the L2. */
-	if (!ptdev->reset.fast)
-		panthor_gpu_soft_reset(ptdev);
-	else
-		panthor_gpu_power_off(ptdev, L2, 1, 20000);
+	/*
+	 * Power off the L2 and soft reset the GPU, that makes
+	 * iMX95 Mali G310 resume without firmware boot timeout.
+	 */
+	panthor_gpu_power_off(ptdev, L2, 1, 20000);
+	panthor_gpu_soft_reset(ptdev);
 
 	panthor_gpu_irq_suspend(&ptdev->gpu->irq);
 }
-- 
2.47.2

[PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 is the
Mali G310, document support for this variant.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
index 0efa06822a543..3ab62bd424e41 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
@@ -18,6 +18,7 @@ properties:
     oneOf:
       - items:
           - enum:
+              - fsl,imx95-mali            # G310
               - rockchip,rk3588-mali
           - const: arm,mali-valhall-csf   # Mali Valhall GPU model/revision is fully discoverable
 
-- 
2.47.2

[PATCH 8/9] drm/panthor: Add i.MX95 support

[Marek Vasut]

The instance of the GPU populated in Freescale i.MX95 is the
Mali G310, add support for this variant.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/panthor/panthor_drv.c | 1 +
 drivers/gpu/drm/panthor/panthor_gpu.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 06fe46e320738..2504a456d45c4 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
 ATTRIBUTE_GROUPS(panthor);
 
 static const struct of_device_id dt_match[] = {
+	{ .compatible = "fsl,imx95-mali" },	/* G310 */
 	{ .compatible = "rockchip,rk3588-mali" },
 	{ .compatible = "arm,mali-valhall-csf" },
 	{}
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index 0f07ef7d9aea7..2371ab8e50627 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -67,6 +67,7 @@ struct panthor_model {
 }
 
 static const struct panthor_model gpu_models[] = {
+	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
 	GPU_MODEL(g610, 10, 7),
 	{},
 };
-- 
2.47.2

[PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

The instance of the GPU populated in i.MX95 is the G310,
describe this GPU in the DT. Include description of the
GPUMIX block controller, which can be operated as a simple
reset. Include dummy GPU voltage regulator and OPP tables.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Steven Price <steven.price@arm.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/boot/dts/freescale/imx95.dtsi | 62 ++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 3af13173de4bd..36bad211e5558 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -249,6 +249,37 @@ dummy: clock-dummy {
 		clock-output-names = "dummy";
 	};
 
+	gpu_fixed_reg: fixed-gpu-reg {
+		compatible = "regulator-fixed";
+		regulator-min-microvolt = <920000>;
+		regulator-max-microvolt = <920000>;
+		regulator-name = "vdd_gpu";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
+	gpu_opp_table: opp_table {
+		compatible = "operating-points-v2";
+
+		opp-500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-hz-real = /bits/ 64 <500000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-hz-real = /bits/ 64 <800000000>;
+			opp-microvolt = <920000>;
+		};
+
+		opp-1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-hz-real = /bits/ 64 <1000000000>;
+			opp-microvolt = <920000>;
+		};
+	};
+
 	clk_ext1: clock-ext1 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
 			};
 		};
 
+		gpu_blk_ctrl: reset-controller@4d810000 {
+			compatible = "fsl,imx95-gpu-blk-ctrl";
+			reg = <0x0 0x4d810000 0x0 0xc>;
+			#reset-cells = <1>;
+			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
+			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
+			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
+			assigned-clock-rates = <133333333>;
+			power-domains = <&scmi_devpd IMX95_PD_GPU>;
+			status = "disabled";
+		};
+
+		gpu: gpu@4d900000 {
+			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
+			reg = <0 0x4d900000 0 0x480000>;
+			clocks = <&scmi_clk IMX95_CLK_GPU>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "gpu", "job", "mmu";
+			mali-supply = <&gpu_fixed_reg>;
+			operating-points-v2 = <&gpu_opp_table>;
+			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
+			power-domain-names = "mix", "perf";
+			resets = <&gpu_blk_ctrl 0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <1013>;
+			status = "disabled";
+		};
+
 		ddr-pmu@4e090dc0 {
 			compatible = "fsl,imx95-ddr-pmu", "fsl,imx93-ddr-pmu";
 			reg = <0x0 0x4e090dc0 0x0 0x200>;
-- 
2.47.2

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Boris Brezillon]

On Thu, 27 Feb 2025 17:58:06 +0100
Marek Vasut <marex@denx.de> wrote:

> This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
> from runtime PM suspend. Without this, if only the L2 is powered down
> on RPM entry, the GPU gets stuck and does not indicate the firmware is
> booted after RPM resume.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 671049020afaa..0f07ef7d9aea7 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -470,11 +470,12 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
>   */
>  void panthor_gpu_suspend(struct panthor_device *ptdev)
>  {
> -	/* On a fast reset, simply power down the L2. */
> -	if (!ptdev->reset.fast)
> -		panthor_gpu_soft_reset(ptdev);
> -	else
> -		panthor_gpu_power_off(ptdev, L2, 1, 20000);
> +	/*
> +	 * Power off the L2 and soft reset the GPU, that makes
> +	 * iMX95 Mali G310 resume without firmware boot timeout.
> +	 */
> +	panthor_gpu_power_off(ptdev, L2, 1, 20000);
> +	panthor_gpu_soft_reset(ptdev);

Unfortunately, if you do that unconditionally we no longer have a
fast-reset. Would be good to figure out why the fast-reset doesn't work
on this platform.

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Marek Vasut]

On 2/27/25 6:17 PM, Boris Brezillon wrote:

[...]

>> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
>> index 671049020afaa..0f07ef7d9aea7 100644
>> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
>> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
>> @@ -470,11 +470,12 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
>>    */
>>   void panthor_gpu_suspend(struct panthor_device *ptdev)
>>   {
>> -	/* On a fast reset, simply power down the L2. */
>> -	if (!ptdev->reset.fast)
>> -		panthor_gpu_soft_reset(ptdev);
>> -	else
>> -		panthor_gpu_power_off(ptdev, L2, 1, 20000);
>> +	/*
>> +	 * Power off the L2 and soft reset the GPU, that makes
>> +	 * iMX95 Mali G310 resume without firmware boot timeout.
>> +	 */
>> +	panthor_gpu_power_off(ptdev, L2, 1, 20000);
>> +	panthor_gpu_soft_reset(ptdev);
> 
> Unfortunately, if you do that unconditionally we no longer have a
> fast-reset. Would be good to figure out why the fast-reset doesn't work
> on this platform.

I was hoping to get some hint on this one, I spent quite a while trying 
to narrow this down, finally got it down to this particular bit.

The NXP downstream vendor kernel vendor Mali driver does not seem to 
have anything interesting regarding the L2 power handling, but I might 
have missed it, the code is difficult to read.

Have you ever seen anything problematic in this specific L2 department ?

Do you have any hints how I can debug this further ?

Re: [PATCH 1/9] dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU reset

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:01PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Document support for this reset register.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  .../reset/fsl,imx95-gpu-blk-ctrl.yaml         | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml b/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
> new file mode 100644
> index 0000000000000..dc701bd556c0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/fsl,imx95-gpu-blk-ctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX95 GPU Block Controller
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +description: |

Needn't |

> +  This reset controller is a block of ad-hoc debug registers, one of
> +  which is a single-bit GPU reset.
> +
> +properties:
> +  compatible:
> +    - const: fsl,imx95-gpu-blk-ctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    reset-controller@4d810000 {
> +        compatible = "fsl,imx95-gpu-blk-ctrl";
> +        reg = <0x0 0x4d810000 0x0 0xc>;

No sure if it pass dt_binding_check, I remember default 32bit address
reg = <0x4d810000 0xc>

> +        clocks = <&scmi_clk IMX95_CLK_GPUAPB>;

suppose you missed dt-binding include file for IMX95_CLK_GPUAPB

Frank
> +        power-domains = <&scmi_devpd IMX95_PD_GPU>;
> +        #reset-cells = <1>;
> +    };
> --
> 2.47.2
>

Re: [PATCH 2/9] reset: simple: Add support for Freescale i.MX95 GPU reset

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:02PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Implement support for this reset register.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/reset/reset-simple.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 2760678398308..1415a941fd6eb 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -133,9 +133,17 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>  	.status_active_low = true,
>  };
>
> +static const struct reset_simple_devdata reset_simple_fsl_imx95_gpu_blk_ctrl = {
> +	.reg_offset = 0x8,
> +	.active_low = true,
> +	.status_active_low = true,
> +};
> +
>  static const struct of_device_id reset_simple_dt_ids[] = {
>  	{ .compatible = "altr,stratix10-rst-mgr",
>  		.data = &reset_simple_socfpga },
> +	{ .compatible = "fsl,imx95-gpu-blk-ctrl",
> +		.data = &reset_simple_fsl_imx95_gpu_blk_ctrl },
>  	{ .compatible = "st,stm32-rcc", },
>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>  		.data = &reset_simple_active_low },
> --
> 2.47.2
>

Re: [PATCH 3/9] dt-bindings: gpu: mali-valhall-csf: Document optional reset

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:03PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Document support for one optional reset.
>
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml          | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index a5b4e00217587..0efa06822a543 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -61,6 +61,9 @@ properties:
>      minItems: 1
>      maxItems: 5
>
> +  resets:
> +    maxItems: 1
> +
>    sram-supply: true
>
>    "#cooling-cells":
> --
> 2.47.2
>

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:07PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, document support for this variant.
>
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index 0efa06822a543..3ab62bd424e41 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -18,6 +18,7 @@ properties:
>      oneOf:
>        - items:
>            - enum:
> +              - fsl,imx95-mali            # G310
>                - rockchip,rk3588-mali
>            - const: arm,mali-valhall-csf   # Mali Valhall GPU model/revision is fully discoverable
>
> --
> 2.47.2
>

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:08PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, add support for this variant.
>
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 1 +
>  drivers/gpu/drm/panthor/panthor_gpu.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 06fe46e320738..2504a456d45c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>  ATTRIBUTE_GROUPS(panthor);
>
>  static const struct of_device_id dt_match[] = {
> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
>  	{ .compatible = "rockchip,rk3588-mali" },
>  	{ .compatible = "arm,mali-valhall-csf" },
>  	{}
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 0f07ef7d9aea7..2371ab8e50627 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -67,6 +67,7 @@ struct panthor_model {
>  }
>
>  static const struct panthor_model gpu_models[] = {
> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
>  	GPU_MODEL(g610, 10, 7),
>  	{},
>  };
> --
> 2.47.2
>

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Frank Li]

On Thu, Feb 27, 2025 at 05:58:09PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in i.MX95 is the G310,
> describe this GPU in the DT. Include description of the
> GPUMIX block controller, which can be operated as a simple
> reset. Include dummy GPU voltage regulator and OPP tables.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/boot/dts/freescale/imx95.dtsi | 62 ++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> index 3af13173de4bd..36bad211e5558 100644
> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> @@ -249,6 +249,37 @@ dummy: clock-dummy {
>  		clock-output-names = "dummy";
>  	};
>
> +	gpu_fixed_reg: fixed-gpu-reg {
> +		compatible = "regulator-fixed";
> +		regulator-min-microvolt = <920000>;
> +		regulator-max-microvolt = <920000>;
> +		regulator-name = "vdd_gpu";
> +		regulator-always-on;
> +		regulator-boot-on;

Does really need regulator-boot-on and regulator-always-on ?

> +	};
> +
> +	gpu_opp_table: opp_table {
> +		compatible = "operating-points-v2";
> +
> +		opp-500000000 {
> +			opp-hz = /bits/ 64 <500000000>;
> +			opp-hz-real = /bits/ 64 <500000000>;
> +			opp-microvolt = <920000>;
> +		};
> +
> +		opp-800000000 {
> +			opp-hz = /bits/ 64 <800000000>;
> +			opp-hz-real = /bits/ 64 <800000000>;
> +			opp-microvolt = <920000>;
> +		};
> +
> +		opp-1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +			opp-hz-real = /bits/ 64 <1000000000>;
> +			opp-microvolt = <920000>;
> +		};
> +	};
> +
>  	clk_ext1: clock-ext1 {
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
> @@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
>  			};
>  		};
>
> +		gpu_blk_ctrl: reset-controller@4d810000 {
> +			compatible = "fsl,imx95-gpu-blk-ctrl";
> +			reg = <0x0 0x4d810000 0x0 0xc>;
> +			#reset-cells = <1>;
> +			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> +			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> +			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> +			assigned-clock-rates = <133333333>;
> +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
> +			status = "disabled";
> +		};
> +
> +		gpu: gpu@4d900000 {
> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> +			reg = <0 0x4d900000 0 0x480000>;
> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> +			clock-names = "core";
> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "gpu", "job", "mmu";
> +			mali-supply = <&gpu_fixed_reg>;
> +			operating-points-v2 = <&gpu_opp_table>;
> +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
> +			power-domain-names = "mix", "perf";
> +			resets = <&gpu_blk_ctrl 0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <1013>;
> +			status = "disabled";

GPU is internal module, which have not much dependence with other module
such as pinmux. why not default status is "disabled". Supposed gpu driver
will turn off clock and power if not used.

Frank

> +		};
> +
>  		ddr-pmu@4e090dc0 {
>  			compatible = "fsl,imx95-ddr-pmu", "fsl,imx93-ddr-pmu";
>  			reg = <0x0 0x4e090dc0 0x0 0x200>;
> --
> 2.47.2
>

Re: [PATCH 1/9] dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU reset

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 17:58:01 +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Document support for this reset register.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  .../reset/fsl,imx95-gpu-blk-ctrl.yaml         | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml: ignoring, error in schema: properties: compatible
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml: properties:compatible: [{'const': 'fsl,imx95-gpu-blk-ctrl'}] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml: properties:compatible: [{'const': 'fsl,imx95-gpu-blk-ctrl'}] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
Error: Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.example.dts:21.33-34 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1511: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250227170012.124768-2-marex@denx.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/9] dt-bindings: gpu: mali-valhall-csf: Document optional reset

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 17:58:03 +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Document support for one optional reset.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml          | 3 +++
>  1 file changed, 3 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:


doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250227170012.124768-4-marex@denx.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 17:58:07 +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, document support for this variant.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:


doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250227170012.124768-8-marex@denx.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marco Felsch]

Hi Marek,

On 25-02-27, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, add support for this variant.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 1 +
>  drivers/gpu/drm/panthor/panthor_gpu.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 06fe46e320738..2504a456d45c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>  ATTRIBUTE_GROUPS(panthor);
>  
>  static const struct of_device_id dt_match[] = {
> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
			  ^
			 nxp?

Can we switch to nxp instead?

Regards,
  Marco

>  	{ .compatible = "rockchip,rk3588-mali" },
>  	{ .compatible = "arm,mali-valhall-csf" },
>  	{}
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 0f07ef7d9aea7..2371ab8e50627 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -67,6 +67,7 @@ struct panthor_model {
>  }
>  
>  static const struct panthor_model gpu_models[] = {
> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
>  	GPU_MODEL(g610, 10, 7),
>  	{},
>  };
> -- 
> 2.47.2
> 
> 
> 

Re: [PATCH 1/9] dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU reset

[Marek Vasut]

On 2/27/25 6:30 PM, Frank Li wrote:

[...]

>> +examples:
>> +  - |
>> +    reset-controller@4d810000 {
>> +        compatible = "fsl,imx95-gpu-blk-ctrl";
>> +        reg = <0x0 0x4d810000 0x0 0xc>;
> 
> No sure if it pass dt_binding_check, I remember default 32bit address
> reg = <0x4d810000 0xc>
> 
>> +        clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> 
> suppose you missed dt-binding include file for IMX95_CLK_GPUAPB
Sigh, I knew I forgot something before sending this series out, thanks.

But we have a bit of a problem here, the IMX95_CLK_GPUAPB macro is 
defined in a header in arch/arm64/boot/dts:

arch/arm64/boot/dts/freescale/imx95-clock.h:#define IMX95_CLK_GPUAPB 
               (IMX95_CCM_NUM_CLK_SRC + 42)

Shouldn't this header be moved into dt-bindings/ ?

I can use fixed number 83 to reference the GPUAPB clock in this binding 
document until this is solved, even if that's not a nice thing.

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Marek Vasut]

On 2/27/25 7:38 PM, Rob Herring (Arm) wrote:
> 
> On Thu, 27 Feb 2025 17:58:07 +0100, Marek Vasut wrote:
>> The instance of the GPU populated in Freescale i.MX95 is the
>> Mali G310, document support for this variant.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Sebastian Reichel <sre@kernel.org>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: devicetree@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: imx@lists.linux.dev
>> Cc: linux-arm-kernel@lists.infradead.org
>> ---
>>   Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> 
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250227170012.124768-8-marex@denx.de
It seems there are no errors in this list ?

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 2/27/25 6:43 PM, Frank Li wrote:
[...]

>> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
>> index 3af13173de4bd..36bad211e5558 100644
>> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
>> @@ -249,6 +249,37 @@ dummy: clock-dummy {
>>   		clock-output-names = "dummy";
>>   	};
>>
>> +	gpu_fixed_reg: fixed-gpu-reg {
>> +		compatible = "regulator-fixed";
>> +		regulator-min-microvolt = <920000>;
>> +		regulator-max-microvolt = <920000>;
>> +		regulator-name = "vdd_gpu";
>> +		regulator-always-on;
>> +		regulator-boot-on;
> 
> Does really need regulator-boot-on and regulator-always-on ?

I don't think so, this is a development remnant, fixed, thanks.

[...]

>> +		gpu: gpu@4d900000 {
>> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
>> +			reg = <0 0x4d900000 0 0x480000>;
>> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
>> +			clock-names = "core";
>> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "gpu", "job", "mmu";
>> +			mali-supply = <&gpu_fixed_reg>;
>> +			operating-points-v2 = <&gpu_opp_table>;
>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
>> +			power-domain-names = "mix", "perf";
>> +			resets = <&gpu_blk_ctrl 0>;
>> +			#cooling-cells = <2>;
>> +			dynamic-power-coefficient = <1013>;
>> +			status = "disabled";
> 
> GPU is internal module, which have not much dependence with other module
> such as pinmux. why not default status is "disabled". Supposed gpu driver
> will turn off clock and power if not used.
My thinking was that there are MX95 SoC with GPU fused off, hence it is 
better to keep the GPU disabled in DT by default. But I can also keep it 
enabled and the few boards which do not have MX95 SoC with GPU can 
explicitly disable it in board DT.

What do you think ?

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marek Vasut]

On 2/27/25 9:17 PM, Marco Felsch wrote:

[...]

>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
>> index 06fe46e320738..2504a456d45c4 100644
>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>>   ATTRIBUTE_GROUPS(panthor);
>>   
>>   static const struct of_device_id dt_match[] = {
>> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
> 			  ^
> 			 nxp?
> 
> Can we switch to nxp instead?
We can ... is that the current recommendation ?

Why not stick with fsl , is that deprecated now ?

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Frank Li]

On Thu, Feb 27, 2025 at 09:36:55PM +0100, Marek Vasut wrote:
> On 2/27/25 6:43 PM, Frank Li wrote:
> [...]
>
> > > diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > index 3af13173de4bd..36bad211e5558 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > @@ -249,6 +249,37 @@ dummy: clock-dummy {
> > >   		clock-output-names = "dummy";
> > >   	};
> > >
> > > +	gpu_fixed_reg: fixed-gpu-reg {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-min-microvolt = <920000>;
> > > +		regulator-max-microvolt = <920000>;
> > > +		regulator-name = "vdd_gpu";
> > > +		regulator-always-on;
> > > +		regulator-boot-on;
> >
> > Does really need regulator-boot-on and regulator-always-on ?
>
> I don't think so, this is a development remnant, fixed, thanks.
>
> [...]
>
> > > +		gpu: gpu@4d900000 {
> > > +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> > > +			reg = <0 0x4d900000 0 0x480000>;
> > > +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> > > +			clock-names = "core";
> > > +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "gpu", "job", "mmu";
> > > +			mali-supply = <&gpu_fixed_reg>;
> > > +			operating-points-v2 = <&gpu_opp_table>;
> > > +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
> > > +			power-domain-names = "mix", "perf";
> > > +			resets = <&gpu_blk_ctrl 0>;
> > > +			#cooling-cells = <2>;
> > > +			dynamic-power-coefficient = <1013>;
> > > +			status = "disabled";
> >
> > GPU is internal module, which have not much dependence with other module
> > such as pinmux. why not default status is "disabled". Supposed gpu driver
> > will turn off clock and power if not used.
> My thinking was that there are MX95 SoC with GPU fused off, hence it is
> better to keep the GPU disabled in DT by default. But I can also keep it
> enabled and the few boards which do not have MX95 SoC with GPU can
> explicitly disable it in board DT.
>
> What do you think ?

GPU Fuse off should use access-control, see thread
https://lore.kernel.org/imx/20250207120213.GD14860@localhost.localdomain/

Frank

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 2/27/25 10:27 PM, Frank Li wrote:

[...]

>>>> +		gpu: gpu@4d900000 {
>>>> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
>>>> +			reg = <0 0x4d900000 0 0x480000>;
>>>> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
>>>> +			clock-names = "core";
>>>> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
>>>> +			interrupt-names = "gpu", "job", "mmu";
>>>> +			mali-supply = <&gpu_fixed_reg>;
>>>> +			operating-points-v2 = <&gpu_opp_table>;
>>>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
>>>> +			power-domain-names = "mix", "perf";
>>>> +			resets = <&gpu_blk_ctrl 0>;
>>>> +			#cooling-cells = <2>;
>>>> +			dynamic-power-coefficient = <1013>;
>>>> +			status = "disabled";
>>>
>>> GPU is internal module, which have not much dependence with other module
>>> such as pinmux. why not default status is "disabled". Supposed gpu driver
>>> will turn off clock and power if not used.
>> My thinking was that there are MX95 SoC with GPU fused off, hence it is
>> better to keep the GPU disabled in DT by default. But I can also keep it
>> enabled and the few boards which do not have MX95 SoC with GPU can
>> explicitly disable it in board DT.
>>
>> What do you think ?
> 
> GPU Fuse off should use access-control, see thread
> https://lore.kernel.org/imx/20250207120213.GD14860@localhost.localdomain/
Did that thread ever go anywhere ? It seems there is no real conclusion, 
is there ? +Cc Alex .

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Frank Li]

On Thu, Feb 27, 2025 at 10:34:20PM +0100, Marek Vasut wrote:
> On 2/27/25 10:27 PM, Frank Li wrote:
>
> [...]
>
> > > > > +		gpu: gpu@4d900000 {
> > > > > +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> > > > > +			reg = <0 0x4d900000 0 0x480000>;
> > > > > +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> > > > > +			clock-names = "core";
> > > > > +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> > > > > +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> > > > > +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +			interrupt-names = "gpu", "job", "mmu";
> > > > > +			mali-supply = <&gpu_fixed_reg>;
> > > > > +			operating-points-v2 = <&gpu_opp_table>;
> > > > > +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
> > > > > +			power-domain-names = "mix", "perf";
> > > > > +			resets = <&gpu_blk_ctrl 0>;
> > > > > +			#cooling-cells = <2>;
> > > > > +			dynamic-power-coefficient = <1013>;
> > > > > +			status = "disabled";
> > > >
> > > > GPU is internal module, which have not much dependence with other module
> > > > such as pinmux. why not default status is "disabled". Supposed gpu driver
> > > > will turn off clock and power if not used.
> > > My thinking was that there are MX95 SoC with GPU fused off, hence it is
> > > better to keep the GPU disabled in DT by default. But I can also keep it
> > > enabled and the few boards which do not have MX95 SoC with GPU can
> > > explicitly disable it in board DT.
> > >
> > > What do you think ?
> >
> > GPU Fuse off should use access-control, see thread
> > https://lore.kernel.org/imx/20250207120213.GD14860@localhost.localdomain/
> Did that thread ever go anywhere ? It seems there is no real conclusion, is
> there ? +Cc Alex .

The direction is use access-control to indicate fuse disable. Only
implement detail is under discussion.

Frank

Re: [PATCH 2/9] reset: simple: Add support for Freescale i.MX95 GPU reset

[Alexander Stein]

Hi Marek,

Am Donnerstag, 27. Februar 2025, 17:58:02 CET schrieb Marek Vasut:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Implement support for this reset register.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/reset/reset-simple.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index 2760678398308..1415a941fd6eb 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -133,9 +133,17 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>  	.status_active_low = true,
>  };
>  
> +static const struct reset_simple_devdata reset_simple_fsl_imx95_gpu_blk_ctrl = {
> +	.reg_offset = 0x8,

Shouldn't you add ".nr_resets = 1"? Otherwise this will have 8 resets
(resource_size(res) * BITS_PER_BYTE).

On a side note: RM says this is a write-once register. Do we consider writing
this register again?
BTW: Would it be possible to disable it completely (until reset) by writing 1?

Best regards
Alexander

> +	.active_low = true,
> +	.status_active_low = true,
> +};
> +
>  static const struct of_device_id reset_simple_dt_ids[] = {
>  	{ .compatible = "altr,stratix10-rst-mgr",
>  		.data = &reset_simple_socfpga },
> +	{ .compatible = "fsl,imx95-gpu-blk-ctrl",
> +		.data = &reset_simple_fsl_imx95_gpu_blk_ctrl },
>  	{ .compatible = "st,stm32-rcc", },
>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>  		.data = &reset_simple_active_low },
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 4/9] drm/panthor: Implement optional reset

[Alexander Stein]

Hi Marek,

Am Donnerstag, 27. Februar 2025, 17:58:04 CET schrieb Marek Vasut:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Implement support for one optional reset.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/Kconfig          |  1 +
>  drivers/gpu/drm/panthor/panthor_device.c | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_device.h |  3 +++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> index 55b40ad07f3b0..ab62bd6a0750f 100644
> --- a/drivers/gpu/drm/panthor/Kconfig
> +++ b/drivers/gpu/drm/panthor/Kconfig
> @@ -14,6 +14,7 @@ config DRM_PANTHOR
>  	select IOMMU_IO_PGTABLE_LPAE
>  	select IOMMU_SUPPORT
>  	select PM_DEVFREQ
> +	select RESET_SIMPLE if SOC_IMX9
>  	help
>  	  DRM driver for ARM Mali CSF-based GPUs.
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index a9da1d1eeb707..51ee9cae94504 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev)
>  	return 0;
>  }
>  
> +static int panthor_reset_init(struct panthor_device *ptdev)
> +{
> +	ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL);

If the description as a write-once register is true, wouldn't this
already write to it?

> +	if (IS_ERR(ptdev->resets))
> +		return dev_err_probe(ptdev->base.dev,
> +				     PTR_ERR(ptdev->resets),
> +				     "get reset failed");
> +
> +	return 0;
> +}
> +
>  void panthor_device_unplug(struct panthor_device *ptdev)
>  {
>  	/* This function can be called from two different path: the reset work
> @@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = panthor_reset_init(ptdev);
> +	if (ret)
> +		return ret;
> +
>  	ret = panthor_devfreq_init(ptdev);
>  	if (ret)
>  		return ret;
> @@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev)
>  	if (ret)
>  		goto err_disable_stacks_clk;
>  
> +	ret = reset_control_deassert(ptdev->resets);
> +	if (ret)
> +		goto err_disable_coregroup_clk;
> +

This wouldn't work at all on a write-once register, no? Same for resume.

Best regards
Alexander

>  	panthor_devfreq_resume(ptdev);
>  
>  	if (panthor_device_is_initialized(ptdev) &&
> @@ -512,6 +531,9 @@ int panthor_device_resume(struct device *dev)
>  
>  err_suspend_devfreq:
>  	panthor_devfreq_suspend(ptdev);
> +	reset_control_assert(ptdev->resets);
> +
> +err_disable_coregroup_clk:
>  	clk_disable_unprepare(ptdev->clks.coregroup);
>  
>  err_disable_stacks_clk:
> @@ -563,6 +585,7 @@ int panthor_device_suspend(struct device *dev)
>  
>  	panthor_devfreq_suspend(ptdev);
>  
> +	reset_control_assert(ptdev->resets);
>  	clk_disable_unprepare(ptdev->clks.coregroup);
>  	clk_disable_unprepare(ptdev->clks.stacks);
>  	clk_disable_unprepare(ptdev->clks.core);
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index da6574021664b..fea3a05778e2e 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -111,6 +111,9 @@ struct panthor_device {
>  		struct clk *coregroup;
>  	} clks;
>  
> +	/** @resets: GPU reset. */
> +	struct reset_control *resets;

Your commit message says "one optional reset", so I would name this just
reset.

> +
>  	/** @coherent: True if the CPU/GPU are memory coherent. */
>  	bool coherent;
>  
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 5/9] drm/panthor: Implement support for multiple power domains

[Alexander Stein]

Am Donnerstag, 27. Februar 2025, 17:58:05 CET schrieb Marek Vasut:
> The driver code power domain binding to driver instances only works
> for single power domain, in case there are multiple power domains,
> it is necessary to explicitly attach via dev_pm_domain_attach*().
> As DT bindings list support for up to 5 power domains, add support
> for attaching them all. This is useful on Freescale i.MX95 which
> does have two power domains.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_device.c | 56 ++++++++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_device.h |  5 +++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index 51ee9cae94504..4348b7e917b64 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -75,6 +75,58 @@ static int panthor_reset_init(struct panthor_device *ptdev)
>  	return 0;
>  }
>  
> +/* Generic power domain handling code, see drivers/gpu/drm/tiny/simpledrm.c */
> +static void panthor_detach_genpd(void *res)
> +{
> +	struct panthor_device *ptdev = res;
> +	int i;
> +
> +	if (ptdev->pwr_dom_count <= 1)
> +		return;
> +
> +	for (i = ptdev->pwr_dom_count - 1; i >= 0; i--)
> +		dev_pm_domain_detach(ptdev->pwr_dom_devs[i], true);
> +}
> +
> +static int panthor_genpd_init(struct panthor_device *ptdev)
> +{
> +	struct device *dev = ptdev->base.dev;
> +	int i, ret;
> +
> +	ptdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node, "power-domains",
> +							  "#power-domain-cells");
> +	/*
> +	 * Single power-domain devices are handled by driver core nothing to do
> +	 * here. The same for device nodes without "power-domains" property.
> +	 */
> +	if (ptdev->pwr_dom_count <= 1)
> +		return 0;
> +
> +	if (ptdev->pwr_dom_count > ARRAY_SIZE(ptdev->pwr_dom_devs)) {
> +		drm_warn(&ptdev->base, "Too many power domains (%d) for this device\n",
> +			 ptdev->pwr_dom_count);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < ptdev->pwr_dom_count; i++) {
> +		ptdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i);
> +		if (!IS_ERR(ptdev->pwr_dom_devs[i]))
> +			continue;
> +
> +		ret = PTR_ERR(ptdev->pwr_dom_devs[i]);
> +		if (ret != -EPROBE_DEFER) {
> +			drm_warn(&ptdev->base, "pm_domain_attach_by_id(%u) failed: %d\n", i, ret);
> +			continue;

Is it a good idea to continue if a power-domain is missing? Any access might
stuck completely. IMHO returning an error is more sensible.
Also some dev_err_probe() should be added here.

Best regards
Alexander

> +		}
> +
> +		/* Missing dependency, try again. */
> +		panthor_detach_genpd(ptdev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(dev, panthor_detach_genpd, ptdev);
> +}
> +
>  void panthor_device_unplug(struct panthor_device *ptdev)
>  {
>  	/* This function can be called from two different path: the reset work
> @@ -232,6 +284,10 @@ int panthor_device_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = panthor_genpd_init(ptdev);
> +	if (ret)
> +		return ret;
> +
>  	ret = panthor_devfreq_init(ptdev);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index fea3a05778e2e..7fb65447253e9 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -114,6 +114,11 @@ struct panthor_device {
>  	/** @resets: GPU reset. */
>  	struct reset_control *resets;
>  
> +	/** @pwr_dom_count: Power domain count */
> +	int pwr_dom_count;
> +	/** @pwr_dom_dev: Power domain devices */
> +	struct device *pwr_dom_devs[5];
> +
>  	/** @coherent: True if the CPU/GPU are memory coherent. */
>  	bool coherent;
>  
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Alexander Stein]

Am Donnerstag, 27. Februar 2025, 17:58:07 CET schrieb Marek Vasut:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, document support for this variant.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index 0efa06822a543..3ab62bd424e41 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -18,6 +18,7 @@ properties:
>      oneOf:
>        - items:
>            - enum:
> +              - fsl,imx95-mali            # G310
>                - rockchip,rk3588-mali
>            - const: arm,mali-valhall-csf   # Mali Valhall GPU model/revision is fully discoverable
>  
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Alexander Stein]

Hi Marek,

Am Donnerstag, 27. Februar 2025, 17:58:08 CET schrieb Marek Vasut:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, add support for this variant.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 1 +
>  drivers/gpu/drm/panthor/panthor_gpu.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 06fe46e320738..2504a456d45c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>  ATTRIBUTE_GROUPS(panthor);
>  
>  static const struct of_device_id dt_match[] = {
> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
>  	{ .compatible = "rockchip,rk3588-mali" },
>  	{ .compatible = "arm,mali-valhall-csf" },
>  	{}
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 0f07ef7d9aea7..2371ab8e50627 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -67,6 +67,7 @@ struct panthor_model {
>  }
>  
>  static const struct panthor_model gpu_models[] = {
> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */

Are you sure about 0, 0? With this I get
> [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 status 0x1

Using GPU_MODEL(g310, 10, 4) I get:
> [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 status 0x1

Note the output is GPU_VER_MAJOR() but this table is for GPU_ARCH_MAJOR()
(and the corresponding minor).

Best regards,
Alexander

>  	GPU_MODEL(g610, 10, 7),
>  	{},
>  };
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marco Felsch]

On 25-02-27, Marek Vasut wrote:
> On 2/27/25 9:17 PM, Marco Felsch wrote:
> 
> [...]
> 
> > > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> > > index 06fe46e320738..2504a456d45c4 100644
> > > --- a/drivers/gpu/drm/panthor/panthor_drv.c
> > > +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> > > @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
> > >   ATTRIBUTE_GROUPS(panthor);
> > >   static const struct of_device_id dt_match[] = {
> > > +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
> > 			  ^
> > 			 nxp?
> > 
> > Can we switch to nxp instead?
> We can ... is that the current recommendation ?
> 
> Why not stick with fsl , is that deprecated now ?

Nope I don't think so but I do see patches adding 'nxp' as vendor
(mostly external chips) and some with 'fsl' (mostly soc internal ip
cores).

My hope was that at some point we could switch to 'nxp' only and make
use of this vendor-prefix. Of course NXP should start with that switch
but this seems not to happen :/

It's more a nit but maybe this triggers NXP to make use of the 'nxp'
vendor-prefix as well once they upstream a new base dtsi.

Regards,
  Marco

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Alexander Stein]

Hi Marek,

Am Donnerstag, 27. Februar 2025, 17:58:09 CET schrieb Marek Vasut:
> The instance of the GPU populated in i.MX95 is the G310,
> describe this GPU in the DT. Include description of the
> GPUMIX block controller, which can be operated as a simple
> reset. Include dummy GPU voltage regulator and OPP tables.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/boot/dts/freescale/imx95.dtsi | 62 ++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> index 3af13173de4bd..36bad211e5558 100644
> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> @@ -249,6 +249,37 @@ dummy: clock-dummy {
>  		clock-output-names = "dummy";
>  	};
>  
> +	gpu_fixed_reg: fixed-gpu-reg {
> +		compatible = "regulator-fixed";
> +		regulator-min-microvolt = <920000>;
> +		regulator-max-microvolt = <920000>;
> +		regulator-name = "vdd_gpu";
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};

Is this an internal voltage?

> +
> +	gpu_opp_table: opp_table {

Node-Names use dash instead of underscore.

> +		compatible = "operating-points-v2";
> +
> +		opp-500000000 {
> +			opp-hz = /bits/ 64 <500000000>;
> +			opp-hz-real = /bits/ 64 <500000000>;
> +			opp-microvolt = <920000>;
> +		};
> +
> +		opp-800000000 {
> +			opp-hz = /bits/ 64 <800000000>;
> +			opp-hz-real = /bits/ 64 <800000000>;
> +			opp-microvolt = <920000>;
> +		};
> +
> +		opp-1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +			opp-hz-real = /bits/ 64 <1000000000>;
> +			opp-microvolt = <920000>;
> +		};
> +	};
> +
>  	clk_ext1: clock-ext1 {
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
> @@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
>  			};
>  		};
>  
> +		gpu_blk_ctrl: reset-controller@4d810000 {
> +			compatible = "fsl,imx95-gpu-blk-ctrl";
> +			reg = <0x0 0x4d810000 0x0 0xc>;

Mh, GPU_BLK_CTRL is /just a bit) more than the GPU reset. Does it make sense
to make this an gpu-reset-only node, located at 0x4d810008?

> +			#reset-cells = <1>;
> +			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> +			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> +			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> +			assigned-clock-rates = <133333333>;
> +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
> +			status = "disabled";
> +		};
> +
> +		gpu: gpu@4d900000 {
> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> +			reg = <0 0x4d900000 0 0x480000>;
> +			clocks = <&scmi_clk IMX95_CLK_GPU>;

There is also IMX95_CLK_GPUAPB. Is this only required for the rese control above?

> +			clock-names = "core";
> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "gpu", "job", "mmu";

DT bindings say this order: job, mmu, gpu

Best regards
Alexander

> +			mali-supply = <&gpu_fixed_reg>;
> +			operating-points-v2 = <&gpu_opp_table>;
> +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
> +			power-domain-names = "mix", "perf";
> +			resets = <&gpu_blk_ctrl 0>;
> +			#cooling-cells = <2>;
> +			dynamic-power-coefficient = <1013>;
> +			status = "disabled";
> +		};
> +
>  		ddr-pmu@4e090dc0 {
>  			compatible = "fsl,imx95-ddr-pmu", "fsl,imx93-ddr-pmu";
>  			reg = <0x0 0x4e090dc0 0x0 0x200>;
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Alexander Stein]

Am Donnerstag, 27. Februar 2025, 23:21:22 CET schrieb Frank Li:
> On Thu, Feb 27, 2025 at 10:34:20PM +0100, Marek Vasut wrote:
> > On 2/27/25 10:27 PM, Frank Li wrote:
> >
> > [...]
> >
> > > > > > +		gpu: gpu@4d900000 {
> > > > > > +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> > > > > > +			reg = <0 0x4d900000 0 0x480000>;
> > > > > > +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> > > > > > +			clock-names = "core";
> > > > > > +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +			interrupt-names = "gpu", "job", "mmu";
> > > > > > +			mali-supply = <&gpu_fixed_reg>;
> > > > > > +			operating-points-v2 = <&gpu_opp_table>;
> > > > > > +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
> > > > > > +			power-domain-names = "mix", "perf";
> > > > > > +			resets = <&gpu_blk_ctrl 0>;
> > > > > > +			#cooling-cells = <2>;
> > > > > > +			dynamic-power-coefficient = <1013>;
> > > > > > +			status = "disabled";
> > > > >
> > > > > GPU is internal module, which have not much dependence with other module
> > > > > such as pinmux. why not default status is "disabled". Supposed gpu driver
> > > > > will turn off clock and power if not used.
> > > > My thinking was that there are MX95 SoC with GPU fused off, hence it is
> > > > better to keep the GPU disabled in DT by default. But I can also keep it
> > > > enabled and the few boards which do not have MX95 SoC with GPU can
> > > > explicitly disable it in board DT.
> > > >
> > > > What do you think ?
> > >
> > > GPU Fuse off should use access-control, see thread
> > > https://lore.kernel.org/imx/20250207120213.GD14860@localhost.localdomain/
> > Did that thread ever go anywhere ? It seems there is no real conclusion, is
> > there ? +Cc Alex .
> 
> The direction is use access-control to indicate fuse disable. Only
> implement detail is under discussion.

Well, the discussion ended up to be more complicated for i.MX8M. For i.MX95
things are a bit easier, as fuses and clocks are controlled by System
Manager (SM), accessed using SCMI. [1] is more important for imx95.

Best regards
Alexander

[1] https://lore.kernel.org/all/20250204-imx-ocotp-v8-0-01be4a4bb045@nxp.com/
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Liviu Dudau]

Hi Marek,

On Thu, Feb 27, 2025 at 05:58:06PM +0100, Marek Vasut wrote:
> This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
> from runtime PM suspend. Without this, if only the L2 is powered down
> on RPM entry, the GPU gets stuck and does not indicate the firmware is
> booted after RPM resume.

That doesn't sound right. Can you tell me what GPU firmware are you running
(we are now printing the git sha of the image at boot time).

Best regards,
Liviu

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 671049020afaa..0f07ef7d9aea7 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -470,11 +470,12 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
>   */
>  void panthor_gpu_suspend(struct panthor_device *ptdev)
>  {
> -	/* On a fast reset, simply power down the L2. */
> -	if (!ptdev->reset.fast)
> -		panthor_gpu_soft_reset(ptdev);
> -	else
> -		panthor_gpu_power_off(ptdev, L2, 1, 20000);
> +	/*
> +	 * Power off the L2 and soft reset the GPU, that makes
> +	 * iMX95 Mali G310 resume without firmware boot timeout.
> +	 */
> +	panthor_gpu_power_off(ptdev, L2, 1, 20000);
> +	panthor_gpu_soft_reset(ptdev);
>  
>  	panthor_gpu_irq_suspend(&ptdev->gpu->irq);
>  }
> -- 
> 2.47.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Florent Tomasin]

Hi Marek and Alexander,

On 28/02/2025 10:21, Alexander Stein wrote:
> Hi Marek,
> 
> Am Donnerstag, 27. Februar 2025, 17:58:08 CET schrieb Marek Vasut:
>> The instance of the GPU populated in Freescale i.MX95 is the
>> Mali G310, add support for this variant.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Fabio Estevam <festevam@gmail.com>
>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Sebastian Reichel <sre@kernel.org>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: devicetree@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: imx@lists.linux.dev
>> Cc: linux-arm-kernel@lists.infradead.org
>> ---
>>  drivers/gpu/drm/panthor/panthor_drv.c | 1 +
>>  drivers/gpu/drm/panthor/panthor_gpu.c | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
>> index 06fe46e320738..2504a456d45c4 100644
>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>>  ATTRIBUTE_GROUPS(panthor);
>>  
>>  static const struct of_device_id dt_match[] = {
>> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
No sure the comment about G310 is needed.

>>  	{ .compatible = "rockchip,rk3588-mali" },
>>  	{ .compatible = "arm,mali-valhall-csf" },
>>  	{}
>> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
>> index 0f07ef7d9aea7..2371ab8e50627 100644
>> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
>> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
>> @@ -67,6 +67,7 @@ struct panthor_model {
>>  }
>>  
>>  static const struct panthor_model gpu_models[] = {
>> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
We can remove the comment about the SoC. This GPU model could be found
in other vendor platforms.

> 
> Are you sure about 0, 0? With this I get
>> [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 status 0x1
> 
> Using GPU_MODEL(g310, 10, 4) I get:
>> [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 status 0x1

I'd agree it should be GPU_MODEL(g310, 10, 4). Unless the GPU
version has been changed by the vendor?

> Note the output is GPU_VER_MAJOR() but this table is for GPU_ARCH_MAJOR()
> (and the corresponding minor).
> 
> Best regards,
> Alexander
> 
>>  	GPU_MODEL(g610, 10, 7),
>>  	{},
>>  };
>>
> 
> 

Kind regards,
Florent

Re: [PATCH 3/9] dt-bindings: gpu: mali-valhall-csf: Document optional reset

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 17:58:03 +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 does require
> release from reset by writing into a single GPUMIX block controller
> GPURESET register bit 0. Document support for one optional reset.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml          | 3 +++
>  1 file changed, 3 insertions(+)
> 

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

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Rob Herring]

On Thu, Feb 27, 2025 at 09:31:48PM +0100, Marek Vasut wrote:
> On 2/27/25 7:38 PM, Rob Herring (Arm) wrote:
> > 
> > On Thu, 27 Feb 2025 17:58:07 +0100, Marek Vasut wrote:
> > > The instance of the GPU populated in Freescale i.MX95 is the
> > > Mali G310, document support for this variant.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Cc: Boris Brezillon <boris.brezillon@collabora.com>
> > > Cc: Conor Dooley <conor+dt@kernel.org>
> > > Cc: David Airlie <airlied@gmail.com>
> > > Cc: Fabio Estevam <festevam@gmail.com>
> > > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Sebastian Reichel <sre@kernel.org>
> > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > Cc: Simona Vetter <simona@ffwll.ch>
> > > Cc: Steven Price <steven.price@arm.com>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: imx@lists.linux.dev
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > ---
> > >   Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > 
> > My bot found errors running 'make dt_binding_check' on your patch:
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > 
> > 
> > doc reference errors (make refcheckdocs):
> > 
> > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250227170012.124768-8-marex@denx.de
> It seems there are no errors in this list ?

That's an artifact that a prior patch in this series failed.

Rob

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Rob Herring]

On Thu, Feb 27, 2025 at 05:58:07PM +0100, Marek Vasut wrote:
> The instance of the GPU populated in Freescale i.MX95 is the
> Mali G310, document support for this variant.

You should combine patch 4 with this one and make resets required for 
imx95 since you said it is required.

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index 0efa06822a543..3ab62bd424e41 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -18,6 +18,7 @@ properties:
>      oneOf:
>        - items:
>            - enum:
> +              - fsl,imx95-mali            # G310
>                - rockchip,rk3588-mali
>            - const: arm,mali-valhall-csf   # Mali Valhall GPU model/revision is fully discoverable
>  
> -- 
> 2.47.2
> 

Re: [PATCH 0/9] arm64: dts: imx95: Add support for Mali G310 GPU

[Rob Herring (Arm)]

On Thu, 27 Feb 2025 17:58:00 +0100, Marek Vasut wrote:
> The instance of the GPU populated in i.MX95 is the G310.
> Add support for the GPUMIX reset via simple-reset driver,
> add reset and multiple power domains support into panthor
> GPU driver, add iMX95 GPU support into panthor driver and
> describe the iMX95 GPU in imx95.dtsi DT.
> 
> Marek Vasut (9):
>   dt-bindings: reset: imx95-gpu-blk-ctrl: Document Freescale i.MX95 GPU
>     reset
>   reset: simple: Add support for Freescale i.MX95 GPU reset
>   dt-bindings: gpu: mali-valhall-csf: Document optional reset
>   drm/panthor: Implement optional reset
>   drm/panthor: Implement support for multiple power domains
>   drm/panthor: Reset GPU after L2 cache power off
>   dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support
>   drm/panthor: Add i.MX95 support
>   arm64: dts: imx95: Describe Mali G310 GPU
> 
>  .../bindings/gpu/arm,mali-valhall-csf.yaml    |  4 +
>  .../reset/fsl,imx95-gpu-blk-ctrl.yaml         | 49 ++++++++++++
>  arch/arm64/boot/dts/freescale/imx95.dtsi      | 62 +++++++++++++++
>  drivers/gpu/drm/panthor/Kconfig               |  1 +
>  drivers/gpu/drm/panthor/panthor_device.c      | 79 +++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_device.h      |  8 ++
>  drivers/gpu/drm/panthor/panthor_drv.c         |  1 +
>  drivers/gpu/drm/panthor/panthor_gpu.c         | 12 +--
>  drivers/reset/reset-simple.c                  |  8 ++
>  9 files changed, 219 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx95-gpu-blk-ctrl.yaml
> 
> ---
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> 
> --
> 2.47.2
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/freescale/' for 20250227170012.124768-1-marex@denx.de:

arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: opp_table: $nodename:0: 'opp_table' does not match '^opp-table(-[a-z0-9]+)?$'
	from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: opp_table: Unevaluated properties are not allowed ('opp-1000000000', 'opp-500000000', 'opp-800000000' were unexpected)
	from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: /soc/reset-controller@4d810000: failed to match any schema with compatible: ['fsl,imx95-gpu-blk-ctrl']
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: gpu@4d900000: interrupt-names:0: 'job' was expected
	from schema $id: http://devicetree.org/schemas/gpu/arm,mali-valhall-csf.yaml#
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: gpu@4d900000: interrupt-names:1: 'mmu' was expected
	from schema $id: http://devicetree.org/schemas/gpu/arm,mali-valhall-csf.yaml#
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dtb: gpu@4d900000: interrupt-names:2: 'gpu' was expected
	from schema $id: http://devicetree.org/schemas/gpu/arm,mali-valhall-csf.yaml#

Re: [PATCH 2/9] reset: simple: Add support for Freescale i.MX95 GPU reset

[Marek Vasut]

On 2/28/25 10:58 AM, Alexander Stein wrote:

Hi,

>>   drivers/reset/reset-simple.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
>> index 2760678398308..1415a941fd6eb 100644
>> --- a/drivers/reset/reset-simple.c
>> +++ b/drivers/reset/reset-simple.c
>> @@ -133,9 +133,17 @@ static const struct reset_simple_devdata reset_simple_active_low = {
>>   	.status_active_low = true,
>>   };
>>   
>> +static const struct reset_simple_devdata reset_simple_fsl_imx95_gpu_blk_ctrl = {
>> +	.reg_offset = 0x8,
> 
> Shouldn't you add ".nr_resets = 1"? Otherwise this will have 8 resets
> (resource_size(res) * BITS_PER_BYTE).

I should, thanks.

> On a side note: RM says this is a write-once register. Do we consider writing
> this register again?
> BTW: Would it be possible to disable it completely (until reset) by writing 1?
My observation is that this register is set and cannot be cleared by 
writing 0 into it, if I write 0 into this register and read it back, it 
stays at 1 .

Maybe NXP can clarify whether modelling this as reset-simple is really a 
good idea or whether it would be better to rewrite this as custom reset 
driver ?

[...]

Re: [PATCH 4/9] drm/panthor: Implement optional reset

[Marek Vasut]

On 2/28/25 11:06 AM, Alexander Stein wrote:

Hi,

>> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
>> index a9da1d1eeb707..51ee9cae94504 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.c
>> +++ b/drivers/gpu/drm/panthor/panthor_device.c
>> @@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev)
>>   	return 0;
>>   }
>>   
>> +static int panthor_reset_init(struct panthor_device *ptdev)
>> +{
>> +	ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL);
> 
> If the description as a write-once register is true, wouldn't this
> already write to it?

It would. I believe the reset handling in the GPU driver should be 
generic. The reset has to be deasserted here to access the GPU registers.

The question is, whether the GPUMIX GPURESET bit should be modeled as 
simple reset, even if it really behaves as a 
set-and-never-can-be-cleared latched bit.

>> +	if (IS_ERR(ptdev->resets))
>> +		return dev_err_probe(ptdev->base.dev,
>> +				     PTR_ERR(ptdev->resets),
>> +				     "get reset failed");
>> +
>> +	return 0;
>> +}
>> +
>>   void panthor_device_unplug(struct panthor_device *ptdev)
>>   {
>>   	/* This function can be called from two different path: the reset work
>> @@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	ret = panthor_reset_init(ptdev);
>> +	if (ret)
>> +		return ret;
>> +
>>   	ret = panthor_devfreq_init(ptdev);
>>   	if (ret)
>>   		return ret;
>> @@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev)
>>   	if (ret)
>>   		goto err_disable_stacks_clk;
>>   
>> +	ret = reset_control_deassert(ptdev->resets);
>> +	if (ret)
>> +		goto err_disable_coregroup_clk;
>> +
> 
> This wouldn't work at all on a write-once register, no? Same for resume.
See above and also my reply to 2/9 .

Re: [PATCH 5/9] drm/panthor: Implement support for multiple power domains

[Marek Vasut]

On 2/28/25 11:10 AM, Alexander Stein wrote:

Hi,

>> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
>> index 51ee9cae94504..4348b7e917b64 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.c
>> +++ b/drivers/gpu/drm/panthor/panthor_device.c
>> @@ -75,6 +75,58 @@ static int panthor_reset_init(struct panthor_device *ptdev)
>>   	return 0;
>>   }
>>   
>> +/* Generic power domain handling code, see drivers/gpu/drm/tiny/simpledrm.c */
>> +static void panthor_detach_genpd(void *res)
>> +{
>> +	struct panthor_device *ptdev = res;
>> +	int i;
>> +
>> +	if (ptdev->pwr_dom_count <= 1)
>> +		return;
>> +
>> +	for (i = ptdev->pwr_dom_count - 1; i >= 0; i--)
>> +		dev_pm_domain_detach(ptdev->pwr_dom_devs[i], true);
>> +}
>> +
>> +static int panthor_genpd_init(struct panthor_device *ptdev)
>> +{
>> +	struct device *dev = ptdev->base.dev;
>> +	int i, ret;
>> +
>> +	ptdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node, "power-domains",
>> +							  "#power-domain-cells");
>> +	/*
>> +	 * Single power-domain devices are handled by driver core nothing to do
>> +	 * here. The same for device nodes without "power-domains" property.
>> +	 */
>> +	if (ptdev->pwr_dom_count <= 1)
>> +		return 0;
>> +
>> +	if (ptdev->pwr_dom_count > ARRAY_SIZE(ptdev->pwr_dom_devs)) {
>> +		drm_warn(&ptdev->base, "Too many power domains (%d) for this device\n",
>> +			 ptdev->pwr_dom_count);
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (i = 0; i < ptdev->pwr_dom_count; i++) {
>> +		ptdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i);
>> +		if (!IS_ERR(ptdev->pwr_dom_devs[i]))
>> +			continue;
>> +
>> +		ret = PTR_ERR(ptdev->pwr_dom_devs[i]);
>> +		if (ret != -EPROBE_DEFER) {
>> +			drm_warn(&ptdev->base, "pm_domain_attach_by_id(%u) failed: %d\n", i, ret);
>> +			continue;
> 
> Is it a good idea to continue if a power-domain is missing? Any access might
> stuck completely. IMHO returning an error is more sensible.
> Also some dev_err_probe() should be added here.
Fixed both, thanks .

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Marek Vasut]

On 2/28/25 12:06 PM, Liviu Dudau wrote:
> Hi Marek,

Hi,

> On Thu, Feb 27, 2025 at 05:58:06PM +0100, Marek Vasut wrote:
>> This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
>> from runtime PM suspend. Without this, if only the L2 is powered down
>> on RPM entry, the GPU gets stuck and does not indicate the firmware is
>> booted after RPM resume.
> 
> That doesn't sound right. Can you tell me what GPU firmware are you running
> (we are now printing the git sha of the image at boot time).
Please see below. It could be some sort of NXP firmware fork ?

It comes from the NXP firmware repo , mali-imx-r50.2-710cfb6 .

panthor 4d900000.gpu: [drm] clock rate = 1000000000
panthor 4d900000.gpu: EM: created perf domain
panthor 4d900000.gpu: [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 
status 0x1
panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1 
MMU:0x2830 AS:0xff
panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1 
tiler_present=0x1
panthor 4d900000.gpu: [drm] Firmware protected mode entry not be 
supported, ignoring
panthor 4d900000.gpu: [drm] Firmware git sha: 
8e5cfcfec20cc8aff8509d37e72babc935d34a3b
panthor 4d900000.gpu: [drm] CSF FW using interface v1.1.0, Features 0x0 
Instrumentation features 0x71
[drm] Initialized panthor 1.3.0 for 4d900000.gpu on minor 0

$ sha256sum mali_csffw.bin
befd23b9279b26010ebf35c835cc3aa7d0f50758930c7306ff4f5abbe5218624 
mali_csffw.bin

[...]

Re: [PATCH 7/9] dt-bindings: gpu: mali-valhall-csf: Document i.MX95 support

[Marek Vasut]

On 2/28/25 1:30 PM, Rob Herring wrote:
> On Thu, Feb 27, 2025 at 05:58:07PM +0100, Marek Vasut wrote:
>> The instance of the GPU populated in Freescale i.MX95 is the
>> Mali G310, document support for this variant.
> 
> You should combine patch 4 with this one and make resets required for
> imx95 since you said it is required.
The reset support is orthogonal from MX95, but I did add the resets 
requirement for MX95 , thanks !

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marek Vasut]

On 2/28/25 11:33 AM, Marco Felsch wrote:
> On 25-02-27, Marek Vasut wrote:
>> On 2/27/25 9:17 PM, Marco Felsch wrote:
>>
>> [...]
>>
>>>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
>>>> index 06fe46e320738..2504a456d45c4 100644
>>>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>>>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>>>> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>>>>    ATTRIBUTE_GROUPS(panthor);
>>>>    static const struct of_device_id dt_match[] = {
>>>> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
>>> 			  ^
>>> 			 nxp?
>>>
>>> Can we switch to nxp instead?
>> We can ... is that the current recommendation ?
>>
>> Why not stick with fsl , is that deprecated now ?
> 
> Nope I don't think so but I do see patches adding 'nxp' as vendor
> (mostly external chips) and some with 'fsl' (mostly soc internal ip
> cores).
> 
> My hope was that at some point we could switch to 'nxp' only and make
> use of this vendor-prefix. Of course NXP should start with that switch
> but this seems not to happen :/
> 
> It's more a nit but maybe this triggers NXP to make use of the 'nxp'
> vendor-prefix as well once they upstream a new base dtsi.
OK, fixed.

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marek Vasut]

On 2/28/25 11:21 AM, Alexander Stein wrote:

Hi,

>> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
>> index 0f07ef7d9aea7..2371ab8e50627 100644
>> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
>> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
>> @@ -67,6 +67,7 @@ struct panthor_model {
>>   }
>>   
>>   static const struct panthor_model gpu_models[] = {
>> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
> 
> Are you sure about 0, 0?

I am not.

> With this I get
>> [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 status 0x1
> 
> Using GPU_MODEL(g310, 10, 4) I get:
>> [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 status 0x1
> 
> Note the output is GPU_VER_MAJOR() but this table is for GPU_ARCH_MAJOR()
> (and the corresponding minor).
The major 0x0 minor 0x0 is where I took the 0,0 from. Lemme update this.

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Marek Vasut]

On 2/28/25 12:23 PM, Florent Tomasin wrote:

Hi,

>>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
>>> index 06fe46e320738..2504a456d45c4 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>>> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>>>   ATTRIBUTE_GROUPS(panthor);
>>>   
>>>   static const struct of_device_id dt_match[] = {
>>> +	{ .compatible = "fsl,imx95-mali" },	/* G310 */
> No sure the comment about G310 is needed.
> 
>>>   	{ .compatible = "rockchip,rk3588-mali" },
>>>   	{ .compatible = "arm,mali-valhall-csf" },
>>>   	{}
>>> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
>>> index 0f07ef7d9aea7..2371ab8e50627 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
>>> @@ -67,6 +67,7 @@ struct panthor_model {
>>>   }
>>>   
>>>   static const struct panthor_model gpu_models[] = {
>>> +	GPU_MODEL(g310, 0, 0),	/* NXP i.MX95 */
> We can remove the comment about the SoC. This GPU model could be found
> in other vendor platforms.

Maybe it is a good idea to keep track of which GPU is in which SoC at 
least this way ?

>> Are you sure about 0, 0? With this I get
>>> [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 status 0x1
>>
>> Using GPU_MODEL(g310, 10, 4) I get:
>>> [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 status 0x1
> 
> I'd agree it should be GPU_MODEL(g310, 10, 4). Unless the GPU
> version has been changed by the vendor?
With the fix suggested by Alexander, I get the following output on boot, 
so I think 10/4 is OK ?

panthor 4d900000.gpu: [drm] clock rate = 1000000000
panthor 4d900000.gpu: EM: created perf domain
panthor 4d900000.gpu: [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 
status 0x1
panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1 
MMU:0x2830 AS:0xff
panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1 
tiler_present=0x1
panthor 4d900000.gpu: [drm] Firmware protected mode entry not be 
supported, ignoring
panthor 4d900000.gpu: [drm] Firmware git sha: 
8e5cfcfec20cc8aff8509d37e72babc935d34a3b
panthor 4d900000.gpu: [drm] CSF FW using interface v1.1.0, Features 0x0 
Instrumentation features 0x71
[drm] Initialized panthor 1.3.0 for 4d900000.gpu on minor 0

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 2/28/25 11:39 AM, Alexander Stein wrote:
> Am Donnerstag, 27. Februar 2025, 23:21:22 CET schrieb Frank Li:
>> On Thu, Feb 27, 2025 at 10:34:20PM +0100, Marek Vasut wrote:
>>> On 2/27/25 10:27 PM, Frank Li wrote:
>>>
>>> [...]
>>>
>>>>>>> +		gpu: gpu@4d900000 {
>>>>>>> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
>>>>>>> +			reg = <0 0x4d900000 0 0x480000>;
>>>>>>> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
>>>>>>> +			clock-names = "core";
>>>>>>> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
>>>>>>> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
>>>>>>> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
>>>>>>> +			interrupt-names = "gpu", "job", "mmu";
>>>>>>> +			mali-supply = <&gpu_fixed_reg>;
>>>>>>> +			operating-points-v2 = <&gpu_opp_table>;
>>>>>>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>, <&scmi_perf IMX95_PERF_GPU>;
>>>>>>> +			power-domain-names = "mix", "perf";
>>>>>>> +			resets = <&gpu_blk_ctrl 0>;
>>>>>>> +			#cooling-cells = <2>;
>>>>>>> +			dynamic-power-coefficient = <1013>;
>>>>>>> +			status = "disabled";
>>>>>>
>>>>>> GPU is internal module, which have not much dependence with other module
>>>>>> such as pinmux. why not default status is "disabled". Supposed gpu driver
>>>>>> will turn off clock and power if not used.
>>>>> My thinking was that there are MX95 SoC with GPU fused off, hence it is
>>>>> better to keep the GPU disabled in DT by default. But I can also keep it
>>>>> enabled and the few boards which do not have MX95 SoC with GPU can
>>>>> explicitly disable it in board DT.
>>>>>
>>>>> What do you think ?
>>>>
>>>> GPU Fuse off should use access-control, see thread
>>>> https://lore.kernel.org/imx/20250207120213.GD14860@localhost.localdomain/
>>> Did that thread ever go anywhere ? It seems there is no real conclusion, is
>>> there ? +Cc Alex .
>>
>> The direction is use access-control to indicate fuse disable. Only
>> implement detail is under discussion.
> 
> Well, the discussion ended up to be more complicated for i.MX8M.

Aren't we missing the access controller in MX95 DT so far ?
And maybe some driver to match ?

> For i.MX95
> things are a bit easier, as fuses and clocks are controlled by System
> Manager (SM), accessed using SCMI. [1] is more important for imx95.
I think SCMI is replacing GPL code in kernel with stubs that call 
non-free firmware, which removes flexibility and makes long term (or 
any) maintenance much more complicated.

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 2/28/25 11:36 AM, Alexander Stein wrote:
> Hi Marek,

Hi,

>> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
>> index 3af13173de4bd..36bad211e5558 100644
>> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
>> @@ -249,6 +249,37 @@ dummy: clock-dummy {
>>   		clock-output-names = "dummy";
>>   	};
>>   
>> +	gpu_fixed_reg: fixed-gpu-reg {
>> +		compatible = "regulator-fixed";
>> +		regulator-min-microvolt = <920000>;
>> +		regulator-max-microvolt = <920000>;
>> +		regulator-name = "vdd_gpu";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +	};
> 
> Is this an internal voltage?

I think so.

>> +
>> +	gpu_opp_table: opp_table {
> 
> Node-Names use dash instead of underscore.

Fixed, thanks.

[...]

>> @@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
>>   			};
>>   		};
>>   
>> +		gpu_blk_ctrl: reset-controller@4d810000 {
>> +			compatible = "fsl,imx95-gpu-blk-ctrl";
>> +			reg = <0x0 0x4d810000 0x0 0xc>;
> 
> Mh, GPU_BLK_CTRL is /just a bit) more than the GPU reset. Does it make sense
> to make this an gpu-reset-only node, located at 0x4d810008?

The block controller itself is larger, it spans 3 or 4 registers, so 
this should describe the entire block controller here.

>> +			#reset-cells = <1>;
>> +			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
>> +			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
>> +			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
>> +			assigned-clock-rates = <133333333>;
>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
>> +			status = "disabled";
>> +		};
>> +
>> +		gpu: gpu@4d900000 {
>> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
>> +			reg = <0 0x4d900000 0 0x480000>;
>> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> 
> There is also IMX95_CLK_GPUAPB. Is this only required for the rese control above?

I think I have to describe those clock here too, possibly as 'coregroup' 
clock ?

>> +			clock-names = "core";
>> +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "gpu", "job", "mmu";
> 
> DT bindings say this order: job, mmu, gpu
Yes, currently it is sorted by IRQ number, fixed.

Re: [PATCH 4/9] drm/panthor: Implement optional reset

[kernel test robot]

Hi Marek,

kernel test robot noticed the following build warnings:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on robh/for-next arm64/for-next/core kvmarm/next rockchip/for-next soc/for-next linus/master v6.14-rc4 next-20250228]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marek-Vasut/dt-bindings-reset-imx95-gpu-blk-ctrl-Document-Freescale-i-MX95-GPU-reset/20250228-011031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
patch link:    https://lore.kernel.org/r/20250227170012.124768-5-marex%40denx.de
patch subject: [PATCH 4/9] drm/panthor: Implement optional reset
config: x86_64-kismet-CONFIG_RESET_SIMPLE-CONFIG_DRM_PANTHOR-0-0 (https://download.01.org/0day-ci/archive/20250302/202503020521.MBUsoVte-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20250302/202503020521.MBUsoVte-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503020521.MBUsoVte-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for RESET_SIMPLE when selected by DRM_PANTHOR
   WARNING: unmet direct dependencies detected for RESET_SIMPLE
     Depends on [n]: RESET_CONTROLLER [=n] && HAS_IOMEM [=y]
     Selected by [y]:
     - DRM_PANTHOR [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARM || ARM64 || COMPILE_TEST [=y]) && !GENERIC_ATOMIC64 [=n] && MMU [=y] && SOC_IMX9 [=y]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Liviu Dudau]

On Fri, Feb 28, 2025 at 06:05:36PM +0100, Marek Vasut wrote:
> On 2/28/25 12:06 PM, Liviu Dudau wrote:
> > Hi Marek,
> 
> Hi,
> 
> > On Thu, Feb 27, 2025 at 05:58:06PM +0100, Marek Vasut wrote:
> > > This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
> > > from runtime PM suspend. Without this, if only the L2 is powered down
> > > on RPM entry, the GPU gets stuck and does not indicate the firmware is
> > > booted after RPM resume.
> > 
> > That doesn't sound right. Can you tell me what GPU firmware are you running
> > (we are now printing the git sha of the image at boot time).
> Please see below. It could be some sort of NXP firmware fork ?
> 
> It comes from the NXP firmware repo , mali-imx-r50.2-710cfb6 .
> 
> panthor 4d900000.gpu: [drm] clock rate = 1000000000
> panthor 4d900000.gpu: EM: created perf domain
> panthor 4d900000.gpu: [drm] mali-unknown id 0xac74 major 0x0 minor 0x0
> status 0x1
> panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1
> MMU:0x2830 AS:0xff
> panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1
> tiler_present=0x1
> panthor 4d900000.gpu: [drm] Firmware protected mode entry not be supported,
> ignoring
> panthor 4d900000.gpu: [drm] Firmware git sha:
> 8e5cfcfec20cc8aff8509d37e72babc935d34a3b

This looks like it has been part of a R50 release of the DDK, which is recent
enough to consider it up-to-date. The issues you're seeing with fast resume are
probably due to some integration issues or other quirks.

Boris has the most recent experience with playing with fast resume, maybe
he can share some tips on where to add messages in Panthor to try to debug
your problem.

Best regards,
Liviu

> panthor 4d900000.gpu: [drm] CSF FW using interface v1.1.0, Features 0x0
> Instrumentation features 0x71
> [drm] Initialized panthor 1.3.0 for 4d900000.gpu on minor 0
> 
> $ sha256sum mali_csffw.bin
> befd23b9279b26010ebf35c835cc3aa7d0f50758930c7306ff4f5abbe5218624
> mali_csffw.bin
> 
> [...]

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Boris Brezillon]

On Mon, 3 Mar 2025 12:08:51 +0000
Liviu Dudau <liviu.dudau@arm.com> wrote:

> On Fri, Feb 28, 2025 at 06:05:36PM +0100, Marek Vasut wrote:
> > On 2/28/25 12:06 PM, Liviu Dudau wrote:  
> > > Hi Marek,  
> > 
> > Hi,
> >   
> > > On Thu, Feb 27, 2025 at 05:58:06PM +0100, Marek Vasut wrote:  
> > > > This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
> > > > from runtime PM suspend. Without this, if only the L2 is powered down
> > > > on RPM entry, the GPU gets stuck and does not indicate the firmware is
> > > > booted after RPM resume.  
> > > 
> > > That doesn't sound right. Can you tell me what GPU firmware are you running
> > > (we are now printing the git sha of the image at boot time).  
> > Please see below. It could be some sort of NXP firmware fork ?
> > 
> > It comes from the NXP firmware repo , mali-imx-r50.2-710cfb6 .
> > 
> > panthor 4d900000.gpu: [drm] clock rate = 1000000000
> > panthor 4d900000.gpu: EM: created perf domain
> > panthor 4d900000.gpu: [drm] mali-unknown id 0xac74 major 0x0 minor 0x0
> > status 0x1
> > panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1
> > MMU:0x2830 AS:0xff
> > panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1
> > tiler_present=0x1
> > panthor 4d900000.gpu: [drm] Firmware protected mode entry not be supported,
> > ignoring
> > panthor 4d900000.gpu: [drm] Firmware git sha:
> > 8e5cfcfec20cc8aff8509d37e72babc935d34a3b  
> 
> This looks like it has been part of a R50 release of the DDK, which is recent
> enough to consider it up-to-date. The issues you're seeing with fast resume are
> probably due to some integration issues or other quirks.
> 
> Boris has the most recent experience with playing with fast resume, maybe
> he can share some tips on where to add messages in Panthor to try to debug
> your problem.

If you end up with fast_reset=true, that means the FW claims it entered
a suspend state from which is can resume quickly (fast-reset), and in
that case, we're only supposed to power on the L2 block at resume time
AFAIK. If there's a component that can tell us more, it's probably the
FW, unfortunately, it doesn't tell us much through the FW interface,
especially if it's not booting properly. Do you hit this error [1]
without this patch, and if you do, what's the status reported there?

[1]https://elixir.bootlin.com/linux/v6.14-rc4/source/drivers/gpu/drm/panthor/panthor_fw.c#L1051

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Boris Brezillon]

On Mon, 3 Mar 2025 13:35:23 +0100
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> On Mon, 3 Mar 2025 12:08:51 +0000
> Liviu Dudau <liviu.dudau@arm.com> wrote:
> 
> > On Fri, Feb 28, 2025 at 06:05:36PM +0100, Marek Vasut wrote:  
> > > On 2/28/25 12:06 PM, Liviu Dudau wrote:    
> > > > Hi Marek,    
> > > 
> > > Hi,
> > >     
> > > > On Thu, Feb 27, 2025 at 05:58:06PM +0100, Marek Vasut wrote:    
> > > > > This seems necessary on Freescale i.MX95 Mali G310 to reliably resume
> > > > > from runtime PM suspend. Without this, if only the L2 is powered down
> > > > > on RPM entry, the GPU gets stuck and does not indicate the firmware is
> > > > > booted after RPM resume.    
> > > > 
> > > > That doesn't sound right. Can you tell me what GPU firmware are you running
> > > > (we are now printing the git sha of the image at boot time).    
> > > Please see below. It could be some sort of NXP firmware fork ?
> > > 
> > > It comes from the NXP firmware repo , mali-imx-r50.2-710cfb6 .
> > > 
> > > panthor 4d900000.gpu: [drm] clock rate = 1000000000
> > > panthor 4d900000.gpu: EM: created perf domain
> > > panthor 4d900000.gpu: [drm] mali-unknown id 0xac74 major 0x0 minor 0x0
> > > status 0x1
> > > panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1
> > > MMU:0x2830 AS:0xff
> > > panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1
> > > tiler_present=0x1
> > > panthor 4d900000.gpu: [drm] Firmware protected mode entry not be supported,
> > > ignoring
> > > panthor 4d900000.gpu: [drm] Firmware git sha:
> > > 8e5cfcfec20cc8aff8509d37e72babc935d34a3b    
> > 
> > This looks like it has been part of a R50 release of the DDK, which is recent
> > enough to consider it up-to-date. The issues you're seeing with fast resume are
> > probably due to some integration issues or other quirks.
> > 
> > Boris has the most recent experience with playing with fast resume, maybe
> > he can share some tips on where to add messages in Panthor to try to debug
> > your problem.  
> 
> If you end up with fast_reset=true, that means the FW claims it entered
> a suspend state from which is can resume quickly (fast-reset), and in
> that case, we're only supposed to power on the L2 block at resume time
> AFAIK.

Can you try deasserting the reset lines at probe time and keeping them
deasserted until the device is removed instead of re-asserting at
suspend time? If that doesn't work, try doing the same with power
domains.

> If there's a component that can tell us more, it's probably the
> FW, unfortunately, it doesn't tell us much through the FW interface,
> especially if it's not booting properly. Do you hit this error [1]
> without this patch, and if you do, what's the status reported there?
> 
> [1]https://elixir.bootlin.com/linux/v6.14-rc4/source/drivers/gpu/drm/panthor/panthor_fw.c#L1051

Re: [PATCH 8/9] drm/panthor: Add i.MX95 support

[Florent Tomasin]

Hi Marek,

On 28/02/2025 17:26, Marek Vasut wrote:
> On 2/28/25 12:23 PM, Florent Tomasin wrote:
> 
> Hi,
> 
>>>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/
>>>> drm/panthor/panthor_drv.c
>>>> index 06fe46e320738..2504a456d45c4 100644
>>>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>>>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>>>> @@ -1591,6 +1591,7 @@ static struct attribute *panthor_attrs[] = {
>>>>   ATTRIBUTE_GROUPS(panthor);
>>>>     static const struct of_device_id dt_match[] = {
>>>> +    { .compatible = "fsl,imx95-mali" },    /* G310 */
>> No sure the comment about G310 is needed.
>>
>>>>       { .compatible = "rockchip,rk3588-mali" },
>>>>       { .compatible = "arm,mali-valhall-csf" },
>>>>       {}
>>>> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/
>>>> drm/panthor/panthor_gpu.c
>>>> index 0f07ef7d9aea7..2371ab8e50627 100644
>>>> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
>>>> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
>>>> @@ -67,6 +67,7 @@ struct panthor_model {
>>>>   }
>>>>     static const struct panthor_model gpu_models[] = {
>>>> +    GPU_MODEL(g310, 0, 0),    /* NXP i.MX95 */
>> We can remove the comment about the SoC. This GPU model could be found
>> in other vendor platforms.
> 
> Maybe it is a good idea to keep track of which GPU is in which SoC at
> least this way ?

I am worried the list gets bigger and bigger.
Maybe a comment in the DTB binding would be me more appropriate and
easier to search?

>>> Are you sure about 0, 0? With this I get
>>>> [drm] mali-unknown id 0xac74 major 0x0 minor 0x0 status 0x1
>>>
>>> Using GPU_MODEL(g310, 10, 4) I get:
>>>> [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 status 0x1
>>
>> I'd agree it should be GPU_MODEL(g310, 10, 4). Unless the GPU
>> version has been changed by the vendor?
> With the fix suggested by Alexander, I get the following output on boot,
> so I think 10/4 is OK ?
> 
> panthor 4d900000.gpu: [drm] clock rate = 1000000000
> panthor 4d900000.gpu: EM: created perf domain
> panthor 4d900000.gpu: [drm] mali-g310 id 0xac74 major 0x0 minor 0x0
> status 0x1
> panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1
> MMU:0x2830 AS:0xff
> panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1
> tiler_present=0x1
> panthor 4d900000.gpu: [drm] Firmware protected mode entry not be
> supported, ignoring
> panthor 4d900000.gpu: [drm] Firmware git sha:
> 8e5cfcfec20cc8aff8509d37e72babc935d34a3b
> panthor 4d900000.gpu: [drm] CSF FW using interface v1.1.0, Features 0x0
> Instrumentation features 0x71
> [drm] Initialized panthor 1.3.0 for 4d900000.gpu on minor 0

It looks about right

Regards,
Florent

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Liviu Dudau]

On Fri, Feb 28, 2025 at 06:43:53PM +0100, Marek Vasut wrote:
> On 2/28/25 11:36 AM, Alexander Stein wrote:
> > Hi Marek,
> 
> Hi,
> 
> > > diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > index 3af13173de4bd..36bad211e5558 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> > > @@ -249,6 +249,37 @@ dummy: clock-dummy {
> > >   		clock-output-names = "dummy";
> > >   	};
> > > +	gpu_fixed_reg: fixed-gpu-reg {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-min-microvolt = <920000>;
> > > +		regulator-max-microvolt = <920000>;
> > > +		regulator-name = "vdd_gpu";
> > > +		regulator-always-on;
> > > +		regulator-boot-on;
> > > +	};
> > 
> > Is this an internal voltage?
> 
> I think so.
> 
> > > +
> > > +	gpu_opp_table: opp_table {
> > 
> > Node-Names use dash instead of underscore.
> 
> Fixed, thanks.
> 
> [...]
> 
> > > @@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
> > >   			};
> > >   		};
> > > +		gpu_blk_ctrl: reset-controller@4d810000 {
> > > +			compatible = "fsl,imx95-gpu-blk-ctrl";
> > > +			reg = <0x0 0x4d810000 0x0 0xc>;
> > 
> > Mh, GPU_BLK_CTRL is /just a bit) more than the GPU reset. Does it make sense
> > to make this an gpu-reset-only node, located at 0x4d810008?
> 
> The block controller itself is larger, it spans 3 or 4 registers, so this
> should describe the entire block controller here.
> 
> > > +			#reset-cells = <1>;
> > > +			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> > > +			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
> > > +			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> > > +			assigned-clock-rates = <133333333>;
> > > +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		gpu: gpu@4d900000 {
> > > +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
> > > +			reg = <0 0x4d900000 0 0x480000>;
> > > +			clocks = <&scmi_clk IMX95_CLK_GPU>;
> > 
> > There is also IMX95_CLK_GPUAPB. Is this only required for the rese control above?
> 
> I think I have to describe those clock here too, possibly as 'coregroup'
> clock ?

The 'coregroup' clock does indeed control the MMU and L2$ blocks as well as the AXI interface,
so if that is indeed a separate external clock source it should be defined. Could it be why
you're seeing issues with L2$ resume on the fast reset path?

Best regards,
Liviu

> 
> > > +			clock-names = "core";
> > > +			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "gpu", "job", "mmu";
> > 
> > DT bindings say this order: job, mmu, gpu
> Yes, currently it is sorted by IRQ number, fixed.

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 3/3/25 4:04 PM, Liviu Dudau wrote:

[...]

>>>> +			#reset-cells = <1>;
>>>> +			clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
>>>> +			assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
>>>> +			assigned-clock-parents = <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
>>>> +			assigned-clock-rates = <133333333>;
>>>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
>>>> +			status = "disabled";
>>>> +		};
>>>> +
>>>> +		gpu: gpu@4d900000 {
>>>> +			compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
>>>> +			reg = <0 0x4d900000 0 0x480000>;
>>>> +			clocks = <&scmi_clk IMX95_CLK_GPU>;
>>>
>>> There is also IMX95_CLK_GPUAPB. Is this only required for the rese control above?
>>
>> I think I have to describe those clock here too, possibly as 'coregroup'
>> clock ?
> 
> The 'coregroup' clock does indeed control the MMU and L2$ blocks as well as the AXI interface,
> so if that is indeed a separate external clock source it should be defined. Could it be why
> you're seeing issues with L2$ resume on the fast reset path?
Sorry for the delayed reply.

I did define these clock, they are enabled, but the L2 reset problem is 
still present .

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Marek Vasut]

On 3/3/25 1:35 PM, Boris Brezillon wrote:

Hi,

>> This looks like it has been part of a R50 release of the DDK, which is recent
>> enough to consider it up-to-date. The issues you're seeing with fast resume are
>> probably due to some integration issues or other quirks.
>>
>> Boris has the most recent experience with playing with fast resume, maybe
>> he can share some tips on where to add messages in Panthor to try to debug
>> your problem.
> 
> If you end up with fast_reset=true, that means the FW claims it entered
> a suspend state from which is can resume quickly (fast-reset), and in
> that case, we're only supposed to power on the L2 block at resume time
> AFAIK. If there's a component that can tell us more, it's probably the
> FW, unfortunately, it doesn't tell us much through the FW interface,
> especially if it's not booting properly. Do you hit this error [1]
> without this patch, and if you do, what's the status reported there?
> 
> [1]https://elixir.bootlin.com/linux/v6.14-rc4/source/drivers/gpu/drm/panthor/panthor_fw.c#L1051

Sorry for the late reply. I get the following, so I think the status is 
"fatal" ?

panthor 4d900000.gpu: [drm] clock rate = 1000000000
panthor 4d900000.gpu: EM: created perf domain
panthor 4d900000.gpu: [drm] mali-g310 id 0xac74 major 0x0 minor 0x0 
status 0x1
panthor 4d900000.gpu: [drm] Features: L2:0x7110306 Tiler:0x809 Mem:0x1 
MMU:0x2830 AS:0xff
panthor 4d900000.gpu: [drm] shader_present=0x1 l2_present=0x1 
tiler_present=0x1
panthor 4d900000.gpu: [drm] Firmware protected mode entry not be 
supported, ignoring
panthor 4d900000.gpu: [drm] Firmware git sha: 
8e5cfcfec20cc8aff8509d37e72babc935d34a3b
panthor 4d900000.gpu: [drm] CSF FW using interface v1.1.0, Features 0x0 
Instrumentation features 0x71
[drm] Initialized panthor 1.3.0 for 4d900000.gpu on minor 0
panthor 4d900000.gpu: [drm] *ERROR* Failed to boot MCU (status=fatal)
panthor 4d900000.gpu: [drm] *ERROR* FW fast reset failed
panthor 4d900000.gpu: [drm] *ERROR* Fast reset failed, trying a slow reset
panthor 4d900000.gpu: [drm] *ERROR* Failed to boot MCU (status=fatal)
panthor 4d900000.gpu: [drm] *ERROR* FW slow reset failed

Re: [PATCH 6/9] drm/panthor: Reset GPU after L2 cache power off

[Marek Vasut]

On 3/3/25 1:42 PM, Boris Brezillon wrote:

Hi,

>>> This looks like it has been part of a R50 release of the DDK, which is recent
>>> enough to consider it up-to-date. The issues you're seeing with fast resume are
>>> probably due to some integration issues or other quirks.
>>>
>>> Boris has the most recent experience with playing with fast resume, maybe
>>> he can share some tips on where to add messages in Panthor to try to debug
>>> your problem.
>>
>> If you end up with fast_reset=true, that means the FW claims it entered
>> a suspend state from which is can resume quickly (fast-reset), and in
>> that case, we're only supposed to power on the L2 block at resume time
>> AFAIK.
> 
> Can you try deasserting the reset lines at probe time and keeping them
> deasserted until the device is removed instead of re-asserting at
> suspend time? If that doesn't work, try doing the same with power
> domains.
I'm afraid either of this makes no difference, no.

I'll at least send a V2 now, so others can use the latest state of my work.