[PATCH v2 0/2] Two small perfcnt fixes for Panfrost

[PATCH v2 0/2] Two small perfcnt fixes for Panfrost

[Adrián Larumbe]

To: Boris Brezillon <boris.brezillon@collabora.com>
To: Rob Herring <robh@kernel.org>
To: Steven Price <steven.price@arm.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>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

First fix makes sure cycle counter is enabled when starting a new
session. This is required by some profiling frameworks like
Perfetto. Second fix deals with a recently-introduced race condition.

---
Changes in v2:
- Added review trailers.
- Removed warning from cycle count reference check.
- Link to v1: https://patch.msgid.link/20261015-panfrost-reset-races-fixes-v1-0-bb74c1786a0b@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>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org

---
Adrián Larumbe (1):
      drm/panfrost: Fix race when restoring reset state in perfcnt

Boris Brezillon (1):
      drm/panfrost: Enable cycle counting when perfcnt is enabled

 drivers/gpu/drm/panfrost/panfrost_gpu.c     |  6 +++---
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 18 ++++++++++--------
 2 files changed, 13 insertions(+), 11 deletions(-)
---
base-commit: 6f7098738fff3ec7393f924343188cb8e6ebc5d2
change-id: 20261014-panfrost-reset-races-fixes-6f5f2459dc9b

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

[PATCH v2 1/2] drm/panfrost: Enable cycle counting when perfcnt is enabled

[Adrián Larumbe]

From: Boris Brezillon <boris.brezillon@collabora.com>

If we don't do that, we end up with invalid timestamps in the block
headers.

We also need to avoid the warning on the cycle counter reference being
other than 0 during device reset, because of perfcnt's lingering ref.

Ideally this would be dealt with by splitting the reset sequence into
pre and post stages, so that a hypothetical panfrost_perfcnt_pre_reset()
would release the cycle counter reference. In the absence thereof, we
just make do.

Fixes: 7786fd108777 ("drm/panfrost: Expose performance counters through unstable ioctls")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_gpu.c     | 6 +++---
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 6081b27829e1..7d66b1b50968 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -94,10 +94,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
 		  GPU_IRQ_CLEAN_CACHES_COMPLETED);
 
 	/*
-	 * All in-flight jobs should have released their cycle
-	 * counter references upon reset, but let us make sure
+	 * Even though all in-flight jobs should have released their cycle counter
+	 * references upon reset, perfcnt's might still hang about.
 	 */
-	if (drm_WARN_ON(&pfdev->base, atomic_read(&pfdev->cycle_counter.use_count) != 0))
+	if (atomic_read(&pfdev->cycle_counter.use_count) != 0)
 		atomic_set(&pfdev->cycle_counter.use_count, 0);
 
 	return 0;
diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
index a9def452d5ef..b26da057a1a0 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
@@ -17,6 +17,7 @@
 #include "panfrost_device.h"
 #include "panfrost_features.h"
 #include "panfrost_gem.h"
+#include "panfrost_gpu.h"
 #include "panfrost_issues.h"
 #include "panfrost_job.h"
 #include "panfrost_mmu.h"
@@ -52,6 +53,7 @@ static void panfrost_perfcnt_hw_disable(struct panfrost_device *pfdev)
 	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
 
 	if (perfcnt->owns_as_ref) {
+		panfrost_cycle_counter_put(pfdev);
 		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
 		perfcnt->owns_as_ref = false;
 	}
@@ -84,6 +86,7 @@ static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
 		return ret;
 
 	perfcnt->owns_as_ref = true;
+	panfrost_cycle_counter_get(pfdev);
 
 	as = ret;
 	cfg = GPU_PERFCNT_CFG_AS(as) |

-- 
2.56.0

Re: [PATCH v2 1/2] drm/panfrost: Enable cycle counting when perfcnt is enabled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panfrost: cycle_counter ref leak in panfrost_perfcnt_reset() on timeout
--

commit bc0ac72ceaf78a79c6a160afa6594381db03f37c
Author: Boris Brezillon <boris.brezillon@collabora.com>

drm/panfrost: Enable cycle counting when perfcnt is enabled

This commit enables the GPU cycle counter while performance counters are
enabled so block headers receive valid timestamps, and drops the warning on
a non-zero cycle counter reference count during GPU soft reset.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 6081b27829e1f..7d66b1b509689 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -94,10 +94,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
>  		  GPU_IRQ_CLEAN_CACHES_COMPLETED);
>  
>  	/*
> -	 * All in-flight jobs should have released their cycle
> -	 * counter references upon reset, but let us make sure
> +	 * Even though all in-flight jobs should have released their cycle counter
> +	 * references upon reset, perfcnt's might still hang about.
>  	 */
> -	if (drm_WARN_ON(&pfdev->base, atomic_read(&pfdev->cycle_counter.use_count) != 0))
> +	if (atomic_read(&pfdev->cycle_counter.use_count) != 0)
>  		atomic_set(&pfdev->cycle_counter.use_count, 0);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index a9def452d5ef9..b26da057a1a01 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c

[ ... ]

> @@ -84,6 +86,7 @@ static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
>  		return ret;
>  
>  	perfcnt->owns_as_ref = true;
> +	panfrost_cycle_counter_get(pfdev);

[Severity: Medium]
Can this leak a reference on pfdev->cycle_counter.use_count if the GPU reset
times out in panfrost_gpu_soft_reset()?

In drivers/gpu/drm/panfrost/panfrost_gpu.c, panfrost_gpu_soft_reset() returns
early if both soft and hard resets time out, before resetting
pfdev->cycle_counter.use_count to 0:

drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_soft_reset() {
	...
		if (ret) {
			dev_err(pfdev->base.dev, "gpu hard reset timed out\n");
			return ret;
		}
	}
	...
	if (atomic_read(&pfdev->cycle_counter.use_count) != 0)
		atomic_set(&pfdev->cycle_counter.use_count, 0);
}

However, in drivers/gpu/drm/panfrost/panfrost_device.c,
panfrost_device_reset() ignores the return value of panfrost_gpu_soft_reset()
and still proceeds to call panfrost_mmu_reset() and panfrost_perfcnt_reset():

drivers/gpu/drm/panfrost/panfrost_device.c:panfrost_device_reset() {
	guard(rwsem_write)(&pfdev->reset.lock);

	panfrost_gpu_soft_reset(pfdev);

	panfrost_gpu_power_on(pfdev);
	panfrost_mmu_reset(pfdev);
	...
	panfrost_perfcnt_reset(pfdev);
}

When an active perfcnt session is present, panfrost_perfcnt_reset() clears
perfcnt->owns_as_ref without dropping the existing cycle_counter reference
and calls panfrost_perfcnt_hw_enable():

drivers/gpu/drm/panfrost/panfrost_perfcnt.c:panfrost_perfcnt_reset() {
	...
	/* All active AS are released during the MMU post_reset. */
	perfcnt->owns_as_ref = false;
	perfcnt->state |= PANFROST_PERFCNT_SESSION_INTERRUPTED_BY_RESET;
	if (panfrost_perfcnt_hw_enable(pfdev))
		perfcnt->state |= PANFROST_PERFCNT_SESSION_DEAD;
	...
}

Because panfrost_mmu_reset() cleared pfdev->as_alloc_mask,
panfrost_mmu_as_get() succeeds in panfrost_perfcnt_hw_enable() and
panfrost_cycle_counter_get() increments pfdev->cycle_counter.use_count a
second time without the prior reference having been cleared or released. When
panfrost_perfcnt_hw_disable() is later called, panfrost_cycle_counter_put()
only decrements pfdev->cycle_counter.use_count once, leaving it non-zero and
preventing GPU_CMD_CYCLE_COUNT_STOP from being issued.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-panfrost-reset-races-fixes-v2-0-454207b1d4e3@collabora.com?part=1

Re: [PATCH v2 1/2] drm/panfrost: Enable cycle counting when perfcnt is enabled

[Steven Price]

On 03/11/2026 20:36, Adrián Larumbe wrote:
> From: Boris Brezillon <boris.brezillon@collabora.com>
> 
> If we don't do that, we end up with invalid timestamps in the block
> headers.
> 
> We also need to avoid the warning on the cycle counter reference being
> other than 0 during device reset, because of perfcnt's lingering ref.
> 
> Ideally this would be dealt with by splitting the reset sequence into
> pre and post stages, so that a hypothetical panfrost_perfcnt_pre_reset()
> would release the cycle counter reference. In the absence thereof, we
> just make do.
> 
> Fixes: 7786fd108777 ("drm/panfrost: Expose performance counters through unstable ioctls")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_gpu.c     | 6 +++---
>  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 3 +++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 6081b27829e1..7d66b1b50968 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -94,10 +94,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
>  		  GPU_IRQ_CLEAN_CACHES_COMPLETED);
>  
>  	/*
> -	 * All in-flight jobs should have released their cycle
> -	 * counter references upon reset, but let us make sure
> +	 * Even though all in-flight jobs should have released their cycle counter
> +	 * references upon reset, perfcnt's might still hang about.
>  	 */
> -	if (drm_WARN_ON(&pfdev->base, atomic_read(&pfdev->cycle_counter.use_count) != 0))
> +	if (atomic_read(&pfdev->cycle_counter.use_count) != 0)
>  		atomic_set(&pfdev->cycle_counter.use_count, 0);

There's no point in doing the read - we might as well just
unconditionally do the atomic_set().

With that fixed:

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

>  
>  	return 0;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index a9def452d5ef..b26da057a1a0 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -17,6 +17,7 @@
>  #include "panfrost_device.h"
>  #include "panfrost_features.h"
>  #include "panfrost_gem.h"
> +#include "panfrost_gpu.h"
>  #include "panfrost_issues.h"
>  #include "panfrost_job.h"
>  #include "panfrost_mmu.h"
> @@ -52,6 +53,7 @@ static void panfrost_perfcnt_hw_disable(struct panfrost_device *pfdev)
>  	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
>  
>  	if (perfcnt->owns_as_ref) {
> +		panfrost_cycle_counter_put(pfdev);
>  		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
>  		perfcnt->owns_as_ref = false;
>  	}
> @@ -84,6 +86,7 @@ static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
>  		return ret;
>  
>  	perfcnt->owns_as_ref = true;
> +	panfrost_cycle_counter_get(pfdev);
>  
>  	as = ret;
>  	cfg = GPU_PERFCNT_CFG_AS(as) |
> 

[PATCH v2 2/2] drm/panfrost: Fix race when restoring reset state in perfcnt

[Adrián Larumbe]

struct panfrost_perfcnt::state should always be updated atomically, both
in the dump ioctl and the reset sequence.

Fixes: 9944dd07f3cf ("drm/panfrost: Fix races between perfcnt and reset sequence")
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 | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
index b26da057a1a0..d50b6cc2ecfa 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
@@ -148,18 +148,17 @@ static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev, u32 *stat
 	 */
 	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
 							msecs_to_jiffies(1000));
-
 	if (!ret)
 		ret = -ETIMEDOUT;
 
-	/* We need to restore perfcnt state so the next successful dump can relay it to UM */
-	if (ret < 0) {
-		perfcnt->state |= *state;
-		*state = 0;
-		return ret;
-	}
-
 	scoped_guard(rwsem_read, &pfdev->reset.lock) {
+		/* Restore perfcnt state so the next successful dump can relay it to UM */
+		if (ret < 0) {
+			perfcnt->state |= *state;
+			*state = 0;
+			return ret;
+		}
+
 		*state |= perfcnt->state;
 
 		/* UM must re-enable their session before requesting new dumps. */

-- 
2.56.0

Re: [PATCH v2 2/2] drm/panfrost: Fix race when restoring reset state in perfcnt

[Steven Price]

On 03/11/2026 20:36, Adrián Larumbe wrote:
> struct panfrost_perfcnt::state should always be updated atomically, both
> in the dump ioctl and the reset sequence.
> 
> Fixes: 9944dd07f3cf ("drm/panfrost: Fix races between perfcnt and reset sequence")
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>

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

> ---
>  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index b26da057a1a0..d50b6cc2ecfa 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -148,18 +148,17 @@ static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev, u32 *stat
>  	 */
>  	ret = wait_for_completion_interruptible_timeout(&pfdev->perfcnt->dump_comp,
>  							msecs_to_jiffies(1000));
> -
>  	if (!ret)
>  		ret = -ETIMEDOUT;
>  
> -	/* We need to restore perfcnt state so the next successful dump can relay it to UM */
> -	if (ret < 0) {
> -		perfcnt->state |= *state;
> -		*state = 0;
> -		return ret;
> -	}
> -
>  	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		/* Restore perfcnt state so the next successful dump can relay it to UM */
> +		if (ret < 0) {
> +			perfcnt->state |= *state;
> +			*state = 0;
> +			return ret;
> +		}
> +
>  		*state |= perfcnt->state;
>  
>  		/* UM must re-enable their session before requesting new dumps. */
>