Messages in this thread | | | Subject | Re: [PATCH 1/5] drm/omap: Fix suspend resume regression after platform data removal | From | Tomi Valkeinen <> | Date | Tue, 16 Jun 2020 16:01:39 +0300 |
| |
On 11/06/2020 17:00, Grygorii Strashko wrote: > > > On 09/06/2020 18:26, Tomi Valkeinen wrote: >> On 09/06/2020 18:19, Tony Lindgren wrote: >>>> But there's an extra runtime PM reference (dev.power.usage_count) that seems >>>> to come out of nowhere. So when omap_drm_suspend is finished, there's still >>>> usage_count of 1, and dispc never suspends fully. >>> >>> Hmm no idea about that. My guess is that there might be an issue that was >>> masked earlier with omap_device calling the child runtime_suspend. >> >> Yes. It's how PM works. It calls pm_runtime_get_noresume() before starting the suspend of a >> device. So I guess omapdrm's suspend has been broken all the time, but it was "fixed" by omap_device. >> > > I think I might have an idea what is going wrong. > > Before: > +----------------------+ > |omap_device_pm_domain | > +---------------+------+------+ > | device | > +-------------+ > | omap_device | > +-------------+ > > omap_device is embedded in DD device and PM handled by omap_device_pm_domain. > > static int _od_suspend_noirq(struct device *dev) > { > ... > > ret = pm_generic_suspend_noirq(dev); > [1] ^^ device suspend_noirq call > > if (!ret && !pm_runtime_status_suspended(dev)) { > if (pm_generic_runtime_suspend(dev) == 0) { > [2] ^^ device pm_runtime_suspend force call > > omap_device_idle(pdev); > [3] ^^ omap_device disable > od->flags |= OMAP_DEVICE_SUSPENDED; > } > } > > return ret; > } > > Now: > +------------+ > |ti sysc dev | > +-+----------+ > | > | > | +-------------+ > | | device | > +-->+ | > +-------------+ > > With new approach the omap_device is not embedded in DD Device anymore, > instead ti-sysc (hwmod replacement) became parent of DD Device. > > As result suspend sequence became the following > (Note. All PM runtime PUT calls became NOP during suspend by design): > > device > |-> suspend() - in case of dss omap_drm_suspend() and Co if defined > |-> suspend_noirq() - in case of dss *not defined", equal to step [1] above > .. > > ti sysc dev (ti-sysc is parent, so called after device) > |-> sysc_noirq_suspend > |-> pm_runtime_force_suspend() > |-> sysc_runtime_suspend() - equal to step [3] above > > And step [2] is missing as of now! > > I think, suspend might be fixed if all devices, which are now child of ti-sysc, will do > pm_runtime_force_xxx() calls at noirq suspend stage by adding: > > SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > pm_runtime_force_resume) > > Am I missing smth?
Isn't this almost exactly the same my patch does? I just used suspend_late and resume_early. Is noirq phase better than late & early?
Tomi
-- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
| |