From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C69A8C433E6 for ; Tue, 19 Jan 2021 18:27:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8D0AB22C9E for ; Tue, 19 Jan 2021 18:27:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726732AbhASQS1 (ORCPT ); Tue, 19 Jan 2021 11:18:27 -0500 Received: from a1.mail.mailgun.net ([198.61.254.60]:17976 "EHLO a1.mail.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389567AbhASQNB (ORCPT ); Tue, 19 Jan 2021 11:13:01 -0500 X-Greylist: delayed 402 seconds by postgrey-1.27 at vger.kernel.org; Tue, 19 Jan 2021 11:13:00 EST DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1611072758; h=In-Reply-To: Content-Type: MIME-Version: References: Message-ID: Subject: Cc: To: From: Date: Sender; bh=AtbnlRvAS8yN8UKf7OYvL9rmN50YvFPDLcQ5jq6qWlk=; b=f301rqoo+6dng0em3bVaBLlZhS/mcr1Ku72nae6sYkXH35DK1Qzuw7j+JJLzgZR+0b1Zt81a 0hR1WLdTfTmaDa9O8TNrFKJs7PUb8LhFx6dnlERTihdIOowEfTILX/whhes265z/2vGXuVy1 S55BbZ0msuVrYH1eRbY697hWl8Q= X-Mailgun-Sending-Ip: 198.61.254.60 X-Mailgun-Sid: WyI5ZDFmMiIsICJsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmciLCAiYmU5ZTRhIl0= Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n06.prod.us-west-2.postgun.com with SMTP id 6007034321210999ed44ba35 (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Tue, 19 Jan 2021 16:05:23 GMT Sender: ilina=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 0DEA5C43464; Tue, 19 Jan 2021 16:05:23 +0000 (UTC) Received: from localhost (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: ilina) by smtp.codeaurora.org (Postfix) with ESMTPSA id 8F9B1C43461; Tue, 19 Jan 2021 16:05:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 8F9B1C43461 Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=fail smtp.mailfrom=ilina@codeaurora.org Date: Tue, 19 Jan 2021 09:05:20 -0700 From: Lina Iyer To: Ulf Hansson Cc: "Rafael J. Wysocki" , Linux PM , linux-arm-msm Subject: Re: [PATCH v8 1/2] PM / domains: inform PM domain of a device's next wakeup Message-ID: References: <20210115165004.22385-1-ilina@codeaurora.org> <20210115165004.22385-2-ilina@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Tue, Jan 19 2021 at 03:01 -0700, Ulf Hansson wrote: >On Fri, 15 Jan 2021 at 17:50, Lina Iyer wrote: >> >> Some devices may have a predictable interrupt pattern while executing >> usecases. An example would be the VSYNC interrupt associated with >> display devices. A 60 Hz display could cause a interrupt every 16 ms. If >> the device were in a PM domain, the domain would need to be powered up >> for device to resume and handle the interrupt. >> >> Entering a domain idle state saves power, only if the residency of the >> idle state is met. Without knowing the idle duration of the domain, the >> governor would just choose the deepest idle state that matches the QoS >> requirements. The domain might be powered off just as the device is >> expecting to wake up. If devices could inform PM frameworks of their >> next event, the parent PM domain's idle duration can be determined. >> >> So let's add the dev_pm_genpd_set_next_wakeup() API for the device to >> inform PM domains of the impending wakeup. This information will be the >> domain governor to determine the best idle state given the wakeup. >> >> Signed-off-by: Lina Iyer >> Reviewed-by: Ulf Hansson >> --- >> Changes in v8: >> - Update documentation. Add Reviewed-by tag. >> Changes in v7: >> - Simplify and set next-wakeup locklessly >> Changes in v6: >> - Update documentation >> Changes in v5: >> - Fix commit text as pointed by Ulf >> - Use -EOPNOTSUPP >> Changes in v4: >> - Use PM domain data to store next_wakeup >> - Drop runtime PM documentation >> Changes in v3: >> - Fix unwanted change >> Changes in v2: >> - Update documentation >> - Remove runtime PM enabled check >> - Update commit text >> --- >> drivers/base/power/domain.c | 25 +++++++++++++++++++++++++ >> include/linux/pm_domain.h | 6 ++++++ >> 2 files changed, 31 insertions(+) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index 9a14eedacb92..10a960bd3204 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -423,6 +423,30 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state) >> } >> EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state); >> >> +/** >> + * dev_pm_genpd_set_next_wakeup - Notify PM framework of an impending wakeup. >> + * >> + * @dev: Device to handle >> + * @next: impending interrupt/wakeup for the device >> + * >> + * >> + * Allow devices to inform of the next wakeup. It's assumed that the users >> + * guarantee that the genpd wouldn't be detached while this routine is getting >> + * called. Additionally, it's also assumed that @dev isn't runtime suspended >> + * (RPM_SUSPENDED)." >> + * Although devices are expected to update the next_wakeup after the end of >> + * their usecase as well, it is possible the devices themselves may not know >> + * about that, so stale @next will be ignored when powering off the domain. >> + */ >> +void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next) >> +{ >> + struct generic_pm_domain_data *gpd_data; > >Looks like you have dropped one of the needed sanity checks, to make >sure the device is attached to a genpd. My apologies if I missed that >in the previous version. So you need something like this: > Oops. I assumed that we leave that to the driver to ensure that requirement. I will send another version with this. Thanks for catching this and the review. --Lina >genpd = dev_to_genpd_safe(dev); >if (!genpd) > return; > >> + >> + gpd_data = to_gpd_data(dev->power.subsys_data->domain_data); >> + gpd_data->next_wakeup = next; >> +} >> +EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup); >> + >> static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) >> { >> unsigned int state_idx = genpd->state_idx; >> @@ -1465,6 +1489,7 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev) >> gpd_data->td.constraint_changed = true; >> gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; >> gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; >> + gpd_data->next_wakeup = KTIME_MAX; >> >> spin_lock_irq(&dev->power.lock); >> >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h >> index 2ca919ae8d36..735583c0bc6d 100644 >> --- a/include/linux/pm_domain.h >> +++ b/include/linux/pm_domain.h >> @@ -9,6 +9,7 @@ >> #define _LINUX_PM_DOMAIN_H >> >> #include >> +#include >> #include >> #include >> #include >> @@ -191,6 +192,7 @@ struct generic_pm_domain_data { >> struct notifier_block *power_nb; >> int cpu; >> unsigned int performance_state; >> + ktime_t next_wakeup; >> void *data; >> }; >> >> @@ -217,6 +219,7 @@ int pm_genpd_remove(struct generic_pm_domain *genpd); >> int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state); >> int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb); >> int dev_pm_genpd_remove_notifier(struct device *dev); >> +void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next); >> >> extern struct dev_power_governor simple_qos_governor; >> extern struct dev_power_governor pm_domain_always_on_gov; >> @@ -275,6 +278,9 @@ static inline int dev_pm_genpd_remove_notifier(struct device *dev) >> return -EOPNOTSUPP; >> } >> >> +static inline void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next) >> +{ } >> + >> #define simple_qos_governor (*(struct dev_power_governor *)(NULL)) >> #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL)) >> #endif >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >> a Linux Foundation Collaborative Project >> > >Kind regards >Uffe