From patchwork Thu Jul 30 08:01:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephan Gerhold X-Patchwork-Id: 11692433 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 62C3A138A for ; Thu, 30 Jul 2020 08:02:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 491E620809 for ; Thu, 30 Jul 2020 08:02:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gerhold.net header.i=@gerhold.net header.b="LKR+bBGj" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725974AbgG3ICF (ORCPT ); Thu, 30 Jul 2020 04:02:05 -0400 Received: from mo4-p01-ob.smtp.rzone.de ([81.169.146.165]:25948 "EHLO mo4-p01-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726194AbgG3ICE (ORCPT ); Thu, 30 Jul 2020 04:02:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1596096120; s=strato-dkim-0002; d=gerhold.net; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=n/qF6XkwQZ048amOMDhjZoS46CKxlEYFuVpG5WDcxyg=; b=LKR+bBGjh57RtT4olMQPPg8NJeLM+10MnfPU2Dz4KnRelAAZexkysltAkOUC4s5ipu TdVTdCOJ1bkuF7IWVcU3QyznoISLyRN7SI/GiXB3cLewAUHE5PR+mblBwHUXxy4RMqh6 SLuasgnkvlV/5dMwsw4sOraqU6VySzs4JEppLx2TonzhE6PEnBg7A/Nb/B5IsukxDyXA 6LzOe7Xu1VRfePjPMAbcC3OCQWon+cJ7x1OZajb5h2tKZdDc5SRYAyI9YZIDdNg4/0Mo YV6B/BaIW2qrCDOZp28F1TBtcKu/kaxUVMk6MIgkQQltqgk6SCGHh3MXkj2QPtIUb2uO LYTQ== X-RZG-AUTH: ":P3gBZUipdd93FF5ZZvYFPugejmSTVR2nRPhVORvLd4SsytBXS7IYBkLahKxB4G6NeHYC" X-RZG-CLASS-ID: mo00 Received: from localhost.localdomain by smtp.strato.de (RZmta 46.10.5 DYNA|AUTH) with ESMTPSA id Y0939ew6U81xgup (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Thu, 30 Jul 2020 10:01:59 +0200 (CEST) From: Stephan Gerhold To: Viresh Kumar Cc: Stephan Gerhold , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , Nishanth Menon , Stephen Boyd , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Niklas Cassel Subject: [RFC PATCH 1/3] opp: Reduce code duplication in _set_required_opps() Date: Thu, 30 Jul 2020 10:01:44 +0200 Message-Id: <20200730080146.25185-2-stephan@gerhold.net> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200730080146.25185-1-stephan@gerhold.net> References: <20200730080146.25185-1-stephan@gerhold.net> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org Move call to dev_pm_genpd_set_performance_state() to a separate function so we can avoid duplicating the code for the single and multiple genpd case. Signed-off-by: Stephan Gerhold --- drivers/opp/core.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 9d7fb45b1786..f7a476b55069 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -781,6 +781,21 @@ static int _set_opp_custom(const struct opp_table *opp_table, return opp_table->set_opp(data); } +static int _set_required_opp(struct device *dev, struct device *pd_dev, + struct dev_pm_opp *opp, int i) +{ + unsigned int pstate = likely(opp) ? opp->required_opps[i]->pstate : 0; + int ret; + + ret = dev_pm_genpd_set_performance_state(pd_dev, pstate); + if (ret) { + dev_err(dev, "Failed to set performance rate of %s: %d (%d)\n", + dev_name(pd_dev), pstate, ret); + } + + return ret; +} + /* This is only called for PM domain for now */ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, @@ -788,22 +803,15 @@ static int _set_required_opps(struct device *dev, { struct opp_table **required_opp_tables = opp_table->required_opp_tables; struct device **genpd_virt_devs = opp_table->genpd_virt_devs; - unsigned int pstate; + struct device *pd_dev; int i, ret = 0; if (!required_opp_tables) return 0; /* Single genpd case */ - if (!genpd_virt_devs) { - pstate = likely(opp) ? opp->required_opps[0]->pstate : 0; - ret = dev_pm_genpd_set_performance_state(dev, pstate); - if (ret) { - dev_err(dev, "Failed to set performance state of %s: %d (%d)\n", - dev_name(dev), pstate, ret); - } - return ret; - } + if (!genpd_virt_devs) + return _set_required_opp(dev, dev, opp, 0); /* Multiple genpd case */ @@ -814,17 +822,11 @@ 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++) { - pstate = likely(opp) ? opp->required_opps[i]->pstate : 0; - - if (!genpd_virt_devs[i]) - continue; + pd_dev = genpd_virt_devs[i]; - ret = dev_pm_genpd_set_performance_state(genpd_virt_devs[i], pstate); - if (ret) { - dev_err(dev, "Failed to set performance rate of %s: %d (%d)\n", - dev_name(genpd_virt_devs[i]), pstate, ret); + ret = _set_required_opp(dev, pd_dev, opp, i); + if (ret) break; - } } mutex_unlock(&opp_table->genpd_virt_dev_lock); From patchwork Thu Jul 30 08:01:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephan Gerhold X-Patchwork-Id: 11692435 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 03E6B722 for ; Thu, 30 Jul 2020 08:02:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DE42C20809 for ; Thu, 30 Jul 2020 08:02:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gerhold.net header.i=@gerhold.net header.b="tPj+che6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728724AbgG3ICI (ORCPT ); Thu, 30 Jul 2020 04:02:08 -0400 Received: from mo4-p02-ob.smtp.rzone.de ([85.215.255.84]:31352 "EHLO mo4-p02-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726804AbgG3ICF (ORCPT ); Thu, 30 Jul 2020 04:02:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1596096121; s=strato-dkim-0002; d=gerhold.net; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=HaJwBOdIkjvs8im4/IU2I0knSpAyk7I2vquFxeVH9ME=; b=tPj+che6PdMsJZIu4uEyy4FRuJ6xYfsz/4Yk62XyHhquRNQBi0twzgD8LZYK/+gKRL gEXRtJucChVLPL9Ibjmb1WYMp03P5lIp0Ua8VcDm9cnZM4vByEHqyilghG5hSIGlebM4 F/ojWJDAcxtH+PzrAtOzK89KHg0SYTK4G6iBLQ3IwnaSicrevGjGyPKbtVeX83nqWQMc aHEG3HSJZMvnG5pJNfEMsc5Vm6/5Zr+ndMzSGPmhwQdNc3EMhdmEt0VMOAUXXuwlg+4E n/j6mNHPrkGZ6emI6Crhl8SL9Da5MiqsgWXcRoXbqAU4K8WFOY4YqKhgvUgh5oYXd+IG zLDw== X-RZG-AUTH: ":P3gBZUipdd93FF5ZZvYFPugejmSTVR2nRPhVORvLd4SsytBXS7IYBkLahKxB4G6NeHYC" X-RZG-CLASS-ID: mo00 Received: from localhost.localdomain by smtp.strato.de (RZmta 46.10.5 DYNA|AUTH) with ESMTPSA id Y0939ew6U81xgut (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Thu, 30 Jul 2020 10:01:59 +0200 (CEST) From: Stephan Gerhold To: Viresh Kumar Cc: Stephan Gerhold , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , Nishanth Menon , Stephen Boyd , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Niklas Cassel Subject: [RFC PATCH 2/3] opp: Set required OPPs in reverse order when scaling down Date: Thu, 30 Jul 2020 10:01:45 +0200 Message-Id: <20200730080146.25185-3-stephan@gerhold.net> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200730080146.25185-1-stephan@gerhold.net> References: <20200730080146.25185-1-stephan@gerhold.net> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org 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 --- 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); } From patchwork Thu Jul 30 08:01:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephan Gerhold X-Patchwork-Id: 11692437 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1E11B722 for ; Thu, 30 Jul 2020 08:02:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F083522B40 for ; Thu, 30 Jul 2020 08:02:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gerhold.net header.i=@gerhold.net header.b="U7m9vs2E" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728646AbgG3ICH (ORCPT ); Thu, 30 Jul 2020 04:02:07 -0400 Received: from mo4-p02-ob.smtp.rzone.de ([81.169.146.168]:27376 "EHLO mo4-p02-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726774AbgG3ICG (ORCPT ); Thu, 30 Jul 2020 04:02:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1596096121; s=strato-dkim-0002; d=gerhold.net; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=+WtiI0ygGdiZUF68UUWGgtshE7pdk3Dx6AuxN7BGWgU=; b=U7m9vs2ExhWUf5A617LAEWY9hr4EN4A25dYKIxRFAAbkVxGBCGTPRvCxIcVjZA4fD9 +Ay4+bVLVNqGoJoVg33GpXoVcHFzcTy399+Lwb/NT1/rwaxnvWjljXI8O/XJcedswHxM xMIGFdB0T4macqh+Bh9lwVn6nubqytU3vJ5fJYyP+ZuKpVsgjx4klRiRa5YhnWPSfTUJ yPshvxB1A2Cf+KyJ1mC0wp2hwUFA1yU2tPv748OHn7xq2uRgZ1twWPRbrdunv9aKeZ6e 30OG3BVO1HWLkhVxu5QUlC6PV6W6gaPBzO36uSoxAaXe56u5FyzKdKjT0ikYDguBj7pR IH9Q== X-RZG-AUTH: ":P3gBZUipdd93FF5ZZvYFPugejmSTVR2nRPhVORvLd4SsytBXS7IYBkLahKxB4G6NeHYC" X-RZG-CLASS-ID: mo00 Received: from localhost.localdomain by smtp.strato.de (RZmta 46.10.5 DYNA|AUTH) with ESMTPSA id Y0939ew6U820guw (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Thu, 30 Jul 2020 10:02:00 +0200 (CEST) From: Stephan Gerhold To: Viresh Kumar Cc: Stephan Gerhold , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , Nishanth Menon , Stephen Boyd , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Niklas Cassel Subject: [RFC PATCH 3/3] opp: Power on (virtual) power domains managed by the OPP core Date: Thu, 30 Jul 2020 10:01:46 +0200 Message-Id: <20200730080146.25185-4-stephan@gerhold.net> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200730080146.25185-1-stephan@gerhold.net> References: <20200730080146.25185-1-stephan@gerhold.net> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org dev_pm_opp_attach_genpd() allows attaching an arbitrary number of power domains to an OPP table. In that case, the genpd core will create a virtual device for each of the power domains. At the moment, the OPP core only calls dev_pm_genpd_set_performance_state() on these virtual devices. It does not attempt to power on the power domains. Therefore the required power domain might never get turned on. So far, dev_pm_opp_attach_genpd() is only used in qcom-cpufreq-nvmem.c to attach the CPR power domain to the CPU OPP table. The CPR driver does not check if it was actually powered on so this did not cause any problems. However, other drivers (e.g. rpmpd) might ignore the performance state until the power domain is actually powered on. Since these virtual devices are managed exclusively by the OPP core, I would say that it should also be responsible to ensure they are enabled. A similar approach is already used for regulators, see commit 8d45719caaf5 ("opp: core: add regulators enable and disable"). This commit implements similar functionality for the virtual genpd devices managed by the OPP core. The power domains are turned on the first time dev_pm_opp_set_rate() is called. They are turned off again when dev_pm_opp_set_rate(dev, 0) is called. Signed-off-by: Stephan Gerhold --- Related discussion: https://lore.kernel.org/linux-arm-msm/20200426123140.GA190483@gerhold.net/ There would be also other ways to implement this, e.g. device links, assuming that the device using the OPP table also makes use of runtime PM. My first thought was that it would be most consistent to handle this like regulators, bandwidth votes etc. RFC :) --- drivers/opp/core.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/opp/opp.h | 1 + 2 files changed, 56 insertions(+) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index f93f551c911e..66ecffe12f01 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "opp.h" @@ -796,6 +797,26 @@ static int _set_required_opp(struct device *dev, struct device *pd_dev, return ret; } +static int _enable_required_opp(struct device *dev, struct device *pd_dev, + bool on) +{ + int ret; + + if (on) { + ret = pm_runtime_get_sync(pd_dev); + if (ret < 0) { + pm_runtime_put_noidle(pd_dev); + dev_err(dev, "Failed to enable %s: %d\n", + dev_name(pd_dev), ret); + return ret; + } + } else { + pm_runtime_put(pd_dev); + } + + return 0; +} + /* This is only called for PM domain for now */ static int _set_required_opps(struct device *dev, struct opp_table *opp_table, @@ -803,6 +824,8 @@ static int _set_required_opps(struct device *dev, { struct opp_table **required_opp_tables = opp_table->required_opp_tables; struct device **genpd_virt_devs = opp_table->genpd_virt_devs; + bool power_on = opp != NULL; + bool already_enabled = power_on == opp_table->genpd_virt_enabled; struct device *pd_dev; int i, ret = 0; @@ -829,6 +852,20 @@ static int _set_required_opps(struct device *dev, ret = _set_required_opp(dev, pd_dev, opp, i); if (ret) break; + + if (likely(already_enabled)) + continue; + + ret = _enable_required_opp(dev, pd_dev, power_on); + if (ret) + break; + } + + if (ret && !already_enabled) { + /* Rollback (skip current since it failed) */ + for (i--; i >= 0; i--) + _enable_required_opp(dev, genpd_virt_devs[i], + !power_on); } } else { /* Scaling down? Set required OPPs in reverse order */ @@ -838,8 +875,26 @@ static int _set_required_opps(struct device *dev, ret = _set_required_opp(dev, pd_dev, opp, i); if (ret) break; + + if (likely(already_enabled)) + continue; + + ret = _enable_required_opp(dev, pd_dev, power_on); + if (ret) + break; + } + + if (ret && !already_enabled) { + /* Rollback (skip current since it failed) */ + for (i++; i < opp_table->required_opp_count; i++) + _enable_required_opp(dev, genpd_virt_devs[i], + !power_on); } } + + if (ret == 0 && !already_enabled) + opp_table->genpd_virt_enabled = power_on; + mutex_unlock(&opp_table->genpd_virt_dev_lock); return ret; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index e51646ff279e..01ad9e136cc8 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -188,6 +188,7 @@ struct opp_table { struct device **genpd_virt_devs; struct opp_table **required_opp_tables; unsigned int required_opp_count; + bool genpd_virt_enabled; unsigned int *supported_hw; unsigned int supported_hw_count;