Message ID | 20230925131715.138411-7-ulf.hansson@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:cae8:0:b0:403:3b70:6f57 with SMTP id r8csp1331835vqu; Mon, 25 Sep 2023 09:22:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEFP14qY9XpaIht2+EBcnm8bDIHDnVXrE7TkcwJoKoHa/PJVlReNiBbieyyYN98b1XMs7/C X-Received: by 2002:a05:6808:2a53:b0:3a8:6a4b:9e34 with SMTP id fa19-20020a0568082a5300b003a86a4b9e34mr8086768oib.47.1695658969632; Mon, 25 Sep 2023 09:22:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695658969; cv=none; d=google.com; s=arc-20160816; b=BXjJwh4biHvfxwKpoJFTolVt963bDlWpm+NI0OiIRAuuDRICSadKQF1PzcAkZYH2Lf ZA86IpU6tv6Fkn9Qtkm1P3wAcvk7AaQ9udNq5MWxLQBUMM1N/jwmP7JKs6hPLjPb1FMD tTLuK4JgSp72ejlfqjk2oKLkAWBjEvK3UxMUhHs49RlZ+vmTPUhOVnwBvu0mpwEU99Ve YW3sbV9ptZ48QpHfc4XP1VXraOBVAIxvUwZ6B7XHPM/5yJqryW0ofzp7CpedehhJYVcj W7io8z0n16sMsT4IVc7rgoFSOE+hOnw87iB354e7FD93YKimoR/2ACmbIQ+d+0XGMaNO 2Vog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=2bFHpy7o1LqMw4W/VuJiDN5LSPCulCxHGcm0dhiriMk=; fh=0iKC8n1xKtkvw+5RDzSud32j7h9ilf6Mvh4d+L1hSv0=; b=SYhrVcbFTmK2F+pId2BWfa8A1XKnKMNrnd+dxlTU59gxZHZtxxmA8PsGQjdpX010NP ICJIM5f9Lk5DcZ39BIgDhUQE8lWJ72A5O43odn5u/UVPo1aVfdG8PovGpBrx9a5wO92o /fpn545H+7ZqNd/cUkgcpq9s5FQrp/pCDumJ4VDAs3EAToYYx8uCupYd4u+UxLVeUpAK n/+L/oqn3ViVH+Vh5a82pWGHPR+hLsRuKFzOAqxaUJ6YZIG7pVQFRdah2p6I6XZFha2a FCPTrmd4vWVtWkYJVp1cf2v/PBGPleOCpjhwrkP+h+fEevFGsjdmCkba3DNvcQWI9src FYaA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=akzsUeXA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from howler.vger.email (howler.vger.email. [2620:137:e000::3:4]) by mx.google.com with ESMTPS id s132-20020a63778a000000b0057822296958si10370166pgc.127.2023.09.25.09.22.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Sep 2023 09:22:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=akzsUeXA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id B0351824E7B9; Mon, 25 Sep 2023 06:18:28 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231868AbjIYNSX (ORCPT <rfc822;pusanteemu@gmail.com> + 29 others); Mon, 25 Sep 2023 09:18:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231797AbjIYNSM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 25 Sep 2023 09:18:12 -0400 Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D48B11F for <linux-kernel@vger.kernel.org>; Mon, 25 Sep 2023 06:18:03 -0700 (PDT) Received: by mail-lf1-x134.google.com with SMTP id 2adb3069b0e04-503f39d3236so10274028e87.0 for <linux-kernel@vger.kernel.org>; Mon, 25 Sep 2023 06:18:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1695647882; x=1696252682; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=2bFHpy7o1LqMw4W/VuJiDN5LSPCulCxHGcm0dhiriMk=; b=akzsUeXATj6phxvbMERpRKxwOFzaqEdoEpSv0qkCkrIv8xSul2dEEqSCp2Wg0ZYNLW TDWdEo8mdnA3tl1Tfq+59V/IelAMic1xq/D9hSJuPe6WHL0AEI6jrP09on29j9JmA0jv 8HhGi5hlL+ihxBkqAEUdmQo+Sb+iJs1FUX9oXHbdvItStVA4p+0O6AA1F2D+B5jwwOHj WqxI1/cLrmnrJPrX8FS/iyu6eOKZjfJvTjL1y5Ajdqzfzi1xYjnWAFkAPmxRx0EXewhZ 40mqR4b6q1256GPxNCzgtd+ud/pdQizE81b+7kCgDY+YesbV3O8uTauH7nh9FK/dpZXV evow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695647882; x=1696252682; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2bFHpy7o1LqMw4W/VuJiDN5LSPCulCxHGcm0dhiriMk=; b=HGxzoKJhgG2nAc5HlAysSRdzaziRQlYFdzdhKVjUk1M0oTUUlsSUSea9rZS9eqrB7v uo2x2Civ9JsIybSWWZ7st4z7Cb2K+rw+jGV6qbRD4KZYWp59MnCfDOILEicRSvwHayUr OKDjuXmkBrTfs3ox9wJ3VhgGb7Uz1CnQQ/dwbHiJboggeDzapNmBvWvjJSwG3rWovA6U W57Hnz2sMu6NKYSSrpCyYx2WPD9Yan2f532A3Pnhawtq/e00UVhSyVppOE1aqw0RVMUQ CWFGcsUQsEUvDETGXMHEzuWx3zRAEpgVG2SNffuRikJFXf98Aby8Zpn/e3ClRn4AHMyp 2ipg== X-Gm-Message-State: AOJu0YxmuIk7WxxBoI8FSlsJB9qrEKu6920er6N4WDw1nXOAj9mCiU0t S9+/v3r6GX/YVwJz79he1kBIGA== X-Received: by 2002:a05:6512:39c5:b0:4fd:faa5:64ed with SMTP id k5-20020a05651239c500b004fdfaa564edmr7421632lfu.11.1695647882083; Mon, 25 Sep 2023 06:18:02 -0700 (PDT) Received: from uffe-tuxpro14.. (h-94-254-63-18.NA.cust.bahnhof.se. [94.254.63.18]) by smtp.gmail.com with ESMTPSA id m29-20020a056512015d00b004fe10276bbfsm1823583lfo.296.2023.09.25.06.18.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Sep 2023 06:18:01 -0700 (PDT) From: Ulf Hansson <ulf.hansson@linaro.org> To: Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi <cristian.marussi@arm.com>, Viresh Kumar <vireshk@kernel.org>, "Rafael J . Wysocki" <rafael@kernel.org> Cc: Nishanth Menon <nm@ti.com>, Stephen Boyd <sboyd@kernel.org>, Nikunj Kela <nkela@quicinc.com>, Prasad Sodagudi <psodagud@quicinc.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Ulf Hansson <ulf.hansson@linaro.org>, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 6/9] OPP: Extend support for the opp-level beyond required-opps Date: Mon, 25 Sep 2023 15:17:12 +0200 Message-Id: <20230925131715.138411-7-ulf.hansson@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230925131715.138411-1-ulf.hansson@linaro.org> References: <20230925131715.138411-1-ulf.hansson@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Mon, 25 Sep 2023 06:18:28 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778027213845823087 X-GMAIL-MSGID: 1778027299695019946 |
Series |
PM: domains/opp/arm_scmi: Extend performance scaling support
|
|
Commit Message
Ulf Hansson
Sept. 25, 2023, 1:17 p.m. UTC
At this point the level (performance state) for an OPP is currently limited
to be requested for a device that is attached to a PM domain. Moreover,
the device needs to have the so called required-opps assigned to it, which
are based upon OPP tables being described in DT.
To extend the support beyond required-opps and DT, let's enable the level
to be set for all OPPs. More precisely, if the requested OPP has a valid
level let's try to request it through the device's optional PM domain, via
calling dev_pm_domain_set_performance_state().
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/opp/core.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Comments
On 25-09-23, 15:33, Ulf Hansson wrote: > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > At this point the level (performance state) for an OPP is currently limited > > to be requested for a device that is attached to a PM domain. Moreover, > > the device needs to have the so called required-opps assigned to it, which > > are based upon OPP tables being described in DT. > > > > To extend the support beyond required-opps and DT, let's enable the level > > to be set for all OPPs. More precisely, if the requested OPP has a valid > > level let's try to request it through the device's optional PM domain, via > > calling dev_pm_domain_set_performance_state(). > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/opp/core.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > index 60dca60ac4af..afb73978cdcb 100644 > > --- a/drivers/opp/core.c > > +++ b/drivers/opp/core.c > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > > opp_table->set_required_opps = _opp_set_required_opps_generic; > > } > > > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > > + struct dev_pm_opp *opp) > > +{ > > + int ret = 0; > > + > > + /* Request a new performance state through the device's PM domain. */ > > + if (opp && opp->level) { > > + ret = dev_pm_domain_set_performance_state(dev, opp->level); > > + if (ret) > > + dev_err(dev, "Failed to set performance state %u (%d)\n", > > + opp->level, ret); > > + } > > Okay, so reviewing my own code found a problem here. We need an "else" > here, that should request the performance state to be set to 0. > > I am not sending a new version at this point, but awaiting more feedback first. I am looking to add below to this patch, is that okay with you ? diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 7b505316bb1c..a113e9caaa5a 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table) static int _set_opp_level(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp) { + unsigned int level = 0; int ret = 0; - /* Request a new performance state through the device's PM domain. */ - if (opp && opp->level) { - ret = dev_pm_domain_set_performance_state(dev, opp->level); - if (ret) - dev_err(dev, "Failed to set performance state %u (%d)\n", - opp->level, ret); + if (opp) { + if (!opp->level) + return 0; + + level = opp->level; } + /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_genpd_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + return ret; } I am switching back to dev_pm_genpd_set_performance_state() as I won't be applying the power domain changes. I will then push out a branch and you can rebase your patches on top of it ? And then probably Sudeep or someone else can apply everything ?
On Tue, 3 Oct 2023 at 10:23, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 25-09-23, 15:33, Ulf Hansson wrote: > > On Mon, 25 Sept 2023 at 15:18, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > > > At this point the level (performance state) for an OPP is currently limited > > > to be requested for a device that is attached to a PM domain. Moreover, > > > the device needs to have the so called required-opps assigned to it, which > > > are based upon OPP tables being described in DT. > > > > > > To extend the support beyond required-opps and DT, let's enable the level > > > to be set for all OPPs. More precisely, if the requested OPP has a valid > > > level let's try to request it through the device's optional PM domain, via > > > calling dev_pm_domain_set_performance_state(). > > > > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > > --- > > > drivers/opp/core.c | 29 +++++++++++++++++++++++++++++ > > > 1 file changed, 29 insertions(+) > > > > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > > > index 60dca60ac4af..afb73978cdcb 100644 > > > --- a/drivers/opp/core.c > > > +++ b/drivers/opp/core.c > > > @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > > > opp_table->set_required_opps = _opp_set_required_opps_generic; > > > } > > > > > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > > > + struct dev_pm_opp *opp) > > > +{ > > > + int ret = 0; > > > + > > > + /* Request a new performance state through the device's PM domain. */ > > > + if (opp && opp->level) { > > > + ret = dev_pm_domain_set_performance_state(dev, opp->level); > > > + if (ret) > > > + dev_err(dev, "Failed to set performance state %u (%d)\n", > > > + opp->level, ret); > > > + } > > > > Okay, so reviewing my own code found a problem here. We need an "else" > > here, that should request the performance state to be set to 0. > > > > I am not sending a new version at this point, but awaiting more feedback first. > > I am looking to add below to this patch, is that okay with you ? > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 7b505316bb1c..a113e9caaa5a 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1135,16 +1135,22 @@ void _update_set_required_opps(struct opp_table *opp_table) > static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > struct dev_pm_opp *opp) > { > + unsigned int level = 0; > int ret = 0; > > - /* Request a new performance state through the device's PM domain. */ > - if (opp && opp->level) { > - ret = dev_pm_domain_set_performance_state(dev, opp->level); > - if (ret) > - dev_err(dev, "Failed to set performance state %u (%d)\n", > - opp->level, ret); > + if (opp) { > + if (!opp->level) > + return 0; > + > + level = opp->level; > } > > + /* Request a new performance state through the device's PM domain. */ > + ret = dev_pm_genpd_set_performance_state(dev, level); > + if (ret) > + dev_err(dev, "Failed to set performance state %u (%d)\n", level, > + ret); > + > return ret; > } > > I am switching back to dev_pm_genpd_set_performance_state() as I won't be > applying the power domain changes. Can you please explain further on this. Rafael has acked those patches, so it should be perfectly fine for you to pick them via your tree too. There is no need to defer them. > I will then push out a branch and you can > rebase your patches on top of it ? And then probably Sudeep or someone else can > apply everything ? Or are you suggesting to just take one of the patches from my series, and then I will re-base everything on top? Just trying to understand the way forward. :-) Kind regards Uffe
On 03-10-23, 14:36, Ulf Hansson wrote: > Can you please explain further on this. Rafael has acked those > patches, so it should be perfectly fine for you to pick them via your > tree too. There is no need to defer them. Ahh, then it is fine. > > I will then push out a branch and you can > > rebase your patches on top of it ? And then probably Sudeep or someone else can > > apply everything ? > > Or are you suggesting to just take one of the patches from my series, > and then I will re-base everything on top? > > Just trying to understand the way forward. :-) Applied patches 1-6/9 and the fixed routine looks like this now: +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, + struct dev_pm_opp *opp) +{ + unsigned int level = 0; + int ret = 0; + + if (opp) { + if (!opp->level) + return 0; + + level = opp->level; + } + + /* Request a new performance state through the device's PM domain. */ + ret = dev_pm_domain_set_performance_state(dev, level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", level, + ret); + + return ret; +} Does it look okay now ?
On Wed, 4 Oct 2023 at 07:04, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 03-10-23, 14:36, Ulf Hansson wrote: > > Can you please explain further on this. Rafael has acked those > > patches, so it should be perfectly fine for you to pick them via your > > tree too. There is no need to defer them. > > Ahh, then it is fine. > > > > I will then push out a branch and you can > > > rebase your patches on top of it ? And then probably Sudeep or someone else can > > > apply everything ? > > > > Or are you suggesting to just take one of the patches from my series, > > and then I will re-base everything on top? > > > > Just trying to understand the way forward. :-) > > Applied patches 1-6/9 and the fixed routine looks like this now: > > +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, > + struct dev_pm_opp *opp) > +{ > + unsigned int level = 0; > + int ret = 0; > + > + if (opp) { > + if (!opp->level) > + return 0; > + > + level = opp->level; > + } > + > + /* Request a new performance state through the device's PM domain. */ > + ret = dev_pm_domain_set_performance_state(dev, level); > + if (ret) > + dev_err(dev, "Failed to set performance state %u (%d)\n", level, > + ret); > + > + return ret; > +} > > Does it look okay now ? Yes, perfect! Thanks for helping out! Kind regards Uffe
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 60dca60ac4af..afb73978cdcb 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1107,6 +1107,22 @@ void _update_set_required_opps(struct opp_table *opp_table) opp_table->set_required_opps = _opp_set_required_opps_generic; } +static int _set_opp_level(struct device *dev, struct opp_table *opp_table, + struct dev_pm_opp *opp) +{ + int ret = 0; + + /* Request a new performance state through the device's PM domain. */ + if (opp && opp->level) { + ret = dev_pm_domain_set_performance_state(dev, opp->level); + if (ret) + dev_err(dev, "Failed to set performance state %u (%d)\n", + opp->level, ret); + } + + return ret; +} + static void _find_current_opp(struct device *dev, struct opp_table *opp_table) { struct dev_pm_opp *opp = ERR_PTR(-ENODEV); @@ -1154,8 +1170,13 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table) if (opp_table->regulators) regulator_disable(opp_table->regulators[0]); + ret = _set_opp_level(dev, opp_table, NULL); + if (ret) + goto out; + ret = _set_required_opps(dev, opp_table, NULL, false); +out: opp_table->enabled = false; return ret; } @@ -1198,6 +1219,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, return ret; } + ret = _set_opp_level(dev, opp_table, opp); + if (ret) + return ret; + ret = _set_opp_bw(opp_table, opp, dev); if (ret) { dev_err(dev, "Failed to set bw: %d\n", ret); @@ -1241,6 +1266,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, return ret; } + ret = _set_opp_level(dev, opp_table, opp); + if (ret) + return ret; + ret = _set_required_opps(dev, opp_table, opp, false); if (ret) { dev_err(dev, "Failed to set required opps: %d\n", ret);