linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephan Gerhold <stephan@gerhold.net>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Stephan Gerhold <stephan@gerhold.net>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Niklas Cassel <nks@flawful.org>
Subject: [RFC PATCH 2/3] opp: Set required OPPs in reverse order when scaling down
Date: Thu, 30 Jul 2020 10:01:45 +0200	[thread overview]
Message-ID: <20200730080146.25185-3-stephan@gerhold.net> (raw)
In-Reply-To: <20200730080146.25185-1-stephan@gerhold.net>

The OPP core already has well-defined semantics to ensure required
OPPs/regulators are set before/after the frequency change, depending
on if we scale up or down.

Similar requirements might exist for the order of required OPPs
when multiple power domains need to be scaled for a frequency change.

For example, on Qualcomm platforms using CPR (Core Power Reduction),
we need to scale the VDDMX and CPR power domain. When scaling up,
MX should be scaled up before CPR. When scaling down, CPR should be
scaled down before MX.

In general, if there are multiple "required-opps" in the device tree
I would expect that the order is either irrelevant, or there is some
dependency between the power domains. In that case, the power domains
should be scaled down in reverse order.

This commit updates _set_required_opps() to set required OPPs in
reverse order when scaling down.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Related discussion: https://lore.kernel.org/linux-arm-msm/20200525194443.GA11851@flawful.org/

The advantage of this approach is that the CPR driver does not need
to bother with the VDDMX power domain at all - the requirements
can be fully described within the device tree, see e.g. [1].
An alternative option would be to modify the CPR driver to make these votes.

[1]: https://lore.kernel.org/linux-arm-msm/20200507104603.GA581328@gerhold.net/2-msm8916-vdd-mx.patch
---
 drivers/opp/core.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index f7a476b55069..f93f551c911e 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -799,7 +799,7 @@ static int _set_required_opp(struct device *dev, struct device *pd_dev,
 /* This is only called for PM domain for now */
 static int _set_required_opps(struct device *dev,
 			      struct opp_table *opp_table,
-			      struct dev_pm_opp *opp)
+			      struct dev_pm_opp *opp, bool up)
 {
 	struct opp_table **required_opp_tables = opp_table->required_opp_tables;
 	struct device **genpd_virt_devs = opp_table->genpd_virt_devs;
@@ -821,12 +821,24 @@ static int _set_required_opps(struct device *dev,
 	 */
 	mutex_lock(&opp_table->genpd_virt_dev_lock);
 
-	for (i = 0; i < opp_table->required_opp_count; i++) {
-		pd_dev = genpd_virt_devs[i];
+	if (up) {
+		/* Scaling up? Set required OPPs in normal order */
+		for (i = 0; i < opp_table->required_opp_count; i++) {
+			pd_dev = genpd_virt_devs[i];
 
-		ret = _set_required_opp(dev, pd_dev, opp, i);
-		if (ret)
-			break;
+			ret = _set_required_opp(dev, pd_dev, opp, i);
+			if (ret)
+				break;
+		}
+	} else {
+		/* Scaling down? Set required OPPs in reverse order */
+		for (i = opp_table->required_opp_count - 1; i >= 0; i--) {
+			pd_dev = genpd_virt_devs[i];
+
+			ret = _set_required_opp(dev, pd_dev, opp, i);
+			if (ret)
+				break;
+		}
 	}
 	mutex_unlock(&opp_table->genpd_virt_dev_lock);
 
@@ -914,7 +926,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 			opp_table->regulator_enabled = false;
 		}
 
-		ret = _set_required_opps(dev, opp_table, NULL);
+		ret = _set_required_opps(dev, opp_table, NULL, false);
 		goto put_opp_table;
 	}
 
@@ -973,7 +985,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Scaling up? Configure required OPPs before frequency */
 	if (freq >= old_freq) {
-		ret = _set_required_opps(dev, opp_table, opp);
+		ret = _set_required_opps(dev, opp_table, opp, true);
 		if (ret)
 			goto put_opp;
 	}
@@ -993,7 +1005,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Scaling down? Configure required OPPs after frequency */
 	if (!ret && freq < old_freq) {
-		ret = _set_required_opps(dev, opp_table, opp);
+		ret = _set_required_opps(dev, opp_table, opp, false);
 		if (ret)
 			dev_err(dev, "Failed to set required opps: %d\n", ret);
 	}
-- 
2.27.0


  parent reply	other threads:[~2020-07-30  8:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-30  8:01 [RFC PATCH 0/3] opp: required_opps: Power on genpd, scale down in reverse order Stephan Gerhold
2020-07-30  8:01 ` [RFC PATCH 1/3] opp: Reduce code duplication in _set_required_opps() Stephan Gerhold
2020-08-24 11:18   ` Viresh Kumar
2020-08-24 11:30     ` Stephan Gerhold
2020-08-24 12:10       ` Viresh Kumar
2020-08-24 12:23         ` Stephan Gerhold
2020-07-30  8:01 ` Stephan Gerhold [this message]
2020-08-21 16:31   ` [RFC PATCH 2/3] opp: Set required OPPs in reverse order when scaling down Stephan Gerhold
2020-08-24 11:30     ` Viresh Kumar
2020-08-24 11:42       ` Stephan Gerhold
2020-07-30  8:01 ` [RFC PATCH 3/3] opp: Power on (virtual) power domains managed by the OPP core Stephan Gerhold
2020-08-24 11:27   ` Viresh Kumar
2020-08-24 11:55     ` Stephan Gerhold
2020-08-24 14:36       ` Ulf Hansson
2020-08-24 15:08         ` Stephan Gerhold
2020-08-25  4:43           ` Viresh Kumar
2020-08-25  6:43             ` Ulf Hansson
2020-08-25  7:33               ` Stephan Gerhold
2020-08-25 12:42                 ` Ulf Hansson
2020-08-26  8:31                   ` Stephan Gerhold
2020-08-12  8:53 ` [RFC PATCH 0/3] opp: required_opps: Power on genpd, scale down in reverse order Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200730080146.25185-3-stephan@gerhold.net \
    --to=stephan@gerhold.net \
    --cc=khilman@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nks@flawful.org \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).