Re: [PATCH v7 02/17] drm/panfrost: Move all DRM device initialisation into device_init()

[Boris Brezillon <boris.brezillon@collabora.com>]

On Wed, 2 Sep 2026 16:38:25 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> > > +	pm_runtime_disable(pfdev->base.dev);  
> > 
> > I think you need a pm_runtime_dont_use_autosuspend() call before
> > pm_runtime_disable().
> >   
> > > +	panfrost_gem_fini(pfdev);
> > >  out_perfcnt:
> > >  	panfrost_perfcnt_fini(pfdev);
> > >  out_job:
> > > @@ -304,11 +331,15 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> > >  	panfrost_reset_fini(pfdev);
> > >  out_pm_domain:
> > >  	panfrost_pm_domain_fini(pfdev);
> > > +	pm_runtime_set_suspended(pfdev->base.dev);  
> > 
> > Do we have a good reason for not flagging the device suspended just
> > after the pm_runtime_disable() call in the error path? I mean, sure
> > it's not truly suspended until the clks/regulators have been turned
> > off, but it also wasn't suspended the before the initial
> > pm_runtime_set_active() call, and I like the idea of undoing things in
> > reverse init order.  
> 
> That was my thought when I put it at the end, although since we manually
> manage device power down here rather than going through the RPM runtime
> suspend callback, I guess it makes no difference.
> 
> > but it also wasn't suspended the before the initial pm_runtime_set_active() call  
> 
> I think devices begin their lifetime with power.runtime_status set to RPM_SUSPENDED:
> drivers/base/power/runtime.c::pm_runtime_init

Yes, the SW state is RPM_SUSPENDED at probe time, but my point is that
the device is actually active before pm_runtime_set_active() is called,
so the disconnect between the SW state and the HW state already exists
and is accepted. I think, as long as we manipulate the state while RPM
is disabled, and we make sure to restore the resources into their
suspended-state before leaving, it doesn't matter when exactly we call
pm_runtime_set_suspended(). If my understanding is correct, I'd rather
have this call just after the pm_runtime_disable() to respect the
reversed-init-order convention we have in error paths.