On 29/09/2026 04:44, Adrián Larumbe wrote: > 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. > > Also do clk (un)prepares and dis/enables at the same time, since the > clk_prepare_* family of functions can simply increase the refcnt of > an already prepared clock. > > Reviewed-by: Boris Brezillon <[email protected]> > Signed-off-by: Adrián Larumbe <[email protected]>
Reviewed-by: Steven Price <[email protected]> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 118 > +++++++++++++---------------- > 1 file changed, 52 insertions(+), 66 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index b3a53504bd01..9f2b1967a398 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); > + if (err) > + return err; > + > + err = clk_prepare_enable(pfdev->bus_clock); > + if (err) > + goto disable_clock; > + > + if (on_resume) { > + err = clk_prepare_enable(pfdev->bus_ace_clock); > + if (err) > + goto disable_bus_clock; > + } > + > + return 0; > + > +disable_bus_clock: > + clk_disable_unprepare(pfdev->bus_clock); > +disable_clock: > + clk_disable_unprepare(pfdev->clock); > + > + return err; > +} > + > +static void panfrost_clks_disable(struct panfrost_device *pfdev, bool > on_suspend) > +{ > + if (on_suspend) > + clk_disable_unprepare(pfdev->bus_ace_clock); > + clk_disable_unprepare(pfdev->bus_clock); > + clk_disable_unprepare(pfdev->clock); > +} > + > +static int panfrost_clk_init(struct panfrost_device *pfdev) > +{ > unsigned long rate; > + int err; > > pfdev->clock = devm_clk_get(pfdev->base.dev, NULL); > if (IS_ERR(pfdev->clock)) { > @@ -48,53 +84,31 @@ 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); > - if (err) > - return err; > - > pfdev->bus_clock = devm_clk_get_optional(pfdev->base.dev, "bus"); > if (IS_ERR(pfdev->bus_clock)) { > - 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; > + dev_err(pfdev->base.dev, "get bus_clock failed %d\n", err); > + return err; > } > > 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); > - if (err) > - goto disable_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; > + return err; > } > > - err = clk_prepare_enable(pfdev->bus_ace_clock); > - if (err) > - goto disable_bus_clock; > - > - return 0; > - > -disable_bus_clock: > - clk_disable_unprepare(pfdev->bus_clock); > -disable_clock: > - clk_disable_unprepare(pfdev->clock); > - > - return err; > + return panfrost_clks_enable(pfdev, true); > } > > 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); > + panfrost_clks_disable(pfdev, true); > } > > static int panfrost_regulator_init(struct panfrost_device *pfdev) > @@ -436,34 +450,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; > + } > } > > 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) > @@ -480,9 +477,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); > } > > @@ -506,13 +501,9 @@ static int panfrost_device_resume(struct device *dev) > } > > if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { > - ret = clk_enable(pfdev->clock); > + ret = panfrost_clks_enable(pfdev, false); > if (ret) > goto err_clk; > - > - ret = clk_enable(pfdev->bus_clock); > - if (ret) > - goto err_bus_clk; > } > > ret = pm_runtime_force_resume(dev); > @@ -523,10 +514,7 @@ static int panfrost_device_resume(struct device *dev) > > err_resume: > if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) > - clk_disable(pfdev->bus_clock); > -err_bus_clk: > - if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) > - clk_disable(pfdev->clock); > + panfrost_clks_disable(pfdev, false); > err_clk: > if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) > dev_pm_opp_set_opp(dev, NULL); > @@ -542,10 +530,8 @@ static int panfrost_device_suspend(struct device *dev) > if (ret) > return ret; > > - if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) { > - clk_disable(pfdev->bus_clock); > - clk_disable(pfdev->clock); > - } > + if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) > + panfrost_clks_disable(pfdev, false); > > if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) > dev_pm_opp_set_opp(dev, NULL); >
