Re: [PATCH RFC 09/10] drm/panthor: devfreq: add pluggable devfreq providers

Steven Price
2025-09-10T13:59:41Z

On 05/09/2025 11:23, Nicolas Frattaroli wrote:
> On some devices, devfreq is not controlled directly by DT OPPs and the
> common clock framework, but through an external devfreq driver. To
> permit this type of usage, add the concept of devfreq providers.
>
> Devfreq providers for panthor register themselves with panthor as a
> provider. panthor then gets whatever driver is suckling on its
> performance-node property, finds the registered devfreq provider init
> function for it, and calls it.
>
> Should the probe order work out such that panthor probes before the
> devfreq provider is finished probing and registering itself, then we
> just defer the probe after adding a device link.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Looks fine to me, but I think there are some updates needed due to the
DT review.

Thanks,
Steve

> ---
> drivers/gpu/drm/panthor/panthor_devfreq.c | 74 ++++++++++++++++++++++++++++++-
> drivers/gpu/drm/panthor/panthor_devfreq.h | 16 +++++++
> 2 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index d495dd856299826c4bddc205087d8914d01d7fc4..1b0c21f6f069b059b8b0e412a79556c602c5f1e7 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -4,6 +4,7 @@
> #include <linux/clk.h>
> #include <linux/devfreq.h>
> #include <linux/devfreq_cooling.h>
> +#include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/pm_opp.h>
>
> @@ -12,6 +13,34 @@
> #include "panthor_devfreq.h"
> #include "panthor_device.h"
>
> +static LIST_HEAD(panthor_devfreq_providers);
> +static DEFINE_MUTEX(panthor_devfreq_providers_lock);
> +
> +int panthor_devfreq_register_provider(struct panthor_devfreq_provider *prov)
> +{
> + guard(mutex)(&panthor_devfreq_providers_lock);
> +
> + list_add(&prov->node, &panthor_devfreq_providers);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(panthor_devfreq_register_provider);
> +
> +static int panthor_devfreq_init_provider(struct panthor_device *ptdev,
> + struct device *provider_dev)
> +{
> + struct panthor_devfreq_provider *prov;
> +
> + guard(mutex)(&panthor_devfreq_providers_lock);
> +
> + list_for_each_entry(prov, &panthor_devfreq_providers, node) {
> + if (prov->dev == provider_dev)
> + return prov->init(ptdev, provider_dev);
> + }
> +
> + return -EPROBE_DEFER;
> +}
> +
> static void panthor_devfreq_update_utilization(struct panthor_devfreq *pdevfreq)
> {
> ktime_t now, last;
> @@ -102,7 +131,7 @@ static struct devfreq_dev_profile panthor_devfreq_profile = {
> .get_cur_freq = panthor_devfreq_get_cur_freq,
> };
>
> -int panthor_devfreq_init(struct panthor_device *ptdev)
> +static int panthor_devfreq_init_of(struct panthor_device *ptdev)
> {
> /* There's actually 2 regulators (mali and sram), but the OPP core only
> * supports one.
> @@ -222,6 +251,49 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
> return 0;
> }
>
> +static int panthor_devfreq_init_platform(struct panthor_device *ptdev)
> +{
> + struct device_node *pcnode;
> + struct platform_device *pdev;
> + struct device_link *link;
> + int ret;
> +
> + pcnode = of_parse_phandle(ptdev->base.dev->of_node,
> + "performance-controller", 0);
> + if (!pcnode)
> + return -EINVAL;
> +
> + pdev = of_find_device_by_node(pcnode);
> + of_node_put(pcnode);
> + if (!pdev)
> + return -ENODEV;
> +
> + link = device_link_add(ptdev->base.dev, &pdev->dev,
> + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
> + if (!link) {
> + dev_err(ptdev->base.dev, "failed to add device link\n");
> + return -ENODEV;
> + }
> +
> + ret = panthor_devfreq_init_provider(ptdev, &pdev->dev);
> + if (ret)
> + return dev_err_probe(ptdev->base.dev, ret,
> + "failed to initialize devfreq provider\n");
> +
> + DRM_DEV_INFO(ptdev->base.dev, "initialized devfreq provider %s\n",
> + dev_name(&pdev->dev));
> +
> + return 0;
> +}
> +
> +int panthor_devfreq_init(struct panthor_device *ptdev)
> +{
> + if (!of_property_present(ptdev->base.dev->of_node, "performance-controller"))
> + return panthor_devfreq_init_of(ptdev);
> +
> + return panthor_devfreq_init_platform(ptdev);
> +}
> +
> void panthor_devfreq_resume(struct panthor_device *ptdev)
> {
> struct panthor_devfreq *pdevfreq = ptdev->devfreq;
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.h b/drivers/gpu/drm/panthor/panthor_devfreq.h
> index a891cb5fdc34636444f141e10f5d45828fc35b51..94c9768d5d038c4ba8516929edb565a1f13443fb 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.h
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.h
> @@ -8,6 +8,7 @@
>
> struct devfreq;
> struct thermal_cooling_device;
> +struct platform_device;
>
> struct panthor_device;
>
> @@ -43,6 +44,19 @@ struct panthor_devfreq {
> spinlock_t lock;
> };
>
> +struct panthor_devfreq_provider {
> + /** @dev: device pointer to the provider device */
> + struct device *dev;
> + /**
> + * @init: the provider's init callback that allocates a
> + * &struct panthor_devfreq, adds it to panthor, and adds a devfreq
> + * device to panthor. Will be called during panthor's probe.
> + */
> + int (*init)(struct panthor_device *ptdev, struct device *dev);
> +
> + struct list_head node;
> +};
> +
>
> int panthor_devfreq_init(struct panthor_device *ptdev);
>
> @@ -57,4 +71,6 @@ int panthor_devfreq_get_dev_status(struct device *dev,
>
> unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev);
>
> +int panthor_devfreq_register_provider(struct panthor_devfreq_provider *prov);
> +
> #endif /* __PANTHOR_DEVFREQ_H__ */
>