From patchwork Mon Feb 1 23:28:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Lindgren X-Patchwork-Id: 8184801 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 74A509F3CD for ; Mon, 1 Feb 2016 23:28:40 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 87F1E202B8 for ; Mon, 1 Feb 2016 23:28:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 764A0202C8 for ; Mon, 1 Feb 2016 23:28:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750950AbcBAX2h (ORCPT ); Mon, 1 Feb 2016 18:28:37 -0500 Received: from muru.com ([72.249.23.125]:59408 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750919AbcBAX2g (ORCPT ); Mon, 1 Feb 2016 18:28:36 -0500 Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 2358D802C; Mon, 1 Feb 2016 23:29:35 +0000 (UTC) Date: Mon, 1 Feb 2016 15:28:33 -0800 From: Tony Lindgren To: "Rafael J. Wysocki" Cc: Ulf Hansson , "Rafael J. Wysocki" , Kevin Hilman , "linux-pm@vger.kernel.org" , Linux OMAP Mailing List , "linux-arm-kernel@lists.infradead.org" Subject: Re: PM regression with commit 5de85b9d57ab PM runtime re-init in v4.5-rc1 Message-ID: <20160201232833.GR19432@atomide.com> References: <20160126235222.GF19432@atomide.com> <20160128165810.GA19432@atomide.com> <20160201181135.GL19432@atomide.com> <20160201220657.GO19432@atomide.com> <20160201222926.GQ19432@atomide.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP * Rafael J. Wysocki [160201 15:11]: > On Mon, Feb 1, 2016 at 11:29 PM, Tony Lindgren wrote: > > * Rafael J. Wysocki [160201 14:18]: > >> On Mon, Feb 1, 2016 at 11:06 PM, Tony Lindgren wrote: > >> > --- a/drivers/base/power/runtime.c > >> > +++ b/drivers/base/power/runtime.c > >> > @@ -1419,17 +1419,25 @@ void pm_runtime_init(struct device *dev) > >> > */ > >> > void pm_runtime_reinit(struct device *dev) > >> > { > >> > - if (!pm_runtime_enabled(dev)) { > >> > - if (dev->power.runtime_status == RPM_ACTIVE) > >> > + if (pm_runtime_enabled(dev)) > >> > + return; > >> > + > >> > + if (dev->power.runtime_status == RPM_ACTIVE) { > >> > + if (dev->power.use_autosuspend) { > >> > + __pm_runtime_use_autosuspend(dev, false); > >> > + pm_runtime_suspend(dev); > >> > >> This won't work, because runtime PM is disabled at this point. > > > > Hmm right OK. It does work from idling the hardware point > > of view though.. > > pm_runtime_suspend() with runtime PM disabled is a NOP. It will do > nothing and return -EACCES. Hmm it makes a difference here for sure :) > >> What about doing this instead: > >> > >> if (dev->power.use_autosuspend) > >> __pm_runtime_use_autosuspend(dev, false); > >> > >> pm_runtime_set_suspended(dev); > > > > ..while this does not work. The hardware is never idled > > in this case. > > I'm not sure what you mean. pm_runtime_set_suspended() sets the > status to RPM_SUSPENDED for devices with runtime PM disabled. It has > nothing to do with "idling" in principle. Well looking at the update_autosuspend(), it seems we're now missing rpm_idle() call that now never happens. Does the patch below make more sense to you where we call rpm_idle? That seems to make things behave here also. > > What else does __pm_runtime_use_autosuspend() set initially > > that changes things here? > > The usage counter, if the delay is negative. Yeah I don't see any difference with those. > I'll look at this in detail, but not right now, sorry. I'm working on > something else ATM and I was hoping that Ulf would be able to figure > out what's going on here. Yeah we need to understand what's going on here. Having the PM runtime framework out of sync with the hardare is not good.. If we can't figure this out we should probably revert the patch until we understand it. Regards, Tony 8< ------------ --- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1419,17 +1419,28 @@ void pm_runtime_init(struct device *dev) */ void pm_runtime_reinit(struct device *dev) { - if (!pm_runtime_enabled(dev)) { - if (dev->power.runtime_status == RPM_ACTIVE) + int (*callback)(struct device *); + int err; + + if (pm_runtime_enabled(dev)) + return; + + if (dev->power.runtime_status == RPM_ACTIVE) { + if (dev->power.use_autosuspend) { + __pm_runtime_use_autosuspend(dev, false); + rpm_idle(dev, RPM_AUTO); + } else { pm_runtime_set_suspended(dev); - if (dev->power.irq_safe) { - spin_lock_irq(&dev->power.lock); - dev->power.irq_safe = 0; - spin_unlock_irq(&dev->power.lock); - if (dev->parent) - pm_runtime_put(dev->parent); } } + + if (dev->power.irq_safe) { + spin_lock_irq(&dev->power.lock); + dev->power.irq_safe = 0; + spin_unlock_irq(&dev->power.lock); + if (dev->parent) + pm_runtime_put(dev->parent); + } } /**