* [PATCH v7 0/5] Add TH1520 GPU support with power sequencing
[not found] <CGME20250626093355eucas1p12c6dc13a987fd498906fb8846b4722c7@eucas1p1.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
[not found] ` <CGME20250626093356eucas1p1adfcd565173d939f82e15252189c316f@eucas1p1.samsung.com>
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski, Krzysztof Kozlowski
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)
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 gpu-clkgen reset property to the aon node in the
TH1520 device tree source.
Patch 4: Adds the device tree node for the IMG BXM-4-64 GPU and its
required fixed-clock.
Patch 5: Enables compilation of the Imagination PowerVR driver on the
RISC-V architecture.
This patchset finishes the work started in bigger series [2] by adding
all 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 v6 of this series - [3].
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/20250623-apr_14_for_sending-v6-0-6583ce0f6c25@samsung.com/
---
Michal Wilczynski (5):
drm/imagination: Use pwrseq for TH1520 GPU power management
dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible
riscv: dts: thead: th1520: Add GPU clkgen reset to AON node
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 | 26 +++-
arch/riscv/boot/dts/thead/th1520.dtsi | 25 ++++
drivers/gpu/drm/imagination/Kconfig | 3 +-
drivers/gpu/drm/imagination/pvr_device.c | 36 +++++-
drivers/gpu/drm/imagination/pvr_device.h | 17 +++
drivers/gpu/drm/imagination/pvr_drv.c | 27 +++-
drivers/gpu/drm/imagination/pvr_power.c | 139 +++++++++++++++------
drivers/gpu/drm/imagination/pvr_power.h | 13 ++
8 files changed, 237 insertions(+), 49 deletions(-)
---
base-commit: b54547357cdc6fec6d1d58945a746cda9c771b3d
change-id: 20250414-apr_14_for_sending-5b3917817acc
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management
[not found] ` <CGME20250626093356eucas1p1adfcd565173d939f82e15252189c316f@eucas1p1.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
2025-07-03 10:21 ` Michal Wilczynski
2025-07-23 10:38 ` Matt Coster
0 siblings, 2 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
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 a `pwr_power_sequence_ops` struct containing
function pointers for power_on and power_off operations. 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 inspects the assigned ops struct. If the
pwrseq variant is detected, the driver calls
devm_pwrseq_get("gpu-power"), deferring probe if the sequencer is not
yet available. Otherwise, it falls back to the existing manual clock and
reset handling. The runtime PM callbacks now call the appropriate
functions via the ops table.
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
drivers/gpu/drm/imagination/pvr_device.c | 36 +++++++-
drivers/gpu/drm/imagination/pvr_device.h | 17 ++++
drivers/gpu/drm/imagination/pvr_drv.c | 27 +++++-
drivers/gpu/drm/imagination/pvr_power.c | 139 ++++++++++++++++++++++---------
drivers/gpu/drm/imagination/pvr_power.h | 13 +++
5 files changed, 185 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..770fc32a6fe485aad41cd92fa1431dd233ac20dc 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -23,8 +23,12 @@
#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>
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+#include <linux/pwrseq/consumer.h>
+#endif
#include <linux/reset.h>
#include <linux/slab.h>
#include <linux/stddef.h>
@@ -618,6 +622,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,10 +638,31 @@ 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);
- if (err)
- return err;
+ /*
+ * For platforms that require it, get the power sequencer.
+ * For all others, perform manual reset initialization.
+ */
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+ if (pvr_dev->device_data->pwr_ops == &pvr_power_sequence_ops_pwrseq) {
+ 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");
+ }
+ } else
+#endif
+ {
+ /* This platform does not use a sequencer, init reset manually. */
+ err = pvr_device_reset_init(pvr_dev);
+ if (err)
+ return err;
+ }
/* Explicitly power the GPU so we can access control registers before the FW is booted. */
err = pm_runtime_resume_and_get(dev);
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..0d7f7c78573a0766a467fb0c3a577ffe152d0892 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..af830e565646daf19555197df492438ef48d5e44 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1480,15 +1480,37 @@ 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,
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+static const struct pvr_device_data pvr_device_data_pwrseq = {
+ .pwr_ops = &pvr_power_sequence_ops_pwrseq,
+};
+#endif
+
static const struct of_device_id dt_match[] = {
- { .compatible = "img,img-rogue", .data = NULL },
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+ {
+ .compatible = "thead,th1520-gpu",
+ .data = &pvr_device_data_pwrseq,
+ },
+#endif
+ {
+ .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 +1535,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 41f5d89e78b854cf6993838868a4416a220b490a..13aef27849d1a71df77406c8d7845836998a35a0 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -18,6 +18,9 @@
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+#include <linux/pwrseq/consumer.h>
+#endif
#include <linux/reset.h>
#include <linux/timer.h>
#include <linux/types.h>
@@ -234,6 +237,96 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
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 = {
+ .power_on = pvr_power_on_sequence_manual,
+ .power_off = pvr_power_off_sequence_manual,
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
+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 = {
+ .power_on = pvr_power_on_sequence_pwrseq,
+ .power_off = pvr_power_off_sequence_pwrseq,
+};
+#else /* IS_ENABLED(CONFIG_POWER_SEQUENCING) */
+static int pvr_power_sequence_pwrseq_stub(struct pvr_device *pvr_dev)
+{
+ WARN_ONCE(1, "pwrseq support not enabled in kernel config\n");
+ return -EOPNOTSUPP;
+}
+
+const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
+ .power_on = pvr_power_sequence_pwrseq_stub,
+ .power_off = pvr_power_sequence_pwrseq_stub,
+};
+#endif /* IS_ENABLED(CONFIG_POWER_SEQUENCING) */
+
int
pvr_power_device_suspend(struct device *dev)
{
@@ -252,11 +345,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,54 +365,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..6a2f3f6213e5ac2254344ad24d9678334c8974ea 100644
--- a/drivers/gpu/drm/imagination/pvr_power.h
+++ b/drivers/gpu/drm/imagination/pvr_power.h
@@ -41,4 +41,17 @@ 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.
+ * @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 (*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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 2/5] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible
[not found] ` <CGME20250626093358eucas1p1e5d763dc86dccb0ed9fa725182ea59e6@eucas1p1.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
0 siblings, 0 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Krzysztof Kozlowski, Bartosz Golaszewski
Update the img,powervr-rogue.yaml to include the T-HEAD TH1520 SoC's
specific GPU compatible string.
The thead,th1520-gpu compatible, along with its full chain
img,img-bxm-4-64, and img,img-rogue, is added to the
list of recognized GPU types.
While the BXM-4-64 GPU IP is designed with two distinct power domains,
the TH1520 SoC integrates it with only a single, unified power gate that
is controllable by the kernel.
To model this reality correctly while keeping the binding accurate for
other devices, add conditional constraints to the `allOf` section:
- If the compatible is "thead,th1520-gpu", enforce a maximum of one
power domain.
- For other "img,img-bxm-4-64" and "img,img-bxs-4-64" devices,
enforce a minimum of two power domains, using a 'not:' clause to
exclude the TH1520.
This makes the binding strict and correct for both the specific TH1520
implementation and for other SoCs that use the B-series Rogue GPUs.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
.../devicetree/bindings/gpu/img,powervr-rogue.yaml | 26 +++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 4450e2e73b3ccf74d29f0e31e2e6687d7cbe5d65..263c40c8438e26efcf1613b5e2af54f2d6599871 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
@@ -89,6 +94,18 @@ allOf:
- power-domains
- power-domain-names
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: thead,th1520-gpu
+ then:
+ properties:
+ power-domains:
+ maxItems: 1
+ power-domain-names:
+ maxItems: 1
+
- if:
properties:
compatible:
@@ -105,7 +122,14 @@ allOf:
properties:
compatible:
contains:
- const: img,img-bxs-4-64
+ enum:
+ - img,img-bxs-4-64
+ - img,img-bxm-4-64
+ not:
+ properties:
+ compatible:
+ contains:
+ const: thead,th1520-gpu
then:
properties:
power-domains:
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 3/5] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node
[not found] ` <CGME20250626093359eucas1p20a737f31b582ef0a2d54082eb172586e@eucas1p2.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
2025-06-30 20:52 ` Drew Fustini
0 siblings, 1 reply; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
Add the "gpu-clkgen" reset property to the AON device tree node. This
allows the AON power domain driver to detect the capability to power
sequence the GPU and spawn the necessary pwrseq-thead-gpu auxiliary
driver for managing the GPU's complex power sequence.
This commit also adds the prerequisite
dt-bindings/reset/thead,th1520-reset.h include to make the
TH1520_RESET_ID_GPU_CLKGEN available. This include was previously
dropped during a conflict resolution [1].
Link: https://lore.kernel.org/all/aAvfn2mq0Ksi8DF2@x1/ [1]
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Drew Fustini <drew@pdp7.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
arch/riscv/boot/dts/thead/th1520.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 1db0054c4e093400e9dbebcee5fcfa5b5cae6e32..f3f5db0201ab8c0306d4d63072a1573431e51893 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -7,6 +7,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/clock/thead,th1520-clk-ap.h>
#include <dt-bindings/power/thead,th1520-power.h>
+#include <dt-bindings/reset/thead,th1520-reset.h>
/ {
compatible = "thead,th1520";
@@ -234,6 +235,8 @@ aon: aon {
compatible = "thead,th1520-aon";
mboxes = <&mbox_910t 1>;
mbox-names = "aon";
+ resets = <&rst TH1520_RESET_ID_GPU_CLKGEN>;
+ reset-names = "gpu-clkgen";
#power-domain-cells = <1>;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 4/5] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node
[not found] ` <CGME20250626093400eucas1p1b50fe1f64ac3f8c2d033740ef8f22424@eucas1p1.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
0 siblings, 0 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
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>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
arch/riscv/boot/dts/thead/th1520.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index f3f5db0201ab8c0306d4d63072a1573431e51893..c8447eef36c3a6e92d768658b6b19dfeb59a47c4 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,21 @@ 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>;
+ power-domain-names = "a";
+ resets = <&rst TH1520_RESET_ID_GPU>;
+ };
+
rst: reset-controller@ffef528000 {
compatible = "thead,th1520-reset";
reg = <0xff 0xef528000 0x0 0x4f>;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 5/5] drm/imagination: Enable PowerVR driver for RISC-V
[not found] ` <CGME20250626093401eucas1p147c296efa1b9d8b747b04abb0b5536b7@eucas1p1.samsung.com>
@ 2025-06-26 9:33 ` Michal Wilczynski
0 siblings, 0 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-06-26 9:33 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michal Wilczynski, Bartosz Golaszewski, Philipp Zabel,
Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Ulf Hansson,
Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
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>
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v7 3/5] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node
2025-06-26 9:33 ` [PATCH v7 3/5] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node Michal Wilczynski
@ 2025-06-30 20:52 ` Drew Fustini
0 siblings, 0 replies; 10+ messages in thread
From: Drew Fustini @ 2025-06-30 20:52 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, linux-riscv,
devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
On Thu, Jun 26, 2025 at 11:33:48AM +0200, Michal Wilczynski wrote:
> Add the "gpu-clkgen" reset property to the AON device tree node. This
> allows the AON power domain driver to detect the capability to power
> sequence the GPU and spawn the necessary pwrseq-thead-gpu auxiliary
> driver for managing the GPU's complex power sequence.
>
> This commit also adds the prerequisite
> dt-bindings/reset/thead,th1520-reset.h include to make the
> TH1520_RESET_ID_GPU_CLKGEN available. This include was previously
> dropped during a conflict resolution [1].
>
> Link: https://lore.kernel.org/all/aAvfn2mq0Ksi8DF2@x1/ [1]
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Reviewed-by: Drew Fustini <drew@pdp7.com>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
> arch/riscv/boot/dts/thead/th1520.dtsi | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index 1db0054c4e093400e9dbebcee5fcfa5b5cae6e32..f3f5db0201ab8c0306d4d63072a1573431e51893 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -7,6 +7,7 @@
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/clock/thead,th1520-clk-ap.h>
> #include <dt-bindings/power/thead,th1520-power.h>
> +#include <dt-bindings/reset/thead,th1520-reset.h>
>
> / {
> compatible = "thead,th1520";
> @@ -234,6 +235,8 @@ aon: aon {
> compatible = "thead,th1520-aon";
> mboxes = <&mbox_910t 1>;
> mbox-names = "aon";
> + resets = <&rst TH1520_RESET_ID_GPU_CLKGEN>;
> + reset-names = "gpu-clkgen";
> #power-domain-cells = <1>;
> };
>
>
> --
> 2.34.1
>
I have applied this patch to thead-dt-for-next [1] as commit cf5e81d [2].
Thanks,
Drew
[1] https://github.com/pdp7/linux/commits/thead-dt-for-next/
[2] https://github.com/pdp7/linux/commit/cf5e81da0ed7f548367a9687ad73956a8dfb54d1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management
2025-06-26 9:33 ` [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management Michal Wilczynski
@ 2025-07-03 10:21 ` Michal Wilczynski
2025-07-14 11:07 ` Michal Wilczynski
2025-07-23 10:38 ` Matt Coster
1 sibling, 1 reply; 10+ messages in thread
From: Michal Wilczynski @ 2025-07-03 10:21 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
On 6/26/25 11:33, 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 a `pwr_power_sequence_ops` struct containing
> function pointers for power_on and power_off operations. 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 inspects the assigned ops struct. If the
> pwrseq variant is detected, the driver calls
> devm_pwrseq_get("gpu-power"), deferring probe if the sequencer is not
> yet available. Otherwise, it falls back to the existing manual clock and
> reset handling. The runtime PM callbacks now call the appropriate
> functions via the ops table.
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
> drivers/gpu/drm/imagination/pvr_device.c | 36 +++++++-
> drivers/gpu/drm/imagination/pvr_device.h | 17 ++++
> drivers/gpu/drm/imagination/pvr_drv.c | 27 +++++-
> drivers/gpu/drm/imagination/pvr_power.c | 139 ++++++++++++++++++++++---------
> drivers/gpu/drm/imagination/pvr_power.h | 13 +++
> 5 files changed, 185 insertions(+), 47 deletions(-)
>
Hi,
I'm checking in on the status of my pwrseq patch above. Is this on track
for the next merge window?
Please let me know if there's anything else needed from my end to help
get it ready.
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management
2025-07-03 10:21 ` Michal Wilczynski
@ 2025-07-14 11:07 ` Michal Wilczynski
0 siblings, 0 replies; 10+ messages in thread
From: Michal Wilczynski @ 2025-07-14 11:07 UTC (permalink / raw)
To: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, Philipp Zabel, Frank Binns, Matt Coster,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, Drew Fustini
Cc: linux-riscv, devicetree, linux-kernel, linux-pm, dri-devel,
Bartosz Golaszewski
On 7/3/25 12:21, Michal Wilczynski wrote:
>
>
> On 6/26/25 11:33, 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 a `pwr_power_sequence_ops` struct containing
>> function pointers for power_on and power_off operations. 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 inspects the assigned ops struct. If the
>> pwrseq variant is detected, the driver calls
>> devm_pwrseq_get("gpu-power"), deferring probe if the sequencer is not
>> yet available. Otherwise, it falls back to the existing manual clock and
>> reset handling. The runtime PM callbacks now call the appropriate
>> functions via the ops table.
>>
>> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>> drivers/gpu/drm/imagination/pvr_device.c | 36 +++++++-
>> drivers/gpu/drm/imagination/pvr_device.h | 17 ++++
>> drivers/gpu/drm/imagination/pvr_drv.c | 27 +++++-
>> drivers/gpu/drm/imagination/pvr_power.c | 139 ++++++++++++++++++++++---------
>> drivers/gpu/drm/imagination/pvr_power.h | 13 +++
>> 5 files changed, 185 insertions(+), 47 deletions(-)
>>
>
> Hi,
>
> I'm checking in on the status of my pwrseq patch above. Is this on track
> for the next merge window?
>
> Please let me know if there's anything else needed from my end to help
> get it ready.
>
> Best regards,
Hi Matt,
I was very happy to see the recent "pvr: various enablement changes" get
merged in Mesa [1]. Congratulations to the team on that progress.
I just wanted to check in and see if you have any more requests for this
series ?
[1] - https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33998
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management
2025-06-26 9:33 ` [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management Michal Wilczynski
2025-07-03 10:21 ` Michal Wilczynski
@ 2025-07-23 10:38 ` Matt Coster
1 sibling, 0 replies; 10+ messages in thread
From: Matt Coster @ 2025-07-23 10:38 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Guo Ren, Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, Philipp Zabel, Frank Binns,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Ulf Hansson, Marek Szyprowski, Drew Fustini,
Bartosz Golaszewski, linux-riscv@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org, dri-devel@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 15057 bytes --]
On 26/06/2025 10:33, 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 a `pwr_power_sequence_ops` struct containing
> function pointers for power_on and power_off operations. 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 inspects the assigned ops struct. If the
> pwrseq variant is detected, the driver calls
> devm_pwrseq_get("gpu-power"), deferring probe if the sequencer is not
> yet available. Otherwise, it falls back to the existing manual clock and
> reset handling. The runtime PM callbacks now call the appropriate
> functions via the ops table.
Hi Michal,
I've replied again on the v6 series regarding the power domain
situation[1], it took me a while to figure out internally what's going on
in this SoC integration but I hope we have a somewhat satisfying answer
now. As for v7, just a couple notes from me on this patch, plus whatever
changes are needed to solve the power domains once and for all. The rest
of the series otherwise looks good to me.
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
> drivers/gpu/drm/imagination/pvr_device.c | 36 +++++++-
> drivers/gpu/drm/imagination/pvr_device.h | 17 ++++
> drivers/gpu/drm/imagination/pvr_drv.c | 27 +++++-
> drivers/gpu/drm/imagination/pvr_power.c | 139 ++++++++++++++++++++++---------
> drivers/gpu/drm/imagination/pvr_power.h | 13 +++
> 5 files changed, 185 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
> index 8b9ba4983c4cb5bc40342fcafc4259078bc70547..770fc32a6fe485aad41cd92fa1431dd233ac20dc 100644
> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -23,8 +23,12 @@
> #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>
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> +#include <linux/pwrseq/consumer.h>
> +#endif
> #include <linux/reset.h>
> #include <linux/slab.h>
> #include <linux/stddef.h>
> @@ -618,6 +622,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,10 +638,31 @@ 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);
> - if (err)
> - return err;
> + /*
> + * For platforms that require it, get the power sequencer.
> + * For all others, perform manual reset initialization.
> + */
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> + if (pvr_dev->device_data->pwr_ops == &pvr_power_sequence_ops_pwrseq) {
> + 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");
> + }
> + } else
> +#endif
> + {
> + /* This platform does not use a sequencer, init reset manually. */
> + err = pvr_device_reset_init(pvr_dev);
> + if (err)
> + return err;
> + }
Can you extract this whole conditional into an ->init() callback on
struct pvr_power_sequence_ops? That would eliminate the #if conditional
code from this file entirely since you'd no longer need
<linux/pwrseq/consumer.h> at all.
>
> /* Explicitly power the GPU so we can access control registers before the FW is booted. */
> err = pm_runtime_resume_and_get(dev);
> diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
> index 7cb01c38d2a9c3fc71effe789d4dfe54eddd93ee..0d7f7c78573a0766a467fb0c3a577ffe152d0892 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 */
Missing '>'.
> +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;
> +
Can you add a note to this doc comment explaining that
CONFIG_POWER_SEQUENCING is not a dependency of this driver, and that
this member should only be accessed behind an IS_ENABLED() check?
> /** @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..af830e565646daf19555197df492438ef48d5e44 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1480,15 +1480,37 @@ 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,
> +};
> +
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> +static const struct pvr_device_data pvr_device_data_pwrseq = {
> + .pwr_ops = &pvr_power_sequence_ops_pwrseq,
> +};
> +#endif
> +
> static const struct of_device_id dt_match[] = {
> - { .compatible = "img,img-rogue", .data = NULL },
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> + {
> + .compatible = "thead,th1520-gpu",
> + .data = &pvr_device_data_pwrseq,
> + },
> +#endif
> + {
> + .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 +1535,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 41f5d89e78b854cf6993838868a4416a220b490a..13aef27849d1a71df77406c8d7845836998a35a0 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -18,6 +18,9 @@
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> +#include <linux/pwrseq/consumer.h>
> +#endif
> #include <linux/reset.h>
> #include <linux/timer.h>
> #include <linux/types.h>
> @@ -234,6 +237,96 @@ pvr_watchdog_init(struct pvr_device *pvr_dev)
> 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 = {
> + .power_on = pvr_power_on_sequence_manual,
> + .power_off = pvr_power_off_sequence_manual,
> +};
> +
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCING)
> +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 = {
> + .power_on = pvr_power_on_sequence_pwrseq,
> + .power_off = pvr_power_off_sequence_pwrseq,
> +};
> +#else /* IS_ENABLED(CONFIG_POWER_SEQUENCING) */
> +static int pvr_power_sequence_pwrseq_stub(struct pvr_device *pvr_dev)
> +{
> + WARN_ONCE(1, "pwrseq support not enabled in kernel config\n");
> + return -EOPNOTSUPP;
> +}
> +
> +const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq = {
> + .power_on = pvr_power_sequence_pwrseq_stub,
> + .power_off = pvr_power_sequence_pwrseq_stub,
> +};
> +#endif /* IS_ENABLED(CONFIG_POWER_SEQUENCING) */
> +
> int
> pvr_power_device_suspend(struct device *dev)
> {
> @@ -252,11 +345,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,54 +365,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);
Nit: can you put the blank line back here please?
Cheers,
Matt
[1]: https://lore.kernel.org/r/f25c1e7f-bef2-47b1-8fa8-14c9c51087a8@imgtec.com
> 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..6a2f3f6213e5ac2254344ad24d9678334c8974ea 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.h
> +++ b/drivers/gpu/drm/imagination/pvr_power.h
> @@ -41,4 +41,17 @@ 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.
> + * @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 (*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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-07-23 10:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250626093355eucas1p12c6dc13a987fd498906fb8846b4722c7@eucas1p1.samsung.com>
2025-06-26 9:33 ` [PATCH v7 0/5] Add TH1520 GPU support with power sequencing Michal Wilczynski
[not found] ` <CGME20250626093356eucas1p1adfcd565173d939f82e15252189c316f@eucas1p1.samsung.com>
2025-06-26 9:33 ` [PATCH v7 1/5] drm/imagination: Use pwrseq for TH1520 GPU power management Michal Wilczynski
2025-07-03 10:21 ` Michal Wilczynski
2025-07-14 11:07 ` Michal Wilczynski
2025-07-23 10:38 ` Matt Coster
[not found] ` <CGME20250626093358eucas1p1e5d763dc86dccb0ed9fa725182ea59e6@eucas1p1.samsung.com>
2025-06-26 9:33 ` [PATCH v7 2/5] dt-bindings: gpu: img,powervr-rogue: Add TH1520 GPU compatible Michal Wilczynski
[not found] ` <CGME20250626093359eucas1p20a737f31b582ef0a2d54082eb172586e@eucas1p2.samsung.com>
2025-06-26 9:33 ` [PATCH v7 3/5] riscv: dts: thead: th1520: Add GPU clkgen reset to AON node Michal Wilczynski
2025-06-30 20:52 ` Drew Fustini
[not found] ` <CGME20250626093400eucas1p1b50fe1f64ac3f8c2d033740ef8f22424@eucas1p1.samsung.com>
2025-06-26 9:33 ` [PATCH v7 4/5] riscv: dts: thead: th1520: Add IMG BXM-4-64 GPU node Michal Wilczynski
[not found] ` <CGME20250626093401eucas1p147c296efa1b9d8b747b04abb0b5536b7@eucas1p1.samsung.com>
2025-06-26 9:33 ` [PATCH v7 5/5] drm/imagination: Enable PowerVR driver for RISC-V Michal Wilczynski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).