From: Roger Lu <roger.lu@mediatek.com>
To: Pi-Hsun Shih <pihsun@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Nicolas Boichat <drinkcat@google.com>,
Angus Lin <Angus.Lin@mediatek.com>,
Kevin Hilman <khilman@kernel.org>,
Stephen Boyd <sboyd@kernel.org>,
"open list:THERMAL" <linux-pm@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
HenryC Chen <HenryC.Chen@mediatek.com>, <yt.lee@mediatek.com>,
Fan Chen <fan.chen@mediatek.com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Nishanth Menon <nm@ti.com>,
"moderated list:ARM/Mediatek SoC support"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v5 3/3] PM / AVS: SVS: Introduce SVS engine
Date: Fri, 27 Dec 2019 15:14:49 +0800 [thread overview]
Message-ID: <1577430889.10290.49.camel@mtksdaap41> (raw)
In-Reply-To: <CANdKZ0eEMQe+OFOJxOs37gyUm2vGQ2F-NeAkcxmgYXVKvRzQBw@mail.gmail.com>
Dear Pi-Hsun,
Sorry for the late reply.
On Thu, 2019-11-14 at 15:41 +0800, Pi-Hsun Shih wrote:
> Hi Roger,
>
> On Fri, Sep 6, 2019 at 6:06 PM Roger Lu <roger.lu@mediatek.com> wrote:
> >
> > The SVS (Smart Voltage Scaling) engine is a piece of hardware which is
> > used to calculate optimized voltage values of several power domains, e.g.
> > CPU/GPU/CCI, according to chip process corner, temperatures, and other
> > factors. Then DVFS driver could apply those optimized voltage values to
> > reduce power consumption.
> >
> > Signed-off-by: Roger Lu <roger.lu@mediatek.com>
> > ---
> > drivers/power/avs/Kconfig | 10 +
> > drivers/power/avs/Makefile | 1 +
> > drivers/power/avs/mtk_svs.c | 2075 +++++++++++++++++++++++++++++++++
> > include/linux/power/mtk_svs.h | 23 +
> > 4 files changed, 2109 insertions(+)
> > create mode 100644 drivers/power/avs/mtk_svs.c
> > create mode 100644 include/linux/power/mtk_svs.h
> >
> > [...]
> > diff --git a/drivers/power/avs/mtk_svs.c b/drivers/power/avs/mtk_svs.c
> > new file mode 100644
> > index 000000000000..78ec93c3a4a5
> > --- /dev/null
> > +++ b/drivers/power/avs/mtk_svs.c
> > [...]
> > +static int svs_set_volts(struct svs_bank *svsb, bool force_update)
> > +{
> > + u32 i, svsb_volt, opp_volt, low_temp_offset = 0;
> > + int zone_temp, ret;
> > +
> > + mutex_lock(&svsb->lock);
> > +
> > + /* If bank is suspended, it means init02 voltage is applied.
> > + * Don't need to update opp voltage anymore.
> > + */
> > + if (svsb->suspended && !force_update) {
> > + pr_notice("%s: bank is suspended\n", svsb->name);
> > + mutex_unlock(&svsb->lock);
> > + return -EPERM;
> > + }
> > +
> > + /* get thermal effect */
> > + if (svsb->phase == SVS_PHASE_MON) {
> > + if (svsb->svs_temp > svsb->upper_temp_bound &&
> > + svsb->svs_temp < svsb->lower_temp_bound) {
> > + pr_err("%s: svs_temp is abnormal (0x%x)?\n",
> > + svsb->name, svsb->svs_temp);
> > + mutex_unlock(&svsb->lock);
> > + return -EINVAL;
> > + }
> > +
> > + ret = svs_get_zone_temperature(svsb, &zone_temp);
> > + if (ret) {
> > + pr_err("%s: cannot get zone \"%s\" temperature\n",
> > + svsb->name, svsb->zone_name);
> > + pr_err("%s: add low_temp_offset = %u\n",
> > + svsb->name, svsb->low_temp_offset);
> > + zone_temp = svsb->low_temp_threashold;
> > + }
> > +
> > + if (zone_temp <= svsb->low_temp_threashold)
> > + low_temp_offset = svsb->low_temp_offset;
> > + }
> > +
> > + /* vmin <= svsb_volt (opp_volt) <= signed-off voltage */
> > + for (i = 0; i < svsb->opp_count; i++) {
> > + if (svsb->phase == SVS_PHASE_MON) {
> > + svsb_volt = max((svsb->volts[i] + svsb->volt_offset +
> > + low_temp_offset), svsb->vmin);
> > + opp_volt = svs_volt_to_opp_volt(svsb_volt,
> > + svsb->volt_step,
> > + svsb->volt_base);
> > + } else if (svsb->phase == SVS_PHASE_INIT02) {
> > + svsb_volt = max((svsb->init02_volts[i] +
> > + svsb->volt_offset), svsb->vmin);
> > + opp_volt = svs_volt_to_opp_volt(svsb_volt,
> > + svsb->volt_step,
> > + svsb->volt_base);
> > + } else if (svsb->phase == SVS_PHASE_ERROR) {
> > + opp_volt = svsb->opp_volts[i];
> > + } else {
> > + pr_err("%s: unknown phase: %u?\n",
> > + svsb->name, svsb->phase);
> > + mutex_unlock(&svsb->lock);
> > + return -EINVAL;
> > + }
> > +
> > + opp_volt = min(opp_volt, svsb->opp_volts[i]);
> > + ret = dev_pm_opp_adjust_voltage(svsb->dev, svsb->opp_freqs[i],
> > + opp_volt);
>
> The version of this function in opp tree
> (https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=25cb20a212a1f989385dfe23230817e69c62bee5)
> has a different function signature, so this should be changed too.
Sure. I'll update it in the next patch.
>
> > + if (ret) {
> > + pr_err("%s: set voltage failed: %d\n", svsb->name, ret);
> > + mutex_unlock(&svsb->lock);
> > + return ret;
> > + }
> > + }
> > +
> > + mutex_unlock(&svsb->lock);
> > +
> > + return 0;
> > +}
> > +
> > [...]
> > +static int svs_init01(struct mtk_svs *svs)
> > +{
> > + const struct svs_platform *svsp = svs->platform;
> > + struct svs_bank *svsb;
> > + struct pm_qos_request qos_request = { {0} };
> > + unsigned long flags, time_left;
> > + bool search_done;
> > + int ret = -EINVAL;
> > + u32 opp_freqs, opp_vboot, buck_volt, idx, i;
> > +
> > + /* Let CPUs leave idle-off state for initializing svs_init01. */
> > + pm_qos_add_request(&qos_request, PM_QOS_CPU_DMA_LATENCY, 0);
> > +
> > + /* Sometimes two svs_bank use the same buck.
> > + * Therefore, we set each svs_bank to vboot voltage first.
> > + */
> > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > + svsb = &svsp->banks[idx];
> > + search_done = false;
> > +
> > + if (!svsb->init01_support)
> > + continue;
> > +
> > + ret = regulator_set_mode(svsb->buck, REGULATOR_MODE_FAST);
> > + if (ret)
> > + pr_notice("%s: fail to set fast mode: %d\n",
> > + svsb->name, ret);
> > +
> > + if (svsb->mtcmos_request) {
> > + ret = regulator_enable(svsb->buck);
> > + if (ret) {
> > + pr_err("%s: fail to enable %s power: %d\n",
> > + svsb->name, svsb->buck_name, ret);
> > + goto init01_finish;
> > + }
> > +
> > + ret = dev_pm_domain_attach(svsb->dev, false);
> > + if (ret) {
> > + pr_err("%s: attach pm domain fail: %d\n",
> > + svsb->name, ret);
> > + goto init01_finish;
> > + }
> > +
> > + pm_runtime_enable(svsb->dev);
> > + ret = pm_runtime_get_sync(svsb->dev);
> > + if (ret < 0) {
> > + pr_err("%s: turn mtcmos on fail: %d\n",
> > + svsb->name, ret);
> > + goto init01_finish;
> > + }
> > + }
> > +
> > + /* Find the fastest freq that can be run at vboot and
> > + * fix to that freq until svs_init01 is done.
> > + */
> > + opp_vboot = svs_volt_to_opp_volt(svsb->vboot,
> > + svsb->volt_step,
> > + svsb->volt_base);
> > +
> > + for (i = 0; i < svsb->opp_count; i++) {
> > + opp_freqs = svsb->opp_freqs[i];
> > + if (!search_done && svsb->opp_volts[i] <= opp_vboot) {
> > + ret = dev_pm_opp_adjust_voltage(svsb->dev,
> > + opp_freqs,
> > + opp_vboot);
>
> Same here.
Sure. I'll update it in the next patch.
>
> > + if (ret) {
> > + pr_err("%s: set voltage failed: %d\n",
> > + svsb->name, ret);
> > + goto init01_finish;
> > + }
> > +
> > + search_done = true;
> > + } else {
> > + dev_pm_opp_disable(svsb->dev,
> > + svsb->opp_freqs[i]);
> > + }
> > + }
> > + }
> > +
> > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > + svsb = &svsp->banks[idx];
> > + svs->bank = svsb;
> > +
> > + if (!svsb->init01_support)
> > + continue;
> > +
> > + opp_vboot = svs_volt_to_opp_volt(svsb->vboot,
> > + svsb->volt_step,
> > + svsb->volt_base);
> > +
> > + buck_volt = regulator_get_voltage(svsb->buck);
> > + if (buck_volt != opp_vboot) {
> > + pr_err("%s: buck voltage: %u, expected vboot: %u\n",
> > + svsb->name, buck_volt, opp_vboot);
> > + ret = -EPERM;
> > + goto init01_finish;
> > + }
> > +
> > + init_completion(&svsb->init_completion);
> > + flags = claim_mtk_svs_lock();
> > + svs_set_phase(svs, SVS_PHASE_INIT01);
> > + release_mtk_svs_lock(flags);
> > + time_left =
> > + wait_for_completion_timeout(&svsb->init_completion,
> > + msecs_to_jiffies(2000));
> > + if (time_left == 0) {
> > + pr_err("%s: init01 completion timeout\n", svsb->name);
> > + ret = -EBUSY;
> > + goto init01_finish;
> > + }
> > + }
> > +
> > +init01_finish:
> > + for (idx = 0; idx < svsp->bank_num; idx++) {
> > + svsb = &svsp->banks[idx];
> > +
> > + if (!svsb->init01_support)
> > + continue;
> > +
> > + for (i = 0; i < svsb->opp_count; i++)
> > + dev_pm_opp_enable(svsb->dev, svsb->opp_freqs[i]);
> > +
> > + if (regulator_set_mode(svsb->buck, REGULATOR_MODE_NORMAL))
> > + pr_notice("%s: fail to set normal mode: %d\n",
> > + svsb->name, ret);
> > +
> > + if (svsb->mtcmos_request) {
> > + if (pm_runtime_put_sync(svsb->dev))
> > + pr_err("%s: turn mtcmos off fail: %d\n",
> > + svsb->name, ret);
> > + pm_runtime_disable(svsb->dev);
> > + dev_pm_domain_detach(svsb->dev, 0);
> > + if (regulator_disable(svsb->buck))
> > + pr_err("%s: fail to disable %s power: %d\n",
> > + svsb->name, svsb->buck_name, ret);
> > + }
> > + }
> > +
> > + pm_qos_remove_request(&qos_request);
> > +
> > + return ret;
> > +}
> > +
> > [...]
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2019-12-27 7:15 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-06 10:05 [PATCH v5 0/3] PM / AVS: SVS: Introduce SVS engine Roger Lu
2019-09-06 10:05 ` [PATCH v5 1/3] dt-bindings: soc: add mtk svs dt-bindings Roger Lu
2019-09-30 13:35 ` Rob Herring
2019-12-27 6:50 ` Roger Lu
2020-01-13 7:02 ` Nicolas Boichat
2019-09-06 10:05 ` [PATCH v5 2/3] arm64: dts: mt8183: add svs device information Roger Lu
2019-09-06 10:05 ` [PATCH v5 3/3] PM / AVS: SVS: Introduce SVS engine Roger Lu
2019-09-26 22:39 ` Kevin Hilman
2020-01-03 5:44 ` Roger Lu
2019-10-21 7:51 ` Pi-Hsun Shih
2019-11-01 6:10 ` Roger Lu
2019-11-14 7:41 ` Pi-Hsun Shih
2019-12-27 7:14 ` Roger Lu [this message]
2019-09-06 10:15 ` [PATCH v5 0/3] " Roger Lu
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=1577430889.10290.49.camel@mtksdaap41 \
--to=roger.lu@mediatek.com \
--cc=Angus.Lin@mediatek.com \
--cc=HenryC.Chen@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=drinkcat@google.com \
--cc=fan.chen@mediatek.com \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=matthias.bgg@gmail.com \
--cc=nm@ti.com \
--cc=pihsun@chromium.org \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=yt.lee@mediatek.com \
/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).