From patchwork Thu Aug 24 01:03:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 9918801 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D4E99602A0 for ; Thu, 24 Aug 2017 01:13:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C70D228AC8 for ; Thu, 24 Aug 2017 01:13:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BBB6528ACA; Thu, 24 Aug 2017 01:13:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_LOW autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3EAB028AC8 for ; Thu, 24 Aug 2017 01:13:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=a4B0PztGOclT1/962ggbsiwpkhCgfDcdetV/xxFhulc=; b=Yr8ky0PIzFi/XM sq93kP0XwBLBXcGp58SPF/INDCWa/fhSnY2Bdf+efSg+hWwY8KPLKG/iswLjYF/0W/KERXu18ZVn6 /wtVp2sUaMPFjttqjOvqSKLRGH1FQa/QfBMZiGBm5HzRBEf11/1RRo5PYvl18kB0DVbRinQov64e1 Uv1bU37KyI1RiQACneHfuvgx62PnC3xTb3VJLGAkRI3yUkrwHZ59yPKgKqZnXm6YSpqjDWsFB6vj5 0L0Ajr/eIlCFN/h0A0uS2cydWMIJm8zxxAMCrxRXmFNtVNRlkmbIBgSuS/X7pl1BlhSGtpTGDLOCF cWxjbfcEmIfRjL5LwK4w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dkghd-00005Y-6e; Thu, 24 Aug 2017 01:12:57 +0000 Received: from cloudserver094114.home.net.pl ([79.96.170.134]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dkgha-0008W3-8K for linux-arm-kernel@lists.infradead.org; Thu, 24 Aug 2017 01:12:56 +0000 Received: from 79.184.253.199.ipv4.supernova.orange.pl (79.184.253.199) (HELO aspire.rjw.lan) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.82) id ae8e68c3714c8cf1; Thu, 24 Aug 2017 03:12:30 +0200 From: "Rafael J. Wysocki" To: Ulf Hansson Subject: Re: [PATCH v2 5/9] PM / ACPI: Provide option to disable direct_complete for ACPI devices Date: Thu, 24 Aug 2017 03:03:57 +0200 Message-ID: <1726767.L8TO24juDO@aspire.rjw.lan> In-Reply-To: <9174364.s7oU13Tavs@aspire.rjw.lan> References: <1503499329-28834-1-git-send-email-ulf.hansson@linaro.org> <3774251.HUIR40xRhc@aspire.rjw.lan> <9174364.s7oU13Tavs@aspire.rjw.lan> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170823_181254_487959_984FB947 X-CRM114-Status: GOOD ( 31.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jisheng Zhang , Guodong Xu , Linux PM , Mika Westerberg , Wolfram Sang , "Rafael J. Wysocki" , linux-i2c , ACPI Devel Maling List , Jarkko Nikula , Haojian Zhuang , "Rafael J. Wysocki" , John Stultz , Andy Shevchenko , Kevin Hilman , Sumit Semwal , "linux-arm-kernel@lists.infradead.org" , Len Brown Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Thursday, August 24, 2017 2:20:00 AM CEST Rafael J. Wysocki wrote: > On Thursday, August 24, 2017 2:13:55 AM CEST Rafael J. Wysocki wrote: > > On Thursday, August 24, 2017 1:39:44 AM CEST Rafael J. Wysocki wrote: > > > On Wed, Aug 23, 2017 at 4:42 PM, Ulf Hansson wrote: > > > > In some cases a driver for an ACPI device needs to be able to prevent the > > > > ACPI PM domain from using the direct_complete path during system sleep. > > > > > > > > One typical case is when the driver for the device needs its device to stay > > > > runtime enabled, during the __device_suspend phase. This isn't the case > > > > when the direct_complete path is being executed by the PM core, as it then > > > > disables runtime PM for the device in __device_suspend(). Any following > > > > attempts to runtime resume the device after that point, just fails. > > > > > > OK, that is a problem. > > > > > > > A workaround to this problem is to let the driver runtime resume its device > > > > from its ->prepare() callback, as that would prevent the direct_complete > > > > path from being executed. However, that may often be a waste, especially if > > > > it turned out that no one really needed the device. > > > > > > > > For this reason, invent acpi_dev_disable|enable_direct_complete(), to allow > > > > drivers to inform the ACPI PM domain to change its default behaviour during > > > > system sleep, and thus control whether it may use the direct_complete path > > > > or not. > > > > > > But I'm not sure this is the right place to address it as it very well > > > may affect a PCI device too. > > > > > > Also, this is about the device and not about its ACPI companion > > > object, so putting the flag in there is somewhat unclean, so to speak. > > > > > > It looks like we need a flag in dev_pm_info saying something along the > > > lines of "my system suspend callback can deal with runtime PM" that > > > will cause the direct_complete update to occur in > > > __device_suspend_late() instead of __device_suspend(). > > > > IOW, something like the patch below. > > > > It actually should work with the ACPI PM domain code as is except that it > > will cause the device to be runtime resumed every time during suspend. > > > > But acpi_subsys_suspend() can be changed to avoid resuming the device if > > power.force_suspend is set. > > Or better yet, if power.direct_complete is not set, because that means the > device needs to be resumed anyway. > > And if power.direct_complete is set at that point, power.force_suspend has to > be set too. Overall, like below, and of course drivers that ser power.force_suspend need to be able to deal with devices that have been runtime resumed during __device_suspend(). --- drivers/acpi/device_pm.c | 8 ++++++++ drivers/base/power/main.c | 11 +++++++++-- include/linux/pm.h | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) Index: linux-pm/drivers/base/power/main.c =================================================================== --- linux-pm.orig/drivers/base/power/main.c +++ linux-pm/drivers/base/power/main.c @@ -1271,9 +1271,16 @@ static int __device_suspend_late(struct goto Complete; } - if (dev->power.syscore || dev->power.direct_complete) + if (dev->power.syscore) goto Complete; + if (dev->power.direct_complete) { + if (pm_runtime_status_suspended(dev)) + goto Complete; + + dev->power.direct_complete = false; + } + if (dev->pm_domain) { info = "late power domain "; callback = pm_late_early_op(&dev->pm_domain->ops, state); @@ -1482,7 +1489,7 @@ static int __device_suspend(struct devic if (dev->power.syscore) goto Complete; - if (dev->power.direct_complete) { + if (dev->power.direct_complete && !dev->power.force_suspend) { if (pm_runtime_status_suspended(dev)) { pm_runtime_disable(dev); if (pm_runtime_status_suspended(dev)) Index: linux-pm/include/linux/pm.h =================================================================== --- linux-pm.orig/include/linux/pm.h +++ linux-pm/include/linux/pm.h @@ -554,6 +554,7 @@ struct dev_pm_info { pm_message_t power_state; unsigned int can_wakeup:1; unsigned int async_suspend:1; + unsigned int force_suspend:1; bool in_dpm_list:1; /* Owned by the PM core */ bool is_prepared:1; /* Owned by the PM core */ bool is_suspended:1; /* Ditto */ Index: linux-pm/drivers/acpi/device_pm.c =================================================================== --- linux-pm.orig/drivers/acpi/device_pm.c +++ linux-pm/drivers/acpi/device_pm.c @@ -1025,9 +1025,17 @@ EXPORT_SYMBOL_GPL(acpi_subsys_prepare); * * Follow PCI and resume devices suspended at run time before running their * system suspend callbacks. + * + * If the power.direct_complete flag is set for the device, though, skip all + * that, because the device doesn't need to be resumed then and if it is + * resumed via runtime PM, the core will notice that and will carry out the + * late suspend for it. */ int acpi_subsys_suspend(struct device *dev) { + if (dev->power.direct_complete) + return 0; + pm_runtime_resume(dev); return pm_generic_suspend(dev); }