| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 24 May 2023 08:43:31 +0000
From: Jia-wei Chang (張佳偉)
<Jia-wei.Chang@...iatek.com>
To: "daniel@...rotopia.org" <daniel@...rotopia.org>,
"angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>
CC: "linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"vincent@...temli.org" <vincent@...temli.org>,
"hsinyi@...gle.com" <hsinyi@...gle.com>,
"viresh.kumar@...aro.org" <viresh.kumar@...aro.org>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"khilman@...libre.com" <khilman@...libre.com>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"rafael@...nel.org" <rafael@...nel.org>,
Rex-BC Chen (陳柏辰)
<Rex-BC.Chen@...iatek.com>, "error27@...il.com" <error27@...il.com>
Subject: Re: [PATCH v2 4/4] cpufreq: mediatek: Raise proc and sram max voltage
for MT7622/7623
On Wed, 2023-05-24 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 23/05/23 19:37, Daniel Golle ha scritto:
> > On Tue, May 23, 2023 at 04:56:47PM +0200, AngeloGioacchino Del
> > Regno wrote:
> > > Il 22/05/23 20:03, Daniel Golle ha scritto:
> > > > Hi Jia-Wei,
> > > > Hi AngeloGioacchino,
> > > >
> > > > On Fri, Mar 24, 2023 at 06:11:30PM +0800, jia-wei.chang wrote:
> > > > > From: AngeloGioacchino Del Regno <
> > > > > angelogioacchino.delregno@...labora.com>
> > > > >
> > > > > During the addition of SRAM voltage tracking for CCI scaling,
> > > > > this
> > > > > driver got some voltage limits set for the vtrack algorithm:
> > > > > these
> > > > > were moved to platform data first, then enforced in a later
> > > > > commit
> > > > > 6a17b3876bc8 ("cpufreq: mediatek: Refine
> > > > > mtk_cpufreq_voltage_tracking()")
> > > > > using these as max values for the regulator_set_voltage()
> > > > > calls.
> > > > >
> > > > > In this case, the vsram/vproc constraints for MT7622 and
> > > > > MT7623
> > > > > were supposed to be the same as MT2701 (and a number of other
> > > > > SoCs),
> > > > > but that turned out to be a mistake because the
> > > > > aforementioned two
> > > > > SoCs' maximum voltage for both VPROC and VPROC_SRAM is 1.36V.
> > > > >
> > > > > Fix that by adding new platform data for MT7622/7623
> > > > > declaring the
> > > > > right {proc,sram}_max_volt parameter.
> > > > >
> > > > > Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits
> > > > > to platform data")
> > > > > Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine
> > > > > mtk_cpufreq_voltage_tracking()")
> > > > > Signed-off-by: AngeloGioacchino Del Regno <
> > > > > angelogioacchino.delregno@...labora.com>
> > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@...iatek.com>
> > > > > ---
> > > > > drivers/cpufreq/mediatek-cpufreq.c | 13 +++++++++++--
> > > > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c
> > > > > b/drivers/cpufreq/mediatek-cpufreq.c
> > > > > index 764e4fbdd536..9a39a7ccfae9 100644
> > > > > --- a/drivers/cpufreq/mediatek-cpufreq.c
> > > > > +++ b/drivers/cpufreq/mediatek-cpufreq.c
> > > > > @@ -693,6 +693,15 @@ static const struct
> > > > > mtk_cpufreq_platform_data mt2701_platform_data = {
> > > > > .ccifreq_supported = false,
> > > > > };
> > > > > +static const struct mtk_cpufreq_platform_data
> > > > > mt7622_platform_data = {
> > > > > + .min_volt_shift = 100000,
> > > > > + .max_volt_shift = 200000,
> > > > > + .proc_max_volt = 1360000,
> > > > > + .sram_min_volt = 0,
> > > > > + .sram_max_volt = 1360000,
> > > >
> > > > This change breaks cpufreq (with ondemand scheduler) on my BPi
> > > > R64
> > > > board (having MT7622AV SoC with MT6380N PMIC).
> > > > ...
> > > > [ 2.540091] cpufreq: __target_index: Failed to change cpu
> > > > frequency: -22
> > > > [ 2.556985] cpu cpu0: cpu0: failed to scale up voltage!
> > > > ...
> > > > (repeating a lot, every time the highest operating point is
> > > > selected
> > > > by the cpufreq governor)
> > > >
> > > > The reason is that the MT6380N doesn't support 1360000uV on the
> > > > supply
> > > > outputs used for SRAM and processor.
> > > >
> > > > As for some reason cpufreq-mediatek tries to rise the SRAM
> > > > supply
> > > > voltage to the maximum for a short moment (probably a side-
> > > > effect of
> > > > the voltage tracking algorithm), this fails because the PMIC
> > > > only
> > > > supports up to 1350000uV. As the highest operating point is
> > > > anyway
> > > > using only 1310000uV the simple fix is setting 1350000uV as the
> > > > maximum
> > > > instead for both proc_max_volt and sram_max_volt.
> > > >
> > > > A similar situation applies also for BPi R2 (MT7623NI with
> > > > MT6323L
> > > > PMIC), here the maximum supported voltage of the PMIC which
> > > > also only
> > > > supports up to 1350000uV, and the SoC having its highest
> > > > operating
> > > > voltage defined at 1300000uV.
> > > >
> > > > If all agree with the simple fix I will post a patch for that.
> > > >
> > > > However, to me it feels fishy to begin with that the tracking
> > > > algorithm
> > > > tries to rise the voltage above the highest operating point
> > > > defined in
> > > > device tree, see here:
> > > >
> > > > 6a17b3876bc830 drivers/cpufreq/mediatek-cpufreq.c (Jia-Wei
> > > > Chang 2022-05-05 19:52:20 +0800 100) new_vsram
> > > > = clamp(new_vproc + soc_data->min_volt_shift,
> > > > 6a17b3876bc830 drivers/cpufreq/mediatek-cpufreq.c (Jia-Wei
> > > > Chang 2022-05-05 19:52:20 +0800
> > > > 101) soc_data->sram_min_volt, soc_data-
> > > > >sram_max_volt);
> > > >
> > > > However, I did not investigate in depth the purpose of this
> > > > initial rise and can impossibly test my modifications to the
> > > > tracking algorithm on all supported SoCs.
> > > >
> > >
> > > Thanks for actually reporting that, I don't think that there's
> > > any
> > > valid reason why the algorithm should set a voltage higher than
> > > the
> > > maximum votage specified in the fastest OPP.
> > >
> > > Anyway - the logic for the platform data of this driver is to
> > > declare
> > > the maximum voltage that SoC model X supports, regardless of the
> > > actual
> > > board-specific OPPs, so that part is right; to solve this issue,
> > > I guess
> > > that the only way is for this driver to parse the OPPs during
> > > .probe()
> > > and then always use in the algorithm
> > >
> > > vproc_max = max(proc_max_volt, opp_vproc_max);
> > > vsram_max = max(sram_max_volt, vsram_vreg_max);
Hi Daniel, Angelo Sir,
Thanks for the issue report and suggestions.
Is it possible to modify the value of proc_max_volt and sram_max_volt
to 1310000 in mt7622_platform_data as the highest voltage declared in
mt7622.dtsi and then give it a try?
Sorry, I need someone help to check this on mt7622 since I don't have
mt7622 platform..
Thanks.
> >
> > You probably meant to write
> > vproc_max = min(proc_max_volt, opp_vproc_max);
> > vsram_max = min(sram_max_volt, vsram_vreg_max);
> >
> > right?
> >
>
> Apparently, some of my braincells was apparently taking a break. :-)
>
> Yes, I was meaning min(), not max() :-)
>
> Cheers!
>
> > >
> > > Jia-Wei, can you please handle this?
> > >
> > > Thanks,
> > > Angelo
> > >
>
>
>
Powered by blists - more mailing lists