From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes Date: Thu, 20 Nov 2014 17:06:30 +0200 Message-ID: <546E0376.3030300@ti.com> References: <20141117220238.GE5258@dtor-ws> <1787068.2MCH8czsD4@vostro.rjw.lan> <3492523.qBADTmdTqo@vostro.rjw.lan> <546DDBCF.205@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-samsung-soc-owner@vger.kernel.org To: Ulf Hansson Cc: "Rafael J. Wysocki" , Len Brown , "linux-samsung-soc@vger.kernel.org" , Sylwester Nawrocki , Geert Uytterhoeven , "linux-pm@vger.kernel.org" , Dmitry Torokhov , Kevin Hilman , Alan Stern , Pavel Machek , "linux-arm-kernel@lists.infradead.org" List-Id: linux-pm@vger.kernel.org On 11/20/2014 03:01 PM, Ulf Hansson wrote: > On 20 November 2014 13:17, Grygorii Strashko wrote: >> On 11/19/2014 10:54 AM, Ulf Hansson wrote: >>> [...] >>> >>>>> >>>>> Scenario 5), a platform driver with/without runtime PM callbacks. >>>>> ->probe() >>>>> - do some initialization >>>>> - may fetch handles to runtime PM resources >>>>> - pm_runtime_enable() >>>> >>>> Well, and now how the driver knows if the device is "on" before ac= cessing it? >>> >>> In this case the driver don't need to access the device during >>> ->probe(). That's postponed until sometime later. >>> >>>> >>>>> Note 1) >>>>> Scenario 1) and 2), both relies on the approach to power on the P= M >>>>> domain by using pm_runtime_get_sync(). That approach didn't work = when >>>>> CONFIG_PM_RUNTIME was unset, but we recently decided to fixed tha= t by >>>>> the below patch, so that's good! >>>>> "[PATCH] PM / domains: Kconfig: always enable PM_RUNTIME when gen= pd enabled" >>>>> >>>>> Note 2) >>>>> Scenario 3) and 4) use the same principles for managing runtime P= M. >>>>> These scenarios needs a way to power on the generic PM domain pri= or >>>>> probing the device. The call to pm_runtime_set_active(), prevents= an >>>>> already powered PM domain from power off until after probe, but t= hat's >>>>> not enough. >>>>> >>>>> Note 3) >>>>> The $subject patch, tried to address the issues for scenario 3) a= nd >>>>> 4). It does so, but will affect scenario 5) which was working nic= ely >>>>> before. In scenario 5), the $subject patch will cause the generic= PM >>>>> domain to potentially stay powered after ->probe() even if the de= vice >>>>> is runtime PM suspended. >>>> >>>> Why would it? If the device is runtime-suspended, the domain will= know >>>> that, because its callbacks will be used for that. At least, that= 's >>>> what I'd expect to happen, so is there a problem here? >>> >>> Genpd do knows about the device but it doesn=E2=80=99t get a "notif= ication" to >>> power off. There are no issues whatsoever for driver. >>> >>> This is a somewhat special case. Let's go through an example. >>> >>> 1. The PM domain is initially in powered off state. >>> 2. The bus ->probe() invokes dev_pm_domain_attach() and then the PM >>> domain gets attached to the device. >>> 3. $subject patch causes the PM domain to power on. >>> 4. A driver ->probe() sequence start, following the Scenario 5). >>> 5. The device is initially in runtime PM suspended state and it wil= l >>> remain so during ->probe(). >>> 6. The pm_request_idle() invoked after really_probe() in driver cor= e, >>> won't trigger a runtime PM suspend callback to be invoked. In other >>> words, genpd don't get a "notification" that it may power off. >>> >>> In this state, genpd will either power off from the late_initcall, >>> genpd_poweroff_unused() (depending on when the driver was probed) o= r >>> wait until some device's runtime PM suspend callback is invoked at = any >>> later point. >> >> if I understand things right (thanks to Russell), the Power domain m= ay not >> be powered off not only in above case, but also in some cases when >> driver is unloaded. >> >> AMBA bus for example: >> static int amba_remove(struct device *dev) >> { >> pm_runtime_get_sync(dev); <---------- GPD=3Don, dev is acti= ve, usage_count >=3D 1 >> ret =3D drv->remove(pcdev); <----------- GPD=3Don, should d= o balancing put() to compensate all get() made by driver, usage_count =3D= =3D 1 >> <----------- GPD=3Don, should do = balancing get() to compensate put() in probe, usage_count =3D=3D 2 >> pm_runtime_put_noidle(dev); <---------- GPD=3Don, dev is ac= tive, usage_count =3D=3D 1 >> >> /* Undo the runtime PM settings in amba_probe() */ >> pm_runtime_disable(dev); <---------- GPD=3Don, dev is acti= ve, usage_count =3D=3D 1 >> pm_runtime_set_suspended(dev); <---------- GPD=3Don, dev is= suspended, usage_count =3D=3D 1 >> pm_runtime_put_noidle(dev); <---------- GPD=3Don, dev is su= spended, usage_count =3D=3D 0 >> >> amba_put_disable_pclk(pcdev); >> dev_pm_domain_detach(dev, true); <---------- GPD=3Don, dev = is suspended, usage_count =3D=3D 0 > > For the generic OF-based PM domain look-up case: > ->dev_pm_domain_detach() > ->genpd_dev_pm_detach() - to remove the device from the PM domai= n. > ->genpd_queue_power_off_work() - to power off unused PM domai= ns. > > That does the trick, right!? True. Thanks a lot for pointing me on right place. regards, -grygorii From mboxrd@z Thu Jan 1 00:00:00 1970 From: grygorii.strashko@ti.com (Grygorii Strashko) Date: Thu, 20 Nov 2014 17:06:30 +0200 Subject: [PATCH] PM / Domains: Power on the PM domain right after attach completes In-Reply-To: References: <20141117220238.GE5258@dtor-ws> <1787068.2MCH8czsD4@vostro.rjw.lan> <3492523.qBADTmdTqo@vostro.rjw.lan> <546DDBCF.205@ti.com> Message-ID: <546E0376.3030300@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 11/20/2014 03:01 PM, Ulf Hansson wrote: > On 20 November 2014 13:17, Grygorii Strashko wrote: >> On 11/19/2014 10:54 AM, Ulf Hansson wrote: >>> [...] >>> >>>>> >>>>> Scenario 5), a platform driver with/without runtime PM callbacks. >>>>> ->probe() >>>>> - do some initialization >>>>> - may fetch handles to runtime PM resources >>>>> - pm_runtime_enable() >>>> >>>> Well, and now how the driver knows if the device is "on" before accessing it? >>> >>> In this case the driver don't need to access the device during >>> ->probe(). That's postponed until sometime later. >>> >>>> >>>>> Note 1) >>>>> Scenario 1) and 2), both relies on the approach to power on the PM >>>>> domain by using pm_runtime_get_sync(). That approach didn't work when >>>>> CONFIG_PM_RUNTIME was unset, but we recently decided to fixed that by >>>>> the below patch, so that's good! >>>>> "[PATCH] PM / domains: Kconfig: always enable PM_RUNTIME when genpd enabled" >>>>> >>>>> Note 2) >>>>> Scenario 3) and 4) use the same principles for managing runtime PM. >>>>> These scenarios needs a way to power on the generic PM domain prior >>>>> probing the device. The call to pm_runtime_set_active(), prevents an >>>>> already powered PM domain from power off until after probe, but that's >>>>> not enough. >>>>> >>>>> Note 3) >>>>> The $subject patch, tried to address the issues for scenario 3) and >>>>> 4). It does so, but will affect scenario 5) which was working nicely >>>>> before. In scenario 5), the $subject patch will cause the generic PM >>>>> domain to potentially stay powered after ->probe() even if the device >>>>> is runtime PM suspended. >>>> >>>> Why would it? If the device is runtime-suspended, the domain will know >>>> that, because its callbacks will be used for that. At least, that's >>>> what I'd expect to happen, so is there a problem here? >>> >>> Genpd do knows about the device but it doesn?t get a "notification" to >>> power off. There are no issues whatsoever for driver. >>> >>> This is a somewhat special case. Let's go through an example. >>> >>> 1. The PM domain is initially in powered off state. >>> 2. The bus ->probe() invokes dev_pm_domain_attach() and then the PM >>> domain gets attached to the device. >>> 3. $subject patch causes the PM domain to power on. >>> 4. A driver ->probe() sequence start, following the Scenario 5). >>> 5. The device is initially in runtime PM suspended state and it will >>> remain so during ->probe(). >>> 6. The pm_request_idle() invoked after really_probe() in driver core, >>> won't trigger a runtime PM suspend callback to be invoked. In other >>> words, genpd don't get a "notification" that it may power off. >>> >>> In this state, genpd will either power off from the late_initcall, >>> genpd_poweroff_unused() (depending on when the driver was probed) or >>> wait until some device's runtime PM suspend callback is invoked at any >>> later point. >> >> if I understand things right (thanks to Russell), the Power domain may not >> be powered off not only in above case, but also in some cases when >> driver is unloaded. >> >> AMBA bus for example: >> static int amba_remove(struct device *dev) >> { >> pm_runtime_get_sync(dev); <---------- GPD=on, dev is active, usage_count >= 1 >> ret = drv->remove(pcdev); <----------- GPD=on, should do balancing put() to compensate all get() made by driver, usage_count == 1 >> <----------- GPD=on, should do balancing get() to compensate put() in probe, usage_count == 2 >> pm_runtime_put_noidle(dev); <---------- GPD=on, dev is active, usage_count == 1 >> >> /* Undo the runtime PM settings in amba_probe() */ >> pm_runtime_disable(dev); <---------- GPD=on, dev is active, usage_count == 1 >> pm_runtime_set_suspended(dev); <---------- GPD=on, dev is suspended, usage_count == 1 >> pm_runtime_put_noidle(dev); <---------- GPD=on, dev is suspended, usage_count == 0 >> >> amba_put_disable_pclk(pcdev); >> dev_pm_domain_detach(dev, true); <---------- GPD=on, dev is suspended, usage_count == 0 > > For the generic OF-based PM domain look-up case: > ->dev_pm_domain_detach() > ->genpd_dev_pm_detach() - to remove the device from the PM domain. > ->genpd_queue_power_off_work() - to power off unused PM domains. > > That does the trick, right!? True. Thanks a lot for pointing me on right place. regards, -grygorii