[PATCH v5 0/7] Power Management for Raspberry Pi V3D GPU

[PATCH v5 0/7] Power Management for Raspberry Pi V3D GPU

[Maíra Canal]

This series introduces Runtime Power Management (PM) support for the
Raspberry Pi V3D GPU.

Currently, the V3D clock remains enabled for the entire system uptime,
even when the GPU is idle. With the introduction of Runtime PM, the
clock can now be disabled during idle periods. For example, with this
series applied on a Raspberry Pi 5, if we check `vcgencmd measure_clock
v3d`, we get:

(idle)

$ vcgencmd measure_clock v3d
frequency(0)=0

(running glmark2)

$ vcgencmd measure_clock v3d
frequency(0)=960016128

To ease testing in Raspberry Pi 4 and 5, I prepared a downstream branch
backporting this series to rpi-6.18.y [1].

[1] https://github.com/mairacanal/linux-rpi/tree/v3d/downstream/power-management-v5

Best regards,
- Maíra

---
v1 -> v2: https://lore.kernel.org/r/20250728-v3d-power-management-v1-0-780f922b1048@igalia.com

- [1/5] NEW PATCH: "clk: bcm: rpi: Add missing logs if firmware fails" (Stefan Wahren)
- [2/5] Remove the "Fixes:" tag (Stefan Wahren)
- [2/5] dev_err_ratelimited() instead of dev_err() (Stefan Wahren)
- [2/5] Instead of logging the clock ID, use clk_hw_get_name(hw) to log the name (Stefan Wahren)
- [2/5] Add a newline character at the end of the log message (Stefan Wahren)
- [2/5] Use CLK_IS_CRITICAL for all clocks that can't be disabled (Maxime Ripard)
- [3/5] NEW PATCH: "clk: bcm: rpi: Maximize V3D clock"
- [4/5] Use devm_reset_control_get_optional_exclusive() (Philipp Zabel)
- [4/5] Make sure that resource are cleaned in the inverse order of allocation (Philipp Zabel)

v2 -> v3: https://lore.kernel.org/r/20250731-v3d-power-management-v2-0-032d56b01964@igalia.com

- Rebased on top of drm-misc-next
- Patches "[PATCH v2 1/5] clk: bcm: rpi: Add missing logs if firmware
  fails", "[PATCH v2 2/5] clk: bcm: rpi: Turn firmware clock on/off when
  preparing/unpreparing", and "[PATCH v2 3/5] clk: bcm: rpi: Maximize
  V3D clock" were applied to clk-next.
- [1/4] NEW PATCH: "clk: bcm: rpi: Let V3D consumers manage clock rate"
- [2/4] NEW PATCH: "clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED"
- [3/4] Added Philipp's R-b (Philipp Zabel)
- [4/4] s/DRM_ERROR/drm_err
- [4/4] Set the clock rate to 0 during suspend and to the maximum rate
  during resume

v3 -> v4: https://lore.kernel.org/r/20260116-v3d-power-management-v3-0-4e1874e81dd6@igalia.com

- Rebased on top of drm-misc-next
- [1/6, 3/6] Add Melissa's A-b (Melissa Wen)
- [2/6] NEW PATCH: "clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE
  behavior" (Stefan Wahren)
- [4/6] NEW PATCH: "drm/v3d: Use devm_reset_control_get_optional_exclusive()" (Melissa Wen)
- [5/6] Include more context to the commit message (Melissa Wen)
- [5/6, 6/6] Instead of creating the function v3d_gem_allocate(), use v3d_gem_init()
  and move HW initialization out of it (Melissa Wen)

v4 -> v5: https://lore.kernel.org/r/20260126-v3d-power-management-v4-0-caf2df16d4e2@igalia.com

- [2/7] Add Stefan's A-b (Stefan Wahren)
- [2/7, 5/7, 6/7] Add Melissa's R-b (Melissa Wen)
- [4/7] NEW PATCH: "pmdomain: bcm: bcm2835-power: Increase ASB control timeout"
- [7/7] Remove redundant pm_runtime_mark_last_busy() from v3d_pm_runtime_put()
- [7/7] Use pm_runtime_get_if_active() in v3d_mmu_flush_all() instead of
  pm_runtime_get_noresume() + pm_runtime_active()
- [7/7] Add missing PM runtime calls to v3d_perfmon_start() and v3d_perfmon_stop()

---
Maíra Canal (7):
      clk: bcm: rpi: Let V3D consumers manage clock rate
      clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE behavior
      clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED
      pmdomain: bcm: bcm2835-power: Increase ASB control timeout
      drm/v3d: Use devm_reset_control_get_optional_exclusive()
      drm/v3d: Allocate all resources before enabling the clock
      drm/v3d: Introduce Runtime Power Management

 drivers/clk/bcm/clk-raspberrypi.c    |  15 ++--
 drivers/gpu/drm/v3d/Makefile         |   3 +-
 drivers/gpu/drm/v3d/v3d_debugfs.c    |  23 ++++-
 drivers/gpu/drm/v3d/v3d_drv.c        | 163 +++++++++++++++++------------------
 drivers/gpu/drm/v3d/v3d_drv.h        |  20 +++++
 drivers/gpu/drm/v3d/v3d_gem.c        |  25 +++---
 drivers/gpu/drm/v3d/v3d_irq.c        |  15 ++--
 drivers/gpu/drm/v3d/v3d_mmu.c        |  10 ++-
 drivers/gpu/drm/v3d/v3d_perfmon.c    |  18 +++-
 drivers/gpu/drm/v3d/v3d_power.c      |  96 +++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_submit.c     |  19 +++-
 drivers/pmdomain/bcm/bcm2835-power.c |   5 +-
 12 files changed, 286 insertions(+), 126 deletions(-)
---
base-commit: df4dc947c46bb9f80038f52c6e38cb2d40c10e50
change-id: 20250728-v3d-power-management-eebb2024dc96

[PATCH v5 1/7] clk: bcm: rpi: Let V3D consumers manage clock rate

[Maíra Canal]

Remove the `maximize` infrastructure and don't set `minimize` for the
V3D clock, giving consumers full control over its rate.

On some firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
actually power off the clock. To achieve meaningful power consumption
reduction, consumers need to set the clock rate to minimum before
disabling it. Forcing the clock to maximum rate in the clock framework
prevents this, as consumers don't have any flexibility over the clock
rate.

This change enables the v3d driver to control the clock rate directly
in its suspend/resume callbacks.

Fixes: 6526402b9bac ("clk: bcm: rpi: Maximize V3D clock")
Acked-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 1a9162f0ae31e330c46f6eafdd00350599b0eede..9783385d5859836898683209e320fcc928dfdc71 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -68,7 +68,6 @@ struct raspberrypi_clk_variant {
 	char		*clkdev;
 	unsigned long	min_rate;
 	bool		minimize;
-	bool		maximize;
 	u32		flags;
 };
 
@@ -136,7 +135,6 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
 	},
 	[RPI_FIRMWARE_V3D_CLK_ID] = {
 		.export = true,
-		.maximize = true,
 	},
 	[RPI_FIRMWARE_PIXEL_CLK_ID] = {
 		.export = true,
@@ -387,9 +385,6 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
 		}
 	}
 
-	if (variant->maximize)
-		variant->min_rate = max_rate;
-
 	if (variant->min_rate) {
 		unsigned long rate;
 

-- 
2.52.0

[PATCH v5 2/7] clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE behavior

[Maíra Canal]

On some firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
actually power off the clock. Document this behavior so that clock
consumers understand they may need to set the rate to minimum before
disabling for meaningful power savings.

Suggested-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 9783385d5859836898683209e320fcc928dfdc71..2e63121af0900ac9b4874b666cf6d02baa8be11c 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -308,6 +308,12 @@ static void raspberrypi_fw_unprepare(struct clk_hw *hw)
 	u32 state = 0;
 	int ret;
 
+	/*
+	 * On some firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
+	 * actually power off the clock. To achieve meaningful power consumption
+	 * reduction, consumers need to set the clock rate to minimum before
+	 * disabling it.
+	 */
 	ret = raspberrypi_clock_property(rpi->firmware, data,
 					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
 	if (ret)

-- 
2.52.0

[PATCH v5 3/7] clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED

[Maíra Canal]

If PIXEL_CLK or HEVC_CLK is disabled during boot, the firmware will skip
HSM initialization, which would result in a bus lockup. However, those
clocks are consumed by drivers (vc4 and HEVC decoder drivers,
respectively), which means that they can be enabled/disabled by the
drivers.

Mark those clocks as CLK_IGNORE_UNUSED to allow them to be disabled by
drivers when appropriate.

Acked-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 2e63121af0900ac9b4874b666cf6d02baa8be11c..b9f2a4dc1510ff162c84d67aa0ddd7f435156398 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -139,12 +139,12 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
 	[RPI_FIRMWARE_PIXEL_CLK_ID] = {
 		.export = true,
 		.minimize = true,
-		.flags = CLK_IS_CRITICAL,
+		.flags = CLK_IGNORE_UNUSED,
 	},
 	[RPI_FIRMWARE_HEVC_CLK_ID] = {
 		.export = true,
 		.minimize = true,
-		.flags = CLK_IS_CRITICAL,
+		.flags = CLK_IGNORE_UNUSED,
 	},
 	[RPI_FIRMWARE_ISP_CLK_ID] = {
 		.export = true,

-- 
2.52.0

[PATCH v5 4/7] pmdomain: bcm: bcm2835-power: Increase ASB control timeout

[Maíra Canal]

The bcm2835_asb_control() function uses a tight polling loop to wait
for the ASB bridge to acknowledge a request. During intensive workloads,
this handshake intermittently fails for V3D's master ASB on BCM2711,
resulting in "Failed to disable ASB master for v3d" errors during
runtime PM suspend. As a consequence, the failed power-off leaves V3D in
a broken state, leading to bus faults or system hangs on later accesses.

As the timeout is insufficient in some scenarios, increase the polling
timeout from 1us to 5us, which is still negligible in the context of a
power domain transition. Also, move the start timestamp to after the
MMIO write, as the write latency is counted against the timeout,
reducing the effective wait time for the hardware to respond.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/pmdomain/bcm/bcm2835-power.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pmdomain/bcm/bcm2835-power.c b/drivers/pmdomain/bcm/bcm2835-power.c
index 1d29addfe036348e82293693b4059e504bb25575..7b9eea10a24e26835deeca84c60ccb616b99a508 100644
--- a/drivers/pmdomain/bcm/bcm2835-power.c
+++ b/drivers/pmdomain/bcm/bcm2835-power.c
@@ -166,8 +166,6 @@ static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable
 		break;
 	}
 
-	start = ktime_get_ns();
-
 	/* Enable the module's async AXI bridges. */
 	if (enable) {
 		val = readl(base + reg) & ~ASB_REQ_STOP;
@@ -176,9 +174,10 @@ static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable
 	}
 	writel(PM_PASSWORD | val, base + reg);
 
+	start = ktime_get_ns();
 	while (!!(readl(base + reg) & ASB_ACK) == enable) {
 		cpu_relax();
-		if (ktime_get_ns() - start >= 1000)
+		if (ktime_get_ns() - start >= 5000)
 			return -ETIMEDOUT;
 	}
 

-- 
2.52.0

[PATCH v5 5/7] drm/v3d: Use devm_reset_control_get_optional_exclusive()

[Maíra Canal]

Simplify optional reset handling by using the function
devm_reset_control_get_optional_exclusive().

Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 8de4f151a5c02cbf970e72933d1a275968088357..257f2fefbdb6f8736411de8965919f1728844a6a 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -398,18 +398,17 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 
 	v3d_perfmon_init(v3d);
 
-	v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
 	if (IS_ERR(v3d->reset)) {
-		ret = PTR_ERR(v3d->reset);
+		ret = dev_err_probe(dev, PTR_ERR(v3d->reset),
+				    "Failed to get reset control\n");
+		goto clk_disable;
+	}
 
-		if (ret == -EPROBE_DEFER)
-			goto clk_disable;
-
-		v3d->reset = NULL;
+	if (!v3d->reset) {
 		ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
 		if (ret) {
-			dev_err(dev,
-				"Failed to get reset control or bridge regs\n");
+			dev_err(dev, "Failed to get bridge registers\n");
 			goto clk_disable;
 		}
 	}

-- 
2.52.0

[PATCH v5 6/7] drm/v3d: Allocate all resources before enabling the clock

[Maíra Canal]

Move all resource allocation operations before actually enabling the
clock, as those operations don't require the GPU to be powered on.

This is a preparation for runtime PM support. The next commit will
move all code related to powering on and initiating the GPU into the
runtime PM resume callback and all resource allocation will happen
before resume().

Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c | 93 ++++++++++++++++++++++---------------------
 drivers/gpu/drm/v3d/v3d_drv.h |  1 +
 drivers/gpu/drm/v3d/v3d_gem.c | 18 ++++-----
 drivers/gpu/drm/v3d/v3d_irq.c | 15 +++----
 4 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 257f2fefbdb6f8736411de8965919f1728844a6a..592ef610d6a4bb7dfe64acf6f7283c947e9d2921 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -360,14 +360,50 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (v3d->ver < V3D_GEN_41) {
+		ret = map_regs(v3d, &v3d->gca_regs, "gca");
+		if (ret)
+			return ret;
+	}
+
+	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(v3d->reset))
+		return dev_err_probe(dev, PTR_ERR(v3d->reset),
+				     "Failed to get reset control\n");
+
+	if (!v3d->reset) {
+		ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
+		if (ret) {
+			dev_err(dev, "Failed to get bridge registers\n");
+			return ret;
+		}
+	}
+
 	v3d->clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(v3d->clk))
 		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
 
+	ret = v3d_irq_init(v3d);
+	if (ret)
+		return ret;
+
+	v3d_perfmon_init(v3d);
+
+	v3d->mmu_scratch = dma_alloc_wc(dev, 4096, &v3d->mmu_scratch_paddr,
+					GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
+	if (!v3d->mmu_scratch) {
+		dev_err(dev, "Failed to allocate MMU scratch page\n");
+		return -ENOMEM;
+	}
+
+	ret = v3d_gem_init(drm);
+	if (ret)
+		goto dma_free;
+
 	ret = clk_prepare_enable(v3d->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "Couldn't enable the V3D clock\n");
-		return ret;
+		goto gem_destroy;
 	}
 
 	v3d_idle_sms(v3d);
@@ -396,44 +432,9 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	ident3 = V3D_READ(V3D_HUB_IDENT3);
 	v3d->rev = V3D_GET_FIELD(ident3, V3D_HUB_IDENT3_IPREV);
 
-	v3d_perfmon_init(v3d);
-
-	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
-	if (IS_ERR(v3d->reset)) {
-		ret = dev_err_probe(dev, PTR_ERR(v3d->reset),
-				    "Failed to get reset control\n");
-		goto clk_disable;
-	}
-
-	if (!v3d->reset) {
-		ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
-		if (ret) {
-			dev_err(dev, "Failed to get bridge registers\n");
-			goto clk_disable;
-		}
-	}
-
-	if (v3d->ver < V3D_GEN_41) {
-		ret = map_regs(v3d, &v3d->gca_regs, "gca");
-		if (ret)
-			goto clk_disable;
-	}
-
-	v3d->mmu_scratch = dma_alloc_wc(dev, 4096, &v3d->mmu_scratch_paddr,
-					GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
-	if (!v3d->mmu_scratch) {
-		dev_err(dev, "Failed to allocate MMU scratch page\n");
-		ret = -ENOMEM;
-		goto clk_disable;
-	}
-
-	ret = v3d_gem_init(drm);
-	if (ret)
-		goto dma_free;
-
-	ret = v3d_irq_init(v3d);
-	if (ret)
-		goto gem_destroy;
+	v3d_init_hw_state(v3d);
+	v3d_mmu_set_page_table(v3d);
+	v3d_irq_enable(v3d);
 
 	ret = drm_dev_register(drm, 0);
 	if (ret)
@@ -449,12 +450,13 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	drm_dev_unregister(drm);
 irq_disable:
 	v3d_irq_disable(v3d);
+clk_disable:
+	v3d_power_off_sms(v3d);
+	clk_disable_unprepare(v3d->clk);
 gem_destroy:
 	v3d_gem_destroy(drm);
 dma_free:
 	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
-clk_disable:
-	clk_disable_unprepare(v3d->clk);
 	return ret;
 }
 
@@ -468,14 +470,13 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(drm);
 
-	v3d_gem_destroy(drm);
-
-	dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch,
-		    v3d->mmu_scratch_paddr);
-
 	v3d_power_off_sms(v3d);
 
 	clk_disable_unprepare(v3d->clk);
+
+	v3d_gem_destroy(drm);
+
+	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
 }
 
 static struct platform_driver v3d_platform_driver = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 314213c2671003862c486a1a7237af5480afa9e4..ff90ef6876d65241975f259b44c6f09941d12ecb 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -562,6 +562,7 @@ struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum v3d_queue q);
 
 /* v3d_gem.c */
 extern bool super_pages;
+void v3d_init_hw_state(struct v3d_dev *v3d);
 int v3d_gem_init(struct drm_device *dev);
 void v3d_gem_destroy(struct drm_device *dev);
 void v3d_reset_sms(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 57965c0d6f6efea0019fb0b1a47addf2f586d138..dd4ac899a489fb7341815592114cc4f1652f35e8 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -36,13 +36,6 @@ v3d_init_core(struct v3d_dev *v3d, int core)
 	V3D_CORE_WRITE(core, V3D_CTL_L2TFLEND, ~0);
 }
 
-/* Sets invariant state for the HW. */
-static void
-v3d_init_hw_state(struct v3d_dev *v3d)
-{
-	v3d_init_core(v3d, 0);
-}
-
 static void
 v3d_idle_axi(struct v3d_dev *v3d, int core)
 {
@@ -259,6 +252,14 @@ v3d_invalidate_caches(struct v3d_dev *v3d)
 	v3d_invalidate_slices(v3d, 0);
 }
 
+/* Sets invariant state for the HW. */
+void
+v3d_init_hw_state(struct v3d_dev *v3d)
+{
+	v3d_init_core(v3d, 0);
+}
+
+
 static void
 v3d_huge_mnt_init(struct v3d_dev *v3d)
 {
@@ -325,9 +326,6 @@ v3d_gem_init(struct drm_device *dev)
 		return -ENOMEM;
 	}
 
-	v3d_init_hw_state(v3d);
-	v3d_mmu_set_page_table(v3d);
-
 	v3d_huge_mnt_init(v3d);
 
 	ret = v3d_sched_init(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 1249f6e64b979fe29cf2b9bfc43b39aa755f71ce..e33cdede7c746ef1e1d6b2a7922f4ced35164649 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -248,17 +248,10 @@ v3d_hub_irq(int irq, void *arg)
 int
 v3d_irq_init(struct v3d_dev *v3d)
 {
-	int irq, ret, core;
+	int irq, ret;
 
 	INIT_WORK(&v3d->overflow_mem_work, v3d_overflow_mem_work);
 
-	/* Clear any pending interrupts someone might have left around
-	 * for us.
-	 */
-	for (core = 0; core < v3d->cores; core++)
-		V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
-	V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));
-
 	irq = platform_get_irq_optional(v3d_to_pdev(v3d), 1);
 	if (irq == -EPROBE_DEFER)
 		return irq;
@@ -296,7 +289,6 @@ v3d_irq_init(struct v3d_dev *v3d)
 			goto fail;
 	}
 
-	v3d_irq_enable(v3d);
 	return 0;
 
 fail:
@@ -310,6 +302,11 @@ v3d_irq_enable(struct v3d_dev *v3d)
 {
 	int core;
 
+	/* Clear any pending interrupts someone might have left around for us. */
+	for (core = 0; core < v3d->cores; core++)
+		V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
+	V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));
+
 	/* Enable our set of interrupts, masking out any others. */
 	for (core = 0; core < v3d->cores; core++) {
 		V3D_CORE_WRITE(core, V3D_CTL_INT_MSK_SET, ~V3D_CORE_IRQS(v3d->ver));

-- 
2.52.0

[PATCH v5 7/7] drm/v3d: Introduce Runtime Power Management

[Maíra Canal]

Commit 90a64adb0876 ("drm/v3d: Get rid of pm code") removed the last
bits of power management code that V3D had, which were actually never
hooked. Therefore, currently, the GPU clock is enabled during probe
and only disabled when removing the driver.

Implement proper power management using the kernel's Runtime PM
framework.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/Makefile      |  3 +-
 drivers/gpu/drm/v3d/v3d_debugfs.c | 23 +++++++++-
 drivers/gpu/drm/v3d/v3d_drv.c     | 87 +++++++++++++++++------------------
 drivers/gpu/drm/v3d/v3d_drv.h     | 19 ++++++++
 drivers/gpu/drm/v3d/v3d_gem.c     |  7 +++
 drivers/gpu/drm/v3d/v3d_mmu.c     | 10 +++-
 drivers/gpu/drm/v3d/v3d_perfmon.c | 18 ++++++--
 drivers/gpu/drm/v3d/v3d_power.c   | 96 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_submit.c  | 19 ++++++--
 9 files changed, 223 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/v3d/Makefile b/drivers/gpu/drm/v3d/Makefile
index b7d673f1153bef16db3800e50b2bfaf36bf8871b..9cb1fd4e4091dbb56e6a73e2b8a51fa0d242698b 100644
--- a/drivers/gpu/drm/v3d/Makefile
+++ b/drivers/gpu/drm/v3d/Makefile
@@ -13,7 +13,8 @@ v3d-y := \
 	v3d_trace_points.o \
 	v3d_sched.o \
 	v3d_sysfs.o \
-	v3d_submit.o
+	v3d_submit.o \
+	v3d_power.o
 
 v3d-$(CONFIG_DEBUG_FS) += v3d_debugfs.o
 
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 89f24eec62a74ec49b28f0b22dbf626ba7a35206..634cc796ba2324dc497694c070f2cfffcc4424c9 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -97,7 +97,11 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_debugfs_entry *entry = m->private;
 	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
-	int i, core;
+	int i, core, ret;
+
+	ret = v3d_pm_runtime_get(v3d);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < ARRAY_SIZE(v3d_hub_reg_defs); i++) {
 		const struct v3d_reg_def *def = &v3d_hub_reg_defs[i];
@@ -139,6 +143,8 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 		}
 	}
 
+	v3d_pm_runtime_put(v3d);
+
 	return 0;
 }
 
@@ -148,7 +154,11 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	u32 ident0, ident1, ident2, ident3, cores;
-	int core;
+	int core, ret;
+
+	ret = v3d_pm_runtime_get(v3d);
+	if (ret)
+		return ret;
 
 	ident0 = V3D_READ(V3D_HUB_IDENT0);
 	ident1 = V3D_READ(V3D_HUB_IDENT1);
@@ -207,6 +217,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 		}
 	}
 
+	v3d_pm_runtime_put(v3d);
+
 	return 0;
 }
 
@@ -234,6 +246,11 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
 	uint32_t cycles;
 	int core = 0;
 	int measure_ms = 1000;
+	int ret;
+
+	ret = v3d_pm_runtime_get(v3d);
+	if (ret)
+		return ret;
 
 	if (v3d->ver >= V3D_GEN_41) {
 		int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d->ver);
@@ -253,6 +270,8 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
 	msleep(measure_ms);
 	cycles = V3D_CORE_READ(core, V3D_PCTR_0_PCTR0);
 
+	v3d_pm_runtime_put(v3d);
+
 	seq_printf(m, "cycles: %d (%d.%d Mhz)\n",
 		   cycles,
 		   cycles / (measure_ms * 1000),
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 592ef610d6a4bb7dfe64acf6f7283c947e9d2921..07a7de54de9432a970c9165bebed51f6984fe43d 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -59,6 +59,7 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 		[DRM_V3D_PARAM_V3D_CORE0_IDENT1] = V3D_CTL_IDENT1,
 		[DRM_V3D_PARAM_V3D_CORE0_IDENT2] = V3D_CTL_IDENT2,
 	};
+	int ret;
 
 	if (args->pad != 0)
 		return -EINVAL;
@@ -75,12 +76,19 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 		if (args->value != 0)
 			return -EINVAL;
 
+		ret = v3d_pm_runtime_get(v3d);
+		if (ret)
+			return ret;
+
 		if (args->param >= DRM_V3D_PARAM_V3D_CORE0_IDENT0 &&
 		    args->param <= DRM_V3D_PARAM_V3D_CORE0_IDENT2) {
 			args->value = V3D_CORE_READ(0, offset);
 		} else {
 			args->value = V3D_READ(offset);
 		}
+
+		v3d_pm_runtime_put(v3d);
+
 		return 0;
 	}
 
@@ -287,36 +295,6 @@ static const struct of_device_id v3d_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, v3d_of_match);
 
-static void
-v3d_idle_sms(struct v3d_dev *v3d)
-{
-	if (v3d->ver < V3D_GEN_71)
-		return;
-
-	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
-
-	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
-				    V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
-		drm_err(&v3d->drm, "Failed to power up SMS\n");
-	}
-
-	v3d_reset_sms(v3d);
-}
-
-static void
-v3d_power_off_sms(struct v3d_dev *v3d)
-{
-	if (v3d->ver < V3D_GEN_71)
-		return;
-
-	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
-
-	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
-				    V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
-		drm_err(&v3d->drm, "Failed to power off SMS\n");
-	}
-}
-
 static int
 map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
 {
@@ -383,6 +361,9 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	if (IS_ERR(v3d->clk))
 		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
 
+	if (v3d->clk)
+		v3d->max_clk_rate = clk_round_rate(v3d->clk, ULONG_MAX);
+
 	ret = v3d_irq_init(v3d);
 	if (ret)
 		return ret;
@@ -400,19 +381,26 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	if (ret)
 		goto dma_free;
 
-	ret = clk_prepare_enable(v3d->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "Couldn't enable the V3D clock\n");
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
 		goto gem_destroy;
-	}
 
-	v3d_idle_sms(v3d);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		goto gem_destroy;
+
+	/* If PM is disabled, we need to call v3d_power_resume() manually. */
+	if (!IS_ENABLED(CONFIG_PM)) {
+		ret = v3d_power_resume(dev);
+		if (ret)
+			goto gem_destroy;
+	}
 
 	mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
 	mask = DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
 	ret = dma_set_mask_and_coherent(dev, mask);
 	if (ret)
-		goto clk_disable;
+		goto runtime_pm_put;
 
 	dma_set_max_seg_size(&pdev->dev, UINT_MAX);
 
@@ -433,26 +421,27 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	v3d->rev = V3D_GET_FIELD(ident3, V3D_HUB_IDENT3_IPREV);
 
 	v3d_init_hw_state(v3d);
-	v3d_mmu_set_page_table(v3d);
-	v3d_irq_enable(v3d);
+
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(dev);
 
 	ret = drm_dev_register(drm, 0);
 	if (ret)
-		goto irq_disable;
+		goto runtime_pm_put;
 
 	ret = v3d_sysfs_init(dev);
 	if (ret)
 		goto drm_unregister;
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 
 drm_unregister:
 	drm_dev_unregister(drm);
-irq_disable:
-	v3d_irq_disable(v3d);
-clk_disable:
-	v3d_power_off_sms(v3d);
-	clk_disable_unprepare(v3d->clk);
+runtime_pm_put:
+	pm_runtime_put_sync_suspend(dev);
 gem_destroy:
 	v3d_gem_destroy(drm);
 dma_free:
@@ -470,21 +459,27 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(drm);
 
-	v3d_power_off_sms(v3d);
+	pm_runtime_suspend(dev);
 
-	clk_disable_unprepare(v3d->clk);
+	/* If PM is disabled, we need to call v3d_power_suspend() manually. */
+	if (!IS_ENABLED(CONFIG_PM))
+		v3d_power_suspend(dev);
 
 	v3d_gem_destroy(drm);
 
 	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
 }
 
+static DEFINE_RUNTIME_DEV_PM_OPS(v3d_pm_ops, v3d_power_suspend,
+				 v3d_power_resume, NULL);
+
 static struct platform_driver v3d_platform_driver = {
 	.probe		= v3d_platform_drm_probe,
 	.remove		= v3d_platform_drm_remove,
 	.driver		= {
 		.name	= "v3d",
 		.of_match_table = v3d_of_match,
+		.pm = pm_ptr(&v3d_pm_ops),
 	},
 };
 
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index ff90ef6876d65241975f259b44c6f09941d12ecb..724fad4311a6c36124b54b07a857d32684398886 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -3,6 +3,7 @@
 
 #include <linux/delay.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/spinlock_types.h>
 #include <linux/workqueue.h>
 
@@ -134,6 +135,8 @@ struct v3d_dev {
 	void __iomem *gca_regs;
 	void __iomem *sms_regs;
 	struct clk *clk;
+	unsigned long max_clk_rate;
+
 	struct reset_control *reset;
 
 	/* Virtual and DMA addresses of the single shared page table. */
@@ -321,6 +324,8 @@ struct v3d_job {
 
 	/* Callback for the freeing of the job on refcount going to 0. */
 	void (*free)(struct kref *ref);
+
+	bool has_pm_ref;
 };
 
 struct v3d_bin_job {
@@ -594,6 +599,20 @@ int v3d_mmu_set_page_table(struct v3d_dev *v3d);
 void v3d_mmu_insert_ptes(struct v3d_bo *bo);
 void v3d_mmu_remove_ptes(struct v3d_bo *bo);
 
+/* v3d_power.c */
+int v3d_power_suspend(struct device *dev);
+int v3d_power_resume(struct device *dev);
+
+static __always_inline int v3d_pm_runtime_get(struct v3d_dev *v3d)
+{
+	return pm_runtime_resume_and_get(v3d->drm.dev);
+}
+
+static __always_inline int v3d_pm_runtime_put(struct v3d_dev *v3d)
+{
+	return pm_runtime_put_autosuspend(v3d->drm.dev);
+}
+
 /* v3d_sched.c */
 void v3d_timestamp_query_info_free(struct v3d_timestamp_query_info *query_info,
 				   unsigned int count);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index dd4ac899a489fb7341815592114cc4f1652f35e8..17f50e145d4b7b206b729bfe34b93d00a7fa80e6 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -118,6 +118,11 @@ void
 v3d_reset(struct v3d_dev *v3d)
 {
 	struct drm_device *dev = &v3d->drm;
+	int ret;
+
+	ret = v3d_pm_runtime_get(v3d);
+	if (ret)
+		return;
 
 	drm_err(dev, "Resetting GPU for hang.\n");
 	drm_err(dev, "V3D_ERR_STAT: 0x%08x\n", V3D_CORE_READ(0, V3D_ERR_STAT));
@@ -140,6 +145,8 @@ v3d_reset(struct v3d_dev *v3d)
 	v3d_perfmon_stop(v3d, v3d->active_perfmon, false);
 
 	trace_v3d_reset_end(dev);
+
+	v3d_pm_runtime_put(v3d);
 }
 
 static void
diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/v3d_mmu.c
index c513a393c0313772650fd6d7236127b2dc4101d9..630c64e51d2f2ad30e59fa2b175487efe0bfba49 100644
--- a/drivers/gpu/drm/v3d/v3d_mmu.c
+++ b/drivers/gpu/drm/v3d/v3d_mmu.c
@@ -39,7 +39,11 @@ static bool v3d_mmu_is_aligned(u32 page, u32 page_address, size_t alignment)
 
 int v3d_mmu_flush_all(struct v3d_dev *v3d)
 {
-	int ret;
+	int ret = 0;
+
+	/* Flush the PTs only if we're already awake */
+	if (!pm_runtime_get_if_active(v3d->drm.dev))
+		return 0;
 
 	V3D_WRITE(V3D_MMUC_CONTROL, V3D_MMUC_CONTROL_FLUSH |
 		  V3D_MMUC_CONTROL_ENABLE);
@@ -48,7 +52,7 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
 			 V3D_MMUC_CONTROL_FLUSHING), 100);
 	if (ret) {
 		dev_err(v3d->drm.dev, "MMUC flush wait idle failed\n");
-		return ret;
+		goto pm_put;
 	}
 
 	V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL) |
@@ -59,6 +63,8 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
 	if (ret)
 		dev_err(v3d->drm.dev, "MMU TLB clear wait idle failed\n");
 
+pm_put:
+	v3d_pm_runtime_put(v3d);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index c090fc30ba4b2c502cbdc4310ee0c35f6c8aefb0..1c3c0b8c46b8fcb7697742011bbd8311dc8cddb7 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -232,6 +232,9 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
 	if (WARN_ON_ONCE(!perfmon || v3d->active_perfmon))
 		return;
 
+	if (!pm_runtime_get_if_active(v3d->drm.dev))
+		return;
+
 	ncounters = perfmon->ncounters;
 	mask = GENMASK(ncounters - 1, 0);
 
@@ -257,6 +260,8 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
 	V3D_CORE_WRITE(0, V3D_PCTR_0_OVERFLOW, mask);
 
 	v3d->active_perfmon = perfmon;
+
+	v3d_pm_runtime_put(v3d);
 }
 
 void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
@@ -268,10 +273,11 @@ void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
 		return;
 
 	mutex_lock(&perfmon->lock);
-	if (perfmon != v3d->active_perfmon) {
-		mutex_unlock(&perfmon->lock);
-		return;
-	}
+	if (perfmon != v3d->active_perfmon)
+		goto out;
+
+	if (!pm_runtime_get_if_active(v3d->drm.dev))
+		goto out_clear;
 
 	if (capture)
 		for (i = 0; i < perfmon->ncounters; i++)
@@ -279,7 +285,11 @@ void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
 
 	V3D_CORE_WRITE(0, V3D_V4_PCTR_0_EN, 0);
 
+	v3d_pm_runtime_put(v3d);
+
+out_clear:
 	v3d->active_perfmon = NULL;
+out:
 	mutex_unlock(&perfmon->lock);
 }
 
diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
new file mode 100644
index 0000000000000000000000000000000000000000..285f56acf544bbfd3d9848253e788a138aacf2af
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_power.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2026 Raspberry Pi */
+
+#include <linux/clk.h>
+#include <linux/reset.h>
+
+#include <drm/drm_print.h>
+
+#include "v3d_drv.h"
+#include "v3d_regs.h"
+
+static void
+v3d_resume_sms(struct v3d_dev *v3d)
+{
+	if (v3d->ver < V3D_GEN_71)
+		return;
+
+	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
+
+	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+				    V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
+		drm_err(&v3d->drm, "Failed to power up SMS\n");
+	}
+
+	v3d_reset_sms(v3d);
+}
+
+static void
+v3d_suspend_sms(struct v3d_dev *v3d)
+{
+	if (v3d->ver < V3D_GEN_71)
+		return;
+
+	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
+
+	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+				    V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
+		drm_err(&v3d->drm, "Failed to power off SMS\n");
+	}
+}
+
+int v3d_power_suspend(struct device *dev)
+{
+	struct drm_device *drm = dev_get_drvdata(dev);
+	struct v3d_dev *v3d = to_v3d_dev(drm);
+
+	v3d_irq_disable(v3d);
+	v3d_suspend_sms(v3d);
+
+	if (v3d->reset)
+		reset_control_assert(v3d->reset);
+
+	/* Some firmware versions might not actually power off the clock
+	 * when we set the clock state to off. Therefore, set the clock
+	 * rate to 0 to ensure it is running in the minimum rate.
+	 */
+	if (v3d->clk)
+		clk_set_rate(v3d->clk, 0);
+
+	clk_disable_unprepare(v3d->clk);
+
+	return 0;
+}
+
+int v3d_power_resume(struct device *dev)
+{
+	struct drm_device *drm = dev_get_drvdata(dev);
+	struct v3d_dev *v3d = to_v3d_dev(drm);
+	int ret;
+
+	ret = clk_prepare_enable(v3d->clk);
+	if (ret)
+		return ret;
+
+	/* Set the clock to the maximum rate and let the firmware decide
+	 * if we can actually keep it.
+	 */
+	if (v3d->clk)
+		clk_set_rate(v3d->clk, v3d->max_clk_rate);
+
+	if (v3d->reset) {
+		ret = reset_control_deassert(v3d->reset);
+		if (ret)
+			goto clk_disable;
+	}
+
+	v3d_resume_sms(v3d);
+	v3d_mmu_set_page_table(v3d);
+	v3d_irq_enable(v3d);
+
+	return 0;
+
+clk_disable:
+	clk_disable_unprepare(v3d->clk);
+	return ret;
+}
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 794c3571662de7eb566bf4c0561571d7618dd234..fea7573505cd0d2b84d21efda88ed4da657d18d4 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -103,6 +103,9 @@ v3d_job_free(struct kref *ref)
 	if (job->perfmon)
 		v3d_perfmon_put(job->perfmon);
 
+	if (job->has_pm_ref)
+		v3d_pm_runtime_put(job->v3d);
+
 	kfree(job);
 }
 
@@ -184,13 +187,13 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
 				if (copy_from_user(&in, handle++, sizeof(in))) {
 					ret = -EFAULT;
 					drm_dbg(&v3d->drm, "Failed to copy wait dep handle.\n");
-					goto fail_deps;
+					goto fail_job_init;
 				}
 				ret = drm_sched_job_add_syncobj_dependency(&job->base, file_priv, in.handle, 0);
 
 				// TODO: Investigate why this was filtered out for the IOCTL.
 				if (ret && ret != -ENOENT)
-					goto fail_deps;
+					goto fail_job_init;
 			}
 		}
 	} else {
@@ -198,14 +201,22 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
 
 		// TODO: Investigate why this was filtered out for the IOCTL.
 		if (ret && ret != -ENOENT)
-			goto fail_deps;
+			goto fail_job_init;
+	}
+
+	/* CPU jobs don't require hardware resources */
+	if (queue != V3D_CPU) {
+		ret = v3d_pm_runtime_get(v3d);
+		if (ret)
+			goto fail_job_init;
+		job->has_pm_ref = true;
 	}
 
 	kref_init(&job->refcount);
 
 	return 0;
 
-fail_deps:
+fail_job_init:
 	drm_sched_job_cleanup(&job->base);
 	return ret;
 }

-- 
2.52.0

Claude review: Power Management for Raspberry Pi V3D GPU

[Claude Code Review Bot]

Overall Series Review

Subject: Power Management for Raspberry Pi V3D GPU
Author: =?utf-8?q?Ma=C3=ADra_Canal?= <mcanal@igalia.com>
Patches: 8
Reviewed: 2026-02-14T07:21:52.077935

---

This series introduces runtime power management for the Raspberry Pi V3D GPU driver, allowing the V3D clock to be disabled during idle periods. The work is split across three subsystems: clock framework changes to give V3D consumers control over clock rates (patches 1-3), a timeout increase for the BCM2835 ASB power domain controller (patch 4), and the V3D driver changes to reorder probe, separate HW init from resource allocation, and implement runtime PM callbacks (patches 5-7).

The architectural approach is sound: patch 6 carefully separates resource allocation (which does not need the GPU powered) from hardware initialization (which does), making the subsequent introduction of runtime PM callbacks in patch 7 clean. The runtime PM integration wraps all hardware-accessing paths -- ioctls, debugfs, job submission, MMU flushing, perfmon, and GPU reset -- with appropriate get/put calls.

There are a few issues worth discussing, primarily around the error cleanup ordering in patch 6's probe function and a missing `v3d_init_hw_state` call in the resume path. The `!CONFIG_PM` handling also has a minor gap. These are detailed in the per-patch reviews below.

---
Generated by Claude Code Patch Reviewer

Claude review: clk: bcm: rpi: Let V3D consumers manage clock rate

[Claude Code Review Bot]

Patch Review

This removes the `maximize` infrastructure and stops forcing the V3D clock to maximum rate, giving consumers (the V3D driver) direct control over clock rate.

> -	bool		maximize;

> -	[RPI_FIRMWARE_V3D_CLK_ID] = {
> -		.export = true,
> -		.maximize = true,
> -	},

> -	if (variant->maximize)
> -		variant->min_rate = max_rate;

The `maximize` field is only used for the V3D clock, so removing it along with the single usage site is clean. The `minimize` field is also not set for V3D, giving full consumer control. The Fixes tag referencing the commit that added `maximize` is appropriate since that commit prevented consumers from managing clock rates for power savings.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE behavior

[Claude Code Review Bot]

Patch Review

> +	/*
> +	 * On some firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> +	 * actually power off the clock. To achieve meaningful power consumption
> +	 * reduction, consumers need to set the clock rate to minimum before
> +	 * disabling it.
> +	 */

Documentation-only change. The comment is placed in `raspberrypi_fw_unprepare()`, which is the right location since this is where consumers would expect the clock to actually power off. No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED

[Claude Code Review Bot]

Patch Review

> -		.flags = CLK_IS_CRITICAL,
> +		.flags = CLK_IGNORE_UNUSED,

Changed for both `PIXEL_CLK` and `HEVC_CLK`. The commit message explains that these clocks are consumed by specific drivers (vc4, HEVC decoder), so marking them `CLK_IGNORE_UNUSED` instead of `CLK_IS_CRITICAL` allows the clock framework's late cleanup to skip them while still permitting their respective drivers to control them. This is the correct flag for clocks that have consumers but might appear unused to the clock framework at certain points during boot.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: pmdomain: bcm: bcm2835-power: Increase ASB control timeout

[Claude Code Review Bot]

Patch Review

> -	start = ktime_get_ns();
> -
>  	/* Enable the module's async AXI bridges. */
>  	if (enable) {
>  		val = readl(base + reg) & ~ASB_REQ_STOP;
> @@ -176,9 +174,10 @@
>  	}
>  	writel(PM_PASSWORD | val, base + reg);
>  
> +	start = ktime_get_ns();
>  	while (!!(readl(base + reg) & ASB_ACK) == enable) {
>  		cpu_relax();
> -		if (ktime_get_ns() - start >= 1000)
> +		if (ktime_get_ns() - start >= 5000)
>  			return -ETIMEDOUT;
>  	}

Moving the timestamp after the `writel()` is a good change -- the previous placement counted the write latency against the timeout budget. The increase from 1us to 5us is modest and, as the commit message says, negligible in a power domain transition context.

The commit message states this fixes intermittent "Failed to disable ASB master for v3d" errors on BCM2711 during runtime PM suspend. Was this timeout value determined empirically, and has 5us been sufficient in all testing? The concern is whether this is papering over a deeper issue versus being a genuine timing margin fix. That said, the change itself is correct.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Use devm_reset_control_get_optional_exclusive()

[Claude Code Review Bot]

Patch Review

> -	v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
> +	v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
>  	if (IS_ERR(v3d->reset)) {
> -		ret = PTR_ERR(v3d->reset);
> +		ret = dev_err_probe(dev, PTR_ERR(v3d->reset),
> +				    "Failed to get reset control\n");
> +		goto clk_disable;
> +	}
>  
> -		if (ret == -EPROBE_DEFER)
> -			goto clk_disable;
> -
> -		v3d->reset = NULL;
> +	if (!v3d->reset) {
>  		ret = map_regs(v3d, &v3d->bridge_regs, "bridge");

This is a clean simplification. `devm_reset_control_get_optional_exclusive()` returns NULL when no reset is specified in DT (instead of an error), making the flow clearer. The `dev_err_probe()` usage also properly handles `-EPROBE_DEFER` without a special case.

Note that this patch is immediately superseded by patch 6, which moves this code block. This is fine for bisectability -- each patch is correct on its own -- but worth noting that this patch exists mainly for clarity of the refactoring.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Allocate all resources before enabling the clock

[Claude Code Review Bot]

Patch Review

This is the key restructuring patch. It reorders probe so that resource allocation happens before clock enable, and hardware initialization happens after.

Looking at the new probe ordering:
1. Register mappings (hub, core0, sms, gca, bridge)
2. Reset control and clock acquisition
3. `v3d_irq_init()` -- requests IRQs but no longer clears pending or enables
4. `v3d_perfmon_init()`
5. MMU scratch page allocation
6. `v3d_gem_init()` -- no longer does HW init
7. `clk_prepare_enable()` -- hardware access begins here
8. SMS idle, HW ident reads, `v3d_init_hw_state()`, `v3d_mmu_set_page_table()`, `v3d_irq_enable()`
9. `drm_dev_register()`

The separation of `v3d_irq_init()` (IRQ request only) from `v3d_irq_enable()` (HW register writes) is well done:

> -	/* Clear any pending interrupts someone might have left around
> -	 * for us.
> -	 */
> -	for (core = 0; core < v3d->cores; core++)
> -		V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
> -	V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));

This pending interrupt clear is moved into `v3d_irq_enable()`:

> +	/* Clear any pending interrupts someone might have left around for us. */
> +	for (core = 0; core < v3d->cores; core++)
> +		V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
> +	V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));

This is correct because `v3d_irq_enable()` is now called after `v3d->cores` is set from the hardware ident register.

However, there is a concern about the error cleanup path:

> +clk_disable:
> +	v3d_power_off_sms(v3d);
> +	clk_disable_unprepare(v3d->clk);
>  gem_destroy:
>  	v3d_gem_destroy(drm);
>  dma_free:
>  	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
> -clk_disable:
> -	clk_disable_unprepare(v3d->clk);
>  	return ret;

If `drm_dev_register()` or `v3d_sysfs_init()` fails, the error path jumps to `irq_disable` then falls through to `clk_disable`, `gem_destroy`, `dma_free`. But if `dma_set_mask_and_coherent()` fails (which goes to `clk_disable`), the path is `clk_disable` -> `gem_destroy` -> `dma_free`. This ordering is correct: clock off, then gem destroy, then free DMA memory.

The remove function ordering also matches:

> +	v3d_power_off_sms(v3d);
> +	clk_disable_unprepare(v3d->clk);
> +	v3d_gem_destroy(drm);
> +	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);

This looks correct -- power off before freeing software resources.

One small note: there's a double blank line after the new `v3d_init_hw_state()` function definition in v3d_gem.c:

> +void
> +v3d_init_hw_state(struct v3d_dev *v3d)
> +{
> +	v3d_init_core(v3d, 0);
> +}
> +
> +

Minor style nit, not a correctness issue.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Introduce Runtime Power Management

[Claude Code Review Bot]

Patch Review

This is the largest and most complex patch in the series. Let me go through the key aspects.

**PM runtime get/put wrappers:**

> +static __always_inline int v3d_pm_runtime_get(struct v3d_dev *v3d)
> +{
> +	return pm_runtime_resume_and_get(v3d->drm.dev);
> +}
> +
> +static __always_inline int v3d_pm_runtime_put(struct v3d_dev *v3d)
> +{
> +	return pm_runtime_put_autosuspend(v3d->drm.dev);
> +}

Clean wrappers. The 50ms autosuspend delay is set in probe.

**Job submission PM reference:**

> +	/* CPU jobs don't require hardware resources */
> +	if (queue != V3D_CPU) {
> +		ret = v3d_pm_runtime_get(v3d);
> +		if (ret)
> +			goto fail_job_init;
> +		job->has_pm_ref = true;
> +	}

This takes a PM reference at job init time and releases it when the job's refcount drops to zero:

> +	if (job->has_pm_ref)
> +		v3d_pm_runtime_put(job->v3d);

This means the GPU stays powered as long as any submitted job exists, even after it finishes execution but before its refcount drops to zero. That's a reasonable approach -- it ensures the autosuspend timer starts ticking after the last job reference is released.

**v3d_reset PM reference:**

> +	ret = v3d_pm_runtime_get(v3d);
> +	if (ret)
> +		return;

`v3d_reset()` is called from the scheduler timeout handler (`v3d_gpu_reset_for_timeout`), which is triggered when a submitted job times out. Since submitted jobs hold a PM reference via `has_pm_ref`, the GPU should already be powered when the timeout handler runs. The extra `pm_runtime_get` here is still correct for safety, and the early return on failure is reasonable since a reset can't proceed without hardware access. However, the scheduler timeout handlers (e.g., `v3d_cl_job_timedout`) also read hardware registers *before* calling `v3d_gpu_reset_for_timeout`:

```c
u32 ctca = V3D_CORE_READ(0, V3D_CLE_CTNCA(q));
u32 ctra = V3D_CORE_READ(0, V3D_CLE_CTNRA(q));
```

These register reads in `v3d_cl_job_timedout()` and `v3d_csd_job_timedout()` happen without an explicit PM reference. They rely on the timed-out job's `has_pm_ref` keeping the GPU powered. Since the job must still be alive for the timeout to fire, this is safe.

**v3d_mmu_flush_all PM guard:**

> +	/* Flush the PTs only if we're already awake */
> +	if (!pm_runtime_get_if_active(v3d->drm.dev))
> +		return 0;

This is correct -- if the GPU is suspended, the MMU doesn't need flushing since state will be fully restored on resume. `pm_runtime_get_if_active()` returns 0 when the device is not active, and the `!0` causes the early return. When active (return 1), the reference is incremented and the flush proceeds, followed by `v3d_pm_runtime_put()`. The `-EINVAL` case (runtime PM disabled) would skip the early return and proceed to flush without taking a reference, but `v3d_pm_runtime_put()` is also effectively a no-op in that scenario, so it works out.

**Perfmon PM guards:**

> +	if (!pm_runtime_get_if_active(v3d->drm.dev))
> +		return;

In `v3d_perfmon_start()`, the early return without setting `v3d->active_perfmon` means the perfmon won't be started if the GPU is suspended. This is called from `v3d_switch_perfmon()` in the scheduler's job run path, where the job already holds a PM reference, so the GPU should be active. The guard is defensive here.

In `v3d_perfmon_stop()`:

> +	if (!pm_runtime_get_if_active(v3d->drm.dev))
> +		goto out_clear;

If the GPU is not active during stop, it skips reading the counter values and writing the disable register, but still clears `v3d->active_perfmon`. This is correct -- if the GPU is powered off, the counters are lost anyway, and we need to clear the active perfmon state.

**Probe sequence with PM:**

> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		goto gem_destroy;
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		goto gem_destroy;
> +
> +	/* If PM is disabled, we need to call v3d_power_resume() manually. */
> +	if (!IS_ENABLED(CONFIG_PM)) {
> +		ret = v3d_power_resume(dev);
> +		if (ret)
> +			goto gem_destroy;
> +	}

When `CONFIG_PM` is enabled, `pm_runtime_resume_and_get()` calls `v3d_power_resume()` via the runtime PM callback. When `CONFIG_PM` is disabled, `pm_runtime_resume_and_get()` returns 1 (from the stub), so `v3d_power_resume()` needs to be called manually. However, in the `!CONFIG_PM` error path, `goto gem_destroy` skips the runtime PM put, which is fine since PM is disabled. But it also skips clock disable -- `v3d_power_resume()` would have called `clk_prepare_enable()`, but there's no corresponding cleanup. This seems like a minor gap: if the `!CONFIG_PM` path succeeds with `v3d_power_resume()` but something later in probe fails, the `runtime_pm_put` label calls `pm_runtime_put_sync_suspend()` which is a no-op when PM is disabled, so the clock won't be disabled. Looking more carefully, the only things that can fail after the `!CONFIG_PM` block are `dma_set_mask_and_coherent`, `drm_dev_register`, and `v3d_sysfs_init`. On those failures, the error path goes to `runtime_pm_put` (no-op) -> `gem_destroy` -> `dma_free`, and the clock remains enabled. You may want to add an explicit `v3d_power_suspend()` call in the `!CONFIG_PM` error cleanup.

**Remove sequence:**

> +	pm_runtime_suspend(dev);
> +
> +	/* If PM is disabled, we need to call v3d_power_suspend() manually. */
> +	if (!IS_ENABLED(CONFIG_PM))
> +		v3d_power_suspend(dev);

`pm_runtime_suspend()` will be a no-op when `!CONFIG_PM`, so the manual call is needed. Calling `pm_runtime_suspend()` directly (rather than through the put/mark_last_busy pattern) makes sense in the remove path since we want immediate suspension.

**v3d_power_resume callback:**

> +	ret = clk_prepare_enable(v3d->clk);
> +	if (ret)
> +		return ret;
> +
> +	if (v3d->clk)
> +		clk_set_rate(v3d->clk, v3d->max_clk_rate);
> +
> +	if (v3d->reset) {
> +		ret = reset_control_deassert(v3d->reset);
> +		if (ret)
> +			goto clk_disable;
> +	}
> +
> +	v3d_resume_sms(v3d);
> +	v3d_mmu_set_page_table(v3d);
> +	v3d_irq_enable(v3d);

One question: should `v3d_init_hw_state()` be called here as well? After the GPU is power-cycled through runtime suspend/resume, the core hardware state (L2T cache configuration set by `v3d_init_core`) would be lost. In the probe path (still visible after this patch), `v3d_init_hw_state()` is called explicitly. But in the runtime resume callback, it's not called. This means after a runtime suspend/resume cycle, the L2T cache flush registers and other invariant state set by `v3d_init_core()` may not be restored. The `v3d_reset()` path does call `v3d_init_hw_state()` (via the patched version that adds a PM get), but the normal resume path does not.

Could you verify whether the hardware requires `v3d_init_hw_state()` to be called on every power-on, or whether the L2T configuration survives power cycling?

**v3d_power_suspend callback:**

> +	v3d_irq_disable(v3d);
> +	v3d_suspend_sms(v3d);
> +
> +	if (v3d->reset)
> +		reset_control_assert(v3d->reset);
> +
> +	if (v3d->clk)
> +		clk_set_rate(v3d->clk, 0);
> +
> +	clk_disable_unprepare(v3d->clk);

The ordering here is: disable IRQs, power off SMS, assert reset, set clock rate to minimum, then disable clock. This is reasonable. The `clk_set_rate(v3d->clk, 0)` before `clk_disable_unprepare()` addresses the firmware quirk where disabling the clock doesn't actually power it off.

**Probe init_hw_state placement:**

> 	v3d_init_hw_state(v3d);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 50);
> +	pm_runtime_use_autosuspend(dev);

`v3d_init_hw_state()` is called after `pm_runtime_resume_and_get()` in probe, which is correct. But note that `v3d_mmu_set_page_table()` and `v3d_irq_enable()` are no longer called explicitly in probe -- they're handled by `v3d_power_resume()` which is called by `pm_runtime_resume_and_get()`. So the `v3d_init_hw_state()` in probe is the only call that initializes core hardware state during first boot. As noted above, this same init is missing from the `v3d_power_resume()` callback for subsequent runtime resumes.

**DEFINE_RUNTIME_DEV_PM_OPS:**

> +static DEFINE_RUNTIME_DEV_PM_OPS(v3d_pm_ops, v3d_power_suspend,
> +				 v3d_power_resume, NULL);

The NULL third argument means no system suspend/resume callbacks beyond runtime PM. This means during system suspend, the runtime PM framework's generic handling (`pm_runtime_force_suspend`/`pm_runtime_force_resume`) will be used. This should work correctly with the runtime PM callbacks already defined.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Allocate all resources before enabling the clock

[Claude Code Review Bot]

Patch Review

This is the key preparation patch that reorders probe to do all resource allocation before enabling the clock. The reordering is correct: `map_regs`, `devm_reset_control_get_optional_exclusive`, `devm_clk_get_optional`, `v3d_irq_init`, `v3d_perfmon_init`, `dma_alloc_wc`, and `v3d_gem_init` all happen before `clk_prepare_enable`.

A few notes:

1. **Error path ordering concern**: The error cleanup labels are:
```c
clk_disable:
	v3d_power_off_sms(v3d);
	clk_disable_unprepare(v3d->clk);
gem_destroy:
	v3d_gem_destroy(drm);
dma_free:
	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
```
The labels fall through, so `clk_disable` falls through to `gem_destroy` and `dma_free`. This is correct — if the clock was enabled and something later fails, all three cleanups happen. But `gem_destroy` and `dma_free` are only reached by `goto` from before the clock is enabled, so `clk_disable` cleanup is never incorrectly reached from those paths. Good.

2. The `v3d_init_hw_state` and `v3d_mmu_set_page_table` calls are correctly moved out of `v3d_gem_init` and into the post-clock-enable section. The `v3d_irq_enable` call is likewise moved out of `v3d_irq_init` and into the post-clock-enable section. This is a clean separation.

3. **Extra blank line** in `v3d_gem.c` after `v3d_init_hw_state()`:
```c
+void
+v3d_init_hw_state(struct v3d_dev *v3d)
+{
+	v3d_init_core(v3d, 0);
+}
+
+
 static void
```
There's a double blank line — minor style nit.

---
Generated by Claude Code Patch Reviewer