From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lina Iyer Subject: Re: [PATCH V4] PM / Domains: Remove intermediate states from the power off sequence Date: Thu, 9 Jul 2015 08:05:32 -0600 Message-ID: <20150709140532.GA2049@linaro.org> References: <1434633473-12908-1-git-send-email-ulf.hansson@linaro.org> <7htwtee84d.fsf@deeprootsystems.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Return-path: Received: from mail-pa0-f47.google.com ([209.85.220.47]:34950 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751430AbbGIOFg (ORCPT ); Thu, 9 Jul 2015 10:05:36 -0400 Received: by pactm7 with SMTP id tm7so151250547pac.2 for ; Thu, 09 Jul 2015 07:05:36 -0700 (PDT) Content-Disposition: inline In-Reply-To: <7htwtee84d.fsf@deeprootsystems.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Kevin Hilman Cc: Ulf Hansson , "Rafael J. Wysocki" , linux-pm@vger.kernel.org, Len Brown , Pavel Machek , Geert Uytterhoeven , Dmitry Torokhov , Axel Haslam , Krzysztof Kozlowski , Russell King , linux-arm-kernel@lists.infradead.org On Wed, Jul 08 2015 at 13:56 -0600, Kevin Hilman wrote: >Ulf Hansson writes: > >> Genpd's ->runtime_suspend() (assigned to pm_genpd_runtime_suspend()) >> doesn't immediately walk the hierarchy of ->runtime_suspend() callbacks. >> Instead, pm_genpd_runtime_suspend() calls pm_genpd_poweroff() which >> postpones that until *all* the devices in the genpd are runtime suspended. >> >> When pm_genpd_poweroff() discovers that the last device in the genpd is >> about to be runtime suspended, it calls __pm_genpd_save_device() for *all* >> the devices in the genpd sequentially. Furthermore, >> __pm_genpd_save_device() invokes the ->start() callback, walks the >> hierarchy of the ->runtime_suspend() callbacks and invokes the ->stop() >> callback. This causes a "thundering herd" problem. >> >> Let's address this issue by having pm_genpd_runtime_suspend() immediately >> walk the hierarchy of the ->runtime_suspend() callbacks, instead of >> postponing that to the power off sequence via pm_genpd_poweroff(). If the >> selected ->runtime_suspend() callback doesn't return an error code, call >> pm_genpd_poweroff() to see if it's feasible to also power off the PM >> domain. >> >> Adopting this change enables us to simplify parts of the code in genpd, >> for example the locking mechanism. Additionally, it gives some positive >> side effects, as described below. >> >> i) >> One device's ->runtime_resume() latency is no longer affected by other >> devices' latencies in a genpd. >> >> The complexity genpd has to support the option to abort the power off >> sequence suffers from latency issues. More precisely, a device that is >> requested to be runtime resumed, may end up waiting for >> __pm_genpd_save_device() to complete its operations for *another* device. >> That's because pm_genpd_poweroff() can't confirm an abort request while it >> waits for __pm_genpd_save_device() to return. >> >> As this patch removes the intermediate states in pm_genpd_poweroff() while >> powering off the PM domain, we no longer need the ability to abort that >> sequence. >> >> ii) >> Make pm_runtime[_status]_suspended() reliable when used with genpd. >> >> Until the last device in a genpd becomes idle, pm_genpd_runtime_suspend() >> will return 0 without actually walking the hierarchy of the >> ->runtime_suspend() callbacks. However, by returning 0 the runtime PM core >> considers the device as runtime_suspended, so >> pm_runtime[_status]_suspended() will return true, even though the device >> isn't (yet) runtime suspended. >> >> After this patch, since pm_genpd_runtime_suspend() immediately walks the >> hierarchy of the ->runtime_suspend() callbacks, >> pm_runtime[_status]_suspended() will accurately reflect the status of the >> device. >> >> iii) >> Enable fine-grained PM through runtime PM callbacks in drivers/subsystems. >> >> There are currently cases were drivers/subsystems implements runtime PM >> callbacks to deploy fine-grained PM (e.g. gate clocks, move pinctrl to >> power-save state, etc.). While using the genpd, pm_genpd_runtime_suspend() >> postpones invoking these callbacks until *all* the devices in the genpd >> are runtime suspended. In essence, one runtime resumed device prevents >> fine-grained PM for other devices within the same genpd. >> >> After this patch, since pm_genpd_runtime_suspend() immediately walks the >> hierarchy of the ->runtime_suspend() callbacks, fine-grained PM is enabled >> throughout all the levels of runtime PM callbacks. >> >> iiii) >> Enable fine-grained PM for IRQ safe devices >> >> Per the definition for an IRQ safe device, its runtime PM callbacks must >> be able to execute in atomic context. In the path while genpd walks the >> hierarchy of the ->runtime_suspend() callbacks for the device, it uses a >> mutex. Therefore, genpd prevents that path to be executed for IRQ safe >> devices. >> >> As this patch changes pm_genpd_runtime_suspend() to immediately walk the >> hierarchy of the ->runtime_suspend() callbacks and without needing to use >> a mutex, fine-grained PM is enabled throughout all the levels of runtime >> PM callbacks for IRQ safe devices. >> >> Unfortunately this patch also comes with a drawback, as described in the >> summary below. >> >> Driver's/subsystem's runtime PM callbacks may be invoked even when the >> genpd hasn't actually powered off the PM domain, potentially introducing >> unnecessary latency. >> >> However, in most cases, saving/restoring register contexts for devices are >> typically fast operations or can be optimized in device specific ways >> (e.g. shadow copies of register contents in memory, device-specific checks >> to see if context has been lost before restoring context, etc.). >> >> Still, in some cases the driver/subsystem may suffer from latency if >> runtime PM is used in a very fine-grained manner (e.g. for each IO request >> or xfer). To prevent that extra overhead, the driver/subsystem may deploy >> the runtime PM autosuspend feature. >> >> Signed-off-by: Ulf Hansson > >Reviewed-by: Kevin Hilman > >I believe Geert and Lina have both tested earlier versions, but would be >nice to see their Tested-by for this one. > I tested verssion 4. Did not see any issues with genpd suspend/resume. Tested-by: Lina Iyer Thanks, Lina