Re: [PATCH v2 6/7] drm/panfrost: Fix PM usage_count mishandling

["Adrián Larumbe" <adrian.larumbe@collabora.com>]

On 04.06.2026 20:36, Boris Brezillon wrote:
> On Thu, 04 Jun 2026 18:35:25 +0100
> Adrián Larumbe <adrian.larumbe@collabora.com> wrote:
> 
> > During device probe(), failure to do a PM get() will leave the usage_count
> > set to 0, which is the value assigned at device creation time. That means
> > when the autosuspend delay expires, runtime suspend callback won't be
> > invoked, so the device will remain powered on forever.
> > 
> > On top of that, failure to call PM put() during device unplug means
> > Panfrost device's PM usage_count increases monotonically for every new
> > module reload.
> > 
> > The combined outcome of both of the above was that devfreq OPP transition
> > notifications would be printed all the time, even when no jobs are being
> > submitted. This quickly fills the kernel ring buffer with junk.
> > 
> > Even direr than that was the fact MMU interrupts are only enabled when
> > the device is reset, so after device probe() the very first job targeting
> > the tiler heap BO would always time out, because the driver's PM runtime
> > resume callback would not be invoked.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> > Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index 2d4b6aa95c66..545fbf2c8d0c 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -989,6 +989,7 @@ static int panfrost_probe(struct platform_device *pdev)
> >  	pm_runtime_set_active(pfdev->base.dev);
> >  	pm_runtime_mark_last_busy(pfdev->base.dev);
> >  	pm_runtime_enable(pfdev->base.dev);
> > +	pm_runtime_get_noresume(pfdev->base.dev);
> >  	pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */
> >  	pm_runtime_use_autosuspend(pfdev->base.dev);
> >  
> > @@ -1000,10 +1001,12 @@ static int panfrost_probe(struct platform_device *pdev)
> >  	if (err < 0)
> >  		goto err_out1;
> >  
> > +	pm_runtime_put_autosuspend(pfdev->base.dev);
> >  
> >  	return 0;
> >  
> >  err_out1:
> > +	pm_runtime_put_noidle(pfdev->base.dev);
> 
> Do we really need this get_noresume/put_noidle dance, can't use call
> pm_runtime_dont_use_autosuspend() instead like is done in panthor, or
> is panthor broken too?

We need get_noresume() because after panfrost_device_init(), the device is powered
up but that is not reflected in the device's PM refcnt. Then pm_runtime_put_autosuspend()
will decrement the refcnt back to 0 and let the autosuspend window expire before
suspending the device, unless someone starts using it immediately.

pm_runtime_dont_use_autosuspend() is not mandatory if we manually call pm_runtime_disable()
at device unplug time or when probe() fails. pm_runtime_use_autosuspend()'s docs say:

pm_runtime_use_autosuspend - Allow autosuspend to be used for a device.
@dev: Target device.
Allow the runtime PM autosuspend mechanism to be used for @dev whenever
requested (or "autosuspend" will be handled as direct runtime-suspend for
it).
NOTE: It's important to undo this with pm_runtime_dont_use_autosuspend()
at driver exit time unless your driver initially enabled pm_runtime
with devm_pm_runtime_enable() (which handles it for you).

Panthor uses devm_pm_runtime_enable(), which makes me suspect perhaps it doesn't need
to explicitly call pm_runtime_dont_use_autosuspend().

Panthor also manages PM refcnt fine at driver probe(), inside panthor_device_init():

``` c
ret = pm_runtime_resume_and_get(ptdev->base.dev);
if (ret)
	return ret;

/* If PM is disabled, we need to call panthor_device_resume() manually. */
if (!IS_ENABLED(CONFIG_PM)) {
	ret = panthor_device_resume(ptdev->base.dev);
	if (ret)
		return ret;
}
```

If we want a similar thing in Panfrost, then we should move all clock and devfreq enablement
into panfrost_device_runtime_resume() and do pm_runtime_resume_and_get() right before
panfrost_gpu_init().

> >  	pm_runtime_disable(pfdev->base.dev);
> >  	panfrost_device_fini(pfdev);
> >  	pm_runtime_set_suspended(pfdev->base.dev);
> > @@ -1018,8 +1021,9 @@ static void panfrost_remove(struct platform_device *pdev)
> >  	drm_dev_unregister(&pfdev->base);
> >  
> >  	pm_runtime_get_sync(pfdev->base.dev);
> > -	pm_runtime_disable(pfdev->base.dev);
> >  	panfrost_device_fini(pfdev);
> > +	pm_runtime_put_noidle(pfdev->base.dev);
> > +	pm_runtime_disable(pfdev->base.dev);
> >  	pm_runtime_set_suspended(pfdev->base.dev);
> >  }
> >  
> > 

Adrian Larumbe