[PATCH v4 00/13] RPM, perfcnt and other minor fixes for Panfrost

[PATCH v4 00/13] RPM, perfcnt and other minor fixes for Panfrost

[Adrián Larumbe]

This patch series comes with some minor fixes for Panfrost. Some of the
issues addressed were uncovered by LLM tools, while the one about RPM was
traditionally debugged.

The only proper bug fixes are the ones for perfcnt and RPM, and the others
are more about ensuring robustness. perfcnt bug was never observed in the
open, because no one had faced a HW reset when a perfcnt session was
ongoing. RPM fix addresses a well-known issue I had been observing for
quite some time, but that I had completely misunderstood.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
Changes in v4:
- Fixed errors and issues uncovered by Sashiko in the previous revision.
- Changed the way RPM checks whether the device is initialised when resuming.
- Simplified perfcnt treatment of potential resets.
- Link to v3: https://patch.msgid.link/20260724-claude-fixes-v3-0-542af61abd0f@collabora.com

Changes in v3:
- Applied some minor suggestions for the first couple of patches in the series.
- Moved shrinker initialisation into gem initialisation.
- Fixed RPM bugs by moving all clock and power initialisation into RPM resume.
- Added patch for reset sequence fixes and also a userspace knob to trigger it.
- Reworked perfcnt fix by having the reset sequence restore its initial state.
- Link to v2: https://patch.msgid.link/20260604-claude-fixes-v2-0-57c6bd4c1655@collabora.com

Changes in v2:
- Fixed race conditions introduced by the previous revision, in the RPM and
HWPerf commits specifically. These can be consulted at [1]
- Don't attempt to suspend the device unconditionally at the end of
probe(), and do it through autosuspend instead.
- Broke the RPM patch into one that fixes PM refcnt proper and another one
for MMU enablement at device init time.
- Moved perfcnt GPU disable helper into its own commmit. Same for
transplating shrinker initialisation and unplug.
- Added 'Fixes' and 'Reported-by' tags to all the relevant commits
- [1] https://sashiko.dev/#/patchset/20260526-claude-fixes-v1-0-16e92eaa4949%40collabora.com
- Link to v1: https://patch.msgid.link/20260526-claude-fixes-v1-0-16e92eaa4949@collabora.com

To: Boris Brezillon <boris.brezillon@collabora.com>
To: Rob Herring <robh@kernel.org>
To: Steven Price <steven.price@arm.com>
To: Adrián Larumbe <adrian.larumbe@collabora.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Faith Ekstrand <faith.ekstrand@collabora.com>
To: "Marty E. Plummer" <hanetzer@startmail.com>
To: Tomeu Vizoso <tomeu@tomeuvizoso.net>
To: Eric Anholt <eric@anholt.net>
To: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
To: Robin Murphy <robin.murphy@arm.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: Neil Armstrong <neil.armstrong@linaro.org>

---
Adrián Larumbe (13):
      drm/panfrost: Check another bo field for cache option query
      drm/panfrost: Prevent division by 0
      drm/panfrost: Remove unnecessary header file include
      drm/panfrost: Move shrinker initialization and unplug one level down
      drm/panfrost: Move all device power up and down into RPM callbacks
      drm/panfrost: Explicitly enable MMU interrupts at device init
      drm/panfrost: Sync with IRQ before MMU disable and reset
      drm/panfrost: Rewire reset sequence to avoid concurrent attempts
      drm/panfrost: Add debugfs knob for manually triggering a GPU reset
      drm/panfrost: Move perfcnt GPU disable sequence into a helper
      drm/panfrost: Introduce a reset lock
      drm/panfrost: Fix races between perfcnt and reset sequence
      drm/panfrost: Bump driver minor to reflect new DUMP IOCTL req field

 drivers/gpu/drm/panfrost/panfrost_devfreq.c      |   2 +-
 drivers/gpu/drm/panfrost/panfrost_device.c       | 400 +++++++++++++++--------
 drivers/gpu/drm/panfrost/panfrost_device.h       |  18 +-
 drivers/gpu/drm/panfrost/panfrost_drv.c          |  31 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c          |  19 +-
 drivers/gpu/drm/panfrost/panfrost_gem.h          |   6 +-
 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |   8 +-
 drivers/gpu/drm/panfrost/panfrost_gpu.c          |   7 +-
 drivers/gpu/drm/panfrost/panfrost_job.c          |  19 +-
 drivers/gpu/drm/panfrost/panfrost_mmu.c          |  25 +-
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c      | 236 ++++++++-----
 drivers/gpu/drm/panfrost/panfrost_perfcnt.h      |   2 +
 include/uapi/drm/panfrost_drm.h                  |   3 +-
 13 files changed, 498 insertions(+), 278 deletions(-)
---
base-commit: 671b7825dbfe9ea6e3ad3001003aeee0df48d1b5
change-id: 20260523-claude-fixes-82b03a134a8c

Best regards,
--  
Adrián Larumbe <adrian.larumbe@collabora.com>

[PATCH v4 01/13] drm/panfrost: Check another bo field for cache option query

[Adrián Larumbe]

When querying the properties of a BO, whether it was created in
Writeback caching mode for userspace mappings, is recorded in a Panfrost
BO's wb_mmap field.

Reported-by: Claude <noreply@anthropic.com>
Closes: https://gitlab.freedesktop.org/panfrost/linux/-/work_items/88
Fixes: 62eedf1ccba5 ("drm/panfrost: Add flag to map GEM object Write-Back Cacheable")
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 784e36d72c2b..2e931fa90da0 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -661,7 +661,7 @@ static int panfrost_ioctl_query_bo_info(struct drm_device *dev, void *data,
 		if (bo->is_heap)
 			args->create_flags |= PANFROST_BO_HEAP;
 
-		if (!bo->base.map_wc)
+		if (bo->wb_mmap)
 			args->create_flags |= PANFROST_BO_WB_MMAP;
 	}
 

-- 
2.55.0

[PATCH v4 02/13] drm/panfrost: Prevent division by 0

[Adrián Larumbe]

When updating and debug-printing devfreq stats, in the very unlikely
off-chance that total device time is less than 100 ns, division by 0
would happen. Fix the divisor when relevant and rearrange operands.

Reported-by: Claude <noreply@anthropic.com>
Closes: https://gitlab.freedesktop.org/panfrost/linux/-/work_items/88
Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index b51c30778811..0fe81d259274 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -78,7 +78,7 @@ static int panfrost_devfreq_get_dev_status(struct device *dev,
 
 	dev_dbg(pfdev->base.dev, "busy %lu total %lu %lu %% freq %lu MHz\n",
 		status->busy_time, status->total_time,
-		status->busy_time / (status->total_time / 100),
+		status->busy_time * 100 / MAX(status->total_time, 1),
 		status->current_frequency / 1000 / 1000);
 
 	return 0;

-- 
2.55.0

[PATCH v4 03/13] drm/panfrost: Remove unnecessary header file include

[Adrián Larumbe]

None of panfrost_features.h definitions are used in panfrost_device.c

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index 7fed22d555a5..d8421fd6a662 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -11,7 +11,6 @@
 
 #include "panfrost_device.h"
 #include "panfrost_devfreq.h"
-#include "panfrost_features.h"
 #include "panfrost_gem.h"
 #include "panfrost_issues.h"
 #include "panfrost_gpu.h"

-- 
2.55.0

[PATCH v4 04/13] drm/panfrost: Move shrinker initialization and unplug one level down

[Adrián Larumbe]

Since the moment we call drm_dev_register() the device should be in a
position to accept jobs, so it's best if the shrinker is already
initialized by then.

On top of that, make shrinker functions take an panfrost_device pointer
like other functions in the same sequence and rename them accordingly.

Essentially mimic the init/fini behaviour in Panthor.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c       |  8 +++++++-
 drivers/gpu/drm/panfrost/panfrost_drv.c          |  6 ------
 drivers/gpu/drm/panfrost/panfrost_gem.c          | 19 ++++++++++---------
 drivers/gpu/drm/panfrost/panfrost_gem.h          |  6 +++---
 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |  8 ++------
 5 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index d8421fd6a662..03905495aee2 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -281,9 +281,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
 	if (err)
 		goto out_job;
 
-	panfrost_gem_init(pfdev);
+	err = panfrost_gem_init(pfdev);
+	if (err)
+		goto out_perfcnt;
 
 	return 0;
+
+out_perfcnt:
+	panfrost_perfcnt_fini(pfdev);
 out_job:
 	panfrost_jm_fini(pfdev);
 out_mmu:
@@ -305,6 +310,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
 
 void panfrost_device_fini(struct panfrost_device *pfdev)
 {
+	panfrost_gem_shrinker_fini(pfdev);
 	panfrost_perfcnt_fini(pfdev);
 	panfrost_jm_fini(pfdev);
 	panfrost_mmu_fini(pfdev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 2e931fa90da0..331a3bd5b98c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -1002,14 +1002,9 @@ static int panfrost_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_out1;
 
-	err = panfrost_gem_shrinker_init(&pfdev->base);
-	if (err)
-		goto err_out2;
 
 	return 0;
 
-err_out2:
-	drm_dev_unregister(&pfdev->base);
 err_out1:
 	pm_runtime_disable(pfdev->base.dev);
 	panfrost_device_fini(pfdev);
@@ -1023,7 +1018,6 @@ static void panfrost_remove(struct platform_device *pdev)
 	struct panfrost_device *pfdev = platform_get_drvdata(pdev);
 
 	drm_dev_unregister(&pfdev->base);
-	panfrost_gem_shrinker_cleanup(&pfdev->base);
 
 	pm_runtime_get_sync(pfdev->base.dev);
 	pm_runtime_disable(pfdev->base.dev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 3a7fce428898..aa3a604b3439 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -15,20 +15,21 @@
 #include "panfrost_gem.h"
 #include "panfrost_mmu.h"
 
-void panfrost_gem_init(struct panfrost_device *pfdev)
+int panfrost_gem_init(struct panfrost_device *pfdev)
 {
 	int err;
 
 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
-	    !panfrost_transparent_hugepage)
-		return;
+	    panfrost_transparent_hugepage) {
+		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
+		if (drm_gem_get_huge_mnt(&pfdev->base))
+			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
+		else if (err)
+			drm_warn(&pfdev->base,
+				 "Can't use Transparent Hugepage (%d)\n", err);
+	}
 
-	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
-	if (drm_gem_get_huge_mnt(&pfdev->base))
-		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
-	else if (err)
-		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
-			 err);
+	return panfrost_gem_shrinker_init(pfdev);
 }
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
index 79d4377019e9..d6e55c2e4d86 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -129,7 +129,7 @@ drm_mm_node_to_panfrost_mapping(struct drm_mm_node *node)
 	return container_of(node, struct panfrost_gem_mapping, mmnode);
 }
 
-void panfrost_gem_init(struct panfrost_device *pfdev);
+int panfrost_gem_init(struct panfrost_device *pfdev);
 
 struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
 
@@ -154,8 +154,8 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
 void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
 void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
 
-int panfrost_gem_shrinker_init(struct drm_device *dev);
-void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
+int panfrost_gem_shrinker_init(struct panfrost_device *pfdev);
+void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev);
 
 void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
 int panfrost_gem_sync(struct drm_gem_object *obj, u32 type,
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
index 2fe967a90bcb..fefae87535d6 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
@@ -95,10 +95,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
  *
  * This function registers and sets up the panfrost shrinker.
  */
-int panfrost_gem_shrinker_init(struct drm_device *dev)
+int panfrost_gem_shrinker_init(struct panfrost_device *pfdev)
 {
-	struct panfrost_device *pfdev = to_panfrost_device(dev);
-
 	pfdev->shrinker = shrinker_alloc(0, "drm-panfrost");
 	if (!pfdev->shrinker)
 		return -ENOMEM;
@@ -118,10 +116,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev)
  *
  * This function unregisters the panfrost shrinker.
  */
-void panfrost_gem_shrinker_cleanup(struct drm_device *dev)
+void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev)
 {
-	struct panfrost_device *pfdev = to_panfrost_device(dev);
-
 	if (pfdev->shrinker)
 		shrinker_free(pfdev->shrinker);
 }

-- 
2.55.0

[PATCH v4 05/13] drm/panfrost: Move all device power up and down into RPM callbacks

[Adrián Larumbe]

During device probe(), failure to do a PM get() will leave the usage_count
set to 0, which is the value assigned at device creation time. That means
when the autosuspend delay expires, runtime suspend callback won't be
invoked, so the device will remain powered on forever.

On top of that, failure to call PM put() during device unplug means
Panfrost device's PM usage_count increases monotonically for every new
module reload.

The combined outcome of both of the above was that devfreq OPP transition
notifications would be printed all the time, even when no jobs are being
submitted. This quickly fills the kernel ring buffer with junk.

Even direr than that was the fact MMU interrupts are only enabled when
the device is reset, so after device probe() the very first job targeting
the tiler heap BO would always time out, because the driver's PM runtime
resume callback would not be invoked.

Fix all that by moving all GPU enabling and disabling into RPM resume and
suspend callbacks, and making sure we resume the device right before
touching any HW registers. This is done in imitation of the Panthor model.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 375 +++++++++++++++++------------
 drivers/gpu/drm/panfrost/panfrost_device.h |   9 +
 drivers/gpu/drm/panfrost/panfrost_drv.c    |  10 +-
 drivers/gpu/drm/panfrost/panfrost_gpu.c    |   7 +-
 drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
 5 files changed, 237 insertions(+), 171 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index 03905495aee2..a9d29d9ee22b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -34,6 +34,40 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
 	reset_control_assert(pfdev->rstc);
 }
 
+static int panfrost_clk_enable(struct panfrost_device *pfdev)
+{
+	int err;
+
+	err = clk_enable(pfdev->clock);
+	if (err)
+		return err;
+
+	err = clk_enable(pfdev->bus_clock);
+	if (err)
+		goto disable_clock;
+
+	err = clk_enable(pfdev->bus_ace_clock);
+	if (err)
+		goto disable_bus_clock;
+
+	return 0;
+
+disable_bus_clock:
+	clk_disable(pfdev->bus_clock);
+disable_clock:
+	clk_disable(pfdev->clock);
+
+	return err;
+}
+
+static void panfrost_clk_disable(struct panfrost_device *pfdev)
+{
+	clk_disable(pfdev->bus_ace_clock);
+	clk_disable(pfdev->bus_clock);
+	clk_disable(pfdev->clock);
+	reset_control_assert(pfdev->rstc);
+}
+
 static int panfrost_clk_init(struct panfrost_device *pfdev)
 {
 	int err;
@@ -48,7 +82,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
 	rate = clk_get_rate(pfdev->clock);
 	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
 
-	err = clk_prepare_enable(pfdev->clock);
+	err = clk_prepare(pfdev->clock);
 	if (err)
 		return err;
 
@@ -57,44 +91,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
 		dev_err(pfdev->base.dev, "get bus_clock failed %ld\n",
 			PTR_ERR(pfdev->bus_clock));
 		err = PTR_ERR(pfdev->bus_clock);
-		goto disable_clock;
+		goto unprepare_clock;
 	}
 
 	if (pfdev->bus_clock) {
 		rate = clk_get_rate(pfdev->bus_clock);
 		dev_info(pfdev->base.dev, "bus_clock rate = %lu\n", rate);
 
-		err = clk_prepare_enable(pfdev->bus_clock);
+		err = clk_prepare(pfdev->bus_clock);
 		if (err)
-			goto disable_clock;
+			goto unprepare_clock;
 	}
 
 	pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, "bus_ace");
 	if (IS_ERR(pfdev->bus_ace_clock)) {
 		err = PTR_ERR(pfdev->bus_ace_clock);
 		dev_err(pfdev->base.dev, "get bus_ace_clock failed %d\n", err);
-		goto disable_bus_clock;
+		goto unprepare_bus_clock;
 	}
 
-	err = clk_prepare_enable(pfdev->bus_ace_clock);
+	err = clk_prepare(pfdev->bus_ace_clock);
 	if (err)
-		goto disable_bus_clock;
+		goto unprepare_bus_clock;
+
+	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT))) {
+		err = panfrost_clk_enable(pfdev);
+		if (err)
+			goto unprepare_bus_ace_clock;
+	}
 
 	return 0;
 
-disable_bus_clock:
-	clk_disable_unprepare(pfdev->bus_clock);
-disable_clock:
-	clk_disable_unprepare(pfdev->clock);
+unprepare_bus_ace_clock:
+	clk_unprepare(pfdev->bus_ace_clock);
+unprepare_bus_clock:
+	clk_unprepare(pfdev->bus_clock);
+unprepare_clock:
+	clk_unprepare(pfdev->clock);
 
 	return err;
 }
 
 static void panfrost_clk_fini(struct panfrost_device *pfdev)
 {
-	clk_disable_unprepare(pfdev->bus_ace_clock);
-	clk_disable_unprepare(pfdev->bus_clock);
-	clk_disable_unprepare(pfdev->clock);
+	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
+		panfrost_clk_disable(pfdev);
+
+	clk_unprepare(pfdev->bus_ace_clock);
+	clk_unprepare(pfdev->bus_clock);
+	clk_unprepare(pfdev->clock);
 }
 
 static int panfrost_regulator_init(struct panfrost_device *pfdev)
@@ -212,6 +257,133 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
 	return err;
 }
 
+static int panfrost_device_runtime_resume(struct device *dev)
+{
+	struct panfrost_device *pfdev = dev_get_drvdata(dev);
+	int ret;
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
+		ret = reset_control_deassert(pfdev->rstc);
+		if (ret)
+			return ret;
+
+		ret = panfrost_clk_enable(pfdev);
+		if (ret)
+			goto err_clk;
+	}
+
+	panfrost_device_reset(pfdev, true);
+	panfrost_devfreq_resume(pfdev);
+
+	return 0;
+
+err_clk:
+	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
+		reset_control_assert(pfdev->rstc);
+	return ret;
+}
+
+static int panfrost_device_runtime_suspend(struct device *dev)
+{
+	struct panfrost_device *pfdev = dev_get_drvdata(dev);
+
+	if (!panfrost_jm_is_idle(pfdev))
+		return -EBUSY;
+
+	panfrost_devfreq_suspend(pfdev);
+	panfrost_jm_suspend_irq(pfdev);
+	panfrost_mmu_suspend_irq(pfdev);
+	panfrost_gpu_suspend_irq(pfdev);
+	panfrost_gpu_power_off(pfdev);
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
+		panfrost_clk_disable(pfdev);
+
+	return 0;
+}
+
+static int panfrost_device_resume(struct device *dev)
+{
+	struct panfrost_device *pfdev = dev_get_drvdata(dev);
+	int ret;
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
+		unsigned long freq = pfdev->pfdevfreq.fast_rate;
+		struct dev_pm_opp *opp;
+
+		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+		if (IS_ERR(opp))
+			return PTR_ERR(opp);
+		dev_pm_opp_set_opp(dev, opp);
+		dev_pm_opp_put(opp);
+	}
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
+		ret = clk_enable(pfdev->clock);
+		if (ret)
+			goto err_clk;
+
+		if (pfdev->bus_clock) {
+			ret = clk_enable(pfdev->bus_clock);
+			if (ret)
+				goto err_bus_clk;
+		}
+	}
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		goto err_resume;
+
+	return 0;
+
+err_resume:
+	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
+		clk_disable(pfdev->bus_clock);
+err_bus_clk:
+	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
+		clk_disable(pfdev->clock);
+err_clk:
+	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
+		dev_pm_opp_set_opp(dev, NULL);
+	return ret;
+}
+
+static int panfrost_device_suspend(struct device *dev)
+{
+	struct panfrost_device *pfdev = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
+		if (pfdev->bus_clock)
+			clk_disable(pfdev->bus_clock);
+
+		clk_disable(pfdev->clock);
+	}
+
+	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
+		dev_pm_opp_set_opp(dev, NULL);
+
+	return 0;
+}
+
+EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
+	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
+};
+
+void panfrost_try_suspend_device(struct panfrost_device *pfdev)
+{
+	pm_runtime_put_sync_suspend(pfdev->base.dev);
+
+	/* If PM is disabled, we need to call the suspend handler manually. */
+	if (!IS_ENABLED(CONFIG_PM))
+		panfrost_device_runtime_suspend(pfdev->base.dev);
+}
+
 int panfrost_device_init(struct panfrost_device *pfdev)
 {
 	int err;
@@ -265,10 +437,25 @@ int panfrost_device_init(struct panfrost_device *pfdev)
 		goto out_regulator;
 	}
 
-	err = panfrost_gpu_init(pfdev);
+	err = devm_pm_runtime_enable(pfdev->base.dev);
 	if (err)
 		goto out_regulator;
 
+	err = pm_runtime_resume_and_get(pfdev->base.dev);
+	if (err)
+		goto out_regulator;
+
+	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
+	if (!IS_ENABLED(CONFIG_PM)) {
+		err = panfrost_device_runtime_resume(pfdev->base.dev);
+		if (err)
+			goto out_regulator;
+	}
+
+	err = panfrost_gpu_init(pfdev);
+	if (err)
+		goto out_rpm_put;
+
 	err = panfrost_mmu_init(pfdev);
 	if (err)
 		goto out_gpu;
@@ -295,6 +482,8 @@ int panfrost_device_init(struct panfrost_device *pfdev)
 	panfrost_mmu_fini(pfdev);
 out_gpu:
 	panfrost_gpu_fini(pfdev);
+out_rpm_put:
+	panfrost_try_suspend_device(pfdev);
 out_regulator:
 	panfrost_regulator_fini(pfdev);
 out_devfreq:
@@ -313,12 +502,10 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
 	panfrost_gem_shrinker_fini(pfdev);
 	panfrost_perfcnt_fini(pfdev);
 	panfrost_jm_fini(pfdev);
-	panfrost_mmu_fini(pfdev);
-	panfrost_gpu_fini(pfdev);
-	panfrost_devfreq_fini(pfdev);
+	panfrost_try_suspend_device(pfdev);
 	panfrost_regulator_fini(pfdev);
+	panfrost_devfreq_fini(pfdev);
 	panfrost_clk_fini(pfdev);
-	panfrost_reset_fini(pfdev);
 	panfrost_pm_domain_fini(pfdev);
 }
 
@@ -424,148 +611,20 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,
 
 void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 {
-	panfrost_gpu_soft_reset(pfdev);
-
-	panfrost_gpu_power_on(pfdev);
-	panfrost_mmu_reset(pfdev);
-
-	panfrost_jm_reset_interrupts(pfdev);
-	if (enable_job_int)
-		panfrost_jm_enable_interrupts(pfdev);
-}
-
-static int panfrost_device_runtime_resume(struct device *dev)
-{
-	struct panfrost_device *pfdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
-		ret = reset_control_deassert(pfdev->rstc);
-		if (ret)
-			return ret;
-
-		ret = clk_enable(pfdev->clock);
-		if (ret)
-			goto err_clk;
-
-		ret = clk_enable(pfdev->bus_clock);
-		if (ret)
-			goto err_bus_clk;
-
-		ret = clk_enable(pfdev->bus_ace_clock);
-		if (ret)
-			goto err_bus_ace_clk;
-	}
-
-	panfrost_device_reset(pfdev, true);
-	panfrost_devfreq_resume(pfdev);
-
-	return 0;
-
-err_bus_ace_clk:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
-		clk_disable(pfdev->bus_clock);
-err_bus_clk:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
-		clk_disable(pfdev->clock);
-err_clk:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
-		reset_control_assert(pfdev->rstc);
-	return ret;
-}
-
-static int panfrost_device_runtime_suspend(struct device *dev)
-{
-	struct panfrost_device *pfdev = dev_get_drvdata(dev);
-
-	if (!panfrost_jm_is_idle(pfdev))
-		return -EBUSY;
-
-	panfrost_devfreq_suspend(pfdev);
-	panfrost_jm_suspend_irq(pfdev);
-	panfrost_mmu_suspend_irq(pfdev);
-	panfrost_gpu_suspend_irq(pfdev);
-	panfrost_gpu_power_off(pfdev);
-
-	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
-		clk_disable(pfdev->bus_ace_clock);
-		clk_disable(pfdev->bus_clock);
-		clk_disable(pfdev->clock);
-		reset_control_assert(pfdev->rstc);
-	}
-
-	return 0;
-}
-
-static int panfrost_device_resume(struct device *dev)
-{
-	struct panfrost_device *pfdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
-		unsigned long freq = pfdev->pfdevfreq.fast_rate;
-		struct dev_pm_opp *opp;
-
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp))
-			return PTR_ERR(opp);
-		dev_pm_opp_set_opp(dev, opp);
-		dev_pm_opp_put(opp);
-	}
-
-	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
-		ret = clk_enable(pfdev->clock);
-		if (ret)
-			goto err_clk;
-
-		if (pfdev->bus_clock) {
-			ret = clk_enable(pfdev->bus_clock);
-			if (ret)
-				goto err_bus_clk;
-		}
-	}
-
-	ret = pm_runtime_force_resume(dev);
-	if (ret)
-		goto err_resume;
-
-	return 0;
-
-err_resume:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
-		clk_disable(pfdev->bus_clock);
-err_bus_clk:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
-		clk_disable(pfdev->clock);
-err_clk:
-	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
-		dev_pm_opp_set_opp(dev, NULL);
-	return ret;
-}
-
-static int panfrost_device_suspend(struct device *dev)
-{
-	struct panfrost_device *pfdev = dev_get_drvdata(dev);
 	int ret;
 
-	ret = pm_runtime_force_suspend(dev);
-	if (ret)
-		return ret;
-
-	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
-		if (pfdev->bus_clock)
-			clk_disable(pfdev->bus_clock);
+	/* Do the actual device reset */
+	ret = panfrost_gpu_soft_reset(pfdev);
+	panfrost_gpu_power_on(pfdev);
 
-		clk_disable(pfdev->clock);
+	/* Post-reset */
+	if (panfrost_device_started(pfdev)) {
+		panfrost_mmu_reset(pfdev);
+		panfrost_jm_reset_interrupts(pfdev);
+		if (enable_job_int)
+			panfrost_jm_enable_interrupts(pfdev);
 	}
 
-	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
-		dev_pm_opp_set_opp(dev, NULL);
-
-	return 0;
+	if (!panfrost_device_started(pfdev) && !ret)
+		pfdev->gpu_started = true;
 }
-
-EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
-	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
-	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
-};
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index ec55c136b1b6..eeca57efcbec 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -142,6 +142,7 @@ struct panfrost_device {
 	/* pm_domains for devices with more than one. */
 	struct device *pm_domain_devs[MAX_PM_DOMAINS];
 	struct device_link *pm_domain_links[MAX_PM_DOMAINS];
+	bool gpu_started;
 	bool coherent;
 
 	struct panfrost_features features;
@@ -253,6 +254,8 @@ int panfrost_device_init(struct panfrost_device *pfdev);
 void panfrost_device_fini(struct panfrost_device *pfdev);
 void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int);
 
+void panfrost_try_suspend_device(struct panfrost_device *pfdev);
+
 extern const struct dev_pm_ops panfrost_pm_ops;
 
 enum drm_panfrost_exception_type {
@@ -342,4 +345,10 @@ panfrost_device_schedule_reset(struct panfrost_device *pfdev)
 	queue_work(pfdev->reset.wq, &pfdev->reset.work);
 }
 
+static inline bool
+panfrost_device_started(struct panfrost_device *pfdev)
+{
+	return pfdev->gpu_started;
+}
+
 #endif
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 331a3bd5b98c..27f4c4b217f5 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -988,9 +988,7 @@ static int panfrost_probe(struct platform_device *pdev)
 		goto err_out0;
 	}
 
-	pm_runtime_set_active(pfdev->base.dev);
 	pm_runtime_mark_last_busy(pfdev->base.dev);
-	pm_runtime_enable(pfdev->base.dev);
 	pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */
 	pm_runtime_use_autosuspend(pfdev->base.dev);
 
@@ -1002,13 +1000,12 @@ static int panfrost_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_out1;
 
+	pm_runtime_put_autosuspend(pfdev->base.dev);
 
 	return 0;
 
 err_out1:
-	pm_runtime_disable(pfdev->base.dev);
 	panfrost_device_fini(pfdev);
-	pm_runtime_set_suspended(pfdev->base.dev);
 err_out0:
 	return err;
 }
@@ -1019,10 +1016,9 @@ static void panfrost_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(&pfdev->base);
 
-	pm_runtime_get_sync(pfdev->base.dev);
-	pm_runtime_disable(pfdev->base.dev);
+	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);
+
 	panfrost_device_fini(pfdev);
-	pm_runtime_set_suspended(pfdev->base.dev);
 }
 
 static ssize_t profiling_show(struct device *dev,
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 7d555e63e21a..416964282138 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -516,9 +516,8 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
 {
 	int err;
 
-	err = panfrost_gpu_soft_reset(pfdev);
-	if (err)
-		return err;
+	if (!panfrost_device_started(pfdev))
+		return -ETIMEDOUT;
 
 	err = panfrost_gpu_init_features(pfdev);
 	if (err)
@@ -543,8 +542,6 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
 		return err;
 	}
 
-	panfrost_gpu_power_on(pfdev);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index 2d12b83e900a..35ff5b0f0013 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -446,7 +446,9 @@ void panfrost_jm_suspend_irq(struct panfrost_device *pfdev)
 	set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended);
 
 	job_write(pfdev, JOB_INT_MASK, 0);
-	synchronize_irq(pfdev->js->irq);
+
+	if (pfdev->js)
+		synchronize_irq(pfdev->js->irq);
 }
 
 static void panfrost_job_handle_err(struct panfrost_device *pfdev,
@@ -971,6 +973,9 @@ int panfrost_jm_is_idle(struct panfrost_device *pfdev)
 	struct panfrost_job_slot *js = pfdev->js;
 	int i;
 
+	if (!js)
+		return true;
+
 	for (i = 0; i < NUM_JOB_SLOTS; i++) {
 		/* If there are any jobs in the HW queue, we're not idle */
 		if (atomic_read(&js->queue[i].sched.credit_count))

-- 
2.55.0

[PATCH v4 06/13] drm/panfrost: Explicitly enable MMU interrupts at device init

[Adrián Larumbe]

Because the device must be in a position to accept jobs between the time
drm_dev_register() is called and autosuspend first kicks in, there's a very
narrow window inbetween during which jobs targeting the tiler buffer
object would time out, since the device's PM status is 'Active', but no MMU
interrupts were enabled at device initialisation time.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
Fixes: 73e467f60acd ("drm/panfrost: Consolidate reset handling")
---
 drivers/gpu/drm/panfrost/panfrost_job.c |  3 ++-
 drivers/gpu/drm/panfrost/panfrost_mmu.c | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index 35ff5b0f0013..bda1494eb430 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -871,7 +871,6 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
 	js = devm_kzalloc(pfdev->base.dev, sizeof(*js), GFP_KERNEL);
 	if (!js)
 		return -ENOMEM;
-	pfdev->js = js;
 
 	INIT_WORK(&pfdev->reset.work, panfrost_reset_work);
 	spin_lock_init(&js->job_lock);
@@ -906,6 +905,8 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
 		}
 	}
 
+	pfdev->js = js;
+
 	panfrost_jm_reset_interrupts(pfdev);
 	panfrost_jm_enable_interrupts(pfdev);
 
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 4a3162c3b659..aad0cd31516d 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -336,6 +336,12 @@ void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu
 	WARN_ON(atomic_read(&mmu->as_count) < 0);
 }
 
+static void panfrost_mmu_enable_interrupts(struct panfrost_device *pfdev)
+{
+	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
+	mmu_write(pfdev, MMU_INT_MASK, ~0);
+}
+
 void panfrost_mmu_reset(struct panfrost_device *pfdev)
 {
 	struct panfrost_mmu *mmu, *mmu_tmp;
@@ -355,8 +361,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
 
 	spin_unlock(&pfdev->as_lock);
 
-	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
-	mmu_write(pfdev, MMU_INT_MASK, ~0);
+	panfrost_mmu_enable_interrupts(pfdev);
 }
 
 static size_t get_pgsize(u64 addr, size_t size, size_t *count)
@@ -880,6 +885,9 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
 	u32 status = mmu_read(pfdev, MMU_INT_RAWSTAT);
 	int ret;
 
+	if (!pfdev->js)
+		return IRQ_NONE;
+
 	while (status) {
 		u32 as = ffs(status | (status >> 16)) - 1;
 		u32 mask = BIT(as) | BIT(as + 16);
@@ -970,6 +978,8 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
 		return err;
 	}
 
+	panfrost_mmu_enable_interrupts(pfdev);
+
 	return 0;
 }
 

-- 
2.55.0

[PATCH v4 07/13] drm/panfrost: Sync with IRQ before MMU disable and reset

[Adrián Larumbe]

There was a pitfall of the MMU threaded IRQ handler accessing HW
registers after a spurious interrupt at the same time the device is
being premature taking down because of a failed probe sequence.

Let's make sure MMU IRQ's are marked suspended and synchronised with
before unwinding further.

Also, during a device reset, make sure we also sync with the MMU IRQ
handler so that AS aren't clobbered while a map operation is ongoing.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index aad0cd31516d..2d0ef6575f06 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -347,6 +347,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
 	struct panfrost_mmu *mmu, *mmu_tmp;
 
 	clear_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended);
+	synchronize_irq(pfdev->mmu_irq);
 
 	spin_lock(&pfdev->as_lock);
 
@@ -983,11 +984,6 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
 	return 0;
 }
 
-void panfrost_mmu_fini(struct panfrost_device *pfdev)
-{
-	mmu_write(pfdev, MMU_INT_MASK, 0);
-}
-
 void panfrost_mmu_suspend_irq(struct panfrost_device *pfdev)
 {
 	set_bit(PANFROST_COMP_BIT_MMU, pfdev->is_suspended);
@@ -995,3 +991,8 @@ void panfrost_mmu_suspend_irq(struct panfrost_device *pfdev)
 	mmu_write(pfdev, MMU_INT_MASK, 0);
 	synchronize_irq(pfdev->mmu_irq);
 }
+
+void panfrost_mmu_fini(struct panfrost_device *pfdev)
+{
+	panfrost_mmu_suspend_irq(pfdev);
+}

-- 
2.55.0

[PATCH v4 08/13] drm/panfrost: Rewire reset sequence to avoid concurrent attempts

[Adrián Larumbe]

This way, and in imitation of Panthor, only a single reset thread can
progress at a time, and other threads wanting to trigger it just wait
on the ongoing one to finish and wake them up.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 1 +
 drivers/gpu/drm/panfrost/panfrost_device.h | 5 +++--
 drivers/gpu/drm/panfrost/panfrost_job.c    | 7 ++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index a9d29d9ee22b..a51f1ef8563c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -8,6 +8,7 @@
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/wait.h>
 
 #include "panfrost_device.h"
 #include "panfrost_devfreq.h"
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index eeca57efcbec..f0acfc634cd4 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -167,6 +167,7 @@ struct panfrost_device {
 	struct {
 		struct workqueue_struct *wq;
 		struct work_struct work;
+		wait_queue_head_t wait;
 		atomic_t pending;
 	} reset;
 
@@ -341,8 +342,8 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,
 static inline void
 panfrost_device_schedule_reset(struct panfrost_device *pfdev)
 {
-	atomic_set(&pfdev->reset.pending, 1);
-	queue_work(pfdev->reset.wq, &pfdev->reset.work);
+	if (!atomic_cmpxchg(&pfdev->reset.pending, 0, 1))
+		queue_work(pfdev->reset.wq, &pfdev->reset.work);
 }
 
 static inline bool
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index bda1494eb430..ed8a1fabd47b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -499,10 +499,8 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev,
 
 	pm_runtime_put_autosuspend(pfdev->base.dev);
 
-	if (panfrost_exception_needs_reset(pfdev, js_status)) {
-		atomic_set(&pfdev->reset.pending, 1);
+	if (panfrost_exception_needs_reset(pfdev, js_status))
 		drm_sched_fault(&pfdev->js->queue[js].sched);
-	}
 }
 
 static void panfrost_jm_handle_done(struct panfrost_device *pfdev,
@@ -757,6 +755,8 @@ panfrost_reset(struct panfrost_device *pfdev,
 	panfrost_jm_enable_interrupts(pfdev);
 
 	dma_fence_end_signalling(cookie);
+
+	wake_up_all(&pfdev->reset.wait);
 }
 
 static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job
@@ -893,6 +893,7 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
 	if (!pfdev->reset.wq)
 		return -ENOMEM;
 	args.timeout_wq = pfdev->reset.wq;
+	init_waitqueue_head(&pfdev->reset.wait);
 
 	for (j = 0; j < NUM_JOB_SLOTS; j++) {
 		js->queue[j].fence_context = dma_fence_context_alloc(1);

-- 
2.55.0

[PATCH v4 09/13] drm/panfrost: Add debugfs knob for manually triggering a GPU reset

[Adrián Larumbe]

This will be of great help when testing potential races between the GPU
reset sequence and other parts of the code accessing HW registers.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 41 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_device.h |  3 +++
 drivers/gpu/drm/panfrost/panfrost_drv.c    |  1 +
 3 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index a51f1ef8563c..c0334b72c70b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -2,6 +2,7 @@
 /* Copyright 2018 Marty E. Plummer <hanetzer@startmail.com> */
 /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */
 
+#include <linux/debugfs.h>
 #include <linux/clk.h>
 #include <linux/reset.h>
 #include <linux/platform_device.h>
@@ -629,3 +630,43 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 	if (!panfrost_device_started(pfdev) && !ret)
 		pfdev->gpu_started = true;
 }
+
+#ifdef CONFIG_DEBUG_FS
+static int reset_get(void *data, u64 *val)
+{
+	struct panfrost_device *pfdev =
+		container_of(data, struct panfrost_device, base);
+
+	*val = atomic_read(&pfdev->reset.pending);
+	return 0;
+}
+
+static int reset_set(void *data, u64 val)
+{
+	struct panfrost_device *pfdev =
+		container_of(data, struct panfrost_device, base);
+	int ret;
+
+	ret = pm_runtime_get_if_in_use(pfdev->base.dev);
+
+	if (ret > 0) {
+		panfrost_device_schedule_reset(pfdev);
+		ret = wait_event_interruptible_timeout(pfdev->reset.wait,
+						       !atomic_read(&pfdev->reset.pending),
+						       msecs_to_jiffies(60));
+		pm_runtime_put(pfdev->base.dev);
+	}
+
+	return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(panfrost_reset_debugfs_fops,
+			 reset_get, reset_set,
+			 "0x%08llx\n");
+
+void panfrost_reset_debugfs_init(struct drm_minor *minor)
+{
+	debugfs_create_file("reset", 0600, minor->debugfs_root,
+			    minor->dev, &panfrost_reset_debugfs_fops);
+}
+#endif // CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index f0acfc634cd4..f96ba50dfa91 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -352,4 +352,7 @@ panfrost_device_started(struct panfrost_device *pfdev)
 	return pfdev->gpu_started;
 }
 
+#ifdef CONFIG_DEBUG_FS
+void panfrost_reset_debugfs_init(struct drm_minor *minor);
+#endif // CONFIG_DEBUG_FS
 #endif
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 27f4c4b217f5..fac5be20ad60 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -916,6 +916,7 @@ static void panfrost_debugfs_init(struct drm_minor *minor)
 {
 	panthor_gems_debugfs_init(minor);
 	panfrost_sched_debugfs_init(minor);
+	panfrost_reset_debugfs_init(minor);
 }
 #endif
 

-- 
2.55.0

[PATCH v4 10/13] drm/panfrost: Move perfcnt GPU disable sequence into a helper

[Adrián Larumbe]

Just for the sake of avoiding repetition.

Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 31 ++++++++++++-----------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
index 7020c0192e18..ad1156678e91 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
@@ -35,6 +35,16 @@ struct panfrost_perfcnt {
 	struct completion dump_comp;
 };
 
+static void panfrost_perfcnt_gpu_disable(struct panfrost_device *pfdev)
+{
+	gpu_write(pfdev, GPU_PERFCNT_CFG,
+		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
+	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
+	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0);
+	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0);
+	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
+}
+
 void panfrost_perfcnt_clean_cache_done(struct panfrost_device *pfdev)
 {
 	complete(&pfdev->perfcnt->dump_comp);
@@ -193,12 +203,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
 	if (user != perfcnt->user)
 		return -EINVAL;
 
-	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
-	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0);
-	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0);
-	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
-	gpu_write(pfdev, GPU_PERFCNT_CFG,
-		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
+	panfrost_perfcnt_gpu_disable(pfdev);
 
 	perfcnt->user = NULL;
 	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
@@ -327,12 +332,7 @@ int panfrost_perfcnt_init(struct panfrost_device *pfdev)
 	perfcnt->bosize = size;
 
 	/* Start with everything disabled. */
-	gpu_write(pfdev, GPU_PERFCNT_CFG,
-		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
-	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
+	panfrost_perfcnt_gpu_disable(pfdev);
 
 	init_completion(&perfcnt->dump_comp);
 	mutex_init(&perfcnt->lock);
@@ -344,10 +344,5 @@ int panfrost_perfcnt_init(struct panfrost_device *pfdev)
 void panfrost_perfcnt_fini(struct panfrost_device *pfdev)
 {
 	/* Disable everything before leaving. */
-	gpu_write(pfdev, GPU_PERFCNT_CFG,
-		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
-	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0);
-	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
+	panfrost_perfcnt_gpu_disable(pfdev);
 }

-- 
2.55.0

[PATCH v4 11/13] drm/panfrost: Introduce a reset lock

[Adrián Larumbe]

So as to avoid accessing HW register while a reset is ongoing, a read/write
semaphore that envelopes the reset sequence will help driver entry points
avoid racing with it. For now, the only such racy entry point is the ioctl
that returns the current GPU timestmap.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 5 +++++
 drivers/gpu/drm/panfrost/panfrost_device.h | 1 +
 drivers/gpu/drm/panfrost/panfrost_drv.c    | 9 ++++++---
 drivers/gpu/drm/panfrost/panfrost_job.c    | 2 ++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index c0334b72c70b..926bdf21389f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -615,6 +615,10 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 {
 	int ret;
 
+	/* Pre-reset */
+	if (panfrost_device_started(pfdev))
+		down_write(&pfdev->reset.lock);
+
 	/* Do the actual device reset */
 	ret = panfrost_gpu_soft_reset(pfdev);
 	panfrost_gpu_power_on(pfdev);
@@ -625,6 +629,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 		panfrost_jm_reset_interrupts(pfdev);
 		if (enable_job_int)
 			panfrost_jm_enable_interrupts(pfdev);
+		up_write(&pfdev->reset.lock);
 	}
 
 	if (!panfrost_device_started(pfdev) && !ret)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index f96ba50dfa91..81f2290d3e86 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -167,6 +167,7 @@ struct panfrost_device {
 	struct {
 		struct workqueue_struct *wq;
 		struct work_struct work;
+		struct rw_semaphore lock;
 		wait_queue_head_t wait;
 		atomic_t pending;
 	} reset;
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index fac5be20ad60..3212dbadcdb8 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -41,9 +41,12 @@ static int panfrost_ioctl_query_timestamp(struct panfrost_device *pfdev,
 	if (ret)
 		return ret;
 
-	panfrost_cycle_counter_get(pfdev);
-	*arg = panfrost_timestamp_read(pfdev);
-	panfrost_cycle_counter_put(pfdev);
+	/* We should not read timestamp register while the GPU  is being reset */
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		panfrost_cycle_counter_get(pfdev);
+		*arg = panfrost_timestamp_read(pfdev);
+		panfrost_cycle_counter_put(pfdev);
+	}
 
 	pm_runtime_put(pfdev->base.dev);
 	return 0;
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index ed8a1fabd47b..f40563682860 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -893,7 +893,9 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
 	if (!pfdev->reset.wq)
 		return -ENOMEM;
 	args.timeout_wq = pfdev->reset.wq;
+
 	init_waitqueue_head(&pfdev->reset.wait);
+	init_rwsem(&pfdev->reset.lock);
 
 	for (j = 0; j < NUM_JOB_SLOTS; j++) {
 		js->queue[j].fence_context = dma_fence_context_alloc(1);

-- 
2.55.0

[PATCH v4 12/13] drm/panfrost: Fix races between perfcnt and reset sequence

[Adrián Larumbe]

Formerly, the reset sequence would race with panfrost_mmu_as_put()
when tearing down a perfcnt session. On top of that, poking GPU
registers to program a perfcnt session or obtaining a dump might lead to
undefined behaviour when done at the same time a reset was ongoing.

Use the reset r/w semaphore to govern access to the hardware at reset
time. On top of that, expand the DRM uAPI for the perfcnt DUMP operation
so that userspace can be made aware of a reset having happened, because
that means counters will go back to 0 and can no longer be accumulated
to values previously kept in user space.

The new perfcnt-aware reset sequence also takes care to reestablish
perfcnt to its original configuration if there was an enabled session.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.c  |   5 +-
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 207 ++++++++++++++++++++--------
 drivers/gpu/drm/panfrost/panfrost_perfcnt.h |   2 +
 include/uapi/drm/panfrost_drm.h             |   3 +-
 4 files changed, 154 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index 926bdf21389f..ae0f10ba7455 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -616,8 +616,10 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 	int ret;
 
 	/* Pre-reset */
-	if (panfrost_device_started(pfdev))
+	if (panfrost_device_started(pfdev)) {
 		down_write(&pfdev->reset.lock);
+		panfrost_perfcnt_reset(pfdev);
+	}
 
 	/* Do the actual device reset */
 	ret = panfrost_gpu_soft_reset(pfdev);
@@ -629,6 +631,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
 		panfrost_jm_reset_interrupts(pfdev);
 		if (enable_job_int)
 			panfrost_jm_enable_interrupts(pfdev);
+		panfrost_perfcnt_postreset(pfdev);
 		up_write(&pfdev->reset.lock);
 	}
 
diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
index ad1156678e91..b92ad9061893 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
@@ -11,6 +11,7 @@
 #include <drm/drm_file.h>
 #include <drm/drm_gem_shmem_helper.h>
 #include <drm/panfrost_drm.h>
+#include <drm/drm_print.h>
 
 #include "panfrost_device.h"
 #include "panfrost_features.h"
@@ -25,14 +26,17 @@
 #define BYTES_PER_COUNTER		4
 #define BLOCKS_PER_COREGROUP		8
 #define V4_SHADERS_PER_COREGROUP	4
+#define PERFCNT_DUMP_MAX_RETRIES	5
 
 struct panfrost_perfcnt {
 	struct panfrost_gem_mapping *mapping;
+	unsigned int counterset;
 	size_t bosize;
 	void *buf;
 	struct panfrost_file_priv *user;
 	struct mutex lock;
 	struct completion dump_comp;
+	bool reset_happened;
 };
 
 static void panfrost_perfcnt_gpu_disable(struct panfrost_device *pfdev)
@@ -55,38 +59,97 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
 	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
 }
 
-static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)
+static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev,
+					u64 *reset_happened)
 {
-	u64 gpuva;
+	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
+	u64 gpuva = perfcnt->mapping->mmnode.start << PAGE_SHIFT;
+	u64 retries = PERFCNT_DUMP_MAX_RETRIES;
 	int ret;
 
-	reinit_completion(&pfdev->perfcnt->dump_comp);
-	gpuva = pfdev->perfcnt->mapping->mmnode.start << PAGE_SHIFT;
-	gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
-	gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
-	gpu_write(pfdev, GPU_INT_CLEAR,
-		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
-		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
-	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
+dump_retry:
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		*reset_happened = perfcnt->reset_happened;
+		perfcnt->reset_happened = false;
+		reinit_completion(&pfdev->perfcnt->dump_comp);
+		gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
+		gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
+		gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_CLEAN_CACHES_COMPLETED |
+						GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
+		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
+	}
+
 	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
 							msecs_to_jiffies(1000));
-	if (!ret)
-		ret = -ETIMEDOUT;
-	else if (ret > 0)
-		ret = 0;
+
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		if (ret > 0) {
+			if (perfcnt->reset_happened) {
+				if (--retries >= 0)
+					goto dump_retry;
+				else
+					ret = -EBUSY;
+			} else {
+				ret = 0;
+			}
+		} else if (!ret) {
+			ret = -ETIMEDOUT;
+		}
+	}
 
 	return ret;
 }
 
+static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
+{
+	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
+	u32 cfg, as;
+	int ret;
+
+	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
+	if (ret < 0)
+		return ret;
+
+	as = ret;
+	cfg = GPU_PERFCNT_CFG_AS(as) |
+	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
+
+	/*
+	 * Bifrost GPUs have 2 set of counters, but we're only interested by
+	 * the first one for now.
+	 */
+	if (panfrost_model_is_bifrost(pfdev))
+		cfg |= GPU_PERFCNT_CFG_SETSEL(perfcnt->counterset);
+
+	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
+	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
+	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
+
+	/*
+	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
+	 * counters.
+	 */
+	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
+		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
+	else
+		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
+
+	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
+
+	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
+		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
+
+	return 0;
+}
+
 static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
 					  struct drm_file *file_priv,
 					  unsigned int counterset)
 {
 	struct panfrost_file_priv *user = file_priv->driver_priv;
 	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
-	struct iosys_map map;
 	struct drm_gem_shmem_object *bo;
-	u32 cfg, as;
+	struct iosys_map map;
 	int ret;
 
 	if (user == perfcnt->user)
@@ -119,7 +182,9 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
 	ret = drm_gem_vmap(&bo->base, &map);
 	if (ret)
 		goto err_put_mapping;
+
 	perfcnt->buf = map.vaddr;
+	perfcnt->counterset = counterset;
 
 	panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer");
 
@@ -127,60 +192,46 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
 	 * Invalidate the cache and clear the counters to start from a fresh
 	 * state.
 	 */
-	reinit_completion(&pfdev->perfcnt->dump_comp);
-	gpu_write(pfdev, GPU_INT_CLEAR,
-		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
-		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
-	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
-	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		reinit_completion(&pfdev->perfcnt->dump_comp);
+		gpu_write(pfdev, GPU_INT_CLEAR,
+			  GPU_IRQ_CLEAN_CACHES_COMPLETED |
+			  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
+		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
+		gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
+		perfcnt->reset_happened = false;
+		perfcnt->user = user;
+	}
+
+	/*
+	 * If a reset happens during the wait for the IRQ notification that caches
+	 * are clean and invalidated, then we know the reset sequence did the job
+	 * for us, even if it takes long enough for the completion to time out.
+	 */
 	ret = wait_for_completion_timeout(&pfdev->perfcnt->dump_comp,
 					  msecs_to_jiffies(1000));
-	if (!ret) {
+	if (!ret && !perfcnt->reset_happened) {
 		ret = -ETIMEDOUT;
 		goto err_vunmap;
 	}
 
-	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
-	if (ret < 0)
-		goto err_vunmap;
-
-	as = ret;
-	cfg = GPU_PERFCNT_CFG_AS(as) |
-	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
-
-	/*
-	 * Bifrost GPUs have 2 set of counters, but we're only interested by
-	 * the first one for now.
-	 */
-	if (panfrost_model_is_bifrost(pfdev))
-		cfg |= GPU_PERFCNT_CFG_SETSEL(counterset);
-
-	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
-	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
-	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
-
-	/*
-	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
-	 * counters.
-	 */
-	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
-		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
-	else
-		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
-
-	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
-
-	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
-		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		if (!perfcnt->reset_happened) {
+			ret = panfrost_perfcnt_hw_enable(pfdev);
+			if (ret)
+				goto err_vunmap;
+		}
+		perfcnt->reset_happened = false;
+	}
 
 	/* The BO ref is retained by the mapping. */
 	drm_gem_object_put(&bo->base);
 
-	perfcnt->user = user;
-
 	return 0;
 
 err_vunmap:
+	scoped_guard(rwsem_read, &pfdev->reset.lock)
+		perfcnt->user = user;
 	drm_gem_vunmap(&bo->base, &map);
 err_put_mapping:
 	panfrost_gem_mapping_put(perfcnt->mapping);
@@ -203,13 +254,15 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
 	if (user != perfcnt->user)
 		return -EINVAL;
 
-	panfrost_perfcnt_gpu_disable(pfdev);
+	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
+		panfrost_perfcnt_gpu_disable(pfdev);
+		perfcnt->user = NULL;
+	}
 
-	perfcnt->user = NULL;
 	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
 	perfcnt->buf = NULL;
 	panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv);
-	panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
 	panfrost_gem_mapping_put(perfcnt->mapping);
 	perfcnt->mapping = NULL;
 	pm_runtime_put_autosuspend(pfdev->base.dev);
@@ -263,7 +316,7 @@ int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
 		goto out;
 	}
 
-	ret = panfrost_perfcnt_dump_locked(pfdev);
+	ret = panfrost_perfcnt_dump_locked(pfdev, &req->hw_reset);
 	if (ret)
 		goto out;
 
@@ -346,3 +399,35 @@ void panfrost_perfcnt_fini(struct panfrost_device *pfdev)
 	/* Disable everything before leaving. */
 	panfrost_perfcnt_gpu_disable(pfdev);
 }
+
+void panfrost_perfcnt_reset(struct panfrost_device *pfdev)
+{
+	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
+
+	if (drm_WARN_ON(&pfdev->base, !perfcnt))
+		return;
+
+	lockdep_assert_held(&pfdev->reset.lock);
+
+	if (!perfcnt->user)
+		return;
+
+	perfcnt->reset_happened = true;
+	complete(&perfcnt->dump_comp);
+	panfrost_perfcnt_gpu_disable(pfdev);
+}
+
+void panfrost_perfcnt_postreset(struct panfrost_device *pfdev)
+{
+	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
+
+	if (drm_WARN_ON(&pfdev->base, !perfcnt))
+		return;
+
+	lockdep_assert_held(&pfdev->reset.lock);
+
+	if (!perfcnt->user)
+		return;
+
+	panfrost_perfcnt_hw_enable(pfdev);
+}
diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
index 8bbcf5f5fb33..e14e760641fd 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
@@ -14,5 +14,7 @@ int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data,
 				  struct drm_file *file_priv);
 int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
+void panfrost_perfcnt_reset(struct panfrost_device *pfdev);
+void panfrost_perfcnt_postreset(struct panfrost_device *pfdev);
 
 #endif
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index 50d5337f35ef..3bbf9220103d 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -47,7 +47,7 @@ extern "C" {
  * them for anything but debugging purpose.
  */
 #define DRM_IOCTL_PANFROST_PERFCNT_ENABLE	DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_ENABLE, struct drm_panfrost_perfcnt_enable)
-#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
+#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
 
 #define PANFROST_JD_REQ_FS (1 << 0)
 #define PANFROST_JD_REQ_CYCLE_COUNT (1 << 1)
@@ -272,6 +272,7 @@ struct drm_panfrost_perfcnt_enable {
 
 struct drm_panfrost_perfcnt_dump {
 	__u64 buf_ptr;
+	__u64 hw_reset;
 };
 
 /* madvise provides a way to tell the kernel in case a buffers contents

-- 
2.55.0

[PATCH v4 13/13] drm/panfrost: Bump driver minor to reflect new DUMP IOCTL req field

[Adrián Larumbe]

Expose expanded PERFCNT_DUMP IOCTL interface to user mode.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 3212dbadcdb8..4384f8f744e5 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -936,6 +936,7 @@ static void panfrost_debugfs_init(struct drm_minor *minor)
  * - 1.6 - adds PANFROST_BO_MAP_WB, PANFROST_IOCTL_SYNC_BO,
  *	   PANFROST_IOCTL_QUERY_BO_INFO and
  *	   DRM_PANFROST_PARAM_SELECTED_COHERENCY
+ * - 1.7 - adds PERFCNT_DUMP req hw_reset field
  */
 static const struct drm_driver panfrost_drm_driver = {
 	.driver_features	= DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ,
@@ -948,7 +949,7 @@ static const struct drm_driver panfrost_drm_driver = {
 	.name			= "panfrost",
 	.desc			= "panfrost DRM",
 	.major			= 1,
-	.minor			= 6,
+	.minor			= 7,
 
 	.gem_create_object	= panfrost_gem_create_object,
 	.gem_prime_import	= panfrost_gem_prime_import,

-- 
2.55.0

Re: [PATCH v4 05/13] drm/panfrost: Move all device power up and down into RPM callbacks

[Philipp Zabel]

On Mi, 2026-07-29 at 03:54 +0100, Adrián Larumbe wrote:
> During device probe(), failure to do a PM get() will leave the usage_count
> set to 0, which is the value assigned at device creation time. That means
> when the autosuspend delay expires, runtime suspend callback won't be
> invoked, so the device will remain powered on forever.
> 
> On top of that, failure to call PM put() during device unplug means
> Panfrost device's PM usage_count increases monotonically for every new
> module reload.
> 
> The combined outcome of both of the above was that devfreq OPP transition
> notifications would be printed all the time, even when no jobs are being
> submitted. This quickly fills the kernel ring buffer with junk.
> 
> Even direr than that was the fact MMU interrupts are only enabled when
> the device is reset, so after device probe() the very first job targeting
> the tiler heap BO would always time out, because the driver's PM runtime
> resume callback would not be invoked.
> 
> Fix all that by moving all GPU enabling and disabling into RPM resume and
> suspend callbacks, and making sure we resume the device right before
> touching any HW registers. This is done in imitation of the Panthor model.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 375 +++++++++++++++++------------
>  drivers/gpu/drm/panfrost/panfrost_device.h |   9 +
>  drivers/gpu/drm/panfrost/panfrost_drv.c    |  10 +-
>  drivers/gpu/drm/panfrost/panfrost_gpu.c    |   7 +-
>  drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
>  5 files changed, 237 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 03905495aee2..a9d29d9ee22b 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -34,6 +34,40 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
>  	reset_control_assert(pfdev->rstc);
>  }
>  
> +static int panfrost_clk_enable(struct panfrost_device *pfdev)
> +{
> +	int err;
> +
> +	err = clk_enable(pfdev->clock);
> +	if (err)
> +		return err;
> +
> +	err = clk_enable(pfdev->bus_clock);
> +	if (err)
> +		goto disable_clock;
> +
> +	err = clk_enable(pfdev->bus_ace_clock);
> +	if (err)
> +		goto disable_bus_clock;
> +
> +	return 0;
> +
> +disable_bus_clock:
> +	clk_disable(pfdev->bus_clock);
> +disable_clock:
> +	clk_disable(pfdev->clock);
> +
> +	return err;
> +}
> +
> +static void panfrost_clk_disable(struct panfrost_device *pfdev)
> +{
> +	clk_disable(pfdev->bus_ace_clock);
> +	clk_disable(pfdev->bus_clock);
> +	clk_disable(pfdev->clock);
> +	reset_control_assert(pfdev->rstc);
> +}

This asymmetry is weird. Why assert the reset control in
panfrost_clock_disable() but not deassert it in panfrost_clk_enable()?
It's also unexpected to have a function called _clk_disable do
something else.

> +
>  static int panfrost_clk_init(struct panfrost_device *pfdev)
>  {
>  	int err;
> @@ -48,7 +82,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>  	rate = clk_get_rate(pfdev->clock);
>  	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
>  
> -	err = clk_prepare_enable(pfdev->clock);
> +	err = clk_prepare(pfdev->clock);
>  	if (err)
>  		return err;
>  
> @@ -57,44 +91,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
[...]
>  static void panfrost_clk_fini(struct panfrost_device *pfdev)
>  {
> -	clk_disable_unprepare(pfdev->bus_ace_clock);
> -	clk_disable_unprepare(pfdev->bus_clock);
> -	clk_disable_unprepare(pfdev->clock);
> +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
> +		panfrost_clk_disable(pfdev);

So here you are effectively adding a reset_control_assert() that wasn't
there before. That seems to be intentional because you are removing the
call to panfrost_reset_fini() in panfrost_device_fini().

It's confusing though, panfrost_clk_fini() now cleans up things that
panfrost_clk_init() didn't set up.

> +
> +	clk_unprepare(pfdev->bus_ace_clock);
> +	clk_unprepare(pfdev->bus_clock);
> +	clk_unprepare(pfdev->clock);
>  }
>  
>  static int panfrost_regulator_init(struct panfrost_device *pfdev)
> @@ -212,6 +257,133 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
>  	return err;
>  }
>  
> +static int panfrost_device_runtime_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> +		ret = reset_control_deassert(pfdev->rstc);
> +		if (ret)
> +			return ret;
> +
> +		ret = panfrost_clk_enable(pfdev);
> +		if (ret)
> +			goto err_clk;

Here we clearly deassert the reset and enable the clocks ...

> +	}
> +
> +	panfrost_device_reset(pfdev, true);
> +	panfrost_devfreq_resume(pfdev);
> +
> +	return 0;
> +
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> +		reset_control_assert(pfdev->rstc);
> +	return ret;
> +}
> +
> +static int panfrost_device_runtime_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +
> +	if (!panfrost_jm_is_idle(pfdev))
> +		return -EBUSY;
> +
> +	panfrost_devfreq_suspend(pfdev);
> +	panfrost_jm_suspend_irq(pfdev);
> +	panfrost_mmu_suspend_irq(pfdev);
> +	panfrost_gpu_suspend_irq(pfdev);
> +	panfrost_gpu_power_off(pfdev);
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> +		panfrost_clk_disable(pfdev);

... and here we don't seem to assert the reset, but secretly do.

> +
> +	return 0;
> +}
> +
> +static int panfrost_device_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> +		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> +		struct dev_pm_opp *opp;
> +
> +		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +		dev_pm_opp_set_opp(dev, opp);
> +		dev_pm_opp_put(opp);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		ret = clk_enable(pfdev->clock);
> +		if (ret)
> +			goto err_clk;
> +
> +		if (pfdev->bus_clock) {

Unnecessary check, clk_enable() handles NULL pointers.

> +			ret = clk_enable(pfdev->bus_clock);
> +			if (ret)
> +				goto err_bus_clk;
> +		}
> +	}
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		goto err_resume;
> +
> +	return 0;
> +
> +err_resume:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)

Unnecessary check.

> +		clk_disable(pfdev->bus_clock);
> +err_bus_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> +		clk_disable(pfdev->clock);
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +	return ret;
> +}
> +
> +static int panfrost_device_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		if (pfdev->bus_clock)

Unnecessary check.

> +			clk_disable(pfdev->bus_clock);
> +
> +		clk_disable(pfdev->clock);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +
> +	return 0;
> +}
> +
> +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> +	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> +	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> +};
> +
> +void panfrost_try_suspend_device(struct panfrost_device *pfdev)
> +{
> +	pm_runtime_put_sync_suspend(pfdev->base.dev);
> +
> +	/* If PM is disabled, we need to call the suspend handler manually. */
> +	if (!IS_ENABLED(CONFIG_PM))
> +		panfrost_device_runtime_suspend(pfdev->base.dev);
> +}
> +
>  int panfrost_device_init(struct panfrost_device *pfdev)
>  {
>  	int err;
> @@ -265,10 +437,25 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  		goto out_regulator;
>  	}
>  
> -	err = panfrost_gpu_init(pfdev);
> +	err = devm_pm_runtime_enable(pfdev->base.dev);
>  	if (err)
>  		goto out_regulator;
>  
> +	err = pm_runtime_resume_and_get(pfdev->base.dev);
> +	if (err)
> +		goto out_regulator;
> +
> +	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
> +	if (!IS_ENABLED(CONFIG_PM)) {
> +		err = panfrost_device_runtime_resume(pfdev->base.dev);
> +		if (err)
> +			goto out_regulator;
> +	}
> +
> +	err = panfrost_gpu_init(pfdev);
> +	if (err)
> +		goto out_rpm_put;
> +
>  	err = panfrost_mmu_init(pfdev);
>  	if (err)
>  		goto out_gpu;
> @@ -295,6 +482,8 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  	panfrost_mmu_fini(pfdev);
>  out_gpu:
>  	panfrost_gpu_fini(pfdev);
> +out_rpm_put:
> +	panfrost_try_suspend_device(pfdev);
>  out_regulator:
>  	panfrost_regulator_fini(pfdev);
>  out_devfreq:

Directly below this:

          panfrost_devfreq_fini(pfdev);                                                                                                       
  out_clk:                                                                                                                                    
          panfrost_clk_fini(pfdev);                                                                                                           
  out_reset:                                                                                                                                  
          panfrost_reset_fini(pfdev);                                                                                                         

_reset_fini is still called right after _clk_fini asserted the reset
control.

> @@ -313,12 +502,10 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
>  	panfrost_gem_shrinker_fini(pfdev);
>  	panfrost_perfcnt_fini(pfdev);
>  	panfrost_jm_fini(pfdev);
> -	panfrost_mmu_fini(pfdev);
> -	panfrost_gpu_fini(pfdev);
> -	panfrost_devfreq_fini(pfdev);
> +	panfrost_try_suspend_device(pfdev);
>  	panfrost_regulator_fini(pfdev);
> +	panfrost_devfreq_fini(pfdev);
>  	panfrost_clk_fini(pfdev);
> -	panfrost_reset_fini(pfdev);

Here _reset_fini is dropped because _clk_fini  already asserted the
reset.

regards
Philipp

Re: [PATCH v4 03/13] drm/panfrost: Remove unnecessary header file include

[Steven Price]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> None of panfrost_features.h definitions are used in panfrost_device.c
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 7fed22d555a5..d8421fd6a662 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -11,7 +11,6 @@
>  
>  #include "panfrost_device.h"
>  #include "panfrost_devfreq.h"
> -#include "panfrost_features.h"
>  #include "panfrost_gem.h"
>  #include "panfrost_issues.h"
>  #include "panfrost_gpu.h"
> 

Re: [PATCH v4 04/13] drm/panfrost: Move shrinker initialization and unplug one level down

[Steven Price]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> Since the moment we call drm_dev_register() the device should be in a
> position to accept jobs, so it's best if the shrinker is already
> initialized by then.
> 
> On top of that, make shrinker functions take an panfrost_device pointer
> like other functions in the same sequence and rename them accordingly.
> 
> Essentially mimic the init/fini behaviour in Panthor.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c       |  8 +++++++-
>  drivers/gpu/drm/panfrost/panfrost_drv.c          |  6 ------
>  drivers/gpu/drm/panfrost/panfrost_gem.c          | 19 ++++++++++---------
>  drivers/gpu/drm/panfrost/panfrost_gem.h          |  6 +++---
>  drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |  8 ++------
>  5 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index d8421fd6a662..03905495aee2 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -281,9 +281,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  	if (err)
>  		goto out_job;
>  
> -	panfrost_gem_init(pfdev);
> +	err = panfrost_gem_init(pfdev);
> +	if (err)
> +		goto out_perfcnt;
>  
>  	return 0;
> +
> +out_perfcnt:
> +	panfrost_perfcnt_fini(pfdev);
>  out_job:
>  	panfrost_jm_fini(pfdev);
>  out_mmu:
> @@ -305,6 +310,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  
>  void panfrost_device_fini(struct panfrost_device *pfdev)
>  {
> +	panfrost_gem_shrinker_fini(pfdev);
>  	panfrost_perfcnt_fini(pfdev);
>  	panfrost_jm_fini(pfdev);
>  	panfrost_mmu_fini(pfdev);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 2e931fa90da0..331a3bd5b98c 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -1002,14 +1002,9 @@ static int panfrost_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_out1;
>  
> -	err = panfrost_gem_shrinker_init(&pfdev->base);
> -	if (err)
> -		goto err_out2;
>  
>  	return 0;
>  
> -err_out2:
> -	drm_dev_unregister(&pfdev->base);
>  err_out1:
>  	pm_runtime_disable(pfdev->base.dev);
>  	panfrost_device_fini(pfdev);
> @@ -1023,7 +1018,6 @@ static void panfrost_remove(struct platform_device *pdev)
>  	struct panfrost_device *pfdev = platform_get_drvdata(pdev);
>  
>  	drm_dev_unregister(&pfdev->base);
> -	panfrost_gem_shrinker_cleanup(&pfdev->base);
>  
>  	pm_runtime_get_sync(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index 3a7fce428898..aa3a604b3439 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -15,20 +15,21 @@
>  #include "panfrost_gem.h"
>  #include "panfrost_mmu.h"
>  
> -void panfrost_gem_init(struct panfrost_device *pfdev)
> +int panfrost_gem_init(struct panfrost_device *pfdev)
>  {
>  	int err;
>  
>  	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> -	    !panfrost_transparent_hugepage)
> -		return;
> +	    panfrost_transparent_hugepage) {

Note you haven't quite inverted the condition here, but I don't think
the new condition is wrong - we skip the otherwise no-op helpers. The
old condition seems a bit backward. Indeed I'm not really sure why we
bother with the IS_ENABLED() check at all. But the new code looks fine.

Reviewed-by: Steven Price <steven.price@arm.com>

Thanks,
Steve

> +		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> +		if (drm_gem_get_huge_mnt(&pfdev->base))
> +			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> +		else if (err)
> +			drm_warn(&pfdev->base,
> +				 "Can't use Transparent Hugepage (%d)\n", err);
> +	}
>  
> -	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> -	if (drm_gem_get_huge_mnt(&pfdev->base))
> -		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> -	else if (err)
> -		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
> -			 err);
> +	return panfrost_gem_shrinker_init(pfdev);
>  }
>  
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> index 79d4377019e9..d6e55c2e4d86 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> @@ -129,7 +129,7 @@ drm_mm_node_to_panfrost_mapping(struct drm_mm_node *node)
>  	return container_of(node, struct panfrost_gem_mapping, mmnode);
>  }
>  
> -void panfrost_gem_init(struct panfrost_device *pfdev);
> +int panfrost_gem_init(struct panfrost_device *pfdev);
>  
>  struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
>  
> @@ -154,8 +154,8 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
>  void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
>  void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
>  
> -int panfrost_gem_shrinker_init(struct drm_device *dev);
> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev);
> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev);
>  
>  void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
>  int panfrost_gem_sync(struct drm_gem_object *obj, u32 type,
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> index 2fe967a90bcb..fefae87535d6 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> @@ -95,10 +95,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
>   *
>   * This function registers and sets up the panfrost shrinker.
>   */
> -int panfrost_gem_shrinker_init(struct drm_device *dev)
> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev)
>  {
> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> -
>  	pfdev->shrinker = shrinker_alloc(0, "drm-panfrost");
>  	if (!pfdev->shrinker)
>  		return -ENOMEM;
> @@ -118,10 +116,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev)
>   *
>   * This function unregisters the panfrost shrinker.
>   */
> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev)
> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev)
>  {
> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> -
>  	if (pfdev->shrinker)
>  		shrinker_free(pfdev->shrinker);
>  }
> 

Re: [PATCH v4 05/13] drm/panfrost: Move all device power up and down into RPM callbacks

[Steven Price]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> During device probe(), failure to do a PM get() will leave the usage_count
> set to 0, which is the value assigned at device creation time. That means
> when the autosuspend delay expires, runtime suspend callback won't be
> invoked, so the device will remain powered on forever.
> 
> On top of that, failure to call PM put() during device unplug means
> Panfrost device's PM usage_count increases monotonically for every new
> module reload.
> 
> The combined outcome of both of the above was that devfreq OPP transition
> notifications would be printed all the time, even when no jobs are being
> submitted. This quickly fills the kernel ring buffer with junk.
> 
> Even direr than that was the fact MMU interrupts are only enabled when
> the device is reset, so after device probe() the very first job targeting
> the tiler heap BO would always time out, because the driver's PM runtime
> resume callback would not be invoked.
> 
> Fix all that by moving all GPU enabling and disabling into RPM resume and
> suspend callbacks, and making sure we resume the device right before
> touching any HW registers. This is done in imitation of the Panthor model.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 375 +++++++++++++++++------------
>  drivers/gpu/drm/panfrost/panfrost_device.h |   9 +
>  drivers/gpu/drm/panfrost/panfrost_drv.c    |  10 +-
>  drivers/gpu/drm/panfrost/panfrost_gpu.c    |   7 +-
>  drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
>  5 files changed, 237 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 03905495aee2..a9d29d9ee22b 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -34,6 +34,40 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
>  	reset_control_assert(pfdev->rstc);
>  }
>  
> +static int panfrost_clk_enable(struct panfrost_device *pfdev)
> +{
> +	int err;
> +
> +	err = clk_enable(pfdev->clock);
> +	if (err)
> +		return err;
> +
> +	err = clk_enable(pfdev->bus_clock);
> +	if (err)
> +		goto disable_clock;
> +
> +	err = clk_enable(pfdev->bus_ace_clock);
> +	if (err)
> +		goto disable_bus_clock;
> +
> +	return 0;
> +
> +disable_bus_clock:
> +	clk_disable(pfdev->bus_clock);
> +disable_clock:
> +	clk_disable(pfdev->clock);
> +
> +	return err;
> +}
> +
> +static void panfrost_clk_disable(struct panfrost_device *pfdev)
> +{
> +	clk_disable(pfdev->bus_ace_clock);
> +	clk_disable(pfdev->bus_clock);
> +	clk_disable(pfdev->clock);
> +	reset_control_assert(pfdev->rstc);
> +}
> +
>  static int panfrost_clk_init(struct panfrost_device *pfdev)
>  {
>  	int err;
> @@ -48,7 +82,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>  	rate = clk_get_rate(pfdev->clock);
>  	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
>  
> -	err = clk_prepare_enable(pfdev->clock);
> +	err = clk_prepare(pfdev->clock);
>  	if (err)
>  		return err;
>  
> @@ -57,44 +91,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>  		dev_err(pfdev->base.dev, "get bus_clock failed %ld\n",
>  			PTR_ERR(pfdev->bus_clock));
>  		err = PTR_ERR(pfdev->bus_clock);
> -		goto disable_clock;
> +		goto unprepare_clock;
>  	}
>  
>  	if (pfdev->bus_clock) {
>  		rate = clk_get_rate(pfdev->bus_clock);
>  		dev_info(pfdev->base.dev, "bus_clock rate = %lu\n", rate);
>  
> -		err = clk_prepare_enable(pfdev->bus_clock);
> +		err = clk_prepare(pfdev->bus_clock);
>  		if (err)
> -			goto disable_clock;
> +			goto unprepare_clock;
>  	}
>  
>  	pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, "bus_ace");
>  	if (IS_ERR(pfdev->bus_ace_clock)) {
>  		err = PTR_ERR(pfdev->bus_ace_clock);
>  		dev_err(pfdev->base.dev, "get bus_ace_clock failed %d\n", err);
> -		goto disable_bus_clock;
> +		goto unprepare_bus_clock;
>  	}
>  
> -	err = clk_prepare_enable(pfdev->bus_ace_clock);
> +	err = clk_prepare(pfdev->bus_ace_clock);
>  	if (err)
> -		goto disable_bus_clock;
> +		goto unprepare_bus_clock;
> +
> +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT))) {
> +		err = panfrost_clk_enable(pfdev);
> +		if (err)
> +			goto unprepare_bus_ace_clock;
> +	}
>  
>  	return 0;
>  
> -disable_bus_clock:
> -	clk_disable_unprepare(pfdev->bus_clock);
> -disable_clock:
> -	clk_disable_unprepare(pfdev->clock);
> +unprepare_bus_ace_clock:
> +	clk_unprepare(pfdev->bus_ace_clock);
> +unprepare_bus_clock:
> +	clk_unprepare(pfdev->bus_clock);
> +unprepare_clock:
> +	clk_unprepare(pfdev->clock);
>  
>  	return err;
>  }
>  
>  static void panfrost_clk_fini(struct panfrost_device *pfdev)
>  {
> -	clk_disable_unprepare(pfdev->bus_ace_clock);
> -	clk_disable_unprepare(pfdev->bus_clock);
> -	clk_disable_unprepare(pfdev->clock);
> +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
> +		panfrost_clk_disable(pfdev);
> +
> +	clk_unprepare(pfdev->bus_ace_clock);
> +	clk_unprepare(pfdev->bus_clock);
> +	clk_unprepare(pfdev->clock);
>  }
>  
>  static int panfrost_regulator_init(struct panfrost_device *pfdev)
> @@ -212,6 +257,133 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
>  	return err;
>  }
>  
> +static int panfrost_device_runtime_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> +		ret = reset_control_deassert(pfdev->rstc);
> +		if (ret)
> +			return ret;
> +
> +		ret = panfrost_clk_enable(pfdev);
> +		if (ret)
> +			goto err_clk;
> +	}
> +
> +	panfrost_device_reset(pfdev, true);
> +	panfrost_devfreq_resume(pfdev);
> +
> +	return 0;
> +
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> +		reset_control_assert(pfdev->rstc);
> +	return ret;
> +}
> +
> +static int panfrost_device_runtime_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +
> +	if (!panfrost_jm_is_idle(pfdev))
> +		return -EBUSY;
> +
> +	panfrost_devfreq_suspend(pfdev);
> +	panfrost_jm_suspend_irq(pfdev);
> +	panfrost_mmu_suspend_irq(pfdev);
> +	panfrost_gpu_suspend_irq(pfdev);
> +	panfrost_gpu_power_off(pfdev);
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> +		panfrost_clk_disable(pfdev);
> +
> +	return 0;
> +}
> +
> +static int panfrost_device_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> +		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> +		struct dev_pm_opp *opp;
> +
> +		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +		dev_pm_opp_set_opp(dev, opp);
> +		dev_pm_opp_put(opp);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		ret = clk_enable(pfdev->clock);
> +		if (ret)
> +			goto err_clk;
> +
> +		if (pfdev->bus_clock) {
> +			ret = clk_enable(pfdev->bus_clock);
> +			if (ret)
> +				goto err_bus_clk;
> +		}
> +	}
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		goto err_resume;
> +
> +	return 0;
> +
> +err_resume:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> +		clk_disable(pfdev->bus_clock);
> +err_bus_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> +		clk_disable(pfdev->clock);
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +	return ret;
> +}
> +
> +static int panfrost_device_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		if (pfdev->bus_clock)
> +			clk_disable(pfdev->bus_clock);
> +
> +		clk_disable(pfdev->clock);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +
> +	return 0;
> +}
> +
> +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> +	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> +	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> +};
> +
> +void panfrost_try_suspend_device(struct panfrost_device *pfdev)
> +{
> +	pm_runtime_put_sync_suspend(pfdev->base.dev);
> +
> +	/* If PM is disabled, we need to call the suspend handler manually. */
> +	if (!IS_ENABLED(CONFIG_PM))
> +		panfrost_device_runtime_suspend(pfdev->base.dev);
> +}
> +
>  int panfrost_device_init(struct panfrost_device *pfdev)
>  {
>  	int err;
> @@ -265,10 +437,25 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  		goto out_regulator;
>  	}
>  
> -	err = panfrost_gpu_init(pfdev);
> +	err = devm_pm_runtime_enable(pfdev->base.dev);
>  	if (err)
>  		goto out_regulator;
>  
> +	err = pm_runtime_resume_and_get(pfdev->base.dev);
> +	if (err)
> +		goto out_regulator;
> +
> +	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
> +	if (!IS_ENABLED(CONFIG_PM)) {
> +		err = panfrost_device_runtime_resume(pfdev->base.dev);
> +		if (err)
> +			goto out_regulator;
> +	}
> +
> +	err = panfrost_gpu_init(pfdev);

We need the call to panfrost_gpu_init_features() (made by
panfrost_gpu_init()) before powering up the GPU with
panfrost_gpu_power_on() (which will now be getting called by the RPM
callbacks). I think what you've got will attempt to power up the GPU
with no cores (because the core masks haven't yet been discovered).
Things will probably work because a suspend/resume cycle afterwards will
'fix' things, but this relies on that suspend/resume cycle happening.

Thanks,
Steve

> +	if (err)
> +		goto out_rpm_put;
> +
>  	err = panfrost_mmu_init(pfdev);
>  	if (err)
>  		goto out_gpu;
> @@ -295,6 +482,8 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  	panfrost_mmu_fini(pfdev);
>  out_gpu:
>  	panfrost_gpu_fini(pfdev);
> +out_rpm_put:
> +	panfrost_try_suspend_device(pfdev);
>  out_regulator:
>  	panfrost_regulator_fini(pfdev);
>  out_devfreq:
> @@ -313,12 +502,10 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
>  	panfrost_gem_shrinker_fini(pfdev);
>  	panfrost_perfcnt_fini(pfdev);
>  	panfrost_jm_fini(pfdev);
> -	panfrost_mmu_fini(pfdev);
> -	panfrost_gpu_fini(pfdev);
> -	panfrost_devfreq_fini(pfdev);
> +	panfrost_try_suspend_device(pfdev);
>  	panfrost_regulator_fini(pfdev);
> +	panfrost_devfreq_fini(pfdev);
>  	panfrost_clk_fini(pfdev);
> -	panfrost_reset_fini(pfdev);
>  	panfrost_pm_domain_fini(pfdev);
>  }
>  
> @@ -424,148 +611,20 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,
>  
>  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
>  {
> -	panfrost_gpu_soft_reset(pfdev);
> -
> -	panfrost_gpu_power_on(pfdev);
> -	panfrost_mmu_reset(pfdev);
> -
> -	panfrost_jm_reset_interrupts(pfdev);
> -	if (enable_job_int)
> -		panfrost_jm_enable_interrupts(pfdev);
> -}
> -
> -static int panfrost_device_runtime_resume(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -	int ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> -		ret = reset_control_deassert(pfdev->rstc);
> -		if (ret)
> -			return ret;
> -
> -		ret = clk_enable(pfdev->clock);
> -		if (ret)
> -			goto err_clk;
> -
> -		ret = clk_enable(pfdev->bus_clock);
> -		if (ret)
> -			goto err_bus_clk;
> -
> -		ret = clk_enable(pfdev->bus_ace_clock);
> -		if (ret)
> -			goto err_bus_ace_clk;
> -	}
> -
> -	panfrost_device_reset(pfdev, true);
> -	panfrost_devfreq_resume(pfdev);
> -
> -	return 0;
> -
> -err_bus_ace_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		clk_disable(pfdev->bus_clock);
> -err_bus_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		clk_disable(pfdev->clock);
> -err_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		reset_control_assert(pfdev->rstc);
> -	return ret;
> -}
> -
> -static int panfrost_device_runtime_suspend(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -
> -	if (!panfrost_jm_is_idle(pfdev))
> -		return -EBUSY;
> -
> -	panfrost_devfreq_suspend(pfdev);
> -	panfrost_jm_suspend_irq(pfdev);
> -	panfrost_mmu_suspend_irq(pfdev);
> -	panfrost_gpu_suspend_irq(pfdev);
> -	panfrost_gpu_power_off(pfdev);
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> -		clk_disable(pfdev->bus_ace_clock);
> -		clk_disable(pfdev->bus_clock);
> -		clk_disable(pfdev->clock);
> -		reset_control_assert(pfdev->rstc);
> -	}
> -
> -	return 0;
> -}
> -
> -static int panfrost_device_resume(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -	int ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> -		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> -		struct dev_pm_opp *opp;
> -
> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> -		if (IS_ERR(opp))
> -			return PTR_ERR(opp);
> -		dev_pm_opp_set_opp(dev, opp);
> -		dev_pm_opp_put(opp);
> -	}
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> -		ret = clk_enable(pfdev->clock);
> -		if (ret)
> -			goto err_clk;
> -
> -		if (pfdev->bus_clock) {
> -			ret = clk_enable(pfdev->bus_clock);
> -			if (ret)
> -				goto err_bus_clk;
> -		}
> -	}
> -
> -	ret = pm_runtime_force_resume(dev);
> -	if (ret)
> -		goto err_resume;
> -
> -	return 0;
> -
> -err_resume:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> -		clk_disable(pfdev->bus_clock);
> -err_bus_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> -		clk_disable(pfdev->clock);
> -err_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> -		dev_pm_opp_set_opp(dev, NULL);
> -	return ret;
> -}
> -
> -static int panfrost_device_suspend(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
>  	int ret;
>  
> -	ret = pm_runtime_force_suspend(dev);
> -	if (ret)
> -		return ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> -		if (pfdev->bus_clock)
> -			clk_disable(pfdev->bus_clock);
> +	/* Do the actual device reset */
> +	ret = panfrost_gpu_soft_reset(pfdev);
> +	panfrost_gpu_power_on(pfdev);
>  
> -		clk_disable(pfdev->clock);
> +	/* Post-reset */
> +	if (panfrost_device_started(pfdev)) {
> +		panfrost_mmu_reset(pfdev);
> +		panfrost_jm_reset_interrupts(pfdev);
> +		if (enable_job_int)
> +			panfrost_jm_enable_interrupts(pfdev);
>  	}
>  
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> -		dev_pm_opp_set_opp(dev, NULL);
> -
> -	return 0;
> +	if (!panfrost_device_started(pfdev) && !ret)
> +		pfdev->gpu_started = true;
>  }
> -
> -EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> -	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> -	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> -};
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index ec55c136b1b6..eeca57efcbec 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -142,6 +142,7 @@ struct panfrost_device {
>  	/* pm_domains for devices with more than one. */
>  	struct device *pm_domain_devs[MAX_PM_DOMAINS];
>  	struct device_link *pm_domain_links[MAX_PM_DOMAINS];
> +	bool gpu_started;
>  	bool coherent;
>  
>  	struct panfrost_features features;
> @@ -253,6 +254,8 @@ int panfrost_device_init(struct panfrost_device *pfdev);
>  void panfrost_device_fini(struct panfrost_device *pfdev);
>  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int);
>  
> +void panfrost_try_suspend_device(struct panfrost_device *pfdev);
> +
>  extern const struct dev_pm_ops panfrost_pm_ops;
>  
>  enum drm_panfrost_exception_type {
> @@ -342,4 +345,10 @@ panfrost_device_schedule_reset(struct panfrost_device *pfdev)
>  	queue_work(pfdev->reset.wq, &pfdev->reset.work);
>  }
>  
> +static inline bool
> +panfrost_device_started(struct panfrost_device *pfdev)
> +{
> +	return pfdev->gpu_started;
> +}
> +
>  #endif
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 331a3bd5b98c..27f4c4b217f5 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -988,9 +988,7 @@ static int panfrost_probe(struct platform_device *pdev)
>  		goto err_out0;
>  	}
>  
> -	pm_runtime_set_active(pfdev->base.dev);
>  	pm_runtime_mark_last_busy(pfdev->base.dev);
> -	pm_runtime_enable(pfdev->base.dev);
>  	pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */
>  	pm_runtime_use_autosuspend(pfdev->base.dev);
>  
> @@ -1002,13 +1000,12 @@ static int panfrost_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_out1;
>  
> +	pm_runtime_put_autosuspend(pfdev->base.dev);
>  
>  	return 0;
>  
>  err_out1:
> -	pm_runtime_disable(pfdev->base.dev);
>  	panfrost_device_fini(pfdev);
> -	pm_runtime_set_suspended(pfdev->base.dev);
>  err_out0:
>  	return err;
>  }
> @@ -1019,10 +1016,9 @@ static void panfrost_remove(struct platform_device *pdev)
>  
>  	drm_dev_unregister(&pfdev->base);
>  
> -	pm_runtime_get_sync(pfdev->base.dev);
> -	pm_runtime_disable(pfdev->base.dev);
> +	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);
> +
>  	panfrost_device_fini(pfdev);
> -	pm_runtime_set_suspended(pfdev->base.dev);
>  }
>  
>  static ssize_t profiling_show(struct device *dev,
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 7d555e63e21a..416964282138 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -516,9 +516,8 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
>  {
>  	int err;
>  
> -	err = panfrost_gpu_soft_reset(pfdev);
> -	if (err)
> -		return err;
> +	if (!panfrost_device_started(pfdev))
> +		return -ETIMEDOUT;
>  
>  	err = panfrost_gpu_init_features(pfdev);
>  	if (err)
> @@ -543,8 +542,6 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
>  		return err;
>  	}
>  
> -	panfrost_gpu_power_on(pfdev);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 2d12b83e900a..35ff5b0f0013 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -446,7 +446,9 @@ void panfrost_jm_suspend_irq(struct panfrost_device *pfdev)
>  	set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended);
>  
>  	job_write(pfdev, JOB_INT_MASK, 0);
> -	synchronize_irq(pfdev->js->irq);
> +
> +	if (pfdev->js)
> +		synchronize_irq(pfdev->js->irq);
>  }
>  
>  static void panfrost_job_handle_err(struct panfrost_device *pfdev,
> @@ -971,6 +973,9 @@ int panfrost_jm_is_idle(struct panfrost_device *pfdev)
>  	struct panfrost_job_slot *js = pfdev->js;
>  	int i;
>  
> +	if (!js)
> +		return true;
> +
>  	for (i = 0; i < NUM_JOB_SLOTS; i++) {
>  		/* If there are any jobs in the HW queue, we're not idle */
>  		if (atomic_read(&js->queue[i].sched.credit_count))
> 

Re: [PATCH v4 06/13] drm/panfrost: Explicitly enable MMU interrupts at device init

[Steven Price]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> Because the device must be in a position to accept jobs between the time
> drm_dev_register() is called and autosuspend first kicks in, there's a very
> narrow window inbetween during which jobs targeting the tiler buffer
> object would time out, since the device's PM status is 'Active', but no MMU
> interrupts were enabled at device initialisation time.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: 73e467f60acd ("drm/panfrost: Consolidate reset handling")
> ---
>  drivers/gpu/drm/panfrost/panfrost_job.c |  3 ++-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 14 ++++++++++++--
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 35ff5b0f0013..bda1494eb430 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -871,7 +871,6 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
>  	js = devm_kzalloc(pfdev->base.dev, sizeof(*js), GFP_KERNEL);
>  	if (!js)
>  		return -ENOMEM;
> -	pfdev->js = js;
>  
>  	INIT_WORK(&pfdev->reset.work, panfrost_reset_work);
>  	spin_lock_init(&js->job_lock);
> @@ -906,6 +905,8 @@ int panfrost_jm_init(struct panfrost_device *pfdev)
>  		}
>  	}
>  
> +	pfdev->js = js;
> +
>  	panfrost_jm_reset_interrupts(pfdev);
>  	panfrost_jm_enable_interrupts(pfdev);
>  
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 4a3162c3b659..aad0cd31516d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -336,6 +336,12 @@ void panfrost_mmu_as_put(struct panfrost_device *pfdev, struct panfrost_mmu *mmu
>  	WARN_ON(atomic_read(&mmu->as_count) < 0);
>  }
>  
> +static void panfrost_mmu_enable_interrupts(struct panfrost_device *pfdev)
> +{
> +	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> +	mmu_write(pfdev, MMU_INT_MASK, ~0);
> +}
> +
>  void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  {
>  	struct panfrost_mmu *mmu, *mmu_tmp;
> @@ -355,8 +361,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  
>  	spin_unlock(&pfdev->as_lock);
>  
> -	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> -	mmu_write(pfdev, MMU_INT_MASK, ~0);
> +	panfrost_mmu_enable_interrupts(pfdev);
>  }
>  
>  static size_t get_pgsize(u64 addr, size_t size, size_t *count)
> @@ -880,6 +885,9 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
>  	u32 status = mmu_read(pfdev, MMU_INT_RAWSTAT);
>  	int ret;
>  
> +	if (!pfdev->js)
> +		return IRQ_NONE;
> +

I find it odd to check the status of the job scheduler here in the MMU
IRQ handler. Personally I'd prefer we just delay the enabling of the MMU
interrupts until the driver is in a state to handle them - i.e. move the
call to panfrost_mmu_enable_interrupts() to panfrost_device_init() once
the call to panfrost_jm_init() has completed.

Also note that simply returning IRQ_NONE without actually clearing the
interrupt is likely to cause problems - panfrost_mmu_irq_handler() will
have disabled the interrupt mask and it won't be re-enabled until the
timeout happens.

Thanks,
Steve

>  	while (status) {
>  		u32 as = ffs(status | (status >> 16)) - 1;
>  		u32 mask = BIT(as) | BIT(as + 16);
> @@ -970,6 +978,8 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>  		return err;
>  	}
>  
> +	panfrost_mmu_enable_interrupts(pfdev);
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH v4 12/13] drm/panfrost: Fix races between perfcnt and reset sequence

[Steven Price]

On 29/07/2026 03:54, Adrián Larumbe wrote:
> Formerly, the reset sequence would race with panfrost_mmu_as_put()
> when tearing down a perfcnt session. On top of that, poking GPU
> registers to program a perfcnt session or obtaining a dump might lead to
> undefined behaviour when done at the same time a reset was ongoing.
> 
> Use the reset r/w semaphore to govern access to the hardware at reset
> time. On top of that, expand the DRM uAPI for the perfcnt DUMP operation
> so that userspace can be made aware of a reset having happened, because
> that means counters will go back to 0 and can no longer be accumulated
> to values previously kept in user space.
> 
> The new perfcnt-aware reset sequence also takes care to reestablish
> perfcnt to its original configuration if there was an enabled session.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c  |   5 +-
>  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 207 ++++++++++++++++++++--------
>  drivers/gpu/drm/panfrost/panfrost_perfcnt.h |   2 +
>  include/uapi/drm/panfrost_drm.h             |   3 +-
>  4 files changed, 154 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 926bdf21389f..ae0f10ba7455 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -616,8 +616,10 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
>  	int ret;
>  
>  	/* Pre-reset */
> -	if (panfrost_device_started(pfdev))
> +	if (panfrost_device_started(pfdev)) {
>  		down_write(&pfdev->reset.lock);
> +		panfrost_perfcnt_reset(pfdev);
> +	}
>  
>  	/* Do the actual device reset */
>  	ret = panfrost_gpu_soft_reset(pfdev);
> @@ -629,6 +631,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
>  		panfrost_jm_reset_interrupts(pfdev);
>  		if (enable_job_int)
>  			panfrost_jm_enable_interrupts(pfdev);
> +		panfrost_perfcnt_postreset(pfdev);
>  		up_write(&pfdev->reset.lock);
>  	}
>  
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index ad1156678e91..b92ad9061893 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -11,6 +11,7 @@
>  #include <drm/drm_file.h>
>  #include <drm/drm_gem_shmem_helper.h>
>  #include <drm/panfrost_drm.h>
> +#include <drm/drm_print.h>
>  
>  #include "panfrost_device.h"
>  #include "panfrost_features.h"
> @@ -25,14 +26,17 @@
>  #define BYTES_PER_COUNTER		4
>  #define BLOCKS_PER_COREGROUP		8
>  #define V4_SHADERS_PER_COREGROUP	4
> +#define PERFCNT_DUMP_MAX_RETRIES	5
>  
>  struct panfrost_perfcnt {
>  	struct panfrost_gem_mapping *mapping;
> +	unsigned int counterset;
>  	size_t bosize;
>  	void *buf;
>  	struct panfrost_file_priv *user;
>  	struct mutex lock;
>  	struct completion dump_comp;
> +	bool reset_happened;
>  };
>  
>  static void panfrost_perfcnt_gpu_disable(struct panfrost_device *pfdev)
> @@ -55,38 +59,97 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>  	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
>  }
>  
> -static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)
> +static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev,
> +					u64 *reset_happened)
>  {
> -	u64 gpuva;
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +	u64 gpuva = perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> +	u64 retries = PERFCNT_DUMP_MAX_RETRIES;
>  	int ret;
>  
> -	reinit_completion(&pfdev->perfcnt->dump_comp);
> -	gpuva = pfdev->perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> -	gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> -	gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> -	gpu_write(pfdev, GPU_INT_CLEAR,
> -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> +dump_retry:
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		*reset_happened = perfcnt->reset_happened;
> +		perfcnt->reset_happened = false;
> +		reinit_completion(&pfdev->perfcnt->dump_comp);
> +		gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> +		gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> +		gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_CLEAN_CACHES_COMPLETED |
> +						GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> +	}
> +
>  	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
>  							msecs_to_jiffies(1000));
> -	if (!ret)
> -		ret = -ETIMEDOUT;
> -	else if (ret > 0)
> -		ret = 0;
> +
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		if (ret > 0) {
> +			if (perfcnt->reset_happened) {
> +				if (--retries >= 0)

retries is defined unsigned, so this statement is always true...

> +					goto dump_retry;
> +				else
> +					ret = -EBUSY;
> +			} else {
> +				ret = 0;
> +			}
> +		} else if (!ret) {
> +			ret = -ETIMEDOUT;
> +		}
> +	}
>  
>  	return ret;
>  }
>  
> +static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
> +{
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +	u32 cfg, as;
> +	int ret;
> +
> +	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
> +	if (ret < 0)
> +		return ret;
> +
> +	as = ret;
> +	cfg = GPU_PERFCNT_CFG_AS(as) |
> +	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
> +
> +	/*
> +	 * Bifrost GPUs have 2 set of counters, but we're only interested by
> +	 * the first one for now.
> +	 */
> +	if (panfrost_model_is_bifrost(pfdev))
> +		cfg |= GPU_PERFCNT_CFG_SETSEL(perfcnt->counterset);
> +
> +	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
> +	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
> +	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
> +
> +	/*
> +	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
> +	 * counters.
> +	 */
> +	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> +	else
> +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> +
> +	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
> +
> +	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> +
> +	return 0;
> +}
> +
>  static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  					  struct drm_file *file_priv,
>  					  unsigned int counterset)
>  {
>  	struct panfrost_file_priv *user = file_priv->driver_priv;
>  	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> -	struct iosys_map map;
>  	struct drm_gem_shmem_object *bo;
> -	u32 cfg, as;
> +	struct iosys_map map;
>  	int ret;
>  
>  	if (user == perfcnt->user)
> @@ -119,7 +182,9 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  	ret = drm_gem_vmap(&bo->base, &map);
>  	if (ret)
>  		goto err_put_mapping;
> +
>  	perfcnt->buf = map.vaddr;
> +	perfcnt->counterset = counterset;
>  
>  	panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer");
>  
> @@ -127,60 +192,46 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  	 * Invalidate the cache and clear the counters to start from a fresh
>  	 * state.
>  	 */
> -	reinit_completion(&pfdev->perfcnt->dump_comp);
> -	gpu_write(pfdev, GPU_INT_CLEAR,
> -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
> -	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		reinit_completion(&pfdev->perfcnt->dump_comp);
> +		gpu_write(pfdev, GPU_INT_CLEAR,
> +			  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> +			  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
> +		perfcnt->reset_happened = false;
> +		perfcnt->user = user;
> +	}
> +
> +	/*
> +	 * If a reset happens during the wait for the IRQ notification that caches
> +	 * are clean and invalidated, then we know the reset sequence did the job
> +	 * for us, even if it takes long enough for the completion to time out.
> +	 */
>  	ret = wait_for_completion_timeout(&pfdev->perfcnt->dump_comp,
>  					  msecs_to_jiffies(1000));
> -	if (!ret) {
> +	if (!ret && !perfcnt->reset_happened) {
>  		ret = -ETIMEDOUT;
>  		goto err_vunmap;
>  	}
>  
> -	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
> -	if (ret < 0)
> -		goto err_vunmap;
> -
> -	as = ret;
> -	cfg = GPU_PERFCNT_CFG_AS(as) |
> -	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
> -
> -	/*
> -	 * Bifrost GPUs have 2 set of counters, but we're only interested by
> -	 * the first one for now.
> -	 */
> -	if (panfrost_model_is_bifrost(pfdev))
> -		cfg |= GPU_PERFCNT_CFG_SETSEL(counterset);
> -
> -	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
> -	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
> -	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
> -
> -	/*
> -	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
> -	 * counters.
> -	 */
> -	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> -	else
> -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> -
> -	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
> -
> -	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		if (!perfcnt->reset_happened) {
> +			ret = panfrost_perfcnt_hw_enable(pfdev);
> +			if (ret)
> +				goto err_vunmap;
> +		}
> +		perfcnt->reset_happened = false;
> +	}
>  
>  	/* The BO ref is retained by the mapping. */
>  	drm_gem_object_put(&bo->base);
>  
> -	perfcnt->user = user;
> -
>  	return 0;
>  
>  err_vunmap:
> +	scoped_guard(rwsem_read, &pfdev->reset.lock)
> +		perfcnt->user = user;

This looks weird - AFAICT perfcnt->user will already have been set to
'user' above if we ever get to this error case. It also unusual to
assign a pointer like this in an error path - I'd expect at least a
comment explaining the reasoning.

Thanks,
Steve

>  	drm_gem_vunmap(&bo->base, &map);
>  err_put_mapping:
>  	panfrost_gem_mapping_put(perfcnt->mapping);
> @@ -203,13 +254,15 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
>  	if (user != perfcnt->user)
>  		return -EINVAL;
>  
> -	panfrost_perfcnt_gpu_disable(pfdev);
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
> +		panfrost_perfcnt_gpu_disable(pfdev);
> +		perfcnt->user = NULL;
> +	}
>  
> -	perfcnt->user = NULL;
>  	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
>  	perfcnt->buf = NULL;
>  	panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv);
> -	panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
>  	panfrost_gem_mapping_put(perfcnt->mapping);
>  	perfcnt->mapping = NULL;
>  	pm_runtime_put_autosuspend(pfdev->base.dev);
> @@ -263,7 +316,7 @@ int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
>  		goto out;
>  	}
>  
> -	ret = panfrost_perfcnt_dump_locked(pfdev);
> +	ret = panfrost_perfcnt_dump_locked(pfdev, &req->hw_reset);
>  	if (ret)
>  		goto out;
>  
> @@ -346,3 +399,35 @@ void panfrost_perfcnt_fini(struct panfrost_device *pfdev)
>  	/* Disable everything before leaving. */
>  	panfrost_perfcnt_gpu_disable(pfdev);
>  }
> +
> +void panfrost_perfcnt_reset(struct panfrost_device *pfdev)
> +{
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +
> +	if (drm_WARN_ON(&pfdev->base, !perfcnt))
> +		return;
> +
> +	lockdep_assert_held(&pfdev->reset.lock);
> +
> +	if (!perfcnt->user)
> +		return;
> +
> +	perfcnt->reset_happened = true;
> +	complete(&perfcnt->dump_comp);
> +	panfrost_perfcnt_gpu_disable(pfdev);
> +}
> +
> +void panfrost_perfcnt_postreset(struct panfrost_device *pfdev)
> +{
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +
> +	if (drm_WARN_ON(&pfdev->base, !perfcnt))
> +		return;
> +
> +	lockdep_assert_held(&pfdev->reset.lock);
> +
> +	if (!perfcnt->user)
> +		return;
> +
> +	panfrost_perfcnt_hw_enable(pfdev);
> +}
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> index 8bbcf5f5fb33..e14e760641fd 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> @@ -14,5 +14,7 @@ int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data,
>  				  struct drm_file *file_priv);
>  int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
>  				struct drm_file *file_priv);
> +void panfrost_perfcnt_reset(struct panfrost_device *pfdev);
> +void panfrost_perfcnt_postreset(struct panfrost_device *pfdev);
>  
>  #endif
> diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> index 50d5337f35ef..3bbf9220103d 100644
> --- a/include/uapi/drm/panfrost_drm.h
> +++ b/include/uapi/drm/panfrost_drm.h
> @@ -47,7 +47,7 @@ extern "C" {
>   * them for anything but debugging purpose.
>   */
>  #define DRM_IOCTL_PANFROST_PERFCNT_ENABLE	DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_ENABLE, struct drm_panfrost_perfcnt_enable)
> -#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
> +#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
>  
>  #define PANFROST_JD_REQ_FS (1 << 0)
>  #define PANFROST_JD_REQ_CYCLE_COUNT (1 << 1)
> @@ -272,6 +272,7 @@ struct drm_panfrost_perfcnt_enable {
>  
>  struct drm_panfrost_perfcnt_dump {
>  	__u64 buf_ptr;
> +	__u64 hw_reset;
>  };
>  
>  /* madvise provides a way to tell the kernel in case a buffers contents
> 

Re: [PATCH v4 05/13] drm/panfrost: Move all device power up and down into RPM callbacks

[Adrián Larumbe]

Hi Philipp,

On 29.07.2026 10:37, Philipp Zabel wrote:
> On Mi, 2026-07-29 at 03:54 +0100, Adrián Larumbe wrote:
> > During device probe(), failure to do a PM get() will leave the usage_count
> > set to 0, which is the value assigned at device creation time. That means
> > when the autosuspend delay expires, runtime suspend callback won't be
> > invoked, so the device will remain powered on forever.
> > 
> > On top of that, failure to call PM put() during device unplug means
> > Panfrost device's PM usage_count increases monotonically for every new
> > module reload.
> > 
> > The combined outcome of both of the above was that devfreq OPP transition
> > notifications would be printed all the time, even when no jobs are being
> > submitted. This quickly fills the kernel ring buffer with junk.
> > 
> > Even direr than that was the fact MMU interrupts are only enabled when
> > the device is reset, so after device probe() the very first job targeting
> > the tiler heap BO would always time out, because the driver's PM runtime
> > resume callback would not be invoked.
> > 
> > Fix all that by moving all GPU enabling and disabling into RPM resume and
> > suspend callbacks, and making sure we resume the device right before
> > touching any HW registers. This is done in imitation of the Panthor model.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> > Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_device.c | 375 +++++++++++++++++------------
> >  drivers/gpu/drm/panfrost/panfrost_device.h |   9 +
> >  drivers/gpu/drm/panfrost/panfrost_drv.c    |  10 +-
> >  drivers/gpu/drm/panfrost/panfrost_gpu.c    |   7 +-
> >  drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
> >  5 files changed, 237 insertions(+), 171 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index 03905495aee2..a9d29d9ee22b 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -34,6 +34,40 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
> >  	reset_control_assert(pfdev->rstc);
> >  }
> >  
> > +static int panfrost_clk_enable(struct panfrost_device *pfdev)
> > +{
> > +	int err;
> > +
> > +	err = clk_enable(pfdev->clock);
> > +	if (err)
> > +		return err;
> > +
> > +	err = clk_enable(pfdev->bus_clock);
> > +	if (err)
> > +		goto disable_clock;
> > +
> > +	err = clk_enable(pfdev->bus_ace_clock);
> > +	if (err)
> > +		goto disable_bus_clock;
> > +
> > +	return 0;
> > +
> > +disable_bus_clock:
> > +	clk_disable(pfdev->bus_clock);
> > +disable_clock:
> > +	clk_disable(pfdev->clock);
> > +
> > +	return err;
> > +}
> > +
> > +static void panfrost_clk_disable(struct panfrost_device *pfdev)
> > +{
> > +	clk_disable(pfdev->bus_ace_clock);
> > +	clk_disable(pfdev->bus_clock);
> > +	clk_disable(pfdev->clock);
> > +	reset_control_assert(pfdev->rstc);
> > +}
> 
> This asymmetry is weird. Why assert the reset control in
> panfrost_clock_disable() but not deassert it in panfrost_clk_enable()?
> It's also unexpected to have a function called _clk_disable do
> something else.

You're right about this. In a newer revision, I've made sure to deassert the reset
control in panfrost_clk_enable() and enable in panfrost_clock_disable().
I've also renamed both functions so that they reflect their handling of the reset
control, besides managing the clocks.

> > +
> >  static int panfrost_clk_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> > @@ -48,7 +82,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
> >  	rate = clk_get_rate(pfdev->clock);
> >  	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
> >  
> > -	err = clk_prepare_enable(pfdev->clock);
> > +	err = clk_prepare(pfdev->clock);
> >  	if (err)
> >  		return err;
> >  
> > @@ -57,44 +91,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
> [...]
> >  static void panfrost_clk_fini(struct panfrost_device *pfdev)
> >  {
> > -	clk_disable_unprepare(pfdev->bus_ace_clock);
> > -	clk_disable_unprepare(pfdev->bus_clock);
> > -	clk_disable_unprepare(pfdev->clock);
> > +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
> > +		panfrost_clk_disable(pfdev);
> 
> So here you are effectively adding a reset_control_assert() that wasn't
> there before. That seems to be intentional because you are removing the
> call to panfrost_reset_fini() in panfrost_device_fini().
> 
> It's confusing though, panfrost_clk_fini() now cleans up things that
> panfrost_clk_init() didn't set up.

Yes, I agree, it looks extremely confusing. As I said above, I've moved all assert
control into the clock disable and enable functions, because they're always meant
to happen in tandem.

> > +
> > +	clk_unprepare(pfdev->bus_ace_clock);
> > +	clk_unprepare(pfdev->bus_clock);
> > +	clk_unprepare(pfdev->clock);
> >  }
> >  
> >  static int panfrost_regulator_init(struct panfrost_device *pfdev)
> > @@ -212,6 +257,133 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
> >  	return err;
> >  }
> >  
> > +static int panfrost_device_runtime_resume(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> > +		ret = reset_control_deassert(pfdev->rstc);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = panfrost_clk_enable(pfdev);
> > +		if (ret)
> > +			goto err_clk;
> 
> Here we clearly deassert the reset and enable the clocks ...
> 
> > +	}
> > +
> > +	panfrost_device_reset(pfdev, true);
> > +	panfrost_devfreq_resume(pfdev);
> > +
> > +	return 0;
> > +
> > +err_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > +		reset_control_assert(pfdev->rstc);
> > +	return ret;
> > +}
> > +
> > +static int panfrost_device_runtime_suspend(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +
> > +	if (!panfrost_jm_is_idle(pfdev))
> > +		return -EBUSY;
> > +
> > +	panfrost_devfreq_suspend(pfdev);
> > +	panfrost_jm_suspend_irq(pfdev);
> > +	panfrost_mmu_suspend_irq(pfdev);
> > +	panfrost_gpu_suspend_irq(pfdev);
> > +	panfrost_gpu_power_off(pfdev);
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > +		panfrost_clk_disable(pfdev);
> 
> ... and here we don't seem to assert the reset, but secretly do.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int panfrost_device_resume(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> > +		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> > +		struct dev_pm_opp *opp;
> > +
> > +		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> > +		if (IS_ERR(opp))
> > +			return PTR_ERR(opp);
> > +		dev_pm_opp_set_opp(dev, opp);
> > +		dev_pm_opp_put(opp);
> > +	}
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > +		ret = clk_enable(pfdev->clock);
> > +		if (ret)
> > +			goto err_clk;
> > +
> > +		if (pfdev->bus_clock) {
> 
> Unnecessary check, clk_enable() handles NULL pointers.

This code I had to shuffle around so that I could call the runtime resume and suspend functions
from panfrost_device_init() and panfrost_device_fini() when the kernel wasn't built with CONFIG_PM.
That means those unnecessary checks were already there. I've decided to postpone their removal until
a next patch series to avoid doing too many things here, but I'll take care of it eventually.

> > +			ret = clk_enable(pfdev->bus_clock);
> > +			if (ret)
> > +				goto err_bus_clk;
> > +		}
> > +	}
> > +
> > +	ret = pm_runtime_force_resume(dev);
> > +	if (ret)
> > +		goto err_resume;
> > +
> > +	return 0;
> > +
> > +err_resume:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> 
> Unnecessary check.
> 
> > +		clk_disable(pfdev->bus_clock);
> > +err_bus_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> > +		clk_disable(pfdev->clock);
> > +err_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > +		dev_pm_opp_set_opp(dev, NULL);
> > +	return ret;
> > +}
> > +
> > +static int panfrost_device_suspend(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = pm_runtime_force_suspend(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > +		if (pfdev->bus_clock)
> 
> Unnecessary check.
> 
> > +			clk_disable(pfdev->bus_clock);
> > +
> > +		clk_disable(pfdev->clock);
> > +	}
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > +		dev_pm_opp_set_opp(dev, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> > +	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> > +	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> > +};
> > +
> > +void panfrost_try_suspend_device(struct panfrost_device *pfdev)
> > +{
> > +	pm_runtime_put_sync_suspend(pfdev->base.dev);
> > +
> > +	/* If PM is disabled, we need to call the suspend handler manually. */
> > +	if (!IS_ENABLED(CONFIG_PM))
> > +		panfrost_device_runtime_suspend(pfdev->base.dev);
> > +}
> > +
> >  int panfrost_device_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> > @@ -265,10 +437,25 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  		goto out_regulator;
> >  	}
> >  
> > -	err = panfrost_gpu_init(pfdev);
> > +	err = devm_pm_runtime_enable(pfdev->base.dev);
> >  	if (err)
> >  		goto out_regulator;
> >  
> > +	err = pm_runtime_resume_and_get(pfdev->base.dev);
> > +	if (err)
> > +		goto out_regulator;
> > +
> > +	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
> > +	if (!IS_ENABLED(CONFIG_PM)) {
> > +		err = panfrost_device_runtime_resume(pfdev->base.dev);
> > +		if (err)
> > +			goto out_regulator;
> > +	}
> > +
> > +	err = panfrost_gpu_init(pfdev);
> > +	if (err)
> > +		goto out_rpm_put;
> > +
> >  	err = panfrost_mmu_init(pfdev);
> >  	if (err)
> >  		goto out_gpu;
> > @@ -295,6 +482,8 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  	panfrost_mmu_fini(pfdev);
> >  out_gpu:
> >  	panfrost_gpu_fini(pfdev);
> > +out_rpm_put:
> > +	panfrost_try_suspend_device(pfdev);
> >  out_regulator:
> >  	panfrost_regulator_fini(pfdev);
> >  out_devfreq:
> 
> Directly below this:
> 
>           panfrost_devfreq_fini(pfdev);                                                                                                       
>   out_clk:                                                                                                                                    
>           panfrost_clk_fini(pfdev);                                                                                                           
>   out_reset:                                                                                                                                  
>           panfrost_reset_fini(pfdev);                                                                                                         
> 
> _reset_fini is still called right after _clk_fini asserted the reset
> control.

Acked. Dealt with this in v5.

> > @@ -313,12 +502,10 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
> >  	panfrost_gem_shrinker_fini(pfdev);
> >  	panfrost_perfcnt_fini(pfdev);
> >  	panfrost_jm_fini(pfdev);
> > -	panfrost_mmu_fini(pfdev);
> > -	panfrost_gpu_fini(pfdev);
> > -	panfrost_devfreq_fini(pfdev);
> > +	panfrost_try_suspend_device(pfdev);
> >  	panfrost_regulator_fini(pfdev);
> > +	panfrost_devfreq_fini(pfdev);
> >  	panfrost_clk_fini(pfdev);
> > -	panfrost_reset_fini(pfdev);
> 
> Here _reset_fini is dropped because _clk_fini  already asserted the
> reset.
> 
> regards
> Philipp

Thanks a lot for the review,
Adrian Larumbe

Re: [PATCH v4 04/13] drm/panfrost: Move shrinker initialization and unplug one level down

[Adrián Larumbe]

Hi Steven,

On 30.07.2026 11:39, Steven Price wrote:
> On 29/07/2026 03:54, Adrián Larumbe wrote:
> > Since the moment we call drm_dev_register() the device should be in a
> > position to accept jobs, so it's best if the shrinker is already
> > initialized by then.
> > 
> > On top of that, make shrinker functions take an panfrost_device pointer
> > like other functions in the same sequence and rename them accordingly.
> > 
> > Essentially mimic the init/fini behaviour in Panthor.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_device.c       |  8 +++++++-
> >  drivers/gpu/drm/panfrost/panfrost_drv.c          |  6 ------
> >  drivers/gpu/drm/panfrost/panfrost_gem.c          | 19 ++++++++++---------
> >  drivers/gpu/drm/panfrost/panfrost_gem.h          |  6 +++---
> >  drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |  8 ++------
> >  5 files changed, 22 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index d8421fd6a662..03905495aee2 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -281,9 +281,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  	if (err)
> >  		goto out_job;
> >  
> > -	panfrost_gem_init(pfdev);
> > +	err = panfrost_gem_init(pfdev);
> > +	if (err)
> > +		goto out_perfcnt;
> >  
> >  	return 0;
> > +
> > +out_perfcnt:
> > +	panfrost_perfcnt_fini(pfdev);
> >  out_job:
> >  	panfrost_jm_fini(pfdev);
> >  out_mmu:
> > @@ -305,6 +310,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  
> >  void panfrost_device_fini(struct panfrost_device *pfdev)
> >  {
> > +	panfrost_gem_shrinker_fini(pfdev);
> >  	panfrost_perfcnt_fini(pfdev);
> >  	panfrost_jm_fini(pfdev);
> >  	panfrost_mmu_fini(pfdev);
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index 2e931fa90da0..331a3bd5b98c 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -1002,14 +1002,9 @@ static int panfrost_probe(struct platform_device *pdev)
> >  	if (err < 0)
> >  		goto err_out1;
> >  
> > -	err = panfrost_gem_shrinker_init(&pfdev->base);
> > -	if (err)
> > -		goto err_out2;
> >  
> >  	return 0;
> >  
> > -err_out2:
> > -	drm_dev_unregister(&pfdev->base);
> >  err_out1:
> >  	pm_runtime_disable(pfdev->base.dev);
> >  	panfrost_device_fini(pfdev);
> > @@ -1023,7 +1018,6 @@ static void panfrost_remove(struct platform_device *pdev)
> >  	struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> >  
> >  	drm_dev_unregister(&pfdev->base);
> > -	panfrost_gem_shrinker_cleanup(&pfdev->base);
> >  
> >  	pm_runtime_get_sync(pfdev->base.dev);
> >  	pm_runtime_disable(pfdev->base.dev);
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > index 3a7fce428898..aa3a604b3439 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > @@ -15,20 +15,21 @@
> >  #include "panfrost_gem.h"
> >  #include "panfrost_mmu.h"
> >  
> > -void panfrost_gem_init(struct panfrost_device *pfdev)
> > +int panfrost_gem_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> >  
> >  	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> > -	    !panfrost_transparent_hugepage)
> > -		return;
> > +	    panfrost_transparent_hugepage) {
> 
> Note you haven't quite inverted the condition here, but I don't think
> the new condition is wrong - we skip the otherwise no-op helpers. The
> old condition seems a bit backward. Indeed I'm not really sure why we
> bother with the IS_ENABLED() check at all. But the new code looks fine.

I've heeded your advice and done away with the IS_ENABLED() check in v5.
I'm curious about why that would work though. Is it because the linker initialises
symbols marked as 'extern' to 0 when they aren't resolved?

Otherwise I would've assumed the compiler would throw out a missing definition
error at link time, but then again I'm unsure about this.

> Reviewed-by: Steven Price <steven.price@arm.com>
> 
> Thanks,
> Steve
> 
> > +		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> > +		if (drm_gem_get_huge_mnt(&pfdev->base))
> > +			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> > +		else if (err)
> > +			drm_warn(&pfdev->base,
> > +				 "Can't use Transparent Hugepage (%d)\n", err);
> > +	}
> >  
> > -	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> > -	if (drm_gem_get_huge_mnt(&pfdev->base))
> > -		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> > -	else if (err)
> > -		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
> > -			 err);
> > +	return panfrost_gem_shrinker_init(pfdev);
> >  }
> >  
> >  #ifdef CONFIG_DEBUG_FS
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> > index 79d4377019e9..d6e55c2e4d86 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> > @@ -129,7 +129,7 @@ drm_mm_node_to_panfrost_mapping(struct drm_mm_node *node)
> >  	return container_of(node, struct panfrost_gem_mapping, mmnode);
> >  }
> >  
> > -void panfrost_gem_init(struct panfrost_device *pfdev);
> > +int panfrost_gem_init(struct panfrost_device *pfdev);
> >  
> >  struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
> >  
> > @@ -154,8 +154,8 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
> >  void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
> >  void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
> >  
> > -int panfrost_gem_shrinker_init(struct drm_device *dev);
> > -void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
> > +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev);
> > +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev);
> >  
> >  void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
> >  int panfrost_gem_sync(struct drm_gem_object *obj, u32 type,
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> > index 2fe967a90bcb..fefae87535d6 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> > @@ -95,10 +95,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
> >   *
> >   * This function registers and sets up the panfrost shrinker.
> >   */
> > -int panfrost_gem_shrinker_init(struct drm_device *dev)
> > +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev)
> >  {
> > -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> > -
> >  	pfdev->shrinker = shrinker_alloc(0, "drm-panfrost");
> >  	if (!pfdev->shrinker)
> >  		return -ENOMEM;
> > @@ -118,10 +116,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev)
> >   *
> >   * This function unregisters the panfrost shrinker.
> >   */
> > -void panfrost_gem_shrinker_cleanup(struct drm_device *dev)
> > +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev)
> >  {
> > -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> > -
> >  	if (pfdev->shrinker)
> >  		shrinker_free(pfdev->shrinker);
> >  }
> > 

Adrian Larumbe

Re: [PATCH v4 05/13] drm/panfrost: Move all device power up and down into RPM callbacks

[Adrián Larumbe]

Hi Steven,

On 30.07.2026 11:40, Steven Price wrote:
> On 29/07/2026 03:54, Adrián Larumbe wrote:
> > During device probe(), failure to do a PM get() will leave the usage_count
> > set to 0, which is the value assigned at device creation time. That means
> > when the autosuspend delay expires, runtime suspend callback won't be
> > invoked, so the device will remain powered on forever.
> > 
> > On top of that, failure to call PM put() during device unplug means
> > Panfrost device's PM usage_count increases monotonically for every new
> > module reload.
> > 
> > The combined outcome of both of the above was that devfreq OPP transition
> > notifications would be printed all the time, even when no jobs are being
> > submitted. This quickly fills the kernel ring buffer with junk.
> > 
> > Even direr than that was the fact MMU interrupts are only enabled when
> > the device is reset, so after device probe() the very first job targeting
> > the tiler heap BO would always time out, because the driver's PM runtime
> > resume callback would not be invoked.
> > 
> > Fix all that by moving all GPU enabling and disabling into RPM resume and
> > suspend callbacks, and making sure we resume the device right before
> > touching any HW registers. This is done in imitation of the Panthor model.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> > Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_device.c | 375 +++++++++++++++++------------
> >  drivers/gpu/drm/panfrost/panfrost_device.h |   9 +
> >  drivers/gpu/drm/panfrost/panfrost_drv.c    |  10 +-
> >  drivers/gpu/drm/panfrost/panfrost_gpu.c    |   7 +-
> >  drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
> >  5 files changed, 237 insertions(+), 171 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index 03905495aee2..a9d29d9ee22b 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -34,6 +34,40 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
> >  	reset_control_assert(pfdev->rstc);
> >  }
> >  
> > +static int panfrost_clk_enable(struct panfrost_device *pfdev)
> > +{
> > +	int err;
> > +
> > +	err = clk_enable(pfdev->clock);
> > +	if (err)
> > +		return err;
> > +
> > +	err = clk_enable(pfdev->bus_clock);
> > +	if (err)
> > +		goto disable_clock;
> > +
> > +	err = clk_enable(pfdev->bus_ace_clock);
> > +	if (err)
> > +		goto disable_bus_clock;
> > +
> > +	return 0;
> > +
> > +disable_bus_clock:
> > +	clk_disable(pfdev->bus_clock);
> > +disable_clock:
> > +	clk_disable(pfdev->clock);
> > +
> > +	return err;
> > +}
> > +
> > +static void panfrost_clk_disable(struct panfrost_device *pfdev)
> > +{
> > +	clk_disable(pfdev->bus_ace_clock);
> > +	clk_disable(pfdev->bus_clock);
> > +	clk_disable(pfdev->clock);
> > +	reset_control_assert(pfdev->rstc);
> > +}
> > +
> >  static int panfrost_clk_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> > @@ -48,7 +82,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
> >  	rate = clk_get_rate(pfdev->clock);
> >  	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
> >  
> > -	err = clk_prepare_enable(pfdev->clock);
> > +	err = clk_prepare(pfdev->clock);
> >  	if (err)
> >  		return err;
> >  
> > @@ -57,44 +91,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
> >  		dev_err(pfdev->base.dev, "get bus_clock failed %ld\n",
> >  			PTR_ERR(pfdev->bus_clock));
> >  		err = PTR_ERR(pfdev->bus_clock);
> > -		goto disable_clock;
> > +		goto unprepare_clock;
> >  	}
> >  
> >  	if (pfdev->bus_clock) {
> >  		rate = clk_get_rate(pfdev->bus_clock);
> >  		dev_info(pfdev->base.dev, "bus_clock rate = %lu\n", rate);
> >  
> > -		err = clk_prepare_enable(pfdev->bus_clock);
> > +		err = clk_prepare(pfdev->bus_clock);
> >  		if (err)
> > -			goto disable_clock;
> > +			goto unprepare_clock;
> >  	}
> >  
> >  	pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, "bus_ace");
> >  	if (IS_ERR(pfdev->bus_ace_clock)) {
> >  		err = PTR_ERR(pfdev->bus_ace_clock);
> >  		dev_err(pfdev->base.dev, "get bus_ace_clock failed %d\n", err);
> > -		goto disable_bus_clock;
> > +		goto unprepare_bus_clock;
> >  	}
> >  
> > -	err = clk_prepare_enable(pfdev->bus_ace_clock);
> > +	err = clk_prepare(pfdev->bus_ace_clock);
> >  	if (err)
> > -		goto disable_bus_clock;
> > +		goto unprepare_bus_clock;
> > +
> > +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT))) {
> > +		err = panfrost_clk_enable(pfdev);
> > +		if (err)
> > +			goto unprepare_bus_ace_clock;
> > +	}
> >  
> >  	return 0;
> >  
> > -disable_bus_clock:
> > -	clk_disable_unprepare(pfdev->bus_clock);
> > -disable_clock:
> > -	clk_disable_unprepare(pfdev->clock);
> > +unprepare_bus_ace_clock:
> > +	clk_unprepare(pfdev->bus_ace_clock);
> > +unprepare_bus_clock:
> > +	clk_unprepare(pfdev->bus_clock);
> > +unprepare_clock:
> > +	clk_unprepare(pfdev->clock);
> >  
> >  	return err;
> >  }
> >  
> >  static void panfrost_clk_fini(struct panfrost_device *pfdev)
> >  {
> > -	clk_disable_unprepare(pfdev->bus_ace_clock);
> > -	clk_disable_unprepare(pfdev->bus_clock);
> > -	clk_disable_unprepare(pfdev->clock);
> > +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
> > +		panfrost_clk_disable(pfdev);
> > +
> > +	clk_unprepare(pfdev->bus_ace_clock);
> > +	clk_unprepare(pfdev->bus_clock);
> > +	clk_unprepare(pfdev->clock);
> >  }
> >  
> >  static int panfrost_regulator_init(struct panfrost_device *pfdev)
> > @@ -212,6 +257,133 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
> >  	return err;
> >  }
> >  
> > +static int panfrost_device_runtime_resume(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> > +		ret = reset_control_deassert(pfdev->rstc);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = panfrost_clk_enable(pfdev);
> > +		if (ret)
> > +			goto err_clk;
> > +	}
> > +
> > +	panfrost_device_reset(pfdev, true);
> > +	panfrost_devfreq_resume(pfdev);
> > +
> > +	return 0;
> > +
> > +err_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > +		reset_control_assert(pfdev->rstc);
> > +	return ret;
> > +}
> > +
> > +static int panfrost_device_runtime_suspend(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +
> > +	if (!panfrost_jm_is_idle(pfdev))
> > +		return -EBUSY;
> > +
> > +	panfrost_devfreq_suspend(pfdev);
> > +	panfrost_jm_suspend_irq(pfdev);
> > +	panfrost_mmu_suspend_irq(pfdev);
> > +	panfrost_gpu_suspend_irq(pfdev);
> > +	panfrost_gpu_power_off(pfdev);
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > +		panfrost_clk_disable(pfdev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int panfrost_device_resume(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> > +		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> > +		struct dev_pm_opp *opp;
> > +
> > +		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> > +		if (IS_ERR(opp))
> > +			return PTR_ERR(opp);
> > +		dev_pm_opp_set_opp(dev, opp);
> > +		dev_pm_opp_put(opp);
> > +	}
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > +		ret = clk_enable(pfdev->clock);
> > +		if (ret)
> > +			goto err_clk;
> > +
> > +		if (pfdev->bus_clock) {
> > +			ret = clk_enable(pfdev->bus_clock);
> > +			if (ret)
> > +				goto err_bus_clk;
> > +		}
> > +	}
> > +
> > +	ret = pm_runtime_force_resume(dev);
> > +	if (ret)
> > +		goto err_resume;
> > +
> > +	return 0;
> > +
> > +err_resume:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> > +		clk_disable(pfdev->bus_clock);
> > +err_bus_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> > +		clk_disable(pfdev->clock);
> > +err_clk:
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > +		dev_pm_opp_set_opp(dev, NULL);
> > +	return ret;
> > +}
> > +
> > +static int panfrost_device_suspend(struct device *dev)
> > +{
> > +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = pm_runtime_force_suspend(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > +		if (pfdev->bus_clock)
> > +			clk_disable(pfdev->bus_clock);
> > +
> > +		clk_disable(pfdev->clock);
> > +	}
> > +
> > +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > +		dev_pm_opp_set_opp(dev, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> > +	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> > +	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> > +};
> > +
> > +void panfrost_try_suspend_device(struct panfrost_device *pfdev)
> > +{
> > +	pm_runtime_put_sync_suspend(pfdev->base.dev);
> > +
> > +	/* If PM is disabled, we need to call the suspend handler manually. */
> > +	if (!IS_ENABLED(CONFIG_PM))
> > +		panfrost_device_runtime_suspend(pfdev->base.dev);
> > +}
> > +
> >  int panfrost_device_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> > @@ -265,10 +437,25 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  		goto out_regulator;
> >  	}
> >  
> > -	err = panfrost_gpu_init(pfdev);
> > +	err = devm_pm_runtime_enable(pfdev->base.dev);
> >  	if (err)
> >  		goto out_regulator;
> >  
> > +	err = pm_runtime_resume_and_get(pfdev->base.dev);
> > +	if (err)
> > +		goto out_regulator;
> > +
> > +	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
> > +	if (!IS_ENABLED(CONFIG_PM)) {
> > +		err = panfrost_device_runtime_resume(pfdev->base.dev);
> > +		if (err)
> > +			goto out_regulator;
> > +	}
> > +
> > +	err = panfrost_gpu_init(pfdev);
> 
> We need the call to panfrost_gpu_init_features() (made by
> panfrost_gpu_init()) before powering up the GPU with
> panfrost_gpu_power_on() (which will now be getting called by the RPM
> callbacks). I think what you've got will attempt to power up the GPU
> with no cores (because the core masks haven't yet been discovered).
> Things will probably work because a suspend/resume cycle afterwards will
> 'fix' things, but this relies on that suspend/resume cycle happening.

I spent some time thinking how to avoid the apparent chicken-and-egg problem of needing
gpu features in order to start the gpu, but also needing the clocks enabled so that I could
poke feature registers. In the end, I left the same gpu_init sequence as before, but
implemented runtime_resume the Panthor way, ie, only when the device is fully initialised
the whole gpu reset sequence takes place in PM resume.

> Thanks,
> Steve
> 
> > +	if (err)
> > +		goto out_rpm_put;
> > +
> >  	err = panfrost_mmu_init(pfdev);
> >  	if (err)
> >  		goto out_gpu;
> > @@ -295,6 +482,8 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >  	panfrost_mmu_fini(pfdev);
> >  out_gpu:
> >  	panfrost_gpu_fini(pfdev);
> > +out_rpm_put:
> > +	panfrost_try_suspend_device(pfdev);
> >  out_regulator:
> >  	panfrost_regulator_fini(pfdev);
> >  out_devfreq:
> > @@ -313,12 +502,10 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
> >  	panfrost_gem_shrinker_fini(pfdev);
> >  	panfrost_perfcnt_fini(pfdev);
> >  	panfrost_jm_fini(pfdev);
> > -	panfrost_mmu_fini(pfdev);
> > -	panfrost_gpu_fini(pfdev);
> > -	panfrost_devfreq_fini(pfdev);
> > +	panfrost_try_suspend_device(pfdev);
> >  	panfrost_regulator_fini(pfdev);
> > +	panfrost_devfreq_fini(pfdev);
> >  	panfrost_clk_fini(pfdev);
> > -	panfrost_reset_fini(pfdev);
> >  	panfrost_pm_domain_fini(pfdev);
> >  }
> >  
> > @@ -424,148 +611,20 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,
> >  
> >  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
> >  {
> > -	panfrost_gpu_soft_reset(pfdev);
> > -
> > -	panfrost_gpu_power_on(pfdev);
> > -	panfrost_mmu_reset(pfdev);
> > -
> > -	panfrost_jm_reset_interrupts(pfdev);
> > -	if (enable_job_int)
> > -		panfrost_jm_enable_interrupts(pfdev);
> > -}
> > -
> > -static int panfrost_device_runtime_resume(struct device *dev)
> > -{
> > -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > -	int ret;
> > -
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> > -		ret = reset_control_deassert(pfdev->rstc);
> > -		if (ret)
> > -			return ret;
> > -
> > -		ret = clk_enable(pfdev->clock);
> > -		if (ret)
> > -			goto err_clk;
> > -
> > -		ret = clk_enable(pfdev->bus_clock);
> > -		if (ret)
> > -			goto err_bus_clk;
> > -
> > -		ret = clk_enable(pfdev->bus_ace_clock);
> > -		if (ret)
> > -			goto err_bus_ace_clk;
> > -	}
> > -
> > -	panfrost_device_reset(pfdev, true);
> > -	panfrost_devfreq_resume(pfdev);
> > -
> > -	return 0;
> > -
> > -err_bus_ace_clk:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > -		clk_disable(pfdev->bus_clock);
> > -err_bus_clk:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > -		clk_disable(pfdev->clock);
> > -err_clk:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> > -		reset_control_assert(pfdev->rstc);
> > -	return ret;
> > -}
> > -
> > -static int panfrost_device_runtime_suspend(struct device *dev)
> > -{
> > -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > -
> > -	if (!panfrost_jm_is_idle(pfdev))
> > -		return -EBUSY;
> > -
> > -	panfrost_devfreq_suspend(pfdev);
> > -	panfrost_jm_suspend_irq(pfdev);
> > -	panfrost_mmu_suspend_irq(pfdev);
> > -	panfrost_gpu_suspend_irq(pfdev);
> > -	panfrost_gpu_power_off(pfdev);
> > -
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> > -		clk_disable(pfdev->bus_ace_clock);
> > -		clk_disable(pfdev->bus_clock);
> > -		clk_disable(pfdev->clock);
> > -		reset_control_assert(pfdev->rstc);
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> > -static int panfrost_device_resume(struct device *dev)
> > -{
> > -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> > -	int ret;
> > -
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> > -		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> > -		struct dev_pm_opp *opp;
> > -
> > -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> > -		if (IS_ERR(opp))
> > -			return PTR_ERR(opp);
> > -		dev_pm_opp_set_opp(dev, opp);
> > -		dev_pm_opp_put(opp);
> > -	}
> > -
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > -		ret = clk_enable(pfdev->clock);
> > -		if (ret)
> > -			goto err_clk;
> > -
> > -		if (pfdev->bus_clock) {
> > -			ret = clk_enable(pfdev->bus_clock);
> > -			if (ret)
> > -				goto err_bus_clk;
> > -		}
> > -	}
> > -
> > -	ret = pm_runtime_force_resume(dev);
> > -	if (ret)
> > -		goto err_resume;
> > -
> > -	return 0;
> > -
> > -err_resume:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> > -		clk_disable(pfdev->bus_clock);
> > -err_bus_clk:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> > -		clk_disable(pfdev->clock);
> > -err_clk:
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > -		dev_pm_opp_set_opp(dev, NULL);
> > -	return ret;
> > -}
> > -
> > -static int panfrost_device_suspend(struct device *dev)
> > -{
> > -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> >  	int ret;
> >  
> > -	ret = pm_runtime_force_suspend(dev);
> > -	if (ret)
> > -		return ret;
> > -
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> > -		if (pfdev->bus_clock)
> > -			clk_disable(pfdev->bus_clock);
> > +	/* Do the actual device reset */
> > +	ret = panfrost_gpu_soft_reset(pfdev);
> > +	panfrost_gpu_power_on(pfdev);
> >  
> > -		clk_disable(pfdev->clock);
> > +	/* Post-reset */
> > +	if (panfrost_device_started(pfdev)) {
> > +		panfrost_mmu_reset(pfdev);
> > +		panfrost_jm_reset_interrupts(pfdev);
> > +		if (enable_job_int)
> > +			panfrost_jm_enable_interrupts(pfdev);
> >  	}
> >  
> > -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> > -		dev_pm_opp_set_opp(dev, NULL);
> > -
> > -	return 0;
> > +	if (!panfrost_device_started(pfdev) && !ret)
> > +		pfdev->gpu_started = true;
> >  }
> > -
> > -EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> > -	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> > -	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> > -};
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> > index ec55c136b1b6..eeca57efcbec 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> > @@ -142,6 +142,7 @@ struct panfrost_device {
> >  	/* pm_domains for devices with more than one. */
> >  	struct device *pm_domain_devs[MAX_PM_DOMAINS];
> >  	struct device_link *pm_domain_links[MAX_PM_DOMAINS];
> > +	bool gpu_started;
> >  	bool coherent;
> >  
> >  	struct panfrost_features features;
> > @@ -253,6 +254,8 @@ int panfrost_device_init(struct panfrost_device *pfdev);
> >  void panfrost_device_fini(struct panfrost_device *pfdev);
> >  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int);
> >  
> > +void panfrost_try_suspend_device(struct panfrost_device *pfdev);
> > +
> >  extern const struct dev_pm_ops panfrost_pm_ops;
> >  
> >  enum drm_panfrost_exception_type {
> > @@ -342,4 +345,10 @@ panfrost_device_schedule_reset(struct panfrost_device *pfdev)
> >  	queue_work(pfdev->reset.wq, &pfdev->reset.work);
> >  }
> >  
> > +static inline bool
> > +panfrost_device_started(struct panfrost_device *pfdev)
> > +{
> > +	return pfdev->gpu_started;
> > +}
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index 331a3bd5b98c..27f4c4b217f5 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -988,9 +988,7 @@ static int panfrost_probe(struct platform_device *pdev)
> >  		goto err_out0;
> >  	}
> >  
> > -	pm_runtime_set_active(pfdev->base.dev);
> >  	pm_runtime_mark_last_busy(pfdev->base.dev);
> > -	pm_runtime_enable(pfdev->base.dev);
> >  	pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */
> >  	pm_runtime_use_autosuspend(pfdev->base.dev);
> >  
> > @@ -1002,13 +1000,12 @@ static int panfrost_probe(struct platform_device *pdev)
> >  	if (err < 0)
> >  		goto err_out1;
> >  
> > +	pm_runtime_put_autosuspend(pfdev->base.dev);
> >  
> >  	return 0;
> >  
> >  err_out1:
> > -	pm_runtime_disable(pfdev->base.dev);
> >  	panfrost_device_fini(pfdev);
> > -	pm_runtime_set_suspended(pfdev->base.dev);
> >  err_out0:
> >  	return err;
> >  }
> > @@ -1019,10 +1016,9 @@ static void panfrost_remove(struct platform_device *pdev)
> >  
> >  	drm_dev_unregister(&pfdev->base);
> >  
> > -	pm_runtime_get_sync(pfdev->base.dev);
> > -	pm_runtime_disable(pfdev->base.dev);
> > +	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);
> > +
> >  	panfrost_device_fini(pfdev);
> > -	pm_runtime_set_suspended(pfdev->base.dev);
> >  }
> >  
> >  static ssize_t profiling_show(struct device *dev,
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> > index 7d555e63e21a..416964282138 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> > @@ -516,9 +516,8 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
> >  {
> >  	int err;
> >  
> > -	err = panfrost_gpu_soft_reset(pfdev);
> > -	if (err)
> > -		return err;
> > +	if (!panfrost_device_started(pfdev))
> > +		return -ETIMEDOUT;
> >  
> >  	err = panfrost_gpu_init_features(pfdev);
> >  	if (err)
> > @@ -543,8 +542,6 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
> >  		return err;
> >  	}
> >  
> > -	panfrost_gpu_power_on(pfdev);
> > -
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> > index 2d12b83e900a..35ff5b0f0013 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> > @@ -446,7 +446,9 @@ void panfrost_jm_suspend_irq(struct panfrost_device *pfdev)
> >  	set_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended);
> >  
> >  	job_write(pfdev, JOB_INT_MASK, 0);
> > -	synchronize_irq(pfdev->js->irq);
> > +
> > +	if (pfdev->js)
> > +		synchronize_irq(pfdev->js->irq);
> >  }
> >  
> >  static void panfrost_job_handle_err(struct panfrost_device *pfdev,
> > @@ -971,6 +973,9 @@ int panfrost_jm_is_idle(struct panfrost_device *pfdev)
> >  	struct panfrost_job_slot *js = pfdev->js;
> >  	int i;
> >  
> > +	if (!js)
> > +		return true;
> > +
> >  	for (i = 0; i < NUM_JOB_SLOTS; i++) {
> >  		/* If there are any jobs in the HW queue, we're not idle */
> >  		if (atomic_read(&js->queue[i].sched.credit_count))
> > 

Adrian Larumbe

Re: [PATCH v4 12/13] drm/panfrost: Fix races between perfcnt and reset sequence

[Adrián Larumbe]

On 30.07.2026 13:49, Steven Price wrote:
> On 29/07/2026 03:54, Adrián Larumbe wrote:
> > Formerly, the reset sequence would race with panfrost_mmu_as_put()
> > when tearing down a perfcnt session. On top of that, poking GPU
> > registers to program a perfcnt session or obtaining a dump might lead to
> > undefined behaviour when done at the same time a reset was ongoing.
> > 
> > Use the reset r/w semaphore to govern access to the hardware at reset
> > time. On top of that, expand the DRM uAPI for the perfcnt DUMP operation
> > so that userspace can be made aware of a reset having happened, because
> > that means counters will go back to 0 and can no longer be accumulated
> > to values previously kept in user space.
> > 
> > The new perfcnt-aware reset sequence also takes care to reestablish
> > perfcnt to its original configuration if there was an enabled session.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_device.c  |   5 +-
> >  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 207 ++++++++++++++++++++--------
> >  drivers/gpu/drm/panfrost/panfrost_perfcnt.h |   2 +
> >  include/uapi/drm/panfrost_drm.h             |   3 +-
> >  4 files changed, 154 insertions(+), 63 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index 926bdf21389f..ae0f10ba7455 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -616,8 +616,10 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
> >  	int ret;
> >  
> >  	/* Pre-reset */
> > -	if (panfrost_device_started(pfdev))
> > +	if (panfrost_device_started(pfdev)) {
> >  		down_write(&pfdev->reset.lock);
> > +		panfrost_perfcnt_reset(pfdev);
> > +	}
> >  
> >  	/* Do the actual device reset */
> >  	ret = panfrost_gpu_soft_reset(pfdev);
> > @@ -629,6 +631,7 @@ void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
> >  		panfrost_jm_reset_interrupts(pfdev);
> >  		if (enable_job_int)
> >  			panfrost_jm_enable_interrupts(pfdev);
> > +		panfrost_perfcnt_postreset(pfdev);
> >  		up_write(&pfdev->reset.lock);
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> > index ad1156678e91..b92ad9061893 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> > @@ -11,6 +11,7 @@
> >  #include <drm/drm_file.h>
> >  #include <drm/drm_gem_shmem_helper.h>
> >  #include <drm/panfrost_drm.h>
> > +#include <drm/drm_print.h>
> >  
> >  #include "panfrost_device.h"
> >  #include "panfrost_features.h"
> > @@ -25,14 +26,17 @@
> >  #define BYTES_PER_COUNTER		4
> >  #define BLOCKS_PER_COREGROUP		8
> >  #define V4_SHADERS_PER_COREGROUP	4
> > +#define PERFCNT_DUMP_MAX_RETRIES	5
> >  
> >  struct panfrost_perfcnt {
> >  	struct panfrost_gem_mapping *mapping;
> > +	unsigned int counterset;
> >  	size_t bosize;
> >  	void *buf;
> >  	struct panfrost_file_priv *user;
> >  	struct mutex lock;
> >  	struct completion dump_comp;
> > +	bool reset_happened;
> >  };
> >  
> >  static void panfrost_perfcnt_gpu_disable(struct panfrost_device *pfdev)
> > @@ -55,38 +59,97 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
> >  	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
> >  }
> >  
> > -static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)
> > +static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev,
> > +					u64 *reset_happened)
> >  {
> > -	u64 gpuva;
> > +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> > +	u64 gpuva = perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> > +	u64 retries = PERFCNT_DUMP_MAX_RETRIES;
> >  	int ret;
> >  
> > -	reinit_completion(&pfdev->perfcnt->dump_comp);
> > -	gpuva = pfdev->perfcnt->mapping->mmnode.start << PAGE_SHIFT;
> > -	gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> > -	gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> > -	gpu_write(pfdev, GPU_INT_CLEAR,
> > -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> > -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> > -	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> > +dump_retry:
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> > +		*reset_happened = perfcnt->reset_happened;
> > +		perfcnt->reset_happened = false;
> > +		reinit_completion(&pfdev->perfcnt->dump_comp);
> > +		gpu_write(pfdev, GPU_PERFCNT_BASE_LO, lower_32_bits(gpuva));
> > +		gpu_write(pfdev, GPU_PERFCNT_BASE_HI, upper_32_bits(gpuva));
> > +		gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_CLEAN_CACHES_COMPLETED |
> > +						GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> > +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_SAMPLE);
> > +	}
> > +
> >  	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
> >  							msecs_to_jiffies(1000));
> > -	if (!ret)
> > -		ret = -ETIMEDOUT;
> > -	else if (ret > 0)
> > -		ret = 0;
> > +
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> > +		if (ret > 0) {
> > +			if (perfcnt->reset_happened) {
> > +				if (--retries >= 0)
> 
> retries is defined unsigned, so this statement is always true...

Sashiko had caught this in a previous iteration and then I forgot to fix it for this one :-/
Anyway, it's already fixed for v5, replaced with an s64.

> > +					goto dump_retry;
> > +				else
> > +					ret = -EBUSY;
> > +			} else {
> > +				ret = 0;
> > +			}
> > +		} else if (!ret) {
> > +			ret = -ETIMEDOUT;
> > +		}
> > +	}
> >  
> >  	return ret;
> >  }
> >  
> > +static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
> > +{
> > +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> > +	u32 cfg, as;
> > +	int ret;
> > +
> > +	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	as = ret;
> > +	cfg = GPU_PERFCNT_CFG_AS(as) |
> > +	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
> > +
> > +	/*
> > +	 * Bifrost GPUs have 2 set of counters, but we're only interested by
> > +	 * the first one for now.
> > +	 */
> > +	if (panfrost_model_is_bifrost(pfdev))
> > +		cfg |= GPU_PERFCNT_CFG_SETSEL(perfcnt->counterset);
> > +
> > +	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
> > +	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
> > +	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
> > +
> > +	/*
> > +	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
> > +	 * counters.
> > +	 */
> > +	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> > +	else
> > +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> > +
> > +	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
> > +
> > +	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > +		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> > +
> > +	return 0;
> > +}
> > +
> >  static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
> >  					  struct drm_file *file_priv,
> >  					  unsigned int counterset)
> >  {
> >  	struct panfrost_file_priv *user = file_priv->driver_priv;
> >  	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> > -	struct iosys_map map;
> >  	struct drm_gem_shmem_object *bo;
> > -	u32 cfg, as;
> > +	struct iosys_map map;
> >  	int ret;
> >  
> >  	if (user == perfcnt->user)
> > @@ -119,7 +182,9 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
> >  	ret = drm_gem_vmap(&bo->base, &map);
> >  	if (ret)
> >  		goto err_put_mapping;
> > +
> >  	perfcnt->buf = map.vaddr;
> > +	perfcnt->counterset = counterset;
> >  
> >  	panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer");
> >  
> > @@ -127,60 +192,46 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
> >  	 * Invalidate the cache and clear the counters to start from a fresh
> >  	 * state.
> >  	 */
> > -	reinit_completion(&pfdev->perfcnt->dump_comp);
> > -	gpu_write(pfdev, GPU_INT_CLEAR,
> > -		  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> > -		  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> > -	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
> > -	gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> > +		reinit_completion(&pfdev->perfcnt->dump_comp);
> > +		gpu_write(pfdev, GPU_INT_CLEAR,
> > +			  GPU_IRQ_CLEAN_CACHES_COMPLETED |
> > +			  GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> > +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
> > +		gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_INV_CACHES);
> > +		perfcnt->reset_happened = false;
> > +		perfcnt->user = user;
> > +	}
> > +
> > +	/*
> > +	 * If a reset happens during the wait for the IRQ notification that caches
> > +	 * are clean and invalidated, then we know the reset sequence did the job
> > +	 * for us, even if it takes long enough for the completion to time out.
> > +	 */
> >  	ret = wait_for_completion_timeout(&pfdev->perfcnt->dump_comp,
> >  					  msecs_to_jiffies(1000));
> > -	if (!ret) {
> > +	if (!ret && !perfcnt->reset_happened) {
> >  		ret = -ETIMEDOUT;
> >  		goto err_vunmap;
> >  	}
> >  
> > -	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
> > -	if (ret < 0)
> > -		goto err_vunmap;
> > -
> > -	as = ret;
> > -	cfg = GPU_PERFCNT_CFG_AS(as) |
> > -	      GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_MANUAL);
> > -
> > -	/*
> > -	 * Bifrost GPUs have 2 set of counters, but we're only interested by
> > -	 * the first one for now.
> > -	 */
> > -	if (panfrost_model_is_bifrost(pfdev))
> > -		cfg |= GPU_PERFCNT_CFG_SETSEL(counterset);
> > -
> > -	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0xffffffff);
> > -	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0xffffffff);
> > -	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0xffffffff);
> > -
> > -	/*
> > -	 * Due to PRLAM-8186 we need to disable the Tiler before we enable HW
> > -	 * counters.
> > -	 */
> > -	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> > -	else
> > -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> > -
> > -	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
> > -
> > -	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> > -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> > +		if (!perfcnt->reset_happened) {
> > +			ret = panfrost_perfcnt_hw_enable(pfdev);
> > +			if (ret)
> > +				goto err_vunmap;
> > +		}
> > +		perfcnt->reset_happened = false;
> > +	}
> >  
> >  	/* The BO ref is retained by the mapping. */
> >  	drm_gem_object_put(&bo->base);
> >  
> > -	perfcnt->user = user;
> > -
> >  	return 0;
> >  
> >  err_vunmap:
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock)
> > +		perfcnt->user = user;
> 
> This looks weird - AFAICT perfcnt->user will already have been set to
> 'user' above if we ever get to this error case. It also unusual to
> assign a pointer like this in an error path - I'd expect at least a
> comment explaining the reasoning.

This was a typo, it was meant to be 'perfcnt->user = NULL. However, I think you're right
that this kind of error path assignment looks rather uncomely. I had fixed the assignment,
but then in v5 Sashiko complained of another error this handling introduces, so for v6
I'll take the semaphore right after the completion is either waited on or expires, reset
the user pointer in the latter case and jump to err_vunmap.

> Thanks,
> Steve
> 
> >  	drm_gem_vunmap(&bo->base, &map);
> >  err_put_mapping:
> >  	panfrost_gem_mapping_put(perfcnt->mapping);
> > @@ -203,13 +254,15 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
> >  	if (user != perfcnt->user)
> >  		return -EINVAL;
> >  
> > -	panfrost_perfcnt_gpu_disable(pfdev);
> > +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> > +		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
> > +		panfrost_perfcnt_gpu_disable(pfdev);
> > +		perfcnt->user = NULL;
> > +	}
> >  
> > -	perfcnt->user = NULL;
> >  	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
> >  	perfcnt->buf = NULL;
> >  	panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv);
> > -	panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
> >  	panfrost_gem_mapping_put(perfcnt->mapping);
> >  	perfcnt->mapping = NULL;
> >  	pm_runtime_put_autosuspend(pfdev->base.dev);
> > @@ -263,7 +316,7 @@ int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
> >  		goto out;
> >  	}
> >  
> > -	ret = panfrost_perfcnt_dump_locked(pfdev);
> > +	ret = panfrost_perfcnt_dump_locked(pfdev, &req->hw_reset);
> >  	if (ret)
> >  		goto out;
> >  
> > @@ -346,3 +399,35 @@ void panfrost_perfcnt_fini(struct panfrost_device *pfdev)
> >  	/* Disable everything before leaving. */
> >  	panfrost_perfcnt_gpu_disable(pfdev);
> >  }
> > +
> > +void panfrost_perfcnt_reset(struct panfrost_device *pfdev)
> > +{
> > +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> > +
> > +	if (drm_WARN_ON(&pfdev->base, !perfcnt))
> > +		return;
> > +
> > +	lockdep_assert_held(&pfdev->reset.lock);
> > +
> > +	if (!perfcnt->user)
> > +		return;
> > +
> > +	perfcnt->reset_happened = true;
> > +	complete(&perfcnt->dump_comp);
> > +	panfrost_perfcnt_gpu_disable(pfdev);
> > +}
> > +
> > +void panfrost_perfcnt_postreset(struct panfrost_device *pfdev)
> > +{
> > +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> > +
> > +	if (drm_WARN_ON(&pfdev->base, !perfcnt))
> > +		return;
> > +
> > +	lockdep_assert_held(&pfdev->reset.lock);
> > +
> > +	if (!perfcnt->user)
> > +		return;
> > +
> > +	panfrost_perfcnt_hw_enable(pfdev);
> > +}
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> > index 8bbcf5f5fb33..e14e760641fd 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.h
> > @@ -14,5 +14,7 @@ int panfrost_ioctl_perfcnt_enable(struct drm_device *dev, void *data,
> >  				  struct drm_file *file_priv);
> >  int panfrost_ioctl_perfcnt_dump(struct drm_device *dev, void *data,
> >  				struct drm_file *file_priv);
> > +void panfrost_perfcnt_reset(struct panfrost_device *pfdev);
> > +void panfrost_perfcnt_postreset(struct panfrost_device *pfdev);
> >  
> >  #endif
> > diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> > index 50d5337f35ef..3bbf9220103d 100644
> > --- a/include/uapi/drm/panfrost_drm.h
> > +++ b/include/uapi/drm/panfrost_drm.h
> > @@ -47,7 +47,7 @@ extern "C" {
> >   * them for anything but debugging purpose.
> >   */
> >  #define DRM_IOCTL_PANFROST_PERFCNT_ENABLE	DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_ENABLE, struct drm_panfrost_perfcnt_enable)
> > -#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
> > +#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
> >  
> >  #define PANFROST_JD_REQ_FS (1 << 0)
> >  #define PANFROST_JD_REQ_CYCLE_COUNT (1 << 1)
> > @@ -272,6 +272,7 @@ struct drm_panfrost_perfcnt_enable {
> >  
> >  struct drm_panfrost_perfcnt_dump {
> >  	__u64 buf_ptr;
> > +	__u64 hw_reset;
> >  };
> >  
> >  /* madvise provides a way to tell the kernel in case a buffers contents
> > 

Regards,
Adrian Larumbe

Re: [PATCH v4 04/13] drm/panfrost: Move shrinker initialization and unplug one level down

[Steven Price]

On 12/08/2026 02:19, Adrián Larumbe wrote:
> Hi Steven,
> 
> On 30.07.2026 11:39, Steven Price wrote:
>> On 29/07/2026 03:54, Adrián Larumbe wrote:
>>> Since the moment we call drm_dev_register() the device should be in a
>>> position to accept jobs, so it's best if the shrinker is already
>>> initialized by then.
>>>
>>> On top of that, make shrinker functions take an panfrost_device pointer
>>> like other functions in the same sequence and rename them accordingly.
>>>
>>> Essentially mimic the init/fini behaviour in Panthor.
>>>
>>> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
>>> ---
>>>  drivers/gpu/drm/panfrost/panfrost_device.c       |  8 +++++++-
>>>  drivers/gpu/drm/panfrost/panfrost_drv.c          |  6 ------
>>>  drivers/gpu/drm/panfrost/panfrost_gem.c          | 19 ++++++++++---------
>>>  drivers/gpu/drm/panfrost/panfrost_gem.h          |  6 +++---
>>>  drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |  8 ++------
>>>  5 files changed, 22 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
>>> index d8421fd6a662..03905495aee2 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
>>> @@ -281,9 +281,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>>>  	if (err)
>>>  		goto out_job;
>>>  
>>> -	panfrost_gem_init(pfdev);
>>> +	err = panfrost_gem_init(pfdev);
>>> +	if (err)
>>> +		goto out_perfcnt;
>>>  
>>>  	return 0;
>>> +
>>> +out_perfcnt:
>>> +	panfrost_perfcnt_fini(pfdev);
>>>  out_job:
>>>  	panfrost_jm_fini(pfdev);
>>>  out_mmu:
>>> @@ -305,6 +310,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>>>  
>>>  void panfrost_device_fini(struct panfrost_device *pfdev)
>>>  {
>>> +	panfrost_gem_shrinker_fini(pfdev);
>>>  	panfrost_perfcnt_fini(pfdev);
>>>  	panfrost_jm_fini(pfdev);
>>>  	panfrost_mmu_fini(pfdev);
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
>>> index 2e931fa90da0..331a3bd5b98c 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
>>> @@ -1002,14 +1002,9 @@ static int panfrost_probe(struct platform_device *pdev)
>>>  	if (err < 0)
>>>  		goto err_out1;
>>>  
>>> -	err = panfrost_gem_shrinker_init(&pfdev->base);
>>> -	if (err)
>>> -		goto err_out2;
>>>  
>>>  	return 0;
>>>  
>>> -err_out2:
>>> -	drm_dev_unregister(&pfdev->base);
>>>  err_out1:
>>>  	pm_runtime_disable(pfdev->base.dev);
>>>  	panfrost_device_fini(pfdev);
>>> @@ -1023,7 +1018,6 @@ static void panfrost_remove(struct platform_device *pdev)
>>>  	struct panfrost_device *pfdev = platform_get_drvdata(pdev);
>>>  
>>>  	drm_dev_unregister(&pfdev->base);
>>> -	panfrost_gem_shrinker_cleanup(&pfdev->base);
>>>  
>>>  	pm_runtime_get_sync(pfdev->base.dev);
>>>  	pm_runtime_disable(pfdev->base.dev);
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
>>> index 3a7fce428898..aa3a604b3439 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
>>> @@ -15,20 +15,21 @@
>>>  #include "panfrost_gem.h"
>>>  #include "panfrost_mmu.h"
>>>  
>>> -void panfrost_gem_init(struct panfrost_device *pfdev)
>>> +int panfrost_gem_init(struct panfrost_device *pfdev)
>>>  {
>>>  	int err;
>>>  
>>>  	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
>>> -	    !panfrost_transparent_hugepage)
>>> -		return;
>>> +	    panfrost_transparent_hugepage) {
>>
>> Note you haven't quite inverted the condition here, but I don't think
>> the new condition is wrong - we skip the otherwise no-op helpers. The
>> old condition seems a bit backward. Indeed I'm not really sure why we
>> bother with the IS_ENABLED() check at all. But the new code looks fine.
> 
> I've heeded your advice and done away with the IS_ENABLED() check in v5.
> I'm curious about why that would work though. Is it because the linker initialises
> symbols marked as 'extern' to 0 when they aren't resolved?
> 
> Otherwise I would've assumed the compiler would throw out a missing definition
> error at link time, but then again I'm unsure about this.

Hmm, I hadn't actually spotted that panfrost_transparent_hugepage wasn't
defined unless CONFIG_TRANSPARENT_HUGEPAGE. Usually this is handled with
a bit of macro magic that #defines the symbol if it's not relevant. I
would expect the linker to complain if this symbol still exists in the
final output.

One possibility is that the compiler is spotting that the
drm_gem_huge_mnt_create() and drm_gem_get_huge_mnt() functions are all
no-ops in this case, so the entire if() branch can be removed (along
with the condition check). Although I'm not sure how safe it is to rely
on such compiler optimisations.

Thanks,
Steve

>> Reviewed-by: Steven Price <steven.price@arm.com>
>>
>> Thanks,
>> Steve
>>
>>> +		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
>>> +		if (drm_gem_get_huge_mnt(&pfdev->base))
>>> +			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
>>> +		else if (err)
>>> +			drm_warn(&pfdev->base,
>>> +				 "Can't use Transparent Hugepage (%d)\n", err);
>>> +	}
>>>  
>>> -	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
>>> -	if (drm_gem_get_huge_mnt(&pfdev->base))
>>> -		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
>>> -	else if (err)
>>> -		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
>>> -			 err);
>>> +	return panfrost_gem_shrinker_init(pfdev);
>>>  }
>>>  
>>>  #ifdef CONFIG_DEBUG_FS
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
>>> index 79d4377019e9..d6e55c2e4d86 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
>>> @@ -129,7 +129,7 @@ drm_mm_node_to_panfrost_mapping(struct drm_mm_node *node)
>>>  	return container_of(node, struct panfrost_gem_mapping, mmnode);
>>>  }
>>>  
>>> -void panfrost_gem_init(struct panfrost_device *pfdev);
>>> +int panfrost_gem_init(struct panfrost_device *pfdev);
>>>  
>>>  struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
>>>  
>>> @@ -154,8 +154,8 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
>>>  void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
>>>  void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
>>>  
>>> -int panfrost_gem_shrinker_init(struct drm_device *dev);
>>> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
>>> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev);
>>> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev);
>>>  
>>>  void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
>>>  int panfrost_gem_sync(struct drm_gem_object *obj, u32 type,
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
>>> index 2fe967a90bcb..fefae87535d6 100644
>>> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
>>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
>>> @@ -95,10 +95,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
>>>   *
>>>   * This function registers and sets up the panfrost shrinker.
>>>   */
>>> -int panfrost_gem_shrinker_init(struct drm_device *dev)
>>> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev)
>>>  {
>>> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
>>> -
>>>  	pfdev->shrinker = shrinker_alloc(0, "drm-panfrost");
>>>  	if (!pfdev->shrinker)
>>>  		return -ENOMEM;
>>> @@ -118,10 +116,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev)
>>>   *
>>>   * This function unregisters the panfrost shrinker.
>>>   */
>>> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev)
>>> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev)
>>>  {
>>> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
>>> -
>>>  	if (pfdev->shrinker)
>>>  		shrinker_free(pfdev->shrinker);
>>>  }
>>>
> 
> Adrian Larumbe

Re: [PATCH v4 04/13] drm/panfrost: Move shrinker initialization and unplug one level down

[Adrián Larumbe]

On 19.08.2026 11:37, Steven Price wrote:
> On 12/08/2026 02:19, Adrián Larumbe wrote:
> > Hi Steven,
> > 
> > On 30.07.2026 11:39, Steven Price wrote:
> >> On 29/07/2026 03:54, Adrián Larumbe wrote:
> >>> Since the moment we call drm_dev_register() the device should be in a
> >>> position to accept jobs, so it's best if the shrinker is already
> >>> initialized by then.
> >>>
> >>> On top of that, make shrinker functions take an panfrost_device pointer
> >>> like other functions in the same sequence and rename them accordingly.
> >>>
> >>> Essentially mimic the init/fini behaviour in Panthor.
> >>>
> >>> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> >>> ---
> >>>  drivers/gpu/drm/panfrost/panfrost_device.c       |  8 +++++++-
> >>>  drivers/gpu/drm/panfrost/panfrost_drv.c          |  6 ------
> >>>  drivers/gpu/drm/panfrost/panfrost_gem.c          | 19 ++++++++++---------
> >>>  drivers/gpu/drm/panfrost/panfrost_gem.h          |  6 +++---
> >>>  drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c |  8 ++------
> >>>  5 files changed, 22 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> >>> index d8421fd6a662..03905495aee2 100644
> >>> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> >>> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> >>> @@ -281,9 +281,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >>>  	if (err)
> >>>  		goto out_job;
> >>>  
> >>> -	panfrost_gem_init(pfdev);
> >>> +	err = panfrost_gem_init(pfdev);
> >>> +	if (err)
> >>> +		goto out_perfcnt;
> >>>  
> >>>  	return 0;
> >>> +
> >>> +out_perfcnt:
> >>> +	panfrost_perfcnt_fini(pfdev);
> >>>  out_job:
> >>>  	panfrost_jm_fini(pfdev);
> >>>  out_mmu:
> >>> @@ -305,6 +310,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> >>>  
> >>>  void panfrost_device_fini(struct panfrost_device *pfdev)
> >>>  {
> >>> +	panfrost_gem_shrinker_fini(pfdev);
> >>>  	panfrost_perfcnt_fini(pfdev);
> >>>  	panfrost_jm_fini(pfdev);
> >>>  	panfrost_mmu_fini(pfdev);
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> >>> index 2e931fa90da0..331a3bd5b98c 100644
> >>> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> >>> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> >>> @@ -1002,14 +1002,9 @@ static int panfrost_probe(struct platform_device *pdev)
> >>>  	if (err < 0)
> >>>  		goto err_out1;
> >>>  
> >>> -	err = panfrost_gem_shrinker_init(&pfdev->base);
> >>> -	if (err)
> >>> -		goto err_out2;
> >>>  
> >>>  	return 0;
> >>>  
> >>> -err_out2:
> >>> -	drm_dev_unregister(&pfdev->base);
> >>>  err_out1:
> >>>  	pm_runtime_disable(pfdev->base.dev);
> >>>  	panfrost_device_fini(pfdev);
> >>> @@ -1023,7 +1018,6 @@ static void panfrost_remove(struct platform_device *pdev)
> >>>  	struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> >>>  
> >>>  	drm_dev_unregister(&pfdev->base);
> >>> -	panfrost_gem_shrinker_cleanup(&pfdev->base);
> >>>  
> >>>  	pm_runtime_get_sync(pfdev->base.dev);
> >>>  	pm_runtime_disable(pfdev->base.dev);
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> >>> index 3a7fce428898..aa3a604b3439 100644
> >>> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> >>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> >>> @@ -15,20 +15,21 @@
> >>>  #include "panfrost_gem.h"
> >>>  #include "panfrost_mmu.h"
> >>>  
> >>> -void panfrost_gem_init(struct panfrost_device *pfdev)
> >>> +int panfrost_gem_init(struct panfrost_device *pfdev)
> >>>  {
> >>>  	int err;
> >>>  
> >>>  	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> >>> -	    !panfrost_transparent_hugepage)
> >>> -		return;
> >>> +	    panfrost_transparent_hugepage) {
> >>
> >> Note you haven't quite inverted the condition here, but I don't think
> >> the new condition is wrong - we skip the otherwise no-op helpers. The
> >> old condition seems a bit backward. Indeed I'm not really sure why we
> >> bother with the IS_ENABLED() check at all. But the new code looks fine.
> > 
> > I've heeded your advice and done away with the IS_ENABLED() check in v5.
> > I'm curious about why that would work though. Is it because the linker initialises
> > symbols marked as 'extern' to 0 when they aren't resolved?
> > 
> > Otherwise I would've assumed the compiler would throw out a missing definition
> > error at link time, but then again I'm unsure about this.
> 
> Hmm, I hadn't actually spotted that panfrost_transparent_hugepage wasn't
> defined unless CONFIG_TRANSPARENT_HUGEPAGE. Usually this is handled with
> a bit of macro magic that #defines the symbol if it's not relevant. I
> would expect the linker to complain if this symbol still exists in the
> final output.

Boris suggested I do this too, namely declaring the symbol as a preprocessor
define in case CONFIG_TRANSPARENT_HUGEPAGE is not enabled.

> One possibility is that the compiler is spotting that the
> drm_gem_huge_mnt_create() and drm_gem_get_huge_mnt() functions are all
> no-ops in this case, so the entire if() branch can be removed (along
> with the condition check). Although I'm not sure how safe it is to rely
> on such compiler optimisations.

My understanding is that one cannot build the kernel without compiler optimisations,
but still for the sake of clarity and to avoid any nasty surprises in the future,
I think I'll just declare panfrost_transparent_hugepage as a macro equal to 0
when the kernel is built without support for transparent huge pages.

> Thanks,
> Steve
> 
> >> Reviewed-by: Steven Price <steven.price@arm.com>
> >>
> >> Thanks,
> >> Steve
> >>
> >>> +		err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> >>> +		if (drm_gem_get_huge_mnt(&pfdev->base))
> >>> +			drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> >>> +		else if (err)
> >>> +			drm_warn(&pfdev->base,
> >>> +				 "Can't use Transparent Hugepage (%d)\n", err);
> >>> +	}
> >>>  
> >>> -	err = drm_gem_huge_mnt_create(&pfdev->base, "within_size");
> >>> -	if (drm_gem_get_huge_mnt(&pfdev->base))
> >>> -		drm_info(&pfdev->base, "Using Transparent Hugepage\n");
> >>> -	else if (err)
> >>> -		drm_warn(&pfdev->base, "Can't use Transparent Hugepage (%d)\n",
> >>> -			 err);
> >>> +	return panfrost_gem_shrinker_init(pfdev);
> >>>  }
> >>>  
> >>>  #ifdef CONFIG_DEBUG_FS
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> >>> index 79d4377019e9..d6e55c2e4d86 100644
> >>> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> >>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> >>> @@ -129,7 +129,7 @@ drm_mm_node_to_panfrost_mapping(struct drm_mm_node *node)
> >>>  	return container_of(node, struct panfrost_gem_mapping, mmnode);
> >>>  }
> >>>  
> >>> -void panfrost_gem_init(struct panfrost_device *pfdev);
> >>> +int panfrost_gem_init(struct panfrost_device *pfdev);
> >>>  
> >>>  struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
> >>>  
> >>> @@ -154,8 +154,8 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo,
> >>>  void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping);
> >>>  void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo);
> >>>  
> >>> -int panfrost_gem_shrinker_init(struct drm_device *dev);
> >>> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
> >>> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev);
> >>> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev);
> >>>  
> >>>  void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
> >>>  int panfrost_gem_sync(struct drm_gem_object *obj, u32 type,
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> >>> index 2fe967a90bcb..fefae87535d6 100644
> >>> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> >>> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> >>> @@ -95,10 +95,8 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
> >>>   *
> >>>   * This function registers and sets up the panfrost shrinker.
> >>>   */
> >>> -int panfrost_gem_shrinker_init(struct drm_device *dev)
> >>> +int panfrost_gem_shrinker_init(struct panfrost_device *pfdev)
> >>>  {
> >>> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> >>> -
> >>>  	pfdev->shrinker = shrinker_alloc(0, "drm-panfrost");
> >>>  	if (!pfdev->shrinker)
> >>>  		return -ENOMEM;
> >>> @@ -118,10 +116,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev)
> >>>   *
> >>>   * This function unregisters the panfrost shrinker.
> >>>   */
> >>> -void panfrost_gem_shrinker_cleanup(struct drm_device *dev)
> >>> +void panfrost_gem_shrinker_fini(struct panfrost_device *pfdev)
> >>>  {
> >>> -	struct panfrost_device *pfdev = to_panfrost_device(dev);
> >>> -
> >>>  	if (pfdev->shrinker)
> >>>  		shrinker_free(pfdev->shrinker);
> >>>  }
> >>>
> > 
> > Adrian Larumbe