On Wed, 2 Sep 2026 16:38:25 +0100 Adrián Larumbe <[email protected]> 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.
