From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH V4] PM / Domains: Remove intermediate states from the power off sequence Date: Wed, 08 Jul 2015 12:56:18 -0700 Message-ID: <7htwtee84d.fsf@deeprootsystems.com> References: <1434633473-12908-1-git-send-email-ulf.hansson@linaro.org> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from mail-pd0-f170.google.com ([209.85.192.170]:33383 "EHLO mail-pd0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932772AbbGHT4W (ORCPT ); Wed, 8 Jul 2015 15:56:22 -0400 Received: by pdbqm3 with SMTP id qm3so7561609pdb.0 for ; Wed, 08 Jul 2015 12:56:22 -0700 (PDT) In-Reply-To: <1434633473-12908-1-git-send-email-ulf.hansson@linaro.org> (Ulf Hansson's message of "Thu, 18 Jun 2015 15:17:53 +0200") Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Ulf Hansson Cc: "Rafael J. Wysocki" , linux-pm@vger.kernel.org, Len Brown , Pavel Machek , Geert Uytterhoeven , Lina Iyer , Dmitry Torokhov , Axel Haslam , Krzysztof Kozlowski , Russell King , linux-arm-kernel@lists.infradead.org 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. Kevin