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 <[email protected]> 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/[email protected]?part=5
