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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5DAFC433EF for ; Thu, 19 May 2022 12:59:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231809AbiESM7j (ORCPT ); Thu, 19 May 2022 08:59:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43458 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236493AbiESM6q (ORCPT ); Thu, 19 May 2022 08:58:46 -0400 Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A3498D4109 for ; Thu, 19 May 2022 05:57:22 -0700 (PDT) Received: by mail-lf1-x136.google.com with SMTP id v8so7036864lfd.8 for ; Thu, 19 May 2022 05:57:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=aR2yuG7QP+r0gTdgSYaStSwxJ8RwX7Q4zTzPF1+Nqpo=; b=ugf1SX9wx9gVA5R+ln6Qge7zlhYGL9u69t98IcOLpR2ZZuDf8lWSidfc/I1zpVDDlu LISZs8hVFvJmY51NVTHChdmyTjuJyPnVNH41hUkedD/zty+U1ScXHzS9LYxEQqXQXxyN 8G8cJYqLju8kBpG/UiZTAywiABeYxP5o4pDDOep916FcuzV2YB2GhqN3YTmNqEp7CCgz h9F4cXdkbqYZlO3mkV027tkz+G5jpERUdi/iCNXiJ82J+lnASxPUkN9XC4ipILd/hJOv xO9v4qBzBuXHqNC2X7InYJm6HP7o2vIlAHSwJUYm8ItFSLeOwqajZAf4zDx8JW8ls6/N YwpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=aR2yuG7QP+r0gTdgSYaStSwxJ8RwX7Q4zTzPF1+Nqpo=; b=DEvQqujXGFzsrJDcbYCE6bU31mei3UTXdIAoMjaZiAUR5pqT4cQCIys9gIkdULVtkD vG7ElEDTHvkb4FBuMM049HZlHR0Vnrrm/e2joUhCHBtdJuNs0rSnU/vGDQahNXewn3RJ Qhd1FizEMF+4COKhoLYKwOvDc/rB/tEU0CMiUsj0DDsLpINm06rHxCEiN13JoQgta08W XKjpWxxzHdCHwj4LXZHDMTtXb7aWdG2nkJXnBlRZd/DHgN4liF3C9uX0fuFMeIo/oCaO Rg9vYI7tChx8Rp7/ZcP7IwHnuPXgAonYKgDMMibNJopMFu6F7JPl+guaHngfV6RqO7av qtvA== X-Gm-Message-State: AOAM531vzMO9TaHaqFyB2n4pBLJYylb43yNkH2PVpCvjVDYlnUwXnZMZ JzkcJLj6MAIjATSlAYSWvNib0mHfF1JVHHU6qjb5XA== X-Google-Smtp-Source: ABdhPJztFL7Pqq1gQj+cNoDZZBXAghknXzjA/pEu2EUSHDZR2mOUec0RwrCi6yWoH7KOlQioB5K2JxOh2ikuEeqNWRA= X-Received: by 2002:a05:6512:3b27:b0:477:96e9:b962 with SMTP id f39-20020a0565123b2700b0047796e9b962mr3293651lfv.373.1652965040195; Thu, 19 May 2022 05:57:20 -0700 (PDT) MIME-Version: 1.0 References: <1652275016-13423-1-git-send-email-quic_mkshah@quicinc.com> <1652275016-13423-5-git-send-email-quic_mkshah@quicinc.com> In-Reply-To: <1652275016-13423-5-git-send-email-quic_mkshah@quicinc.com> From: Ulf Hansson Date: Thu, 19 May 2022 14:56:44 +0200 Message-ID: Subject: Re: [PATCH v2 4/6] PM: domains: Store the closest hrtimer event of the domain CPUs To: Maulik Shah Cc: bjorn.andersson@linaro.org, linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, rafael@kernel.org, daniel.lezcano@linaro.org, quic_lsrao@quicinc.com, quic_rjendra@quicinc.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Wed, 11 May 2022 at 15:17, Maulik Shah wrote: > > The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI) > SoCs when the deepest CPUidle modes results in the SoC also to enter > the low power mode. > > RSC is part of CPU subsystem and APSS rsc device is attached to cluster > power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which > can wakeup the SoC from deepest low power states. The CONTROL_TCS does > this by writing next wakeup in always on domain timer when the SoC is > entering the low power state. > > Add dev_pm_genpd_get_next_hrtimer() to get the genpd wakeup time. > > Signed-off-by: Maulik Shah > --- > drivers/base/power/domain.c | 24 ++++++++++++++++++++++++ > drivers/base/power/domain_governor.c | 1 + > include/linux/pm_domain.h | 7 +++++++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 18cd796..f0d70d0 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -487,6 +487,29 @@ void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next) > } > EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup); > > +/** > + * dev_pm_genpd_get_next_hrtimer - Return genpd domain next_hrtimer. > + * > + * @dev: Device to handle > + * > + * Returns the aggregated domain wakeup time for CPU PM domain > + * when all the subdomains are off. To further clarify when this function should be used, I think that we should state that it should typically be called from a consumer of a genpd on/off-notifier at GENPD_NOTIFY_PRE_OFF. This also means that the genpd's lock is being held across the function. > + */ > +ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev) > +{ > + struct generic_pm_domain *genpd; > + > + genpd = dev_to_genpd_safe(dev); > + if (!genpd) > + return KTIME_MAX; > + > + if (atomic_read(&genpd->sd_count) > 0) > + return KTIME_MAX; This above isn't needed, assuming we clarify the description of the function and when it should be called. > + > + return genpd->next_hrtimer; > +} > +EXPORT_SYMBOL_GPL(dev_pm_genpd_get_next_hrtimer); > + > static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) > { > unsigned int state_idx = genpd->state_idx; > @@ -1998,6 +2021,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd, > genpd->max_off_time_changed = true; > genpd->provider = NULL; > genpd->has_provider = false; > + genpd->next_hrtimer = KTIME_MAX; > genpd->accounting_time = ktime_get_mono_fast_ns(); > genpd->domain.ops.runtime_suspend = genpd_runtime_suspend; > genpd->domain.ops.runtime_resume = genpd_runtime_resume; > diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c > index cd08c58..a4c7dd8 100644 > --- a/drivers/base/power/domain_governor.c > +++ b/drivers/base/power/domain_governor.c > @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd) > domain_wakeup = next_hrtimer; > } > } > + genpd->next_hrtimer = domain_wakeup; There should be no point to set this, unless cpu_power_down_ok() are returning true. Therefore I suggest you move this a few lines further down, where cpu_power_down_ok() actually returns true. > > /* The minimum idle duration is from now - until the next wakeup. */ > idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now)); > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 043d48e..6d9fb79 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > /* > * Flags to control the behaviour of a genpd. > @@ -136,6 +137,7 @@ struct generic_pm_domain { > struct gpd_dev_ops dev_ops; > s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ > ktime_t next_wakeup; /* Maintained by the domain governor */ > + ktime_t next_hrtimer; /* Next hrtimer for the CPU PM domain */ > bool max_off_time_changed; > bool cached_power_down_ok; > bool cached_power_down_state_idx; > @@ -228,6 +230,7 @@ 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); > +ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev); > > extern struct dev_power_governor simple_qos_governor; > extern struct dev_power_governor pm_domain_always_on_gov; > @@ -289,6 +292,10 @@ static inline int dev_pm_genpd_remove_notifier(struct device *dev) > static inline void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next) > { } > > +static inline ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev) > +{ > + return KTIME_MAX; > +} > #define simple_qos_governor (*(struct dev_power_governor *)(NULL)) > #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL)) > #endif > -- > 2.7.4 > Kind regards Uffe