Re: [PATCH v1 09/27] drm/panthor: Skip devfreq when no OPP table is present

[Liviu Dudau]

On Tue, Sep 22, 2026 at 09:45:03PM +0100, Karunika Choo wrote:
> On Mali v15 AM systems, frequency scaling is handled outside panthor by
> the AM_GOVERNOR block, so the GPU DT node may not provide an OPP table.
> 
> Make panthor_devfreq_init() return early when dev_pm_opp_get_opp_table()
> returns -ENODEV, and guard the devfreq helper paths against a missing
> devfreq instance. Also make it so that frequency can be queried even if
> the devfreq instance is missing.
> 
> This keeps devfreq enabled for existing platforms while allowing v15 AM
> systems to probe without a local devfreq setup.
> 
> Signed-off-by: Karunika Choo <karunika.choo@xxxxxxx>
> ---
> v1:
> - Only skip panthor_devfreq init when OPP lookup returns -ENODEV instead
>   of direct DT-property check.
> - Support frequency query without devfreq.
> 
>  drivers/gpu/drm/panthor/panthor_devfreq.c | 31 ++++++++++++++++++-----
>  drivers/gpu/drm/panthor/panthor_hw.h      |  6 +++++
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index 2249b41ca4afe..18c1a7e56ca08 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -12,6 +12,7 @@
>  
>  #include "panthor_devfreq.h"
>  #include "panthor_device.h"
> +#include "panthor_hw.h"
>  
>  /**
>   * struct panthor_devfreq - Device frequency management
> @@ -164,6 +165,19 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  	 */
>  	table = dev_pm_opp_get_opp_table(dev);
>  	if (IS_ERR_OR_NULL(table)) {
> +		/*
> +		 * On v15 GPUs, frequency scaling is handled outside of panthor
> +		 * by the AM_GOVERNOR block, so the GPU DT node may not provide
> +		 * an OPP table. In such cases, do not initialize devfreq for
> +		 * panthor.
> +		 */
> +		if (panthor_hw_is_gen5_am(ptdev->base.dev) &&
> +		    PTR_ERR(table) == -ENODEV) {
> +			drmm_kfree(&ptdev->base, pdevfreq);
> +			ptdev->devfreq = NULL;
> +			return 0;
> +		}
> +
>  		ret = devm_pm_opp_set_regulators(dev, reg_names);
>  		if (ret && ret != -ENODEV) {
>  			if (ret != -EPROBE_DEFER)
> @@ -267,7 +281,7 @@ void panthor_devfreq_resume(struct panthor_device *ptdev)
>  {
>  	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
>  
> -	if (!pdevfreq->devfreq)
> +	if (!pdevfreq || !pdevfreq->devfreq)
>  		return;
>  
>  	panthor_devfreq_reset(pdevfreq);
> @@ -279,7 +293,7 @@ void panthor_devfreq_suspend(struct panthor_device *ptdev)
>  {
>  	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
>  
> -	if (!pdevfreq->devfreq)
> +	if (!pdevfreq || !pdevfreq->devfreq)
>  		return;
>  
>  	drm_WARN_ON(&ptdev->base, devfreq_suspend_device(pdevfreq->devfreq));
> @@ -290,7 +304,7 @@ void panthor_devfreq_record_busy(struct panthor_device *ptdev)
>  	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
>  	unsigned long irqflags;
>  
> -	if (!pdevfreq->devfreq)
> +	if (!pdevfreq || !pdevfreq->devfreq)
>  		return;
>  
>  	spin_lock_irqsave(&pdevfreq->lock, irqflags);
> @@ -306,7 +320,7 @@ void panthor_devfreq_record_idle(struct panthor_device *ptdev)
>  	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
>  	unsigned long irqflags;
>  
> -	if (!pdevfreq->devfreq)
> +	if (!pdevfreq || !pdevfreq->devfreq)
>  		return;
>  
>  	spin_lock_irqsave(&pdevfreq->lock, irqflags);
> @@ -319,14 +333,17 @@ void panthor_devfreq_record_idle(struct panthor_device *ptdev)
>  
>  unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev)
>  {
> +	int (*get_cur_freq)(struct device *dev, unsigned long *freq);
>  	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
>  	unsigned long freq = 0;
>  	int ret;
>  
> -	if (!pdevfreq->devfreq)
> -		return 0;
> +	if (!pdevfreq || !pdevfreq->devfreq)
> +		get_cur_freq = panthor_devfreq_get_cur_freq;
> +	else
> +		get_cur_freq = pdevfreq->devfreq->profile->get_cur_freq;

You don't need the whole pointer to a function dance. You can just do:

--8<-------------------------------------------
+	if (!pdevfreq || !pdevfreq->devfreq)
+		ret = panthor_devfreq_get_cur_freq(ptdev->base.dev, &freq);
+	else
+		ret = pdevfreq->devfreq->profile->get_cur_freq(ptdev->base.dev, &freq);
  	if (ret)
  		return 0;
-->8-------------------------------------------

With that,

Reviewed-by: Liviu Dudau <liviu.dudau@xxxxxxx>

Best regards,
Liviu

>  
> -	ret = pdevfreq->devfreq->profile->get_cur_freq(ptdev->base.dev, &freq);
> +	ret = get_cur_freq(ptdev->base.dev, &freq);
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
> index 48232254fef97..e5bcb908068cb 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.h
> +++ b/drivers/gpu/drm/panthor/panthor_hw.h
> @@ -101,4 +101,10 @@ static inline u64 panthor_hw_get_l2_features(struct panthor_device *ptdev)
>  	return ptdev->gpu_info.l2_features;
>  }
>  
> +static inline bool panthor_hw_is_gen5_am(struct device *dev)
> +{
> +	return device_property_match_string(dev, "compatible",
> +					    "arm,mali-gen5-am") >= 0;
> +}
> +
>  #endif /* __PANTHOR_HW_H__ */
> -- 
> 2.43.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯