Re: [PATCH v14 05/15] drm/panfrost: Consolidate device clock management and reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panfrost: global prepare_lock mutex contention in runtime PM hot path
--

commit 3af44bee15a2caec9722e2975dc04d5f8f206925
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Consolidate device clock management and reset

Gather all clock enables and disables into a single function to avoid
repetitions between driver init/fini and device resume/suspend, since
these clocks are always handled in bulk.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index b3a53504bd016..50f4713bfcfdd 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -34,10 +34,46 @@ static void panfrost_reset_fini(struct panfrost_device *pfdev)
>  	reset_control_assert(pfdev->rstc);
>  }
>  
> -static int panfrost_clk_init(struct panfrost_device *pfdev)
> +static int panfrost_clks_enable(struct panfrost_device *pfdev, bool on_resume)
>  {
>  	int err;
> +
> +	err = clk_prepare_enable(pfdev->clock);

[Severity: Medium]
Does this introduce global prepare_lock mutex contention into the runtime PM
hot path?

By replacing the atomic-safe clk_enable() and clk_disable() calls with
clk_prepare_enable() and clk_disable_unprepare() in the runtime PM suspend
and resume paths, this code will now unconditionally acquire the system-wide
prepare_lock mutex during high-frequency GPU idle transitions.

Even though panfrost_clk_init() appears to still prepare the clocks, the
Common Clock Framework's clk_prepare() unconditionally acquires the global
mutex to manage the software reference count. Acquiring this global mutex
during rapid runtime PM transitions defeats the separation of the slow prepare
phase from the fast enable phase, potentially causing unnecessary
serialization and latency spikes across the system.

[ ... ]

> @@ -436,34 +478,17 @@ static int panfrost_device_runtime_resume(struct device *dev)
>  		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;
> +		ret = panfrost_clks_enable(pfdev, true);
> +		if (ret) {
> +			reset_control_assert(pfdev->rstc);
> +			return ret;
> +		}
>  	}

[ ... ]

> @@ -480,9 +505,7 @@ static int panfrost_device_runtime_suspend(struct device *dev)
>  	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);
> +		panfrost_clks_disable(pfdev, true);
>  		reset_control_assert(pfdev->rstc);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-claude-fixes-v14-0-cb743b3b973b@collabora.com?part=5