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=-5.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,URIBL_DBL_ABUSE_MALW, USER_AGENT_SANE_1 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 5C6BBC35247 for ; Mon, 3 Feb 2020 17:08:49 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 27EC620721 for ; Mon, 3 Feb 2020 17:08:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="LVDbwCLD" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 27EC620721 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bby/9OoMUYTiC9UK4b6zFsjjQVz9J++A9gRwVPfJK1I=; b=LVDbwCLDtnQKzn PTWtMU5ET2dTQOQ6spotKmcUjfIra7TQMP6puTyHRyUNhRR3f8BSfs9Lx3JFK+zG74VCYI25ei1WY ZD7DPR9fa2aqvtsdmpogT345yW9y9kickgioDTwrKt00hgc2peXqfC8y4RzT0qxb3GhBWChA72Qpx AcDh02NUMj9m9Xvr4lxeKZmQuLpVzrbmB6TqlLwENbOxu7oJi73q0YlW2tDc0ffSDKA5VWFyUx3wo HSRo9eUdgypGv49tvtVq7dPJ5yT9iMkwBJ+LgP9/5KwFmX7xwQeh37y3QvLE6LIfw7j2HQmEaGs7/ Vx0DJFBT51kI78EvaLbA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iyfDK-0007Ko-3E; Mon, 03 Feb 2020 17:08:46 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iyfDG-0007K0-5l for linux-arm-kernel@lists.infradead.org; Mon, 03 Feb 2020 17:08:44 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DDFF330E; Mon, 3 Feb 2020 09:08:40 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AE0763F52E; Mon, 3 Feb 2020 09:08:38 -0800 (PST) Date: Mon, 3 Feb 2020 17:08:32 +0000 From: Sudeep Holla To: Maulik Shah Subject: Re: [PATCH v3 5/7] drivers: firmware: psci: Add hierarchical domain idle states converter Message-ID: <20200203170832.GA38466@bogus> References: <1580736940-6985-1-git-send-email-mkshah@codeaurora.org> <1580736940-6985-6-git-send-email-mkshah@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1580736940-6985-6-git-send-email-mkshah@codeaurora.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200203_090842_307549_CA3AC2C0 X-CRM114-Status: GOOD ( 29.08 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dianders@chromium.org, lsrao@codeaurora.org, Lorenzo.Pieralisi@arm.com, rnayak@codeaurora.org, linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, evgreen@chromium.org, swboyd@chromium.org, david.brown@linaro.org, agross@kernel.org, ilina@codeaurora.org, Sudeep Holla , ulf.hansson@linaro.org, bjorn.andersson@linaro.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 03, 2020 at 07:05:38PM +0530, Maulik Shah wrote: > From: Ulf Hansson > > If the hierarchical CPU topology is used, but the OS initiated mode isn't > supported, we need to rely solely on the regular cpuidle framework to > manage the idle state selection, rather than using genpd and its > governor. > > For this reason, introduce a new PSCI DT helper function, > psci_dt_pm_domains_parse_states(), which parses and converts the > hierarchically described domain idle states from DT, into regular flattened > cpuidle states. The converted states are added to the existing cpuidle > driver's array of idle states, which make them available for cpuidle. > And what's the main motivation for this if OSI is not supported in the firmware ? > Signed-off-by: Ulf Hansson > [applied to new path, resolved conflicts] > Signed-off-by: Maulik Shah > --- > drivers/cpuidle/cpuidle-psci-domain.c | 137 +++++++++++++++++++++++++++++----- > drivers/cpuidle/cpuidle-psci.c | 41 +++++----- > drivers/cpuidle/cpuidle-psci.h | 11 +++ > 3 files changed, 153 insertions(+), 36 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > index 423f03b..3c417f7 100644 > --- a/drivers/cpuidle/cpuidle-psci-domain.c > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > @@ -26,13 +26,17 @@ struct psci_pd_provider { > }; > > static LIST_HEAD(psci_pd_providers); > -static bool osi_mode_enabled __initdata; > +static bool osi_mode_enabled; > > static int psci_pd_power_off(struct generic_pm_domain *pd) > { > struct genpd_power_state *state = &pd->states[pd->state_idx]; > u32 *pd_state; > > + /* If we have failed to enable OSI mode, then abort power off. */ > + if ((psci_has_osi_support()) && !osi_mode_enabled) > + return -EBUSY; > + Why is this needed ? IIUC we don't create genpd domains if OSI is not enabled. > if (!state->data) > return 0; > > @@ -101,6 +105,105 @@ static void psci_pd_free_states(struct genpd_power_state *states, > kfree(states); > } > > +static void psci_pd_convert_states(struct cpuidle_state *idle_state, > + u32 *psci_state, struct genpd_power_state *state) > +{ > + u32 *state_data = state->data; > + u64 target_residency_us = state->residency_ns; > + u64 exit_latency_us = state->power_on_latency_ns + > + state->power_off_latency_ns; > + > + *psci_state = *state_data; > + do_div(target_residency_us, 1000); > + idle_state->target_residency = target_residency_us; > + do_div(exit_latency_us, 1000); > + idle_state->exit_latency = exit_latency_us; > + idle_state->enter = &psci_enter_domain_idle_state; > + idle_state->flags |= CPUIDLE_FLAG_TIMER_STOP; > + > + strncpy(idle_state->name, to_of_node(state->fwnode)->name, > + CPUIDLE_NAME_LEN - 1); > + strncpy(idle_state->desc, to_of_node(state->fwnode)->name, > + CPUIDLE_NAME_LEN - 1); > +} > + > +static bool psci_pd_is_provider(struct device_node *np) > +{ > + struct psci_pd_provider *pd_prov, *it; > + > + list_for_each_entry_safe(pd_prov, it, &psci_pd_providers, link) { > + if (pd_prov->node == np) > + return true; > + } > + > + return false; > +} > + > +int __init psci_dt_pm_domains_parse_states(struct cpuidle_driver *drv, > + struct device_node *cpu_node, u32 *psci_states) > +{ > + struct genpd_power_state *pd_states; > + struct of_phandle_args args; > + int ret, pd_state_count, i, state_idx, psci_idx; > + u32 cpu_psci_state = psci_states[drv->state_count - 1]; > + struct device_node *np = of_node_get(cpu_node); > + > + /* Walk the CPU topology to find compatible domain idle states. */ > + while (np) { > + ret = of_parse_phandle_with_args(np, "power-domains", > + "#power-domain-cells", 0, &args); > + of_node_put(np); > + if (ret) > + return 0; > + > + np = args.np; > + > + /* Verify that the node represents a psci pd provider. */ > + if (!psci_pd_is_provider(np)) { > + of_node_put(np); > + return 0; > + } > + > + /* Parse for compatible domain idle states. */ > + ret = psci_pd_parse_states(np, &pd_states, &pd_state_count); > + if (ret) { > + of_node_put(np); > + return ret; > + } > + > + i = 0; > + while (i < pd_state_count) { > + > + state_idx = drv->state_count; > + if (state_idx >= CPUIDLE_STATE_MAX) { > + pr_warn("exceeding max cpuidle states\n"); > + of_node_put(np); > + return 0; > + } > + > + psci_idx = state_idx + i; > + psci_pd_convert_states(&drv->states[state_idx + i], > + &psci_states[psci_idx], &pd_states[i]); > + > + /* > + * In the hierarchical CPU topology the master PM domain > + * idle state's DT property, "arm,psci-suspend-param", > + * don't contain the bits for the idle state of the CPU, > + * let's add those here. > + */ > + psci_states[psci_idx] |= cpu_psci_state; No we can't do that. Refer previous discussions around that. > + pr_debug("psci-power-state %#x index %d\n", > + psci_states[psci_idx], psci_idx); > + > + drv->state_count++; > + i++; > + } > + psci_pd_free_states(pd_states, pd_state_count); > + } > + > + return 0; > +} > + > static int __init psci_pd_init(struct device_node *np) > { > struct generic_pm_domain *pd; > @@ -125,11 +228,14 @@ static int __init psci_pd_init(struct device_node *np) > * Parse the domain idle states and let genpd manage the state selection > * for those being compatible with "domain-idle-state". > */ > - ret = psci_pd_parse_states(np, &states, &state_count); > - if (ret) > - goto free_name; > > - pd->free_states = psci_pd_free_states; > + if (psci_has_osi_support()) { > + ret = psci_pd_parse_states(np, &states, &state_count); > + if (ret) > + goto free_name; > + pd->free_states = psci_pd_free_states; > + } > + > pd->name = kbasename(pd->name); > pd->power_off = psci_pd_power_off; > pd->states = states; > @@ -236,10 +342,6 @@ static int __init psci_idle_init_domains(void) > if (!np) > return -ENODEV; > > - /* Currently limit the hierarchical topology to be used in OSI mode. */ > - if (!psci_has_osi_support()) > - goto out; > - > /* > * Parse child nodes for the "#power-domain-cells" property and > * initialize a genpd/genpd-of-provider pair when it's found. > @@ -265,14 +367,16 @@ static int __init psci_idle_init_domains(void) > goto remove_pd; > > /* Try to enable OSI mode. */ > - ret = psci_set_osi_mode(); > - if (ret) { > - pr_warn("failed to enable OSI mode: %d\n", ret); > - psci_pd_remove_topology(np); > - goto remove_pd; > + if (psci_has_osi_support()) { > + ret = psci_set_osi_mode(); > + if (ret) { > + pr_warn("failed to enable OSI mode: %d\n", ret); > + psci_pd_remove_topology(np); > + goto remove_pd; > + } else > + osi_mode_enabled = true; > } > > - osi_mode_enabled = true; > of_node_put(np); > pr_info("Initialized CPU PM domain topology\n"); > return pd_count; > @@ -293,9 +397,6 @@ struct device __init *psci_dt_attach_cpu(int cpu) > { > struct device *dev; > > - if (!osi_mode_enabled) > - return NULL; > - > dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci"); > if (IS_ERR_OR_NULL(dev)) > return dev; > diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c > index edd7a54..3fa2aee 100644 > --- a/drivers/cpuidle/cpuidle-psci.c > +++ b/drivers/cpuidle/cpuidle-psci.c > @@ -49,7 +49,7 @@ static inline int psci_enter_state(int idx, u32 state) > return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, idx, state); > } > > -static int psci_enter_domain_idle_state(struct cpuidle_device *dev, > +int psci_enter_domain_idle_state(struct cpuidle_device *dev, > struct cpuidle_driver *drv, int idx) > { > struct psci_cpuidle_data *data = this_cpu_ptr(&psci_cpuidle_data); > @@ -193,24 +193,29 @@ static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv, > goto free_mem; > } > > - /* Currently limit the hierarchical topology to be used in OSI mode. */ > - if (psci_has_osi_support()) { > - data->dev = psci_dt_attach_cpu(cpu); > - if (IS_ERR(data->dev)) { > - ret = PTR_ERR(data->dev); > + if (!psci_has_osi_support()) { > + ret = psci_dt_pm_domains_parse_states(drv, cpu_node, > + psci_states); > + if (ret) > goto free_mem; > - } > - > - /* > - * Using the deepest state for the CPU to trigger a potential > - * selection of a shared state for the domain, assumes the > - * domain states are all deeper states. > - */ > - if (data->dev) { > - drv->states[state_count - 1].enter = > - psci_enter_domain_idle_state; > - psci_cpuidle_use_cpuhp = true; > - } > + } > + > + data->dev = psci_dt_attach_cpu(cpu); > + if (IS_ERR(data->dev)) { > + ret = PTR_ERR(data->dev); > + goto free_mem; > + } > + > + /* > + * Using the deepest state for the CPU to trigger a potential > + * selection of a shared state for the domain, assumes the > + * domain states are all deeper states. > + */ > + > + if (data->dev) { > + drv->states[state_count - 1].enter = > + psci_enter_domain_idle_state; > + psci_cpuidle_use_cpuhp = true; > } > > /* Idle states parsed correctly, store them in the per-cpu struct. */ -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel