From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH 3/4] PM / Domains: Allow runtime PM during system PM phases Date: Mon, 23 May 2016 16:06:37 -0700 Message-ID: References: <1463485296-22742-1-git-send-email-ulf.hansson@linaro.org> <1463485296-22742-4-git-send-email-ulf.hansson@linaro.org> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from mail-pf0-f171.google.com ([209.85.192.171]:33738 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752887AbcEWXGk (ORCPT ); Mon, 23 May 2016 19:06:40 -0400 Received: by mail-pf0-f171.google.com with SMTP id b124so31076pfb.0 for ; Mon, 23 May 2016 16:06:40 -0700 (PDT) In-Reply-To: <1463485296-22742-4-git-send-email-ulf.hansson@linaro.org> (Ulf Hansson's message of "Tue, 17 May 2016 13:41:35 +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 , Axel Haslam , Marek Szyprowski , Jon Hunter , Andy Gross , Laurent Pinchart Ulf Hansson writes: > The PM core disables runtime PM at __device_suspend_late() before it calls > a system PM "late" callback for the device. When resuming the device, > after the corresponding "early" callback has been invoked, it re-enables > runtime PM. > > By changing genpd to conform to this behaviour, the device no longer have > to be unconditionally runtime resumed from genpd's ->prepare() callback. In > most cases that avoids unnecessary operations. ...avoids the unnecessary, and energy-wasting operaions of runtime resuming devices that have nothing to do, only to runtime suspend them shortly after. > As runtime PM then isn't disabled/enabled by genpd, the subsystem/driver > can rely on the generic behaviour from PM core. Consequentially runtime PM > is allowed in more phases of system PM than before. OK. I like this change a lot, but it the changelog doesn't go into describing how this gets around the problems that were being worked around by having genpd disable runtime PM in the first place. This changelog should describe how this appraoch deals with those problems, or why they don't exist anymore. > Although, because of this change and due to that genpd powers on the PM > domain unconditionally in the system PM resume "noirq" phase, it could > potentially cause a PM domain to stay powered on even if it's unused after > the system has resumed. To avoid this, let's schedule a power off work > when genpd's system PM ->complete() callback has been invoked for the last > device in the PM domain. OK. > Another issue that arises due to this change in genpd, concerns those > platforms/PM domains that makes use of genpd's device ->stop|start() > callbacks. In these scenarios, the corresponding subsystem/driver needs to > invoke pm_runtime_force_suspend() from a system PM suspend callback to > allow genpd's ->runtime_suspend() to be invoked for an active device, else > genpd can't "stop" a device that is "started". > > The subsystem/driver also needs to invoke pm_runtime_force_resume() > in a system PM resume callback, to restore the runtime PM state for the > device and to re-enable runtime PM. > > Currently not all involved subsystem/drivers makes use of > pm_runtime_force_suspend|resume() accordingly. Therefore, let's invoke > pm_runtime_force_suspend|resume() from genpd's "noirq" system PM > callbacks, in cases when the ->stop|start() callbacks are being used. > > In this way, devices are "stoped" during suspend and "started" during > resume, even in those cases when the subsystem/driver don't call > pm_runtime_force_suspend|resume() themselves. IMO, this last part might be better dealt with as a separate patch, so it's clear which parts are for the main genpd change, and which parts fixup the fallout. On a related note, are there many genpd drivers using dev_ops->start and ->stop out there? A quick grep didn't find anything other than the pm_clk stuff. Kevin