[PATCH v13 0/4] Add TH1520 GPU support with power sequencing

[PATCH v13 0/4] Add TH1520 GPU support with power sequencing

[Michal Wilczynski]

This patch series introduces support for the Imagination IMG BXM-4-64
GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is
managing the GPU's complex power-up and power-down sequence, which
involves multiple clocks and resets.

The TH1520 GPU requires a specific sequence to be followed for its
clocks and resets to ensure correct operation. Initial discussions and
an earlier version of this series explored managing this via the generic
power domain (genpd) framework. However, following further discussions
with kernel maintainers [1], the approach has been reworked to utilize
the dedicated power sequencing (pwrseq) framework.

This revised series now employs a new pwrseq provider driver
(pwrseq-thead-gpu.c) specifically for the TH1520 GPU. This driver
encapsulates the SoC specific power sequence details. The Imagination
GPU driver (pvr_device.c) is updated to act as a consumer of this power
sequencer, requesting the "gpu-power" target. The sequencer driver,
during its match phase with the GPU device, acquires the necessary clock
and reset handles from the GPU device node to perform the full sequence.

This approach aligns with the goal of abstracting SoC specific power
management details away from generic device drivers and leverages the
pwrseq framework as recommended.

The series is structured as follows:

Three patches below are NOT included in this revision, as they were
merged to maintainers trees:

Patch 1: Introduces the pwrseq-thead-gpu auxiliary driver to manage the
         GPU's power-on/off sequence. (already in maintainer tree)
Patch 2: Adds device tree bindings for the gpu-clkgen reset to the
         existing thead,th1520-aon binding. (already in maintainer tree)
Patch 3: Extends the pm-domains driver to detect the gpu-clkgen reset
         and spawn the pwrseq-thead-gpu auxiliary driver. (already in
         maintainer tree)
Patch 4: Adds the gpu-clkgen reset property to the aon node in the
         TH1520 device tree source.

Revised numbering for the rest of un-merged patches:

Patch 1: Updates the Imagination DRM driver to utilize the pwrseq
         framework for TH1520 GPU power management.
Patch 2: Adds the thead,th1520-gpu compatible string to the PowerVR GPU
         device tree bindings.
Patch 3: Adds the device tree node for the IMG BXM-4-64 GPU and its
         required fixed-clock.
Patch 4: Enables compilation of the Imagination PowerVR driver on the
         RISC-V architecture.

This patchset finishes the work started in bigger series [2] by adding
remaining GPU power sequencing piece. After this patchset the GPU probes
correctly.

This series supersedes the previous genpd based approach. Testing on
T-HEAD TH1520 SoC indicates the new pwrseq based solution works
correctly.

Link to v12 of this series - [3].

v13:
- Removed all `#if IS_ENABLED(CONFIG_POWER_SEQUENCING)` guards based on
  reviewer feedback. The pwrseq framework provides stub functions when
  the feature is disabled, which correctly causes the probe to fail for
  devices that depend on it.

v12:
- Re-base on top of 6.17-rc1.

v11:
 - Add the clock constraints in the dt-binding.
 - Add Rev-by from Krzysztof.
 - Remove unnecessary header change in first commit.

v10:
 - Squashed the two dt-binding patches back into a single commit.
 - Simplified the B-series GPU rule by removing the not clause.
 - Reverted the removal of the items definition from the top-level
   power-domain-names property, per maintainer feedback.

v9:
 - Split the dt-bidning patch to two patches: one for refactoring the
   binding and one for adding TH1520 BXM-4-64 support
 - Reworked the device tree binding entirely to define power domain
   constraints on a per-variant basis, per maintainer feedback. This
   replaces the previous generic rules with explicit definitions for
   each GPU variant.
 - Removed Reviewed-by tags from Patch 1 and the dt-binding patches, as
   they have changed significantly since they were provided.

v8:
- Re-base on top of linux-next.
- Refactor the power management logic to use an ->init() callback on
  the pvr_power_sequence_ops struct. This eliminates platform-specific
  initialization code from pvr_device.c, decoupling the generic
  driver from the power sequencing implementation details.
- Improve dt-binding to forbid the power-domain-names for thead.

v7:
- Re-based on linux-next patch 1 from v6 made it there, while I believe
  the 2-3 will join shortly as well

- Implemented conditional devicetree binding constraints. The binding
  now enforces one power domain for the TH1520 SoC while requiring two
  for other BXM/BXS GPUs, using an `if/not` construct to create a
  specific exception for the TH1520

- Rework the Imagination DRM driver's power management. The
  platform-specific logic is now abstracted into a new
  `pvr_power_sequence_ops` struct. The `of_device_id` table uses pointers
  to constant instances of this struct, allowing for a cleaner, more
  robust check at probe time

- Add stubs for the pwrseq functions which return -ENOTSUPP and issue a
  warning if the driver is used on a pwrseq-based platform without
  CONFIG_POWER_SEQUENCING enabled

- Update Kconfig dependencies to restrict RISC-V support to 64-bit
  platforms and ensure correct alphabetical ordering of the
  dependencies

v6:
 - check return values from reset_control_assert() and propagate the
   first error, ensuring all teardown steps are still attempted
 - the driver now stores a reference to the consumer's device node to
   ensure it binds to a single, specific device
 - rename Kconfig option to POWER_SEQUENCING_TH1520_GPU
 - remove COMPILE_TEST

v5:

- reworked the pwrseq-thead-gpu driver, now using manual resource
  management in .match and a .remove callback
- refactored the drm/imagination driver to use function pointers for
  power management instead of a boolean flag
- switched the pmdomain driver to use the generic
  device_property_match_string() helper
- added MMU and COMPILE_TEST dependencies to Kconfig to fix RISC-V
  build warnings.

v4:

- the pwrseq driver is now an auxiliary driver with a robust match
  function based on the power-domains property, spawned from the AON
  node
- Imagination DRM driver now uses of_device_id match data to
  conditionally probe for the pwrseq, solving the cross platform
  probe deferral issue
- add Reviewed-by from Ulf for the entire series

v3:

- re-worked cover letter completely
- complete architectural rework from using extended genpd callbacks to a
  dedicated pwrseq provider driver
- introduced pwrseq-thead-gpu.c and associated DT bindings
   (thead,th1520-gpu-pwrseq)
- the Imagination driver now calls devm_pwrseq_get() and uses
  pwrseq_power_on() / pwrseq_power_off() for the TH1520 GPU
- removed the platform_resources_managed flag from dev_pm_info and
  associated logic
- the new pwrseq driver's match() function now acquires consumer-specific
  resources (GPU clocks, GPU core reset) directly from the consumer device

v2:

Extended the series by adding two new commits:
 - introduced a new platform_resources_managed flag in dev_pm_info along
   with helper functions, allowing drivers to detect when clocks and resets
   are managed by the platform
 - updated the DRM Imagination driver to skip claiming clocks when
   platform_resources_managed is set

Split the original bindings update:
 - the AON firmware bindings now only add the GPU clkgen reset (the GPU
   core reset remains handled by the GPU node)

Reworked the TH1520 PM domain driver to:
 - acquire GPU clocks and reset dynamically using attach_dev/detach_dev
   callbacks
 - handle clkgen reset internally, while GPU core reset is obtained from
   the consumer device node
 - added a check to enforce that only a single device can be attached to
   the GPU PM domain

[1] - https://lore.kernel.org/all/CAPDyKFpi6_CD++a9sbGBvJCuBSQS6YcpNttkRQhQMTWy1yyrRg@mail.gmail.com/
[2] - https://lore.kernel.org/all/20250219140239.1378758-1-m.wilczynski@samsung.com/
[3] - https://lore.kernel.org/all/20250820-apr_14_for_sending-v12-0-4213ccefbd05@samsung.com/

---
Michal Wilczynski (4):
      drm/imagination: Use pwrseq for TH1520 GPU power management
      dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU support
      riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node
      drm/imagination: Enable PowerVR driver for RISC-V

 .../devicetree/bindings/gpu/img,powervr-rogue.yaml |  37 +++--
 arch/riscv/boot/dts/thead/th1520.dtsi              |  21 +++
 drivers/gpu/drm/imagination/Kconfig                |   3 +-
 drivers/gpu/drm/imagination/pvr_device.c           |  22 +--
 drivers/gpu/drm/imagination/pvr_device.h           |  17 +++
 drivers/gpu/drm/imagination/pvr_drv.c              |  23 ++-
 drivers/gpu/drm/imagination/pvr_power.c            | 158 +++++++++++++++------
 drivers/gpu/drm/imagination/pvr_power.h            |  15 ++
 8 files changed, 228 insertions(+), 68 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250414-apr_14_for_sending-5b3917817acc

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>


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

[PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Michal Wilczynski]

Update the Imagination PVR DRM driver to leverage the pwrseq framework
for managing the complex power sequence of the GPU on the T-HEAD TH1520
SoC.

To cleanly separate platform-specific logic from the generic driver,
this patch introduces an `init` callback to the `pwr_power_sequence_ops`
struct. This allows for different power management strategies to be
selected at probe time based on the device's compatible string.

A `pvr_device_data` struct, associated with each compatible in the
of_device_id table, points to the appropriate ops table (manual or
pwrseq).

At probe time, the driver now calls the `->init()` op. For pwrseq-based
platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
probe if the sequencer is not yet available. For other platforms, it
falls back to the existing manual clock and reset handling. The runtime
PM callbacks continue to call the appropriate functions via the ops
table.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/gpu/drm/imagination/pvr_device.c |  22 +----
 drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
 drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
 drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
 drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
 5 files changed, 176 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -23,6 +23,7 @@
 #include <linux/firmware.h>
 #include <linux/gfp.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
@@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
 	return 0;
 }
 
-static int pvr_device_reset_init(struct pvr_device *pvr_dev)
-{
-	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
-	struct reset_control *reset;
-
-	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
-	if (IS_ERR(reset))
-		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
-				     "failed to get gpu reset line\n");
-
-	pvr_dev->reset = reset;
-
-	return 0;
-}
-
 /**
  * pvr_device_process_active_queues() - Process all queue related events.
  * @pvr_dev: PowerVR device to check
@@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
 	struct device *dev = drm_dev->dev;
 	int err;
 
+	/* Get the platform-specific data based on the compatible string. */
+	pvr_dev->device_data = of_device_get_match_data(dev);
+
 	/*
 	 * Setup device parameters. We do this first in case other steps
 	 * depend on them.
@@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
 	if (err)
 		return err;
 
-	/* Get the reset line for the GPU */
-	err = pvr_device_reset_init(pvr_dev);
+	err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
 	if (err)
 		return err;
 
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -37,6 +37,9 @@ struct clk;
 /* Forward declaration from <linux/firmware.h>. */
 struct firmware;
 
+/* Forward declaration from <linux/pwrseq/consumer.h> */
+struct pwrseq_desc;
+
 /**
  * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
  * @b: Branch ID.
@@ -57,6 +60,14 @@ struct pvr_fw_version {
 	u16 major, minor;
 };
 
+/**
+ * struct pvr_device_data - Platform specific data associated with a compatible string.
+ * @pwr_ops: Pointer to a structure with platform-specific power functions.
+ */
+struct pvr_device_data {
+	const struct pvr_power_sequence_ops *pwr_ops;
+};
+
 /**
  * struct pvr_device - powervr-specific wrapper for &struct drm_device
  */
@@ -98,6 +109,9 @@ struct pvr_device {
 	/** @fw_version: Firmware version detected at runtime. */
 	struct pvr_fw_version fw_version;
 
+	/** @device_data: Pointer to platform-specific data. */
+	const struct pvr_device_data *device_data;
+
 	/** @regs_resource: Resource representing device control registers. */
 	struct resource *regs_resource;
 
@@ -148,6 +162,9 @@ struct pvr_device {
 	 */
 	struct reset_control *reset;
 
+	/** @pwrseq: Pointer to a power sequencer, if one is used. */
+	struct pwrseq_desc *pwrseq;
+
 	/** @irq: IRQ number. */
 	int irq;
 
diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
 	pvr_power_domains_fini(pvr_dev);
 }
 
+static const struct pvr_device_data pvr_device_data_manual = {
+	.pwr_ops = &pvr_power_sequence_ops_manual,
+};
+
+static const struct pvr_device_data pvr_device_data_pwrseq = {
+	.pwr_ops = &pvr_power_sequence_ops_pwrseq,
+};
+
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "img,img-rogue", .data = NULL },
+	{
+		.compatible = "thead,th1520-gpu",
+		.data = &pvr_device_data_pwrseq,
+	},
+	{
+		.compatible = "img,img-rogue",
+		.data = &pvr_device_data_manual,
+	},
 
 	/*
 	 * This legacy compatible string was introduced early on before the more generic
 	 * "img,img-rogue" was added. Keep it around here for compatibility, but never use
 	 * "img,img-axe" in new devicetrees.
 	 */
-	{ .compatible = "img,img-axe", .data = NULL },
+	{
+		.compatible = "img,img-axe",
+		.data = &pvr_device_data_manual,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
@@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
 MODULE_LICENSE("Dual MIT/GPL");
 MODULE_IMPORT_NS("DMA_BUF");
 MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
+MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
 MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/pwrseq/consumer.h>
 #include <linux/reset.h>
 #include <linux/timer.h>
 #include <linux/types.h>
@@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
 	return 0;
 }
 
+static int pvr_power_init_manual(struct pvr_device *pvr_dev)
+{
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+	struct reset_control *reset;
+
+	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
+	if (IS_ERR(reset))
+		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
+				     "failed to get gpu reset line\n");
+
+	pvr_dev->reset = reset;
+
+	return 0;
+}
+
+static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
+{
+	int err;
+
+	err = clk_prepare_enable(pvr_dev->core_clk);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(pvr_dev->sys_clk);
+	if (err)
+		goto err_core_clk_disable;
+
+	err = clk_prepare_enable(pvr_dev->mem_clk);
+	if (err)
+		goto err_sys_clk_disable;
+
+	/*
+	 * According to the hardware manual, a delay of at least 32 clock
+	 * cycles is required between de-asserting the clkgen reset and
+	 * de-asserting the GPU reset. Assuming a worst-case scenario with
+	 * a very high GPU clock frequency, a delay of 1 microsecond is
+	 * sufficient to ensure this requirement is met across all
+	 * feasible GPU clock speeds.
+	 */
+	udelay(1);
+
+	err = reset_control_deassert(pvr_dev->reset);
+	if (err)
+		goto err_mem_clk_disable;
+
+	return 0;
+
+err_mem_clk_disable:
+	clk_disable_unprepare(pvr_dev->mem_clk);
+
+err_sys_clk_disable:
+	clk_disable_unprepare(pvr_dev->sys_clk);
+
+err_core_clk_disable:
+	clk_disable_unprepare(pvr_dev->core_clk);
+
+	return err;
+}
+
+static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
+{
+	int err;
+
+	err = reset_control_assert(pvr_dev->reset);
+
+	clk_disable_unprepare(pvr_dev->mem_clk);
+	clk_disable_unprepare(pvr_dev->sys_clk);
+	clk_disable_unprepare(pvr_dev->core_clk);
+
+	return err;
+}
+
+const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
+	.init = pvr_power_init_manual,
+	.power_on = pvr_power_on_sequence_manual,
+	.power_off = pvr_power_off_sequence_manual,
+};
+
+static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
+{
+	struct device *dev = from_pvr_device(pvr_dev)->dev;
+
+	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
+	if (IS_ERR(pvr_dev->pwrseq)) {
+		/*
+		 * This platform requires a sequencer. If we can't get it, we
+		 * must return the error (including -EPROBE_DEFER to wait for
+		 * the provider to appear)
+		 */
+		return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
+				     "Failed to get required power sequencer\n");
+	}
+
+	return 0;
+}
+
+static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
+{
+	return pwrseq_power_on(pvr_dev->pwrseq);
+}
+
+static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
+{
+	return pwrseq_power_off(pvr_dev->pwrseq);
+}
+
+const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
+	.init = pvr_power_init_pwrseq,
+	.power_on = pvr_power_on_sequence_pwrseq,
+	.power_off = pvr_power_off_sequence_pwrseq,
+};
+
 int
 pvr_power_device_suspend(struct device *dev)
 {
@@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
 			goto err_drm_dev_exit;
 	}
 
-	clk_disable_unprepare(pvr_dev->mem_clk);
-	clk_disable_unprepare(pvr_dev->sys_clk);
-	clk_disable_unprepare(pvr_dev->core_clk);
-
-	err = reset_control_assert(pvr_dev->reset);
+	err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
 
 err_drm_dev_exit:
 	drm_dev_exit(idx);
@@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
 	if (!drm_dev_enter(drm_dev, &idx))
 		return -EIO;
 
-	err = clk_prepare_enable(pvr_dev->core_clk);
+	err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
 	if (err)
 		goto err_drm_dev_exit;
 
-	err = clk_prepare_enable(pvr_dev->sys_clk);
-	if (err)
-		goto err_core_clk_disable;
-
-	err = clk_prepare_enable(pvr_dev->mem_clk);
-	if (err)
-		goto err_sys_clk_disable;
-
-	/*
-	 * According to the hardware manual, a delay of at least 32 clock
-	 * cycles is required between de-asserting the clkgen reset and
-	 * de-asserting the GPU reset. Assuming a worst-case scenario with
-	 * a very high GPU clock frequency, a delay of 1 microsecond is
-	 * sufficient to ensure this requirement is met across all
-	 * feasible GPU clock speeds.
-	 */
-	udelay(1);
-
-	err = reset_control_deassert(pvr_dev->reset);
-	if (err)
-		goto err_mem_clk_disable;
-
 	if (pvr_dev->fw_dev.booted) {
 		err = pvr_power_fw_enable(pvr_dev);
 		if (err)
-			goto err_reset_assert;
+			goto err_power_off;
 	}
 
 	drm_dev_exit(idx);
 
 	return 0;
 
-err_reset_assert:
-	reset_control_assert(pvr_dev->reset);
-
-err_mem_clk_disable:
-	clk_disable_unprepare(pvr_dev->mem_clk);
-
-err_sys_clk_disable:
-	clk_disable_unprepare(pvr_dev->sys_clk);
-
-err_core_clk_disable:
-	clk_disable_unprepare(pvr_dev->core_clk);
+err_power_off:
+	pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
 
 err_drm_dev_exit:
 	drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
--- a/drivers/gpu/drm/imagination/pvr_power.h
+++ b/drivers/gpu/drm/imagination/pvr_power.h
@@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
 int pvr_power_domains_init(struct pvr_device *pvr_dev);
 void pvr_power_domains_fini(struct pvr_device *pvr_dev);
 
+/**
+ * struct pvr_power_sequence_ops - Platform specific power sequence operations.
+ * @init: Pointer to the platform-specific initialization function.
+ * @power_on: Pointer to the platform-specific power on function.
+ * @power_off: Pointer to the platform-specific power off function.
+ */
+struct pvr_power_sequence_ops {
+	int (*init)(struct pvr_device *pvr_dev);
+	int (*power_on)(struct pvr_device *pvr_dev);
+	int (*power_off)(struct pvr_device *pvr_dev);
+};
+
+extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
+extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
+
 #endif /* PVR_POWER_H */

-- 
2.34.1


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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Ulf Hansson]

On Fri, 22 Aug 2025 at 00:20, Michal Wilczynski
<m.wilczynski@samsung.com> wrote:
>
> Update the Imagination PVR DRM driver to leverage the pwrseq framework
> for managing the complex power sequence of the GPU on the T-HEAD TH1520
> SoC.
>
> To cleanly separate platform-specific logic from the generic driver,
> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
> struct. This allows for different power management strategies to be
> selected at probe time based on the device's compatible string.
>
> A `pvr_device_data` struct, associated with each compatible in the
> of_device_id table, points to the appropriate ops table (manual or
> pwrseq).
>
> At probe time, the driver now calls the `->init()` op. For pwrseq-based
> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
> probe if the sequencer is not yet available. For other platforms, it
> falls back to the existing manual clock and reset handling. The runtime
> PM callbacks continue to call the appropriate functions via the ops
> table.
>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/gpu/drm/imagination/pvr_device.c |  22 +----
>  drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
>  drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
>  drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
>  drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
>  5 files changed, 176 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -23,6 +23,7 @@
>  #include <linux/firmware.h>
>  #include <linux/gfp.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
> @@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
>         return 0;
>  }
>
> -static int pvr_device_reset_init(struct pvr_device *pvr_dev)
> -{
> -       struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> -       struct reset_control *reset;
> -
> -       reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
> -       if (IS_ERR(reset))
> -               return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
> -                                    "failed to get gpu reset line\n");
> -
> -       pvr_dev->reset = reset;
> -
> -       return 0;
> -}
> -
>  /**
>   * pvr_device_process_active_queues() - Process all queue related events.
>   * @pvr_dev: PowerVR device to check
> @@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
>         struct device *dev = drm_dev->dev;
>         int err;
>
> +       /* Get the platform-specific data based on the compatible string. */
> +       pvr_dev->device_data = of_device_get_match_data(dev);
> +
>         /*
>          * Setup device parameters. We do this first in case other steps
>          * depend on them.
> @@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
>         if (err)
>                 return err;
>
> -       /* Get the reset line for the GPU */
> -       err = pvr_device_reset_init(pvr_dev);
> +       err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
>         if (err)
>                 return err;
>
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.h
> +++ b/drivers/gpu/drm/imagination/pvr_device.h
> @@ -37,6 +37,9 @@ struct clk;
>  /* Forward declaration from <linux/firmware.h>. */
>  struct firmware;
>
> +/* Forward declaration from <linux/pwrseq/consumer.h> */
> +struct pwrseq_desc;
> +
>  /**
>   * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
>   * @b: Branch ID.
> @@ -57,6 +60,14 @@ struct pvr_fw_version {
>         u16 major, minor;
>  };
>
> +/**
> + * struct pvr_device_data - Platform specific data associated with a compatible string.
> + * @pwr_ops: Pointer to a structure with platform-specific power functions.
> + */
> +struct pvr_device_data {
> +       const struct pvr_power_sequence_ops *pwr_ops;
> +};
> +
>  /**
>   * struct pvr_device - powervr-specific wrapper for &struct drm_device
>   */
> @@ -98,6 +109,9 @@ struct pvr_device {
>         /** @fw_version: Firmware version detected at runtime. */
>         struct pvr_fw_version fw_version;
>
> +       /** @device_data: Pointer to platform-specific data. */
> +       const struct pvr_device_data *device_data;
> +
>         /** @regs_resource: Resource representing device control registers. */
>         struct resource *regs_resource;
>
> @@ -148,6 +162,9 @@ struct pvr_device {
>          */
>         struct reset_control *reset;
>
> +       /** @pwrseq: Pointer to a power sequencer, if one is used. */
> +       struct pwrseq_desc *pwrseq;
> +
>         /** @irq: IRQ number. */
>         int irq;
>
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
>         pvr_power_domains_fini(pvr_dev);
>  }
>
> +static const struct pvr_device_data pvr_device_data_manual = {
> +       .pwr_ops = &pvr_power_sequence_ops_manual,
> +};
> +
> +static const struct pvr_device_data pvr_device_data_pwrseq = {
> +       .pwr_ops = &pvr_power_sequence_ops_pwrseq,
> +};
> +
>  static const struct of_device_id dt_match[] = {
> -       { .compatible = "img,img-rogue", .data = NULL },
> +       {
> +               .compatible = "thead,th1520-gpu",
> +               .data = &pvr_device_data_pwrseq,
> +       },
> +       {
> +               .compatible = "img,img-rogue",
> +               .data = &pvr_device_data_manual,
> +       },
>
>         /*
>          * This legacy compatible string was introduced early on before the more generic
>          * "img,img-rogue" was added. Keep it around here for compatibility, but never use
>          * "img,img-axe" in new devicetrees.
>          */
> -       { .compatible = "img,img-axe", .data = NULL },
> +       {
> +               .compatible = "img,img-axe",
> +               .data = &pvr_device_data_manual,
> +       },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, dt_match);
> @@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
>  MODULE_LICENSE("Dual MIT/GPL");
>  MODULE_IMPORT_NS("DMA_BUF");
>  MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
> +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
>  MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pwrseq/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/timer.h>
>  #include <linux/types.h>
> @@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
>         return 0;
>  }
>
> +static int pvr_power_init_manual(struct pvr_device *pvr_dev)
> +{
> +       struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> +       struct reset_control *reset;
> +
> +       reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
> +       if (IS_ERR(reset))
> +               return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
> +                                    "failed to get gpu reset line\n");
> +
> +       pvr_dev->reset = reset;
> +
> +       return 0;
> +}
> +
> +static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
> +{
> +       int err;
> +
> +       err = clk_prepare_enable(pvr_dev->core_clk);
> +       if (err)
> +               return err;
> +
> +       err = clk_prepare_enable(pvr_dev->sys_clk);
> +       if (err)
> +               goto err_core_clk_disable;
> +
> +       err = clk_prepare_enable(pvr_dev->mem_clk);
> +       if (err)
> +               goto err_sys_clk_disable;
> +
> +       /*
> +        * According to the hardware manual, a delay of at least 32 clock
> +        * cycles is required between de-asserting the clkgen reset and
> +        * de-asserting the GPU reset. Assuming a worst-case scenario with
> +        * a very high GPU clock frequency, a delay of 1 microsecond is
> +        * sufficient to ensure this requirement is met across all
> +        * feasible GPU clock speeds.
> +        */
> +       udelay(1);
> +
> +       err = reset_control_deassert(pvr_dev->reset);
> +       if (err)
> +               goto err_mem_clk_disable;
> +
> +       return 0;
> +
> +err_mem_clk_disable:
> +       clk_disable_unprepare(pvr_dev->mem_clk);
> +
> +err_sys_clk_disable:
> +       clk_disable_unprepare(pvr_dev->sys_clk);
> +
> +err_core_clk_disable:
> +       clk_disable_unprepare(pvr_dev->core_clk);
> +
> +       return err;
> +}
> +
> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
> +{
> +       int err;
> +
> +       err = reset_control_assert(pvr_dev->reset);
> +
> +       clk_disable_unprepare(pvr_dev->mem_clk);
> +       clk_disable_unprepare(pvr_dev->sys_clk);
> +       clk_disable_unprepare(pvr_dev->core_clk);
> +
> +       return err;
> +}
> +
> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
> +       .init = pvr_power_init_manual,
> +       .power_on = pvr_power_on_sequence_manual,
> +       .power_off = pvr_power_off_sequence_manual,
> +};
> +
> +static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
> +{
> +       struct device *dev = from_pvr_device(pvr_dev)->dev;
> +
> +       pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
> +       if (IS_ERR(pvr_dev->pwrseq)) {
> +               /*
> +                * This platform requires a sequencer. If we can't get it, we
> +                * must return the error (including -EPROBE_DEFER to wait for
> +                * the provider to appear)
> +                */
> +               return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
> +                                    "Failed to get required power sequencer\n");
> +       }
> +
> +       return 0;
> +}
> +
> +static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
> +{
> +       return pwrseq_power_on(pvr_dev->pwrseq);
> +}
> +
> +static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
> +{
> +       return pwrseq_power_off(pvr_dev->pwrseq);
> +}
> +
> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
> +       .init = pvr_power_init_pwrseq,
> +       .power_on = pvr_power_on_sequence_pwrseq,
> +       .power_off = pvr_power_off_sequence_pwrseq,
> +};
> +
>  int
>  pvr_power_device_suspend(struct device *dev)
>  {
> @@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
>                         goto err_drm_dev_exit;
>         }
>
> -       clk_disable_unprepare(pvr_dev->mem_clk);
> -       clk_disable_unprepare(pvr_dev->sys_clk);
> -       clk_disable_unprepare(pvr_dev->core_clk);
> -
> -       err = reset_control_assert(pvr_dev->reset);
> +       err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>
>  err_drm_dev_exit:
>         drm_dev_exit(idx);
> @@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
>         if (!drm_dev_enter(drm_dev, &idx))
>                 return -EIO;
>
> -       err = clk_prepare_enable(pvr_dev->core_clk);
> +       err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
>         if (err)
>                 goto err_drm_dev_exit;
>
> -       err = clk_prepare_enable(pvr_dev->sys_clk);
> -       if (err)
> -               goto err_core_clk_disable;
> -
> -       err = clk_prepare_enable(pvr_dev->mem_clk);
> -       if (err)
> -               goto err_sys_clk_disable;
> -
> -       /*
> -        * According to the hardware manual, a delay of at least 32 clock
> -        * cycles is required between de-asserting the clkgen reset and
> -        * de-asserting the GPU reset. Assuming a worst-case scenario with
> -        * a very high GPU clock frequency, a delay of 1 microsecond is
> -        * sufficient to ensure this requirement is met across all
> -        * feasible GPU clock speeds.
> -        */
> -       udelay(1);
> -
> -       err = reset_control_deassert(pvr_dev->reset);
> -       if (err)
> -               goto err_mem_clk_disable;
> -
>         if (pvr_dev->fw_dev.booted) {
>                 err = pvr_power_fw_enable(pvr_dev);
>                 if (err)
> -                       goto err_reset_assert;
> +                       goto err_power_off;
>         }
>
>         drm_dev_exit(idx);
>
>         return 0;
>
> -err_reset_assert:
> -       reset_control_assert(pvr_dev->reset);
> -
> -err_mem_clk_disable:
> -       clk_disable_unprepare(pvr_dev->mem_clk);
> -
> -err_sys_clk_disable:
> -       clk_disable_unprepare(pvr_dev->sys_clk);
> -
> -err_core_clk_disable:
> -       clk_disable_unprepare(pvr_dev->core_clk);
> +err_power_off:
> +       pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>
>  err_drm_dev_exit:
>         drm_dev_exit(idx);
> diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
> index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.h
> +++ b/drivers/gpu/drm/imagination/pvr_power.h
> @@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
>  int pvr_power_domains_init(struct pvr_device *pvr_dev);
>  void pvr_power_domains_fini(struct pvr_device *pvr_dev);
>
> +/**
> + * struct pvr_power_sequence_ops - Platform specific power sequence operations.
> + * @init: Pointer to the platform-specific initialization function.
> + * @power_on: Pointer to the platform-specific power on function.
> + * @power_off: Pointer to the platform-specific power off function.
> + */
> +struct pvr_power_sequence_ops {
> +       int (*init)(struct pvr_device *pvr_dev);
> +       int (*power_on)(struct pvr_device *pvr_dev);
> +       int (*power_off)(struct pvr_device *pvr_dev);
> +};
> +
> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
> +
>  #endif /* PVR_POWER_H */
>
> --
> 2.34.1
>

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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Matt Coster]

[-- Attachment #1.1.1: Type: text/plain, Size: 14097 bytes --]

On 21/08/2025 23:20, Michal Wilczynski wrote:
> Update the Imagination PVR DRM driver to leverage the pwrseq framework
> for managing the complex power sequence of the GPU on the T-HEAD TH1520
> SoC.
> 
> To cleanly separate platform-specific logic from the generic driver,
> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
> struct. This allows for different power management strategies to be
> selected at probe time based on the device's compatible string.
> 
> A `pvr_device_data` struct, associated with each compatible in the
> of_device_id table, points to the appropriate ops table (manual or
> pwrseq).
> 
> At probe time, the driver now calls the `->init()` op. For pwrseq-based
> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
> probe if the sequencer is not yet available. For other platforms, it
> falls back to the existing manual clock and reset handling. The runtime
> PM callbacks continue to call the appropriate functions via the ops
> table.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>

Reviewed-by: Matt Coster <matt.coster@imgtec.com>

Would you like me to take the non-DTS changes via drm-misc-next?

Cheers,
Matt

> ---
>  drivers/gpu/drm/imagination/pvr_device.c |  22 +----
>  drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
>  drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
>  drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
>  drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
>  5 files changed, 176 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -23,6 +23,7 @@
>  #include <linux/firmware.h>
>  #include <linux/gfp.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
> @@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
>  	return 0;
>  }
>  
> -static int pvr_device_reset_init(struct pvr_device *pvr_dev)
> -{
> -	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> -	struct reset_control *reset;
> -
> -	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
> -	if (IS_ERR(reset))
> -		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
> -				     "failed to get gpu reset line\n");
> -
> -	pvr_dev->reset = reset;
> -
> -	return 0;
> -}
> -
>  /**
>   * pvr_device_process_active_queues() - Process all queue related events.
>   * @pvr_dev: PowerVR device to check
> @@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
>  	struct device *dev = drm_dev->dev;
>  	int err;
>  
> +	/* Get the platform-specific data based on the compatible string. */
> +	pvr_dev->device_data = of_device_get_match_data(dev);
> +
>  	/*
>  	 * Setup device parameters. We do this first in case other steps
>  	 * depend on them.
> @@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
>  	if (err)
>  		return err;
>  
> -	/* Get the reset line for the GPU */
> -	err = pvr_device_reset_init(pvr_dev);
> +	err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.h
> +++ b/drivers/gpu/drm/imagination/pvr_device.h
> @@ -37,6 +37,9 @@ struct clk;
>  /* Forward declaration from <linux/firmware.h>. */
>  struct firmware;
>  
> +/* Forward declaration from <linux/pwrseq/consumer.h> */
> +struct pwrseq_desc;
> +
>  /**
>   * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
>   * @b: Branch ID.
> @@ -57,6 +60,14 @@ struct pvr_fw_version {
>  	u16 major, minor;
>  };
>  
> +/**
> + * struct pvr_device_data - Platform specific data associated with a compatible string.
> + * @pwr_ops: Pointer to a structure with platform-specific power functions.
> + */
> +struct pvr_device_data {
> +	const struct pvr_power_sequence_ops *pwr_ops;
> +};
> +
>  /**
>   * struct pvr_device - powervr-specific wrapper for &struct drm_device
>   */
> @@ -98,6 +109,9 @@ struct pvr_device {
>  	/** @fw_version: Firmware version detected at runtime. */
>  	struct pvr_fw_version fw_version;
>  
> +	/** @device_data: Pointer to platform-specific data. */
> +	const struct pvr_device_data *device_data;
> +
>  	/** @regs_resource: Resource representing device control registers. */
>  	struct resource *regs_resource;
>  
> @@ -148,6 +162,9 @@ struct pvr_device {
>  	 */
>  	struct reset_control *reset;
>  
> +	/** @pwrseq: Pointer to a power sequencer, if one is used. */
> +	struct pwrseq_desc *pwrseq;
> +
>  	/** @irq: IRQ number. */
>  	int irq;
>  
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
>  	pvr_power_domains_fini(pvr_dev);
>  }
>  
> +static const struct pvr_device_data pvr_device_data_manual = {
> +	.pwr_ops = &pvr_power_sequence_ops_manual,
> +};
> +
> +static const struct pvr_device_data pvr_device_data_pwrseq = {
> +	.pwr_ops = &pvr_power_sequence_ops_pwrseq,
> +};
> +
>  static const struct of_device_id dt_match[] = {
> -	{ .compatible = "img,img-rogue", .data = NULL },
> +	{
> +		.compatible = "thead,th1520-gpu",
> +		.data = &pvr_device_data_pwrseq,
> +	},
> +	{
> +		.compatible = "img,img-rogue",
> +		.data = &pvr_device_data_manual,
> +	},
>  
>  	/*
>  	 * This legacy compatible string was introduced early on before the more generic
>  	 * "img,img-rogue" was added. Keep it around here for compatibility, but never use
>  	 * "img,img-axe" in new devicetrees.
>  	 */
> -	{ .compatible = "img,img-axe", .data = NULL },
> +	{
> +		.compatible = "img,img-axe",
> +		.data = &pvr_device_data_manual,
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, dt_match);
> @@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
>  MODULE_LICENSE("Dual MIT/GPL");
>  MODULE_IMPORT_NS("DMA_BUF");
>  MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
> +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
>  MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pwrseq/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/timer.h>
>  #include <linux/types.h>
> @@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
>  	return 0;
>  }
>  
> +static int pvr_power_init_manual(struct pvr_device *pvr_dev)
> +{
> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> +	struct reset_control *reset;
> +
> +	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
> +	if (IS_ERR(reset))
> +		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
> +				     "failed to get gpu reset line\n");
> +
> +	pvr_dev->reset = reset;
> +
> +	return 0;
> +}
> +
> +static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
> +{
> +	int err;
> +
> +	err = clk_prepare_enable(pvr_dev->core_clk);
> +	if (err)
> +		return err;
> +
> +	err = clk_prepare_enable(pvr_dev->sys_clk);
> +	if (err)
> +		goto err_core_clk_disable;
> +
> +	err = clk_prepare_enable(pvr_dev->mem_clk);
> +	if (err)
> +		goto err_sys_clk_disable;
> +
> +	/*
> +	 * According to the hardware manual, a delay of at least 32 clock
> +	 * cycles is required between de-asserting the clkgen reset and
> +	 * de-asserting the GPU reset. Assuming a worst-case scenario with
> +	 * a very high GPU clock frequency, a delay of 1 microsecond is
> +	 * sufficient to ensure this requirement is met across all
> +	 * feasible GPU clock speeds.
> +	 */
> +	udelay(1);
> +
> +	err = reset_control_deassert(pvr_dev->reset);
> +	if (err)
> +		goto err_mem_clk_disable;
> +
> +	return 0;
> +
> +err_mem_clk_disable:
> +	clk_disable_unprepare(pvr_dev->mem_clk);
> +
> +err_sys_clk_disable:
> +	clk_disable_unprepare(pvr_dev->sys_clk);
> +
> +err_core_clk_disable:
> +	clk_disable_unprepare(pvr_dev->core_clk);
> +
> +	return err;
> +}
> +
> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
> +{
> +	int err;
> +
> +	err = reset_control_assert(pvr_dev->reset);
> +
> +	clk_disable_unprepare(pvr_dev->mem_clk);
> +	clk_disable_unprepare(pvr_dev->sys_clk);
> +	clk_disable_unprepare(pvr_dev->core_clk);
> +
> +	return err;
> +}
> +
> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
> +	.init = pvr_power_init_manual,
> +	.power_on = pvr_power_on_sequence_manual,
> +	.power_off = pvr_power_off_sequence_manual,
> +};
> +
> +static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
> +{
> +	struct device *dev = from_pvr_device(pvr_dev)->dev;
> +
> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
> +	if (IS_ERR(pvr_dev->pwrseq)) {
> +		/*
> +		 * This platform requires a sequencer. If we can't get it, we
> +		 * must return the error (including -EPROBE_DEFER to wait for
> +		 * the provider to appear)
> +		 */
> +		return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
> +				     "Failed to get required power sequencer\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
> +{
> +	return pwrseq_power_on(pvr_dev->pwrseq);
> +}
> +
> +static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
> +{
> +	return pwrseq_power_off(pvr_dev->pwrseq);
> +}
> +
> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
> +	.init = pvr_power_init_pwrseq,
> +	.power_on = pvr_power_on_sequence_pwrseq,
> +	.power_off = pvr_power_off_sequence_pwrseq,
> +};
> +
>  int
>  pvr_power_device_suspend(struct device *dev)
>  {
> @@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
>  			goto err_drm_dev_exit;
>  	}
>  
> -	clk_disable_unprepare(pvr_dev->mem_clk);
> -	clk_disable_unprepare(pvr_dev->sys_clk);
> -	clk_disable_unprepare(pvr_dev->core_clk);
> -
> -	err = reset_control_assert(pvr_dev->reset);
> +	err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>  
>  err_drm_dev_exit:
>  	drm_dev_exit(idx);
> @@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
>  	if (!drm_dev_enter(drm_dev, &idx))
>  		return -EIO;
>  
> -	err = clk_prepare_enable(pvr_dev->core_clk);
> +	err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
>  	if (err)
>  		goto err_drm_dev_exit;
>  
> -	err = clk_prepare_enable(pvr_dev->sys_clk);
> -	if (err)
> -		goto err_core_clk_disable;
> -
> -	err = clk_prepare_enable(pvr_dev->mem_clk);
> -	if (err)
> -		goto err_sys_clk_disable;
> -
> -	/*
> -	 * According to the hardware manual, a delay of at least 32 clock
> -	 * cycles is required between de-asserting the clkgen reset and
> -	 * de-asserting the GPU reset. Assuming a worst-case scenario with
> -	 * a very high GPU clock frequency, a delay of 1 microsecond is
> -	 * sufficient to ensure this requirement is met across all
> -	 * feasible GPU clock speeds.
> -	 */
> -	udelay(1);
> -
> -	err = reset_control_deassert(pvr_dev->reset);
> -	if (err)
> -		goto err_mem_clk_disable;
> -
>  	if (pvr_dev->fw_dev.booted) {
>  		err = pvr_power_fw_enable(pvr_dev);
>  		if (err)
> -			goto err_reset_assert;
> +			goto err_power_off;
>  	}
>  
>  	drm_dev_exit(idx);
>  
>  	return 0;
>  
> -err_reset_assert:
> -	reset_control_assert(pvr_dev->reset);
> -
> -err_mem_clk_disable:
> -	clk_disable_unprepare(pvr_dev->mem_clk);
> -
> -err_sys_clk_disable:
> -	clk_disable_unprepare(pvr_dev->sys_clk);
> -
> -err_core_clk_disable:
> -	clk_disable_unprepare(pvr_dev->core_clk);
> +err_power_off:
> +	pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>  
>  err_drm_dev_exit:
>  	drm_dev_exit(idx);
> diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
> index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.h
> +++ b/drivers/gpu/drm/imagination/pvr_power.h
> @@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
>  int pvr_power_domains_init(struct pvr_device *pvr_dev);
>  void pvr_power_domains_fini(struct pvr_device *pvr_dev);
>  
> +/**
> + * struct pvr_power_sequence_ops - Platform specific power sequence operations.
> + * @init: Pointer to the platform-specific initialization function.
> + * @power_on: Pointer to the platform-specific power on function.
> + * @power_off: Pointer to the platform-specific power off function.
> + */
> +struct pvr_power_sequence_ops {
> +	int (*init)(struct pvr_device *pvr_dev);
> +	int (*power_on)(struct pvr_device *pvr_dev);
> +	int (*power_off)(struct pvr_device *pvr_dev);
> +};
> +
> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
> +
>  #endif /* PVR_POWER_H */
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Michal Wilczynski]

On 8/22/25 11:28, Matt Coster wrote:
> On 21/08/2025 23:20, Michal Wilczynski wrote:
>> Update the Imagination PVR DRM driver to leverage the pwrseq framework
>> for managing the complex power sequence of the GPU on the T-HEAD TH1520
>> SoC.
>>
>> To cleanly separate platform-specific logic from the generic driver,
>> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
>> struct. This allows for different power management strategies to be
>> selected at probe time based on the device's compatible string.
>>
>> A `pvr_device_data` struct, associated with each compatible in the
>> of_device_id table, points to the appropriate ops table (manual or
>> pwrseq).
>>
>> At probe time, the driver now calls the `->init()` op. For pwrseq-based
>> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
>> probe if the sequencer is not yet available. For other platforms, it
>> falls back to the existing manual clock and reset handling. The runtime
>> PM callbacks continue to call the appropriate functions via the ops
>> table.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> 
> Reviewed-by: Matt Coster <matt.coster@imgtec.com>
> 
> Would you like me to take the non-DTS changes via drm-misc-next?

Yeah I think this would be appropriate.
Thanks !

> 
> Cheers,
> Matt
> 
>> ---
>>  drivers/gpu/drm/imagination/pvr_device.c |  22 +----
>>  drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
>>  drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
>>  drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
>>  drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
>>  5 files changed, 176 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/firmware.h>
>>  #include <linux/gfp.h>
>>  #include <linux/interrupt.h>
>> +#include <linux/of.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/reset.h>
>> @@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
>>  	return 0;
>>  }
>>  
>> -static int pvr_device_reset_init(struct pvr_device *pvr_dev)
>> -{
>> -	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>> -	struct reset_control *reset;
>> -
>> -	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>> -	if (IS_ERR(reset))
>> -		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>> -				     "failed to get gpu reset line\n");
>> -
>> -	pvr_dev->reset = reset;
>> -
>> -	return 0;
>> -}
>> -
>>  /**
>>   * pvr_device_process_active_queues() - Process all queue related events.
>>   * @pvr_dev: PowerVR device to check
>> @@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>  	struct device *dev = drm_dev->dev;
>>  	int err;
>>  
>> +	/* Get the platform-specific data based on the compatible string. */
>> +	pvr_dev->device_data = of_device_get_match_data(dev);
>> +
>>  	/*
>>  	 * Setup device parameters. We do this first in case other steps
>>  	 * depend on them.
>> @@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>  	if (err)
>>  		return err;
>>  
>> -	/* Get the reset line for the GPU */
>> -	err = pvr_device_reset_init(pvr_dev);
>> +	err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
>>  	if (err)
>>  		return err;
>>  
>> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
>> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
>> --- a/drivers/gpu/drm/imagination/pvr_device.h
>> +++ b/drivers/gpu/drm/imagination/pvr_device.h
>> @@ -37,6 +37,9 @@ struct clk;
>>  /* Forward declaration from <linux/firmware.h>. */
>>  struct firmware;
>>  
>> +/* Forward declaration from <linux/pwrseq/consumer.h> */
>> +struct pwrseq_desc;
>> +
>>  /**
>>   * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
>>   * @b: Branch ID.
>> @@ -57,6 +60,14 @@ struct pvr_fw_version {
>>  	u16 major, minor;
>>  };
>>  
>> +/**
>> + * struct pvr_device_data - Platform specific data associated with a compatible string.
>> + * @pwr_ops: Pointer to a structure with platform-specific power functions.
>> + */
>> +struct pvr_device_data {
>> +	const struct pvr_power_sequence_ops *pwr_ops;
>> +};
>> +
>>  /**
>>   * struct pvr_device - powervr-specific wrapper for &struct drm_device
>>   */
>> @@ -98,6 +109,9 @@ struct pvr_device {
>>  	/** @fw_version: Firmware version detected at runtime. */
>>  	struct pvr_fw_version fw_version;
>>  
>> +	/** @device_data: Pointer to platform-specific data. */
>> +	const struct pvr_device_data *device_data;
>> +
>>  	/** @regs_resource: Resource representing device control registers. */
>>  	struct resource *regs_resource;
>>  
>> @@ -148,6 +162,9 @@ struct pvr_device {
>>  	 */
>>  	struct reset_control *reset;
>>  
>> +	/** @pwrseq: Pointer to a power sequencer, if one is used. */
>> +	struct pwrseq_desc *pwrseq;
>> +
>>  	/** @irq: IRQ number. */
>>  	int irq;
>>  
>> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
>> index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
>> --- a/drivers/gpu/drm/imagination/pvr_drv.c
>> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
>> @@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
>>  	pvr_power_domains_fini(pvr_dev);
>>  }
>>  
>> +static const struct pvr_device_data pvr_device_data_manual = {
>> +	.pwr_ops = &pvr_power_sequence_ops_manual,
>> +};
>> +
>> +static const struct pvr_device_data pvr_device_data_pwrseq = {
>> +	.pwr_ops = &pvr_power_sequence_ops_pwrseq,
>> +};
>> +
>>  static const struct of_device_id dt_match[] = {
>> -	{ .compatible = "img,img-rogue", .data = NULL },
>> +	{
>> +		.compatible = "thead,th1520-gpu",
>> +		.data = &pvr_device_data_pwrseq,
>> +	},
>> +	{
>> +		.compatible = "img,img-rogue",
>> +		.data = &pvr_device_data_manual,
>> +	},
>>  
>>  	/*
>>  	 * This legacy compatible string was introduced early on before the more generic
>>  	 * "img,img-rogue" was added. Keep it around here for compatibility, but never use
>>  	 * "img,img-axe" in new devicetrees.
>>  	 */
>> -	{ .compatible = "img,img-axe", .data = NULL },
>> +	{
>> +		.compatible = "img,img-axe",
>> +		.data = &pvr_device_data_manual,
>> +	},
>>  	{}
>>  };
>>  MODULE_DEVICE_TABLE(of, dt_match);
>> @@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
>>  MODULE_LICENSE("Dual MIT/GPL");
>>  MODULE_IMPORT_NS("DMA_BUF");
>>  MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
>> +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
>>  MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
>> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
>> index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
>> --- a/drivers/gpu/drm/imagination/pvr_power.c
>> +++ b/drivers/gpu/drm/imagination/pvr_power.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_domain.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/pwrseq/consumer.h>
>>  #include <linux/reset.h>
>>  #include <linux/timer.h>
>>  #include <linux/types.h>
>> @@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
>>  	return 0;
>>  }
>>  
>> +static int pvr_power_init_manual(struct pvr_device *pvr_dev)
>> +{
>> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>> +	struct reset_control *reset;
>> +
>> +	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>> +	if (IS_ERR(reset))
>> +		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>> +				     "failed to get gpu reset line\n");
>> +
>> +	pvr_dev->reset = reset;
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
>> +{
>> +	int err;
>> +
>> +	err = clk_prepare_enable(pvr_dev->core_clk);
>> +	if (err)
>> +		return err;
>> +
>> +	err = clk_prepare_enable(pvr_dev->sys_clk);
>> +	if (err)
>> +		goto err_core_clk_disable;
>> +
>> +	err = clk_prepare_enable(pvr_dev->mem_clk);
>> +	if (err)
>> +		goto err_sys_clk_disable;
>> +
>> +	/*
>> +	 * According to the hardware manual, a delay of at least 32 clock
>> +	 * cycles is required between de-asserting the clkgen reset and
>> +	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>> +	 * a very high GPU clock frequency, a delay of 1 microsecond is
>> +	 * sufficient to ensure this requirement is met across all
>> +	 * feasible GPU clock speeds.
>> +	 */
>> +	udelay(1);
>> +
>> +	err = reset_control_deassert(pvr_dev->reset);
>> +	if (err)
>> +		goto err_mem_clk_disable;
>> +
>> +	return 0;
>> +
>> +err_mem_clk_disable:
>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>> +
>> +err_sys_clk_disable:
>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>> +
>> +err_core_clk_disable:
>> +	clk_disable_unprepare(pvr_dev->core_clk);
>> +
>> +	return err;
>> +}
>> +
>> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
>> +{
>> +	int err;
>> +
>> +	err = reset_control_assert(pvr_dev->reset);
>> +
>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>> +	clk_disable_unprepare(pvr_dev->core_clk);
>> +
>> +	return err;
>> +}
>> +
>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
>> +	.init = pvr_power_init_manual,
>> +	.power_on = pvr_power_on_sequence_manual,
>> +	.power_off = pvr_power_off_sequence_manual,
>> +};
>> +
>> +static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
>> +{
>> +	struct device *dev = from_pvr_device(pvr_dev)->dev;
>> +
>> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
>> +	if (IS_ERR(pvr_dev->pwrseq)) {
>> +		/*
>> +		 * This platform requires a sequencer. If we can't get it, we
>> +		 * must return the error (including -EPROBE_DEFER to wait for
>> +		 * the provider to appear)
>> +		 */
>> +		return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
>> +				     "Failed to get required power sequencer\n");
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
>> +{
>> +	return pwrseq_power_on(pvr_dev->pwrseq);
>> +}
>> +
>> +static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
>> +{
>> +	return pwrseq_power_off(pvr_dev->pwrseq);
>> +}
>> +
>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
>> +	.init = pvr_power_init_pwrseq,
>> +	.power_on = pvr_power_on_sequence_pwrseq,
>> +	.power_off = pvr_power_off_sequence_pwrseq,
>> +};
>> +
>>  int
>>  pvr_power_device_suspend(struct device *dev)
>>  {
>> @@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
>>  			goto err_drm_dev_exit;
>>  	}
>>  
>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>> -	clk_disable_unprepare(pvr_dev->core_clk);
>> -
>> -	err = reset_control_assert(pvr_dev->reset);
>> +	err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>  
>>  err_drm_dev_exit:
>>  	drm_dev_exit(idx);
>> @@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
>>  	if (!drm_dev_enter(drm_dev, &idx))
>>  		return -EIO;
>>  
>> -	err = clk_prepare_enable(pvr_dev->core_clk);
>> +	err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
>>  	if (err)
>>  		goto err_drm_dev_exit;
>>  
>> -	err = clk_prepare_enable(pvr_dev->sys_clk);
>> -	if (err)
>> -		goto err_core_clk_disable;
>> -
>> -	err = clk_prepare_enable(pvr_dev->mem_clk);
>> -	if (err)
>> -		goto err_sys_clk_disable;
>> -
>> -	/*
>> -	 * According to the hardware manual, a delay of at least 32 clock
>> -	 * cycles is required between de-asserting the clkgen reset and
>> -	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>> -	 * a very high GPU clock frequency, a delay of 1 microsecond is
>> -	 * sufficient to ensure this requirement is met across all
>> -	 * feasible GPU clock speeds.
>> -	 */
>> -	udelay(1);
>> -
>> -	err = reset_control_deassert(pvr_dev->reset);
>> -	if (err)
>> -		goto err_mem_clk_disable;
>> -
>>  	if (pvr_dev->fw_dev.booted) {
>>  		err = pvr_power_fw_enable(pvr_dev);
>>  		if (err)
>> -			goto err_reset_assert;
>> +			goto err_power_off;
>>  	}
>>  
>>  	drm_dev_exit(idx);
>>  
>>  	return 0;
>>  
>> -err_reset_assert:
>> -	reset_control_assert(pvr_dev->reset);
>> -
>> -err_mem_clk_disable:
>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>> -
>> -err_sys_clk_disable:
>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>> -
>> -err_core_clk_disable:
>> -	clk_disable_unprepare(pvr_dev->core_clk);
>> +err_power_off:
>> +	pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>  
>>  err_drm_dev_exit:
>>  	drm_dev_exit(idx);
>> diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
>> index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
>> --- a/drivers/gpu/drm/imagination/pvr_power.h
>> +++ b/drivers/gpu/drm/imagination/pvr_power.h
>> @@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
>>  int pvr_power_domains_init(struct pvr_device *pvr_dev);
>>  void pvr_power_domains_fini(struct pvr_device *pvr_dev);
>>  
>> +/**
>> + * struct pvr_power_sequence_ops - Platform specific power sequence operations.
>> + * @init: Pointer to the platform-specific initialization function.
>> + * @power_on: Pointer to the platform-specific power on function.
>> + * @power_off: Pointer to the platform-specific power off function.
>> + */
>> +struct pvr_power_sequence_ops {
>> +	int (*init)(struct pvr_device *pvr_dev);
>> +	int (*power_on)(struct pvr_device *pvr_dev);
>> +	int (*power_off)(struct pvr_device *pvr_dev);
>> +};
>> +
>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
>> +
>>  #endif /* PVR_POWER_H */
>>
> 
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>

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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Michal Wilczynski]

On 8/22/25 12:04, Michal Wilczynski wrote:
> 
> 
> On 8/22/25 11:28, Matt Coster wrote:
>> On 21/08/2025 23:20, Michal Wilczynski wrote:
>>> Update the Imagination PVR DRM driver to leverage the pwrseq framework
>>> for managing the complex power sequence of the GPU on the T-HEAD TH1520
>>> SoC.
>>>
>>> To cleanly separate platform-specific logic from the generic driver,
>>> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
>>> struct. This allows for different power management strategies to be
>>> selected at probe time based on the device's compatible string.
>>>
>>> A `pvr_device_data` struct, associated with each compatible in the
>>> of_device_id table, points to the appropriate ops table (manual or
>>> pwrseq).
>>>
>>> At probe time, the driver now calls the `->init()` op. For pwrseq-based
>>> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
>>> probe if the sequencer is not yet available. For other platforms, it
>>> falls back to the existing manual clock and reset handling. The runtime
>>> PM callbacks continue to call the appropriate functions via the ops
>>> table.
>>>
>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>
>> Reviewed-by: Matt Coster <matt.coster@imgtec.com>
>>
>> Would you like me to take the non-DTS changes via drm-misc-next?
> 
> Yeah I think this would be appropriate.
> Thanks !

Hi Drew,

Matt offered to take the non-DTS patches (1/4 and 4/4) from this series
through the DRM tree.

This leaves the DT binding and TH1520 DT patches (2/4 and 3/4). Would
you be able to pick them up through your tree ?

Thanks !

> 
>>
>> Cheers,
>> Matt
>>
>>> ---
>>>  drivers/gpu/drm/imagination/pvr_device.c |  22 +----
>>>  drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
>>>  drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
>>>  drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
>>>  drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
>>>  5 files changed, 176 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>>> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>>> @@ -23,6 +23,7 @@
>>>  #include <linux/firmware.h>
>>>  #include <linux/gfp.h>
>>>  #include <linux/interrupt.h>
>>> +#include <linux/of.h>
>>>  #include <linux/platform_device.h>
>>>  #include <linux/pm_runtime.h>
>>>  #include <linux/reset.h>
>>> @@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
>>>  	return 0;
>>>  }
>>>  
>>> -static int pvr_device_reset_init(struct pvr_device *pvr_dev)
>>> -{
>>> -	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>>> -	struct reset_control *reset;
>>> -
>>> -	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>>> -	if (IS_ERR(reset))
>>> -		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>>> -				     "failed to get gpu reset line\n");
>>> -
>>> -	pvr_dev->reset = reset;
>>> -
>>> -	return 0;
>>> -}
>>> -
>>>  /**
>>>   * pvr_device_process_active_queues() - Process all queue related events.
>>>   * @pvr_dev: PowerVR device to check
>>> @@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>>  	struct device *dev = drm_dev->dev;
>>>  	int err;
>>>  
>>> +	/* Get the platform-specific data based on the compatible string. */
>>> +	pvr_dev->device_data = of_device_get_match_data(dev);
>>> +
>>>  	/*
>>>  	 * Setup device parameters. We do this first in case other steps
>>>  	 * depend on them.
>>> @@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>>  	if (err)
>>>  		return err;
>>>  
>>> -	/* Get the reset line for the GPU */
>>> -	err = pvr_device_reset_init(pvr_dev);
>>> +	err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
>>>  	if (err)
>>>  		return err;
>>>  
>>> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
>>> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_device.h
>>> +++ b/drivers/gpu/drm/imagination/pvr_device.h
>>> @@ -37,6 +37,9 @@ struct clk;
>>>  /* Forward declaration from <linux/firmware.h>. */
>>>  struct firmware;
>>>  
>>> +/* Forward declaration from <linux/pwrseq/consumer.h> */
>>> +struct pwrseq_desc;
>>> +
>>>  /**
>>>   * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
>>>   * @b: Branch ID.
>>> @@ -57,6 +60,14 @@ struct pvr_fw_version {
>>>  	u16 major, minor;
>>>  };
>>>  
>>> +/**
>>> + * struct pvr_device_data - Platform specific data associated with a compatible string.
>>> + * @pwr_ops: Pointer to a structure with platform-specific power functions.
>>> + */
>>> +struct pvr_device_data {
>>> +	const struct pvr_power_sequence_ops *pwr_ops;
>>> +};
>>> +
>>>  /**
>>>   * struct pvr_device - powervr-specific wrapper for &struct drm_device
>>>   */
>>> @@ -98,6 +109,9 @@ struct pvr_device {
>>>  	/** @fw_version: Firmware version detected at runtime. */
>>>  	struct pvr_fw_version fw_version;
>>>  
>>> +	/** @device_data: Pointer to platform-specific data. */
>>> +	const struct pvr_device_data *device_data;
>>> +
>>>  	/** @regs_resource: Resource representing device control registers. */
>>>  	struct resource *regs_resource;
>>>  
>>> @@ -148,6 +162,9 @@ struct pvr_device {
>>>  	 */
>>>  	struct reset_control *reset;
>>>  
>>> +	/** @pwrseq: Pointer to a power sequencer, if one is used. */
>>> +	struct pwrseq_desc *pwrseq;
>>> +
>>>  	/** @irq: IRQ number. */
>>>  	int irq;
>>>  
>>> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
>>> index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_drv.c
>>> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
>>> @@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
>>>  	pvr_power_domains_fini(pvr_dev);
>>>  }
>>>  
>>> +static const struct pvr_device_data pvr_device_data_manual = {
>>> +	.pwr_ops = &pvr_power_sequence_ops_manual,
>>> +};
>>> +
>>> +static const struct pvr_device_data pvr_device_data_pwrseq = {
>>> +	.pwr_ops = &pvr_power_sequence_ops_pwrseq,
>>> +};
>>> +
>>>  static const struct of_device_id dt_match[] = {
>>> -	{ .compatible = "img,img-rogue", .data = NULL },
>>> +	{
>>> +		.compatible = "thead,th1520-gpu",
>>> +		.data = &pvr_device_data_pwrseq,
>>> +	},
>>> +	{
>>> +		.compatible = "img,img-rogue",
>>> +		.data = &pvr_device_data_manual,
>>> +	},
>>>  
>>>  	/*
>>>  	 * This legacy compatible string was introduced early on before the more generic
>>>  	 * "img,img-rogue" was added. Keep it around here for compatibility, but never use
>>>  	 * "img,img-axe" in new devicetrees.
>>>  	 */
>>> -	{ .compatible = "img,img-axe", .data = NULL },
>>> +	{
>>> +		.compatible = "img,img-axe",
>>> +		.data = &pvr_device_data_manual,
>>> +	},
>>>  	{}
>>>  };
>>>  MODULE_DEVICE_TABLE(of, dt_match);
>>> @@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
>>>  MODULE_LICENSE("Dual MIT/GPL");
>>>  MODULE_IMPORT_NS("DMA_BUF");
>>>  MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
>>> +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
>>>  MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
>>> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
>>> index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_power.c
>>> +++ b/drivers/gpu/drm/imagination/pvr_power.c
>>> @@ -18,6 +18,7 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/pm_domain.h>
>>>  #include <linux/pm_runtime.h>
>>> +#include <linux/pwrseq/consumer.h>
>>>  #include <linux/reset.h>
>>>  #include <linux/timer.h>
>>>  #include <linux/types.h>
>>> @@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
>>>  	return 0;
>>>  }
>>>  
>>> +static int pvr_power_init_manual(struct pvr_device *pvr_dev)
>>> +{
>>> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>>> +	struct reset_control *reset;
>>> +
>>> +	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>>> +	if (IS_ERR(reset))
>>> +		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>>> +				     "failed to get gpu reset line\n");
>>> +
>>> +	pvr_dev->reset = reset;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
>>> +{
>>> +	int err;
>>> +
>>> +	err = clk_prepare_enable(pvr_dev->core_clk);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	err = clk_prepare_enable(pvr_dev->sys_clk);
>>> +	if (err)
>>> +		goto err_core_clk_disable;
>>> +
>>> +	err = clk_prepare_enable(pvr_dev->mem_clk);
>>> +	if (err)
>>> +		goto err_sys_clk_disable;
>>> +
>>> +	/*
>>> +	 * According to the hardware manual, a delay of at least 32 clock
>>> +	 * cycles is required between de-asserting the clkgen reset and
>>> +	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>>> +	 * a very high GPU clock frequency, a delay of 1 microsecond is
>>> +	 * sufficient to ensure this requirement is met across all
>>> +	 * feasible GPU clock speeds.
>>> +	 */
>>> +	udelay(1);
>>> +
>>> +	err = reset_control_deassert(pvr_dev->reset);
>>> +	if (err)
>>> +		goto err_mem_clk_disable;
>>> +
>>> +	return 0;
>>> +
>>> +err_mem_clk_disable:
>>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>>> +
>>> +err_sys_clk_disable:
>>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>>> +
>>> +err_core_clk_disable:
>>> +	clk_disable_unprepare(pvr_dev->core_clk);
>>> +
>>> +	return err;
>>> +}
>>> +
>>> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
>>> +{
>>> +	int err;
>>> +
>>> +	err = reset_control_assert(pvr_dev->reset);
>>> +
>>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>>> +	clk_disable_unprepare(pvr_dev->core_clk);
>>> +
>>> +	return err;
>>> +}
>>> +
>>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
>>> +	.init = pvr_power_init_manual,
>>> +	.power_on = pvr_power_on_sequence_manual,
>>> +	.power_off = pvr_power_off_sequence_manual,
>>> +};
>>> +
>>> +static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
>>> +{
>>> +	struct device *dev = from_pvr_device(pvr_dev)->dev;
>>> +
>>> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
>>> +	if (IS_ERR(pvr_dev->pwrseq)) {
>>> +		/*
>>> +		 * This platform requires a sequencer. If we can't get it, we
>>> +		 * must return the error (including -EPROBE_DEFER to wait for
>>> +		 * the provider to appear)
>>> +		 */
>>> +		return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
>>> +				     "Failed to get required power sequencer\n");
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
>>> +{
>>> +	return pwrseq_power_on(pvr_dev->pwrseq);
>>> +}
>>> +
>>> +static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
>>> +{
>>> +	return pwrseq_power_off(pvr_dev->pwrseq);
>>> +}
>>> +
>>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
>>> +	.init = pvr_power_init_pwrseq,
>>> +	.power_on = pvr_power_on_sequence_pwrseq,
>>> +	.power_off = pvr_power_off_sequence_pwrseq,
>>> +};
>>> +
>>>  int
>>>  pvr_power_device_suspend(struct device *dev)
>>>  {
>>> @@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
>>>  			goto err_drm_dev_exit;
>>>  	}
>>>  
>>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>>> -	clk_disable_unprepare(pvr_dev->core_clk);
>>> -
>>> -	err = reset_control_assert(pvr_dev->reset);
>>> +	err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>>  
>>>  err_drm_dev_exit:
>>>  	drm_dev_exit(idx);
>>> @@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
>>>  	if (!drm_dev_enter(drm_dev, &idx))
>>>  		return -EIO;
>>>  
>>> -	err = clk_prepare_enable(pvr_dev->core_clk);
>>> +	err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
>>>  	if (err)
>>>  		goto err_drm_dev_exit;
>>>  
>>> -	err = clk_prepare_enable(pvr_dev->sys_clk);
>>> -	if (err)
>>> -		goto err_core_clk_disable;
>>> -
>>> -	err = clk_prepare_enable(pvr_dev->mem_clk);
>>> -	if (err)
>>> -		goto err_sys_clk_disable;
>>> -
>>> -	/*
>>> -	 * According to the hardware manual, a delay of at least 32 clock
>>> -	 * cycles is required between de-asserting the clkgen reset and
>>> -	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>>> -	 * a very high GPU clock frequency, a delay of 1 microsecond is
>>> -	 * sufficient to ensure this requirement is met across all
>>> -	 * feasible GPU clock speeds.
>>> -	 */
>>> -	udelay(1);
>>> -
>>> -	err = reset_control_deassert(pvr_dev->reset);
>>> -	if (err)
>>> -		goto err_mem_clk_disable;
>>> -
>>>  	if (pvr_dev->fw_dev.booted) {
>>>  		err = pvr_power_fw_enable(pvr_dev);
>>>  		if (err)
>>> -			goto err_reset_assert;
>>> +			goto err_power_off;
>>>  	}
>>>  
>>>  	drm_dev_exit(idx);
>>>  
>>>  	return 0;
>>>  
>>> -err_reset_assert:
>>> -	reset_control_assert(pvr_dev->reset);
>>> -
>>> -err_mem_clk_disable:
>>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>>> -
>>> -err_sys_clk_disable:
>>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>>> -
>>> -err_core_clk_disable:
>>> -	clk_disable_unprepare(pvr_dev->core_clk);
>>> +err_power_off:
>>> +	pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>>  
>>>  err_drm_dev_exit:
>>>  	drm_dev_exit(idx);
>>> diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
>>> index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_power.h
>>> +++ b/drivers/gpu/drm/imagination/pvr_power.h
>>> @@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
>>>  int pvr_power_domains_init(struct pvr_device *pvr_dev);
>>>  void pvr_power_domains_fini(struct pvr_device *pvr_dev);
>>>  
>>> +/**
>>> + * struct pvr_power_sequence_ops - Platform specific power sequence operations.
>>> + * @init: Pointer to the platform-specific initialization function.
>>> + * @power_on: Pointer to the platform-specific power on function.
>>> + * @power_off: Pointer to the platform-specific power off function.
>>> + */
>>> +struct pvr_power_sequence_ops {
>>> +	int (*init)(struct pvr_device *pvr_dev);
>>> +	int (*power_on)(struct pvr_device *pvr_dev);
>>> +	int (*power_off)(struct pvr_device *pvr_dev);
>>> +};
>>> +
>>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
>>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
>>> +
>>>  #endif /* PVR_POWER_H */
>>>
>>
>>
> 
> Best regards,

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>

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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Michal Wilczynski]

On 8/22/25 12:26, Michal Wilczynski wrote:
> 
> 
> On 8/22/25 12:04, Michal Wilczynski wrote:
>>
>>
>> On 8/22/25 11:28, Matt Coster wrote:
>>> On 21/08/2025 23:20, Michal Wilczynski wrote:
>>>> Update the Imagination PVR DRM driver to leverage the pwrseq framework
>>>> for managing the complex power sequence of the GPU on the T-HEAD TH1520
>>>> SoC.
>>>>
>>>> To cleanly separate platform-specific logic from the generic driver,
>>>> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
>>>> struct. This allows for different power management strategies to be
>>>> selected at probe time based on the device's compatible string.
>>>>
>>>> A `pvr_device_data` struct, associated with each compatible in the
>>>> of_device_id table, points to the appropriate ops table (manual or
>>>> pwrseq).
>>>>
>>>> At probe time, the driver now calls the `->init()` op. For pwrseq-based
>>>> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
>>>> probe if the sequencer is not yet available. For other platforms, it
>>>> falls back to the existing manual clock and reset handling. The runtime
>>>> PM callbacks continue to call the appropriate functions via the ops
>>>> table.
>>>>
>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>>
>>> Reviewed-by: Matt Coster <matt.coster@imgtec.com>
>>>
>>> Would you like me to take the non-DTS changes via drm-misc-next?
>>
>> Yeah I think this would be appropriate.
>> Thanks !
> 
> Hi Drew,
> 
> Matt offered to take the non-DTS patches (1/4 and 4/4) from this series
> through the DRM tree.
> 
> This leaves the DT binding and TH1520 DT patches (2/4 and 3/4). Would
> you be able to pick them up through your tree ?

Aw,
Sorry I think Matt also meant to take the dt-binding patch, so there would be
only patch 3 for your tree.

Thanks !

> 
> Thanks !
> 
>>
>>>
>>> Cheers,
>>> Matt
>>>
>>>> ---
>>>>  drivers/gpu/drm/imagination/pvr_device.c |  22 +----
>>>>  drivers/gpu/drm/imagination/pvr_device.h |  17 ++++
>>>>  drivers/gpu/drm/imagination/pvr_drv.c    |  23 ++++-
>>>>  drivers/gpu/drm/imagination/pvr_power.c  | 158 +++++++++++++++++++++++--------
>>>>  drivers/gpu/drm/imagination/pvr_power.h  |  15 +++
>>>>  5 files changed, 176 insertions(+), 59 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
>>>> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..294b6019b4155bb7fdb7de73ccf7fa8ad867811f 100644
>>>> --- a/drivers/gpu/drm/imagination/pvr_device.c
>>>> +++ b/drivers/gpu/drm/imagination/pvr_device.c
>>>> @@ -23,6 +23,7 @@
>>>>  #include <linux/firmware.h>
>>>>  #include <linux/gfp.h>
>>>>  #include <linux/interrupt.h>
>>>> +#include <linux/of.h>
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/pm_runtime.h>
>>>>  #include <linux/reset.h>
>>>> @@ -121,21 +122,6 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static int pvr_device_reset_init(struct pvr_device *pvr_dev)
>>>> -{
>>>> -	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>>>> -	struct reset_control *reset;
>>>> -
>>>> -	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>>>> -	if (IS_ERR(reset))
>>>> -		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>>>> -				     "failed to get gpu reset line\n");
>>>> -
>>>> -	pvr_dev->reset = reset;
>>>> -
>>>> -	return 0;
>>>> -}
>>>> -
>>>>  /**
>>>>   * pvr_device_process_active_queues() - Process all queue related events.
>>>>   * @pvr_dev: PowerVR device to check
>>>> @@ -618,6 +604,9 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>>>  	struct device *dev = drm_dev->dev;
>>>>  	int err;
>>>>  
>>>> +	/* Get the platform-specific data based on the compatible string. */
>>>> +	pvr_dev->device_data = of_device_get_match_data(dev);
>>>> +
>>>>  	/*
>>>>  	 * Setup device parameters. We do this first in case other steps
>>>>  	 * depend on them.
>>>> @@ -631,8 +620,7 @@ pvr_device_init(struct pvr_device *pvr_dev)
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> -	/* Get the reset line for the GPU */
>>>> -	err = pvr_device_reset_init(pvr_dev);
>>>> +	err = pvr_dev->device_data->pwr_ops->init(pvr_dev);
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
>>>> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..ab8f56ae15df6c2888feb16b1d87b59510961936 100644
>>>> --- a/drivers/gpu/drm/imagination/pvr_device.h
>>>> +++ b/drivers/gpu/drm/imagination/pvr_device.h
>>>> @@ -37,6 +37,9 @@ struct clk;
>>>>  /* Forward declaration from <linux/firmware.h>. */
>>>>  struct firmware;
>>>>  
>>>> +/* Forward declaration from <linux/pwrseq/consumer.h> */
>>>> +struct pwrseq_desc;
>>>> +
>>>>  /**
>>>>   * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
>>>>   * @b: Branch ID.
>>>> @@ -57,6 +60,14 @@ struct pvr_fw_version {
>>>>  	u16 major, minor;
>>>>  };
>>>>  
>>>> +/**
>>>> + * struct pvr_device_data - Platform specific data associated with a compatible string.
>>>> + * @pwr_ops: Pointer to a structure with platform-specific power functions.
>>>> + */
>>>> +struct pvr_device_data {
>>>> +	const struct pvr_power_sequence_ops *pwr_ops;
>>>> +};
>>>> +
>>>>  /**
>>>>   * struct pvr_device - powervr-specific wrapper for &struct drm_device
>>>>   */
>>>> @@ -98,6 +109,9 @@ struct pvr_device {
>>>>  	/** @fw_version: Firmware version detected at runtime. */
>>>>  	struct pvr_fw_version fw_version;
>>>>  
>>>> +	/** @device_data: Pointer to platform-specific data. */
>>>> +	const struct pvr_device_data *device_data;
>>>> +
>>>>  	/** @regs_resource: Resource representing device control registers. */
>>>>  	struct resource *regs_resource;
>>>>  
>>>> @@ -148,6 +162,9 @@ struct pvr_device {
>>>>  	 */
>>>>  	struct reset_control *reset;
>>>>  
>>>> +	/** @pwrseq: Pointer to a power sequencer, if one is used. */
>>>> +	struct pwrseq_desc *pwrseq;
>>>> +
>>>>  	/** @irq: IRQ number. */
>>>>  	int irq;
>>>>  
>>>> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
>>>> index b058ec183bb30ab5c3db17ebaadf2754520a2a1f..916b40ced7eb0408fe985ba1b83b3be2eb024bae 100644
>>>> --- a/drivers/gpu/drm/imagination/pvr_drv.c
>>>> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
>>>> @@ -1480,15 +1480,33 @@ static void pvr_remove(struct platform_device *plat_dev)
>>>>  	pvr_power_domains_fini(pvr_dev);
>>>>  }
>>>>  
>>>> +static const struct pvr_device_data pvr_device_data_manual = {
>>>> +	.pwr_ops = &pvr_power_sequence_ops_manual,
>>>> +};
>>>> +
>>>> +static const struct pvr_device_data pvr_device_data_pwrseq = {
>>>> +	.pwr_ops = &pvr_power_sequence_ops_pwrseq,
>>>> +};
>>>> +
>>>>  static const struct of_device_id dt_match[] = {
>>>> -	{ .compatible = "img,img-rogue", .data = NULL },
>>>> +	{
>>>> +		.compatible = "thead,th1520-gpu",
>>>> +		.data = &pvr_device_data_pwrseq,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "img,img-rogue",
>>>> +		.data = &pvr_device_data_manual,
>>>> +	},
>>>>  
>>>>  	/*
>>>>  	 * This legacy compatible string was introduced early on before the more generic
>>>>  	 * "img,img-rogue" was added. Keep it around here for compatibility, but never use
>>>>  	 * "img,img-axe" in new devicetrees.
>>>>  	 */
>>>> -	{ .compatible = "img,img-axe", .data = NULL },
>>>> +	{
>>>> +		.compatible = "img,img-axe",
>>>> +		.data = &pvr_device_data_manual,
>>>> +	},
>>>>  	{}
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, dt_match);
>>>> @@ -1513,4 +1531,5 @@ MODULE_DESCRIPTION(PVR_DRIVER_DESC);
>>>>  MODULE_LICENSE("Dual MIT/GPL");
>>>>  MODULE_IMPORT_NS("DMA_BUF");
>>>>  MODULE_FIRMWARE("powervr/rogue_33.15.11.3_v1.fw");
>>>> +MODULE_FIRMWARE("powervr/rogue_36.52.104.182_v1.fw");
>>>>  MODULE_FIRMWARE("powervr/rogue_36.53.104.796_v1.fw");
>>>> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
>>>> index 187a07e0bd9adb2f0713ac2c8e091229f4027354..c6e7ff9e935d3b348eff6953c633c72410fdf507 100644
>>>> --- a/drivers/gpu/drm/imagination/pvr_power.c
>>>> +++ b/drivers/gpu/drm/imagination/pvr_power.c
>>>> @@ -18,6 +18,7 @@
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/pm_domain.h>
>>>>  #include <linux/pm_runtime.h>
>>>> +#include <linux/pwrseq/consumer.h>
>>>>  #include <linux/reset.h>
>>>>  #include <linux/timer.h>
>>>>  #include <linux/types.h>
>>>> @@ -234,6 +235,118 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int pvr_power_init_manual(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
>>>> +	struct reset_control *reset;
>>>> +
>>>> +	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL);
>>>> +	if (IS_ERR(reset))
>>>> +		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
>>>> +				     "failed to get gpu reset line\n");
>>>> +
>>>> +	pvr_dev->reset = reset;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int pvr_power_on_sequence_manual(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	int err;
>>>> +
>>>> +	err = clk_prepare_enable(pvr_dev->core_clk);
>>>> +	if (err)
>>>> +		return err;
>>>> +
>>>> +	err = clk_prepare_enable(pvr_dev->sys_clk);
>>>> +	if (err)
>>>> +		goto err_core_clk_disable;
>>>> +
>>>> +	err = clk_prepare_enable(pvr_dev->mem_clk);
>>>> +	if (err)
>>>> +		goto err_sys_clk_disable;
>>>> +
>>>> +	/*
>>>> +	 * According to the hardware manual, a delay of at least 32 clock
>>>> +	 * cycles is required between de-asserting the clkgen reset and
>>>> +	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>>>> +	 * a very high GPU clock frequency, a delay of 1 microsecond is
>>>> +	 * sufficient to ensure this requirement is met across all
>>>> +	 * feasible GPU clock speeds.
>>>> +	 */
>>>> +	udelay(1);
>>>> +
>>>> +	err = reset_control_deassert(pvr_dev->reset);
>>>> +	if (err)
>>>> +		goto err_mem_clk_disable;
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_mem_clk_disable:
>>>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>>>> +
>>>> +err_sys_clk_disable:
>>>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>>>> +
>>>> +err_core_clk_disable:
>>>> +	clk_disable_unprepare(pvr_dev->core_clk);
>>>> +
>>>> +	return err;
>>>> +}
>>>> +
>>>> +static int pvr_power_off_sequence_manual(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	int err;
>>>> +
>>>> +	err = reset_control_assert(pvr_dev->reset);
>>>> +
>>>> +	clk_disable_unprepare(pvr_dev->mem_clk);
>>>> +	clk_disable_unprepare(pvr_dev->sys_clk);
>>>> +	clk_disable_unprepare(pvr_dev->core_clk);
>>>> +
>>>> +	return err;
>>>> +}
>>>> +
>>>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual = {
>>>> +	.init = pvr_power_init_manual,
>>>> +	.power_on = pvr_power_on_sequence_manual,
>>>> +	.power_off = pvr_power_off_sequence_manual,
>>>> +};
>>>> +
>>>> +static int pvr_power_init_pwrseq(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	struct device *dev = from_pvr_device(pvr_dev)->dev;
>>>> +
>>>> +	pvr_dev->pwrseq = devm_pwrseq_get(dev, "gpu-power");
>>>> +	if (IS_ERR(pvr_dev->pwrseq)) {
>>>> +		/*
>>>> +		 * This platform requires a sequencer. If we can't get it, we
>>>> +		 * must return the error (including -EPROBE_DEFER to wait for
>>>> +		 * the provider to appear)
>>>> +		 */
>>>> +		return dev_err_probe(dev, PTR_ERR(pvr_dev->pwrseq),
>>>> +				     "Failed to get required power sequencer\n");
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	return pwrseq_power_on(pvr_dev->pwrseq);
>>>> +}
>>>> +
>>>> +static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
>>>> +{
>>>> +	return pwrseq_power_off(pvr_dev->pwrseq);
>>>> +}
>>>> +
>>>> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
>>>> +	.init = pvr_power_init_pwrseq,
>>>> +	.power_on = pvr_power_on_sequence_pwrseq,
>>>> +	.power_off = pvr_power_off_sequence_pwrseq,
>>>> +};
>>>> +
>>>>  int
>>>>  pvr_power_device_suspend(struct device *dev)
>>>>  {
>>>> @@ -252,11 +365,7 @@ pvr_power_device_suspend(struct device *dev)
>>>>  			goto err_drm_dev_exit;
>>>>  	}
>>>>  
>>>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>>>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>>>> -	clk_disable_unprepare(pvr_dev->core_clk);
>>>> -
>>>> -	err = reset_control_assert(pvr_dev->reset);
>>>> +	err = pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>>>  
>>>>  err_drm_dev_exit:
>>>>  	drm_dev_exit(idx);
>>>> @@ -276,53 +385,22 @@ pvr_power_device_resume(struct device *dev)
>>>>  	if (!drm_dev_enter(drm_dev, &idx))
>>>>  		return -EIO;
>>>>  
>>>> -	err = clk_prepare_enable(pvr_dev->core_clk);
>>>> +	err = pvr_dev->device_data->pwr_ops->power_on(pvr_dev);
>>>>  	if (err)
>>>>  		goto err_drm_dev_exit;
>>>>  
>>>> -	err = clk_prepare_enable(pvr_dev->sys_clk);
>>>> -	if (err)
>>>> -		goto err_core_clk_disable;
>>>> -
>>>> -	err = clk_prepare_enable(pvr_dev->mem_clk);
>>>> -	if (err)
>>>> -		goto err_sys_clk_disable;
>>>> -
>>>> -	/*
>>>> -	 * According to the hardware manual, a delay of at least 32 clock
>>>> -	 * cycles is required between de-asserting the clkgen reset and
>>>> -	 * de-asserting the GPU reset. Assuming a worst-case scenario with
>>>> -	 * a very high GPU clock frequency, a delay of 1 microsecond is
>>>> -	 * sufficient to ensure this requirement is met across all
>>>> -	 * feasible GPU clock speeds.
>>>> -	 */
>>>> -	udelay(1);
>>>> -
>>>> -	err = reset_control_deassert(pvr_dev->reset);
>>>> -	if (err)
>>>> -		goto err_mem_clk_disable;
>>>> -
>>>>  	if (pvr_dev->fw_dev.booted) {
>>>>  		err = pvr_power_fw_enable(pvr_dev);
>>>>  		if (err)
>>>> -			goto err_reset_assert;
>>>> +			goto err_power_off;
>>>>  	}
>>>>  
>>>>  	drm_dev_exit(idx);
>>>>  
>>>>  	return 0;
>>>>  
>>>> -err_reset_assert:
>>>> -	reset_control_assert(pvr_dev->reset);
>>>> -
>>>> -err_mem_clk_disable:
>>>> -	clk_disable_unprepare(pvr_dev->mem_clk);
>>>> -
>>>> -err_sys_clk_disable:
>>>> -	clk_disable_unprepare(pvr_dev->sys_clk);
>>>> -
>>>> -err_core_clk_disable:
>>>> -	clk_disable_unprepare(pvr_dev->core_clk);
>>>> +err_power_off:
>>>> +	pvr_dev->device_data->pwr_ops->power_off(pvr_dev);
>>>>  
>>>>  err_drm_dev_exit:
>>>>  	drm_dev_exit(idx);
>>>> diff --git a/drivers/gpu/drm/imagination/pvr_power.h b/drivers/gpu/drm/imagination/pvr_power.h
>>>> index ada85674a7ca762dcf92df40424230e1c3910342..b853d092242cc90cb98cf66100679a309055a1dc 100644
>>>> --- a/drivers/gpu/drm/imagination/pvr_power.h
>>>> +++ b/drivers/gpu/drm/imagination/pvr_power.h
>>>> @@ -41,4 +41,19 @@ pvr_power_put(struct pvr_device *pvr_dev)
>>>>  int pvr_power_domains_init(struct pvr_device *pvr_dev);
>>>>  void pvr_power_domains_fini(struct pvr_device *pvr_dev);
>>>>  
>>>> +/**
>>>> + * struct pvr_power_sequence_ops - Platform specific power sequence operations.
>>>> + * @init: Pointer to the platform-specific initialization function.
>>>> + * @power_on: Pointer to the platform-specific power on function.
>>>> + * @power_off: Pointer to the platform-specific power off function.
>>>> + */
>>>> +struct pvr_power_sequence_ops {
>>>> +	int (*init)(struct pvr_device *pvr_dev);
>>>> +	int (*power_on)(struct pvr_device *pvr_dev);
>>>> +	int (*power_off)(struct pvr_device *pvr_dev);
>>>> +};
>>>> +
>>>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_manual;
>>>> +extern const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq;
>>>> +
>>>>  #endif /* PVR_POWER_H */
>>>>
>>>
>>>
>>
>> Best regards,
> 
> Best regards,

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>

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

Re: [PATCH v13 1/4] drm/imagination: Use pwrseq for TH1520 GPU power management

[Drew Fustini]

On Fri, Aug 22, 2025 at 01:49:38PM +0200, Michal Wilczynski wrote:
> 
> 
> On 8/22/25 12:26, Michal Wilczynski wrote:
> > 
> > 
> > On 8/22/25 12:04, Michal Wilczynski wrote:
> >>
> >>
> >> On 8/22/25 11:28, Matt Coster wrote:
> >>> On 21/08/2025 23:20, Michal Wilczynski wrote:
> >>>> Update the Imagination PVR DRM driver to leverage the pwrseq framework
> >>>> for managing the complex power sequence of the GPU on the T-HEAD TH1520
> >>>> SoC.
> >>>>
> >>>> To cleanly separate platform-specific logic from the generic driver,
> >>>> this patch introduces an `init` callback to the `pwr_power_sequence_ops`
> >>>> struct. This allows for different power management strategies to be
> >>>> selected at probe time based on the device's compatible string.
> >>>>
> >>>> A `pvr_device_data` struct, associated with each compatible in the
> >>>> of_device_id table, points to the appropriate ops table (manual or
> >>>> pwrseq).
> >>>>
> >>>> At probe time, the driver now calls the `->init()` op. For pwrseq-based
> >>>> platforms, this callback calls `devm_pwrseq_get("gpu-power")`, deferring
> >>>> probe if the sequencer is not yet available. For other platforms, it
> >>>> falls back to the existing manual clock and reset handling. The runtime
> >>>> PM callbacks continue to call the appropriate functions via the ops
> >>>> table.
> >>>>
> >>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> >>>
> >>> Reviewed-by: Matt Coster <matt.coster@imgtec.com>
> >>>
> >>> Would you like me to take the non-DTS changes via drm-misc-next?
> >>
> >> Yeah I think this would be appropriate.
> >> Thanks !
> > 
> > Hi Drew,
> > 
> > Matt offered to take the non-DTS patches (1/4 and 4/4) from this series
> > through the DRM tree.
> > 
> > This leaves the DT binding and TH1520 DT patches (2/4 and 3/4). Would
> > you be able to pick them up through your tree ?
> 
> Aw,
> Sorry I think Matt also meant to take the dt-binding patch, so there would be
> only patch 3 for your tree.

Yes, that makes sense, I will the dts patch through thead-dt-for-next:

[PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

Thanks,
Drew

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

[PATCH v13 2/4] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU support

[Michal Wilczynski]

Rework the PowerVR Rogue GPU binding to use an explicit, per variant
style for defining power domain properties and add support for the
T-HEAD TH1520 SoC's GPU.

To improve clarity and precision, the binding is refactored so that
power domain items are listed explicitly for each variant [1]. The
previous method relied on an implicit, positional mapping between the
`power-domains` and `power-domain-names` properties. This change
replaces the generic rules with self contained if/then blocks for each
GPU variant, making the relationship between power domains and their
names explicit and unambiguous.

The generic if block for img,img-rogue, which previously required
power-domains and power-domain-names for all variants, is removed.
Instead, each specific GPU variant now defines its own power domain
requirements within a self-contained if/then block, making the schema
more explicit.

This new structure is then used to add support for the
`thead,th1520-gpu`. While its BXM-4-64 IP has two conceptual power
domains, the TH1520 SoC integrates them behind a single power gate. The
new binding models this with a specific rule that enforces a single
`power-domains` entry and disallows the `power-domain-names` property.

Link: https://lore.kernel.org/all/4d79c8dd-c5fb-442c-ac65-37e7176b0cdd@linaro.org/ [1]

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Matt Coster <matt.coster@imgtec.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 37 +++++++++++++++++-----
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..c87d7bece0ecd6331fc7d1a479bbdaf68bac6e6d 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -21,6 +21,11 @@ properties:
           # work with newer dts.
           - const: img,img-axe
           - const: img,img-rogue
+      - items:
+          - enum:
+              - thead,th1520-gpu
+          - const: img,img-bxm-4-64
+          - const: img,img-rogue
       - items:
           - enum:
               - ti,j721s2-gpu
@@ -77,14 +82,18 @@ required:
 additionalProperties: false
 
 allOf:
-  # Constraints added alongside the new compatible strings that would otherwise
-  # create an ABI break.
   - if:
       properties:
         compatible:
           contains:
-            const: img,img-rogue
+            const: img,img-axe-1-16m
     then:
+      properties:
+        power-domains:
+          items:
+            - description: Power domain A
+        power-domain-names:
+          maxItems: 1
       required:
         - power-domains
         - power-domain-names
@@ -93,13 +102,20 @@ allOf:
       properties:
         compatible:
           contains:
-            const: img,img-axe-1-16m
+            const: thead,th1520-gpu
     then:
       properties:
+        clocks:
+          minItems: 3
+        clock-names:
+          minItems: 3
         power-domains:
-          maxItems: 1
-        power-domain-names:
-          maxItems: 1
+          items:
+            - description: The single, unified power domain for the GPU on the
+                TH1520 SoC, integrating all internal IP power domains.
+        power-domain-names: false
+      required:
+        - power-domains
 
   - if:
       properties:
@@ -109,9 +125,14 @@ allOf:
     then:
       properties:
         power-domains:
-          minItems: 2
+          items:
+            - description: Power domain A
+            - description: Power domain B
         power-domain-names:
           minItems: 2
+      required:
+        - power-domains
+        - power-domain-names
 
   - if:
       properties:

-- 
2.34.1


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

[PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Michal Wilczynski]

Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
the GPU using the drm/imagination driver.

By adding this node, the kernel can recognize and initialize the GPU,
providing graphics acceleration capabilities on the Lichee Pi 4A and
other boards based on the TH1520 SoC.

Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be
controlled programatically.

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Drew Fustini <drew@pdp7.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Matt Coster <matt.coster@imgtec.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 42724bf7e90e08fac326c464d0f080e3bd2cd59b..6ae5c632205ba63248c0a119c03bdfc084aac7a0 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -225,6 +225,13 @@ aonsys_clk: clock-73728000 {
 		#clock-cells = <0>;
 	};
 
+	gpu_mem_clk: mem-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <0>;
+		clock-output-names = "gpu_mem_clk";
+		#clock-cells = <0>;
+	};
+
 	stmmac_axi_config: stmmac-axi-config {
 		snps,wr_osr_lmt = <15>;
 		snps,rd_osr_lmt = <15>;
@@ -500,6 +507,20 @@ clk: clock-controller@ffef010000 {
 			#clock-cells = <1>;
 		};
 
+		gpu: gpu@ffef400000 {
+			compatible = "thead,th1520-gpu", "img,img-bxm-4-64",
+				     "img,img-rogue";
+			reg = <0xff 0xef400000 0x0 0x100000>;
+			interrupt-parent = <&plic>;
+			interrupts = <102 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk_vo CLK_GPU_CORE>,
+				 <&gpu_mem_clk>,
+				 <&clk_vo CLK_GPU_CFG_ACLK>;
+			clock-names = "core", "mem", "sys";
+			power-domains = <&aon TH1520_GPU_PD>;
+			resets = <&rst TH1520_RESET_ID_GPU>;
+		};
+
 		rst: reset-controller@ffef528000 {
 			compatible = "thead,th1520-reset";
 			reg = <0xff 0xef528000 0x0 0x4f>;

-- 
2.34.1


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

Re: [PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Drew Fustini]

On Fri, Aug 22, 2025 at 12:20:17AM +0200, Michal Wilczynski wrote:
> Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
> TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
> the GPU using the drm/imagination driver.
> 
> By adding this node, the kernel can recognize and initialize the GPU,
> providing graphics acceleration capabilities on the Lichee Pi 4A and
> other boards based on the TH1520 SoC.
> 
> Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be
> controlled programatically.
> 
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Drew Fustini <drew@pdp7.com>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Acked-by: Matt Coster <matt.coster@imgtec.com>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

I've applied this to thead-dt-for-next [1]:

0f78e44fb857 ("riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node")

Thanks,
Drew

[1] https://git.kernel.org/pub/scm/linux/kernel/git/fustini/linux.git/log/?h=thead-dt-for-next


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

Re: [PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Drew Fustini]

On Fri, Aug 22, 2025 at 01:43:53PM -0700, Drew Fustini wrote:
> On Fri, Aug 22, 2025 at 12:20:17AM +0200, Michal Wilczynski wrote:
> > Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
> > TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
> > the GPU using the drm/imagination driver.
> > 
> > By adding this node, the kernel can recognize and initialize the GPU,
> > providing graphics acceleration capabilities on the Lichee Pi 4A and
> > other boards based on the TH1520 SoC.
> > 
> > Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be
> > controlled programatically.
> > 
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Reviewed-by: Drew Fustini <drew@pdp7.com>
> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > Acked-by: Matt Coster <matt.coster@imgtec.com>
> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > ---
> >  arch/riscv/boot/dts/thead/th1520.dtsi | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> 
> I've applied this to thead-dt-for-next [1]:
> 
> 0f78e44fb857 ("riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node")
> 
> Thanks,
> Drew
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/fustini/linux.git/log/?h=thead-dt-for-next

Hi Matt,

Do you know when the dt binding patch will be applied to
the drm-misc/for-linux-next tree?

I applied the dts patch but it is creating a warning in next right now.
If the binding won't show up soon in drm-misc, then I'll remove this dts
patch from next as dtbs_check is now failing in next. I can add it back
once the binding makes it to next.

Thanks,
Drew

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

Re: [PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Drew Fustini]

On Wed, Aug 27, 2025 at 03:08:01PM -0700, Drew Fustini wrote:
> On Fri, Aug 22, 2025 at 01:43:53PM -0700, Drew Fustini wrote:
> > On Fri, Aug 22, 2025 at 12:20:17AM +0200, Michal Wilczynski wrote:
> > > Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
> > > TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
> > > the GPU using the drm/imagination driver.
> > > 
> > > By adding this node, the kernel can recognize and initialize the GPU,
> > > providing graphics acceleration capabilities on the Lichee Pi 4A and
> > > other boards based on the TH1520 SoC.
> > > 
> > > Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be
> > > controlled programatically.
> > > 
> > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > Reviewed-by: Drew Fustini <drew@pdp7.com>
> > > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > Acked-by: Matt Coster <matt.coster@imgtec.com>
> > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > > ---
> > >  arch/riscv/boot/dts/thead/th1520.dtsi | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > 
> > I've applied this to thead-dt-for-next [1]:
> > 
> > 0f78e44fb857 ("riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node")
> > 
> > Thanks,
> > Drew
> > 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/fustini/linux.git/log/?h=thead-dt-for-next
> 
> Hi Matt,
> 
> Do you know when the dt binding patch will be applied to
> the drm-misc/for-linux-next tree?
> 
> I applied the dts patch but it is creating a warning in next right now.
> If the binding won't show up soon in drm-misc, then I'll remove this dts
> patch from next as dtbs_check is now failing in next. I can add it back
> once the binding makes it to next.

I've now removed this patch from thead-dt-for-next and will add it back
once the bindings show up in next.

Thanks,
Drew

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

Re: [PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Matt Coster]

[-- Attachment #1.1.1: Type: text/plain, Size: 2341 bytes --]

On 28/08/2025 22:56, Drew Fustini wrote:
> On Wed, Aug 27, 2025 at 03:08:01PM -0700, Drew Fustini wrote:
>> On Fri, Aug 22, 2025 at 01:43:53PM -0700, Drew Fustini wrote:
>>> On Fri, Aug 22, 2025 at 12:20:17AM +0200, Michal Wilczynski wrote:
>>>> Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
>>>> TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
>>>> the GPU using the drm/imagination driver.
>>>>
>>>> By adding this node, the kernel can recognize and initialize the GPU,
>>>> providing graphics acceleration capabilities on the Lichee Pi 4A and
>>>> other boards based on the TH1520 SoC.
>>>>
>>>> Add fixed clock gpu_mem_clk, as the MEM clock on the T-HEAD SoC can't be
>>>> controlled programatically.
>>>>
>>>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> Reviewed-by: Drew Fustini <drew@pdp7.com>
>>>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>> Acked-by: Matt Coster <matt.coster@imgtec.com>
>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>>> ---
>>>>  arch/riscv/boot/dts/thead/th1520.dtsi | 21 +++++++++++++++++++++
>>>>  1 file changed, 21 insertions(+)
>>>
>>> I've applied this to thead-dt-for-next [1]:
>>>
>>> 0f78e44fb857 ("riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node")
>>>
>>> Thanks,
>>> Drew
>>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/fustini/linux.git/log/?h=thead-dt-for-next
>>
>> Hi Matt,
>>
>> Do you know when the dt binding patch will be applied to
>> the drm-misc/for-linux-next tree?
>>
>> I applied the dts patch but it is creating a warning in next right now.
>> If the binding won't show up soon in drm-misc, then I'll remove this dts
>> patch from next as dtbs_check is now failing in next. I can add it back
>> once the binding makes it to next.
> 
> I've now removed this patch from thead-dt-for-next and will add it back
> once the bindings show up in next.

Hi Drew,

Apologies for the delay, I was on holiday last week.

I've just applied the non-dts patches to drm-misc-next [1], would you
mind re-adding the dts patch to thead-dt-for-next?

Cheers,
Matt

[1]: https://lore.kernel.org/r/175672521205.30950.2944854121832397083.b4-ty@imgtec.com

> 
> Thanks,
> Drew


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v13 3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node

[Drew Fustini]

[-- Attachment #1.1: Type: text/plain, Size: 478 bytes --]

On Mon, Sep 01, 2025 at 11:16:18AM +0000, Matt Coster wrote:
> Hi Drew,
> 
> Apologies for the delay, I was on holiday last week.
> 
> I've just applied the non-dts patches to drm-misc-next [1], would you
> mind re-adding the dts patch to thead-dt-for-next?

Thanks for the update.

I've now pushed the dts patch back to thead-dt-for-next:

[3/4] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node
      commit: 5052d5cf1359e9057ec311788c12997406fdb2fc

-Drew

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

[PATCH v13 4/4] drm/imagination: Enable PowerVR driver for RISC-V

[Michal Wilczynski]

Several RISC-V boards feature Imagination GPUs that are compatible with
the PowerVR driver. An example is the IMG BXM-4-64 GPU on the Lichee Pi
4A board. This commit adjusts the driver's Kconfig dependencies to allow
the PowerVR driver to be compiled on the RISC-V architecture.

By enabling compilation on RISC-V, we expand support for these GPUs,
providing graphics acceleration capabilities and enhancing hardware
compatibility on RISC-V platforms.

The RISC-V support is restricted to 64-bit systems (RISCV && 64BIT) as
the driver currently has an implicit dependency on a 64-bit platform.

Add a dependency on MMU to fix a build warning on RISC-V configurations
without an MMU.

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Matt Coster <matt.coster@imgtec.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/gpu/drm/imagination/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
index 3bfa2ac212dccb73c53bdc2bc259bcba636e7cfc..682dd2633d0c012df18d0f7144d029b67a14d241 100644
--- a/drivers/gpu/drm/imagination/Kconfig
+++ b/drivers/gpu/drm/imagination/Kconfig
@@ -3,8 +3,9 @@
 
 config DRM_POWERVR
 	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
-	depends on ARM64
+	depends on (ARM64 || RISCV && 64BIT)
 	depends on DRM
+	depends on MMU
 	depends on PM
 	select DRM_EXEC
 	select DRM_GEM_SHMEM_HELPER

-- 
2.34.1


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

Re: (subset) [PATCH v13 0/4] Add TH1520 GPU support with power sequencing

[Matt Coster]

On Fri, 22 Aug 2025 00:20:14 +0200, Michal Wilczynski wrote:
> This patch series introduces support for the Imagination IMG BXM-4-64
> GPU found on the T-HEAD TH1520 SoC. A key aspect of this support is
> managing the GPU's complex power-up and power-down sequence, which
> involves multiple clocks and resets.
> 
> The TH1520 GPU requires a specific sequence to be followed for its
> clocks and resets to ensure correct operation. Initial discussions and
> an earlier version of this series explored managing this via the generic
> power domain (genpd) framework. However, following further discussions
> with kernel maintainers [1], the approach has been reworked to utilize
> the dedicated power sequencing (pwrseq) framework.
> 
> [...]

Applied, thanks!

[1/4] drm/imagination: Use pwrseq for TH1520 GPU power management
      commit: e38e8391f30b41c5a24bb46dc6ef4161921e782d
[2/4] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU support
      commit: 337ebfda8a4f2627bf52e200cacf6f3a2f5ccf48
[4/4] drm/imagination: Enable PowerVR driver for RISC-V
      commit: 6b53cf48d9339c75fa51927b0a67d8a6751066bd

Best regards,
-- 
Matt Coster <matt.coster@imgtec.com>


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