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

[sashiko-bot@kernel.org]

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