[PATCH v6 0/6] Power Management for Raspberry Pi V3D GPU

[PATCH v6 0/6] 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 to 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 on 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-v6

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 resources 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] Add 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 in 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()

v5 -> v6: https://lore.kernel.org/r/20260213-v3d-power-management-v5-0-7a8b381eb379@igalia.com

- [1/6] NEW PATCH: "clk: bcm: rpi: Manage clock rate in prepare/unprepare
  callbacks" (Maxime Ripard)
    - Replaces "clk: bcm: rpi: Let V3D consumers manage clock rate" and
      "clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE
      behavior" 
- [6/6] Stop setting min and max clock rates directly in v3d (Maxime Ripard)

---
Maíra Canal (6):
      clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
      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    |  65 ++++++++------
 drivers/gpu/drm/v3d/Makefile         |   3 +-
 drivers/gpu/drm/v3d/v3d_debugfs.c    |  23 ++++-
 drivers/gpu/drm/v3d/v3d_drv.c        | 160 +++++++++++++++++------------------
 drivers/gpu/drm/v3d/v3d_drv.h        |  18 ++++
 drivers/gpu/drm/v3d/v3d_gem.c        |  18 ++--
 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      |  83 ++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_submit.c     |  19 ++++-
 drivers/pmdomain/bcm/bcm2835-power.c |   5 +-
 12 files changed, 290 insertions(+), 147 deletions(-)
---
base-commit: 948e195dfaa56e48eabda591f97630502ff7e27e
change-id: 20250728-v3d-power-management-eebb2024dc96

[PATCH v6 1/6] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks

[Maíra Canal]

On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
actually power off the clock. To achieve meaningful power savings, the
clock rate must be set to the minimum before disabling. This might be
fixed in future firmware releases.

Rather than pushing rate management to clock consumers, handle it
directly in the clock framework's prepare/unprepare callbacks. In
unprepare, set the rate to the firmware-reported minimum before
disabling the clock. In prepare, for clocks marked with `maximize`
(currently v3d), restore the rate to the maximum after enabling.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 61 ++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 1a9162f0ae31e330c46f6eafdd00350599b0eede..0d6e4f43c3bac0e7b38934c5c6e4db51211233de 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -66,7 +66,8 @@ const struct raspberrypi_clk_data *clk_hw_to_data(const struct clk_hw *hw)
 struct raspberrypi_clk_variant {
 	bool		export;
 	char		*clkdev;
-	unsigned long	min_rate;
+	u32		min_rate;
+	u32		max_rate;
 	bool		minimize;
 	bool		maximize;
 	u32		flags;
@@ -289,16 +290,22 @@ static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
 static int raspberrypi_fw_prepare(struct clk_hw *hw)
 {
 	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
+	struct raspberrypi_clk_variant *variant = data->variant;
 	struct raspberrypi_clk *rpi = data->rpi;
 	u32 state = RPI_FIRMWARE_STATE_ENABLE_BIT;
 	int ret;
 
 	ret = raspberrypi_clock_property(rpi->firmware, data,
 					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
-	if (ret)
+	if (ret) {
 		dev_err_ratelimited(rpi->dev,
 				    "Failed to set clock %s state to on: %d\n",
 				    clk_hw_get_name(hw), ret);
+		return ret;
+	}
+
+	if (variant->maximize)
+		ret = raspberrypi_fw_set_rate(hw, variant->max_rate, 0);
 
 	return ret;
 }
@@ -306,10 +313,19 @@ static int raspberrypi_fw_prepare(struct clk_hw *hw)
 static void raspberrypi_fw_unprepare(struct clk_hw *hw)
 {
 	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
+	struct raspberrypi_clk_variant *variant = data->variant;
 	struct raspberrypi_clk *rpi = data->rpi;
 	u32 state = 0;
 	int ret;
 
+	/*
+	 * On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
+	 * actually power off the clock. To achieve meaningful power consumption
+	 * reduction, the driver needs to set the clock rate to minimum before
+	 * disabling it.
+	 */
+	raspberrypi_fw_set_rate(hw, variant->min_rate, 0);
+
 	ret = raspberrypi_clock_property(rpi->firmware, data,
 					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
 	if (ret)
@@ -334,7 +350,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
 {
 	struct raspberrypi_clk_data *data;
 	struct clk_init_data init = {};
-	u32 min_rate, max_rate;
+	unsigned long rate;
 	int ret;
 
 	data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
@@ -354,18 +370,20 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
 
 	data->hw.init = &init;
 
-	ret = raspberrypi_clock_property(rpi->firmware, data,
-					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
-					 &min_rate);
-	if (ret) {
-		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
-			id, ret);
-		return ERR_PTR(ret);
+	if (!variant->min_rate) {
+		ret = raspberrypi_clock_property(rpi->firmware, data,
+						 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
+						 &variant->min_rate);
+		if (ret) {
+			dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
+				id, ret);
+			return ERR_PTR(ret);
+		}
 	}
 
 	ret = raspberrypi_clock_property(rpi->firmware, data,
 					 RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
-					 &max_rate);
+					 &variant->max_rate);
 	if (ret) {
 		dev_err(rpi->dev, "Failed to get clock %d max freq: %d\n",
 			id, ret);
@@ -376,7 +394,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
 	if (ret)
 		return ERR_PTR(ret);
 
-	clk_hw_set_rate_range(&data->hw, min_rate, max_rate);
+	clk_hw_set_rate_range(&data->hw, variant->min_rate, variant->max_rate);
 
 	if (variant->clkdev) {
 		ret = devm_clk_hw_register_clkdev(rpi->dev, &data->hw,
@@ -387,20 +405,11 @@ 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;
-
-		clk_hw_set_rate_range(&data->hw, variant->min_rate, max_rate);
-
-		rate = raspberrypi_fw_get_rate(&data->hw, 0);
-		if (rate < variant->min_rate) {
-			ret = raspberrypi_fw_set_rate(&data->hw, variant->min_rate, 0);
-			if (ret)
-				return ERR_PTR(ret);
-		}
+	rate = raspberrypi_fw_get_rate(&data->hw, 0);
+	if (rate < variant->min_rate) {
+		ret = raspberrypi_fw_set_rate(&data->hw, variant->min_rate, 0);
+		if (ret)
+			return ERR_PTR(ret);
 	}
 
 	return &data->hw;

-- 
2.52.0

[PATCH v6 2/6] 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 0d6e4f43c3bac0e7b38934c5c6e4db51211233de..83cdb2a34c84f34ba834608259d22e2b48b90c9d 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -142,12 +142,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 v6 3/6] 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 v6 4/6] 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 v6 5/6] 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 v6 6/6] 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     | 84 ++++++++++++++++++---------------------
 drivers/gpu/drm/v3d/v3d_drv.h     | 17 ++++++++
 drivers/gpu/drm/v3d/v3d_mmu.c     | 10 ++++-
 drivers/gpu/drm/v3d/v3d_perfmon.c | 18 +++++++--
 drivers/gpu/drm/v3d/v3d_power.c   | 83 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_submit.c  | 19 +++++++--
 8 files changed, 198 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..b46fa42d2a7c105d0e4617b200cc3c549c9e0e4d 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)
 {
@@ -400,19 +378,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 +418,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 +456,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..ff61a2510742e46f246f3935d2d0487d7202b201 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>
 
@@ -321,6 +322,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 +597,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_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..f3d30ef5de4ea6ecdbd21c438b8af54bdf4d0dba
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_power.c
@@ -0,0 +1,83 @@
+// 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);
+
+	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;
+
+	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

Re: [PATCH v6 4/6] drm/v3d: Use devm_reset_control_get_optional_exclusive()

[Philipp Zabel]

On Mi, 2026-02-18 at 17:45 -0300, Maíra Canal wrote:
> 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>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH v6 1/6] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4953 bytes --]

Hi Maira,

On Wed, Feb 18, 2026 at 05:44:59PM -0300, Maíra Canal wrote:
> On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> actually power off the clock. To achieve meaningful power savings, the
> clock rate must be set to the minimum before disabling. This might be
> fixed in future firmware releases.
> 
> Rather than pushing rate management to clock consumers, handle it
> directly in the clock framework's prepare/unprepare callbacks. In
> unprepare, set the rate to the firmware-reported minimum before
> disabling the clock. In prepare, for clocks marked with `maximize`
> (currently v3d), restore the rate to the maximum after enabling.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/clk/bcm/clk-raspberrypi.c | 61 ++++++++++++++++++++++-----------------
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index 1a9162f0ae31e330c46f6eafdd00350599b0eede..0d6e4f43c3bac0e7b38934c5c6e4db51211233de 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -66,7 +66,8 @@ const struct raspberrypi_clk_data *clk_hw_to_data(const struct clk_hw *hw)
>  struct raspberrypi_clk_variant {
>  	bool		export;
>  	char		*clkdev;
> -	unsigned long	min_rate;
> +	u32		min_rate;
> +	u32		max_rate;
>  	bool		minimize;
>  	bool		maximize;
>  	u32		flags;
> @@ -289,16 +290,22 @@ static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
>  static int raspberrypi_fw_prepare(struct clk_hw *hw)
>  {
>  	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
> +	struct raspberrypi_clk_variant *variant = data->variant;
>  	struct raspberrypi_clk *rpi = data->rpi;
>  	u32 state = RPI_FIRMWARE_STATE_ENABLE_BIT;
>  	int ret;
>  
>  	ret = raspberrypi_clock_property(rpi->firmware, data,
>  					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
> -	if (ret)
> +	if (ret) {
>  		dev_err_ratelimited(rpi->dev,
>  				    "Failed to set clock %s state to on: %d\n",
>  				    clk_hw_get_name(hw), ret);
> +		return ret;
> +	}
> +
> +	if (variant->maximize)
> +		ret = raspberrypi_fw_set_rate(hw, variant->max_rate, 0);


It's not clear to me why you would want to force it to the max there,
and especially the max of the clock. It would make more sense to me to
set it to whatever maximum rate clk_hw_get_rate_range would return
(which should be the minimum of variant->max_rate + all clk->max_rate),
but even then it would erase every call to clk_set_rate before calling
clk_prepare().

I'd understand lowering the clock rate in unprepare to avoid wasting too
much power, but why do we need to raise it here?

>  	return ret;
>  }
> @@ -306,10 +313,19 @@ static int raspberrypi_fw_prepare(struct clk_hw *hw)
>  static void raspberrypi_fw_unprepare(struct clk_hw *hw)
>  {
>  	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
> +	struct raspberrypi_clk_variant *variant = data->variant;
>  	struct raspberrypi_clk *rpi = data->rpi;
>  	u32 state = 0;
>  	int ret;
>  
> +	/*
> +	 * On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> +	 * actually power off the clock. To achieve meaningful power consumption
> +	 * reduction, the driver needs to set the clock rate to minimum before
> +	 * disabling it.
> +	 */
> +	raspberrypi_fw_set_rate(hw, variant->min_rate, 0);

I'm not sure setting it to variant->min_rate would work directly either,
since it would break consumers expectations. Also, can we guard it with
a version check if we know that there's some good and bad firmwares?

>  	ret = raspberrypi_clock_property(rpi->firmware, data,
>  					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
>  	if (ret)
> @@ -334,7 +350,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>  {
>  	struct raspberrypi_clk_data *data;
>  	struct clk_init_data init = {};
> -	u32 min_rate, max_rate;
> +	unsigned long rate;
>  	int ret;
>  
>  	data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
> @@ -354,18 +370,20 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>  
>  	data->hw.init = &init;
>  
> -	ret = raspberrypi_clock_property(rpi->firmware, data,
> -					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> -					 &min_rate);
> -	if (ret) {
> -		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
> -			id, ret);
> -		return ERR_PTR(ret);
> +	if (!variant->min_rate) {
> +		ret = raspberrypi_clock_property(rpi->firmware, data,
> +						 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> +						 &variant->min_rate);
> +		if (ret) {
> +			dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
> +				id, ret);
> +			return ERR_PTR(ret);
> +		}
>  	}

It feels to me like it would need to be a separate patch. Why do you
need to override the minimum clock rate reported by the firmware?

Maxime

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

Re: [PATCH v6 1/6] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks

[Maíra Canal]

Hi Maxime,

On 24/02/26 11:11, Maxime Ripard wrote:
> Hi Maira,
> 

[...]

>> @@ -289,16 +290,22 @@ static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
>>   static int raspberrypi_fw_prepare(struct clk_hw *hw)
>>   {
>>   	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
>> +	struct raspberrypi_clk_variant *variant = data->variant;
>>   	struct raspberrypi_clk *rpi = data->rpi;
>>   	u32 state = RPI_FIRMWARE_STATE_ENABLE_BIT;
>>   	int ret;
>>   
>>   	ret = raspberrypi_clock_property(rpi->firmware, data,
>>   					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
>> -	if (ret)
>> +	if (ret) {
>>   		dev_err_ratelimited(rpi->dev,
>>   				    "Failed to set clock %s state to on: %d\n",
>>   				    clk_hw_get_name(hw), ret);
>> +		return ret;
>> +	}
>> +
>> +	if (variant->maximize)
>> +		ret = raspberrypi_fw_set_rate(hw, variant->max_rate, 0);
> 
> 
> It's not clear to me why you would want to force it to the max there,
> and especially the max of the clock. It would make more sense to me to
> set it to whatever maximum rate clk_hw_get_rate_range would return
> (which should be the minimum of variant->max_rate + all clk->max_rate),
> but even then it would erase every call to clk_set_rate before calling
> clk_prepare().
> 
> I'd understand lowering the clock rate in unprepare to avoid wasting too
> much power, but why do we need to raise it here?

This only applies to clocks with variant->maximize == true, which is
exclusively the V3D clock. The V3D driver doesn't do fine-grained rate
control, it always wants maximum performance. In the case of V3D, there
are no intermediate clk_set_rate() calls to preserve.

> 
>>   	return ret;
>>   }
>> @@ -306,10 +313,19 @@ static int raspberrypi_fw_prepare(struct clk_hw *hw)
>>   static void raspberrypi_fw_unprepare(struct clk_hw *hw)
>>   {
>>   	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
>> +	struct raspberrypi_clk_variant *variant = data->variant;
>>   	struct raspberrypi_clk *rpi = data->rpi;
>>   	u32 state = 0;
>>   	int ret;
>>   
>> +	/*
>> +	 * On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
>> +	 * actually power off the clock. To achieve meaningful power consumption
>> +	 * reduction, the driver needs to set the clock rate to minimum before
>> +	 * disabling it.
>> +	 */
>> +	raspberrypi_fw_set_rate(hw, variant->min_rate, 0);
> 
> I'm not sure setting it to variant->min_rate would work directly either,
> since it would break consumers expectations. Also, can we guard it with

The clock is being unprepared, which means that any consumer that wants
to use it again must call clk_prepare() first, at which point the rate
gets restored (for maximize clocks) or re-established by the consumer
via clk_set_rate(). Considering that no consumer should be relying on
the rate of an unprepared clock, I understand that there are no
expectations to break here.

> a version check if we know that there's some good and bad firmwares?

So far, all firmware versions have this issue. For future firmware
releases, it might change, but so far, all firmware versions have this
issue.

> 
>>   	ret = raspberrypi_clock_property(rpi->firmware, data,
>>   					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
>>   	if (ret)
>> @@ -334,7 +350,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>>   {
>>   	struct raspberrypi_clk_data *data;
>>   	struct clk_init_data init = {};
>> -	u32 min_rate, max_rate;
>> +	unsigned long rate;
>>   	int ret;
>>   
>>   	data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
>> @@ -354,18 +370,20 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>>   
>>   	data->hw.init = &init;
>>   
>> -	ret = raspberrypi_clock_property(rpi->firmware, data,
>> -					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
>> -					 &min_rate);
>> -	if (ret) {
>> -		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
>> -			id, ret);
>> -		return ERR_PTR(ret);
>> +	if (!variant->min_rate) {
>> +		ret = raspberrypi_clock_property(rpi->firmware, data,
>> +						 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
>> +						 &variant->min_rate);
>> +		if (ret) {
>> +			dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
>> +				id, ret);
>> +			return ERR_PTR(ret);
>> +		}
>>   	}
> 
> It feels to me like it would need to be a separate patch. Why do you
> need to override the minimum clock rate reported by the firmware?

This change is needed because the prepare/unprepare callbacks need
access to the min/max rates. In the current code, these are local
variables in raspberrypi_clk_register(). Storing them in the variant
struct makes them available to the callbacks. The `if (!variant-
 >min_rate)` guard only preserves the existing behavior for clocks like
M2MC that have a hard-coded minimum rate.

Having said that, I can split this into a separate preparatory patch if
you prefer.

Thanks for your review!

Best regards,
- Maíra

> 
> Maxime

Re: [PATCH v6 3/6] pmdomain: bcm: bcm2835-power: Increase ASB control timeout

[Maíra Canal]

[+cc Ulf Hansson, linux-pm]

Stefan, Florian, could you take a look at this patch when you get a
chance? It would be great to have this patch reviewed as it can go
independently through the linux-pm tree.

I'm sorry, I previously forgot to re-run --auto-to-cc, so the linux-pm
mailing list was missing from the recipients.

Best regards,
- Maíra

On 2/18/26 17:45, Maíra Canal wrote:
> 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;
>   	}
>   
> 

Re: [PATCH v6 3/6] pmdomain: bcm: bcm2835-power: Increase ASB control timeout

[Stefan Wahren]

Hi Maíra,

Am 18.02.26 um 21:45 schrieb 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.
so this bug has been discovered by this series and doesn't need to 
backported?
I remember complete system freezes during suspend to idle causes by V3D 
on Raspberry CM4 [1]. But I never had the time to investigate further.

Regardless, this patch is
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>

Thanks

[1] - 
https://github.com/lategoodbye/linux-dev/commit/6af3f79da5b5a41b37aefa6abe3d368c9ef09805 

>
> 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;
>   	}
>   
>

Re: [PATCH v6 6/6] drm/v3d: Introduce Runtime Power Management

[Stefan Wahren]

Hi Maíra,

just 2 nits.

Am 18.02.26 um 21:45 schrieb 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     | 84 ++++++++++++++++++---------------------
>   drivers/gpu/drm/v3d/v3d_drv.h     | 17 ++++++++
>   drivers/gpu/drm/v3d/v3d_mmu.c     | 10 ++++-
>   drivers/gpu/drm/v3d/v3d_perfmon.c | 18 +++++++--
>   drivers/gpu/drm/v3d/v3d_power.c   | 83 ++++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/v3d/v3d_submit.c  | 19 +++++++--
>   8 files changed, 198 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
Please keep the alphabetical order.
>   
>   v3d-$(CONFIG_DEBUG_FS) += v3d_debugfs.o
>   
...
>   
> diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f3d30ef5de4ea6ecdbd21c438b8af54bdf4d0dba
> --- /dev/null
> +++ b/drivers/gpu/drm/v3d/v3d_power.c
> @@ -0,0 +1,83 @@
> +// 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);
Shouldn't we handle the error case?
> +
> +	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;
> +
> +	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;
>   }
>

Re: [PATCH v6 3/6] pmdomain: bcm: bcm2835-power: Increase ASB control timeout

[Maíra Canal]

Hi Stefan,

On 02/03/26 19:43, Stefan Wahren wrote:
> Hi Maíra,
> 
> Am 18.02.26 um 21:45 schrieb 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.
> so this bug has been discovered by this series and doesn't need to 
> backported?

I only found out about this bug when I was testing this series, but it's
quite possible that this issue also happened in other scenarios.

> I remember complete system freezes during suspend to idle causes by V3D 
> on Raspberry CM4 [1]. But I never had the time to investigate further.
> 
> Regardless, this patch is
> Reviewed-by: Stefan Wahren <wahrenst@gmx.net>

Thanks for the review! Ulf, considering Stefan's comment, I'd appreciate
if you could pick this patch and add the stable tag. Thanks.

Best regards,
- Maíra

> 
> Thanks
> 
> [1] - https://github.com/lategoodbye/linux-dev/ 
> commit/6af3f79da5b5a41b37aefa6abe3d368c9ef09805
>>
>> 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;
>>       }
>>
> 

Re: [PATCH v6 1/6] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5851 bytes --]

On Wed, Feb 25, 2026 at 10:57:11AM -0300, Maíra Canal wrote:
> Hi Maxime,
> 
> On 24/02/26 11:11, Maxime Ripard wrote:
> > Hi Maira,
> > 
> 
> [...]
> 
> > > @@ -289,16 +290,22 @@ static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
> > >   static int raspberrypi_fw_prepare(struct clk_hw *hw)
> > >   {
> > >   	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
> > > +	struct raspberrypi_clk_variant *variant = data->variant;
> > >   	struct raspberrypi_clk *rpi = data->rpi;
> > >   	u32 state = RPI_FIRMWARE_STATE_ENABLE_BIT;
> > >   	int ret;
> > >   	ret = raspberrypi_clock_property(rpi->firmware, data,
> > >   					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
> > > -	if (ret)
> > > +	if (ret) {
> > >   		dev_err_ratelimited(rpi->dev,
> > >   				    "Failed to set clock %s state to on: %d\n",
> > >   				    clk_hw_get_name(hw), ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	if (variant->maximize)
> > > +		ret = raspberrypi_fw_set_rate(hw, variant->max_rate, 0);
> > 
> > 
> > It's not clear to me why you would want to force it to the max there,
> > and especially the max of the clock. It would make more sense to me to
> > set it to whatever maximum rate clk_hw_get_rate_range would return
> > (which should be the minimum of variant->max_rate + all clk->max_rate),
> > but even then it would erase every call to clk_set_rate before calling
> > clk_prepare().
> > 
> > I'd understand lowering the clock rate in unprepare to avoid wasting too
> > much power, but why do we need to raise it here?
> 
> This only applies to clocks with variant->maximize == true, which is
> exclusively the V3D clock. The V3D driver doesn't do fine-grained rate
> control, it always wants maximum performance. In the case of V3D, there
> are no intermediate clk_set_rate() calls to preserve.
> 
> > 
> > >   	return ret;
> > >   }
> > > @@ -306,10 +313,19 @@ static int raspberrypi_fw_prepare(struct clk_hw *hw)
> > >   static void raspberrypi_fw_unprepare(struct clk_hw *hw)
> > >   {
> > >   	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
> > > +	struct raspberrypi_clk_variant *variant = data->variant;
> > >   	struct raspberrypi_clk *rpi = data->rpi;
> > >   	u32 state = 0;
> > >   	int ret;
> > > +	/*
> > > +	 * On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> > > +	 * actually power off the clock. To achieve meaningful power consumption
> > > +	 * reduction, the driver needs to set the clock rate to minimum before
> > > +	 * disabling it.
> > > +	 */
> > > +	raspberrypi_fw_set_rate(hw, variant->min_rate, 0);
> > 
> > I'm not sure setting it to variant->min_rate would work directly either,
> > since it would break consumers expectations. Also, can we guard it with
> 
> The clock is being unprepared, which means that any consumer that wants
> to use it again must call clk_prepare() first, at which point the rate
> gets restored (for maximize clocks) or re-established by the consumer
> via clk_set_rate(). Considering that no consumer should be relying on
> the rate of an unprepared clock, I understand that there are no
> expectations to break here.

For both of those, I still feel like it's a pretty strong deviation from
the general expected behaviour of the CCF API. From what you're saying,
we shouldn't notice it too much, but at the very least we should
document it explicitly, both what the deviation is, and why we think
it's ok.

> > a version check if we know that there's some good and bad firmwares?
> 
> So far, all firmware versions have this issue. For future firmware
> releases, it might change, but so far, all firmware versions have this
> issue.

ack :)

> > 
> > >   	ret = raspberrypi_clock_property(rpi->firmware, data,
> > >   					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
> > >   	if (ret)
> > > @@ -334,7 +350,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
> > >   {
> > >   	struct raspberrypi_clk_data *data;
> > >   	struct clk_init_data init = {};
> > > -	u32 min_rate, max_rate;
> > > +	unsigned long rate;
> > >   	int ret;
> > >   	data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
> > > @@ -354,18 +370,20 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
> > >   	data->hw.init = &init;
> > > -	ret = raspberrypi_clock_property(rpi->firmware, data,
> > > -					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> > > -					 &min_rate);
> > > -	if (ret) {
> > > -		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
> > > -			id, ret);
> > > -		return ERR_PTR(ret);
> > > +	if (!variant->min_rate) {
> > > +		ret = raspberrypi_clock_property(rpi->firmware, data,
> > > +						 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
> > > +						 &variant->min_rate);
> > > +		if (ret) {
> > > +			dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
> > > +				id, ret);
> > > +			return ERR_PTR(ret);
> > > +		}
> > >   	}
> > 
> > It feels to me like it would need to be a separate patch. Why do you
> > need to override the minimum clock rate reported by the firmware?
> 
> This change is needed because the prepare/unprepare callbacks need
> access to the min/max rates. In the current code, these are local
> variables in raspberrypi_clk_register(). Storing them in the variant
> struct makes them available to the callbacks. The `if (!variant-
> >min_rate)` guard only preserves the existing behavior for clocks like
> M2MC that have a hard-coded minimum rate.

min_rate itself is indeed available only in raspberrypi_clk_register(),
but its main use is to call clk_hw_set_rate_range to use it as the
minimum clock rate.

In prepare/unprepare, you should be able to use clk_hw_get_rate_range()
to retrieve it, or am I missing something?

Maxime

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

Re: [PATCH v6 1/6] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks

[Maíra Canal]

Hi Maxime,

On 05/03/26 05:53, Maxime Ripard wrote:
> On Wed, Feb 25, 2026 at 10:57:11AM -0300, Maíra Canal wrote:
>> Hi Maxime,
>>
>> On 24/02/26 11:11, Maxime Ripard wrote:
>>> Hi Maira,
>>>
>>

[...]

>>>> @@ -306,10 +313,19 @@ static int raspberrypi_fw_prepare(struct clk_hw *hw)
>>>>    static void raspberrypi_fw_unprepare(struct clk_hw *hw)
>>>>    {
>>>>    	const struct raspberrypi_clk_data *data = clk_hw_to_data(hw);
>>>> +	struct raspberrypi_clk_variant *variant = data->variant;
>>>>    	struct raspberrypi_clk *rpi = data->rpi;
>>>>    	u32 state = 0;
>>>>    	int ret;
>>>> +	/*
>>>> +	 * On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
>>>> +	 * actually power off the clock. To achieve meaningful power consumption
>>>> +	 * reduction, the driver needs to set the clock rate to minimum before
>>>> +	 * disabling it.
>>>> +	 */
>>>> +	raspberrypi_fw_set_rate(hw, variant->min_rate, 0);
>>>
>>> I'm not sure setting it to variant->min_rate would work directly either,
>>> since it would break consumers expectations. Also, can we guard it with
>>
>> The clock is being unprepared, which means that any consumer that wants
>> to use it again must call clk_prepare() first, at which point the rate
>> gets restored (for maximize clocks) or re-established by the consumer
>> via clk_set_rate(). Considering that no consumer should be relying on
>> the rate of an unprepared clock, I understand that there are no
>> expectations to break here.
> 
> For both of those, I still feel like it's a pretty strong deviation from
> the general expected behaviour of the CCF API. From what you're saying,
> we shouldn't notice it too much, but at the very least we should
> document it explicitly, both what the deviation is, and why we think
> it's ok.
> 

I'll make sure to add such comment in the next version.

>>> a version check if we know that there's some good and bad firmwares?
>>
>> So far, all firmware versions have this issue. For future firmware
>> releases, it might change, but so far, all firmware versions have this
>> issue.
> 
> ack :)
> 
>>>
>>>>    	ret = raspberrypi_clock_property(rpi->firmware, data,
>>>>    					 RPI_FIRMWARE_SET_CLOCK_STATE, &state);
>>>>    	if (ret)
>>>> @@ -334,7 +350,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>>>>    {
>>>>    	struct raspberrypi_clk_data *data;
>>>>    	struct clk_init_data init = {};
>>>> -	u32 min_rate, max_rate;
>>>> +	unsigned long rate;
>>>>    	int ret;
>>>>    	data = devm_kzalloc(rpi->dev, sizeof(*data), GFP_KERNEL);
>>>> @@ -354,18 +370,20 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
>>>>    	data->hw.init = &init;
>>>> -	ret = raspberrypi_clock_property(rpi->firmware, data,
>>>> -					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
>>>> -					 &min_rate);
>>>> -	if (ret) {
>>>> -		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
>>>> -			id, ret);
>>>> -		return ERR_PTR(ret);
>>>> +	if (!variant->min_rate) {
>>>> +		ret = raspberrypi_clock_property(rpi->firmware, data,
>>>> +						 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
>>>> +						 &variant->min_rate);
>>>> +		if (ret) {
>>>> +			dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
>>>> +				id, ret);
>>>> +			return ERR_PTR(ret);
>>>> +		}
>>>>    	}
>>>
>>> It feels to me like it would need to be a separate patch. Why do you
>>> need to override the minimum clock rate reported by the firmware?
>>
>> This change is needed because the prepare/unprepare callbacks need
>> access to the min/max rates. In the current code, these are local
>> variables in raspberrypi_clk_register(). Storing them in the variant
>> struct makes them available to the callbacks. The `if (!variant-
>>> min_rate)` guard only preserves the existing behavior for clocks like
>> M2MC that have a hard-coded minimum rate.
> 
> min_rate itself is indeed available only in raspberrypi_clk_register(),
> but its main use is to call clk_hw_set_rate_range to use it as the
> minimum clock rate.
 > > In prepare/unprepare, you should be able to use clk_hw_get_rate_range()
> to retrieve it, or am I missing something?

Okay, I got now, I didn't know about the function
clk_hw_get_rate_range(). Thanks for your feedback! I'd address it in the
next version.

Best regards,
- Maíra

> 
> Maxime