On 17/10/2025 16:31, Nicolas Frattaroli wrote:
> On SoCs where the GPU's power-domain is in charge of setting performance
> levels, the OPP table of the GPU node will have already been populated
> during said power-domain's attach_dev operation.
>
> To avoid initialising an OPP table twice, only set the OPP regulator and
> the OPPs from DT if there's no OPP table present.
>
> Reviewed-by: Chia-I Wu <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno
> <[email protected]>
> Signed-off-by: Nicolas Frattaroli <[email protected]>
> ---
> drivers/gpu/drm/panthor/panthor_devfreq.c | 32
> ++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c
> b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index a6dca599f0a5..ec63e27f4883 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -141,6 +141,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
> struct thermal_cooling_device *cooling;
> struct device *dev = ptdev->base.dev;
> struct panthor_devfreq *pdevfreq;
> + struct opp_table *table;
> struct dev_pm_opp *opp;
> unsigned long cur_freq;
> unsigned long freq = ULONG_MAX;
> @@ -152,17 +153,30 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>
> ptdev->devfreq = pdevfreq;
>
> - ret = devm_pm_opp_set_regulators(dev, reg_names);
> - if (ret && ret != -ENODEV) {
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> - return ret;
> + /*
> + * The power domain associated with the GPU may have already added an
> + * OPP table, complete with OPPs, as part of the platform bus
> + * initialization. If this is the case, the power domain is in charge of
> + * also controlling the performance, with a set_performance callback.
> + * Only add a new OPP table from DT if there isn't such a table present
> + * already.
> + */
> + table = dev_pm_opp_get_opp_table(dev);
> + if (IS_ERR_OR_NULL(table)) {
> + ret = devm_pm_opp_set_regulators(dev, reg_names);
> + if (ret && ret != -ENODEV) {
Is there a reason to not fail on -ENODEV? I would assume it is a valid
failure path.
Kind regards,
Karunika Choo
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "Couldn't set OPP
> regulators\n");
> + return ret;
> + }
> +
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret)
> + return ret;
> + } else {
> + dev_pm_opp_put_opp_table(table);
> }
>
> - ret = devm_pm_opp_of_add_table(dev);
> - if (ret)
> - return ret;
> -
> spin_lock_init(&pdevfreq->lock);
>
> panthor_devfreq_reset(pdevfreq);
>
