Date:   Wed, 9 Sep 2020 17:53:07 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Wilken Gottwalt <wilken.gottwalt@...lbox.org>
Cc:     linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>
Subject: Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks

On Wed, Sep 09, 2020 at 03:54:46PM +0200, Wilken Gottwalt wrote:
> On Wed, 9 Sep 2020 14:08:59 +0200
> Maxime Ripard <maxime@...no.tech> wrote:
> > Hi!
> > 
> > Thanks for contributing
> > 
> > The prefix isn't right though.
> > 
> > dt-bindings is used when you're modifying the binding itself, ie the
> > description of what the node is supposed to look like, not when you
> > actually use that node in a DT.
> > 
> > In that case, that would be ARM: dts: sunxi:
> > 
> > (we're on the ARM architecture, modifying dts's, for the sunxi platform)
> 
> Ah, I see, it was my first attempt to contribute and wasn't 100% sure, just
> took the line from similar patches on the LKML. Thanks for the correction.
> 
> > On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> > > Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> > > more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> > > these clocks are still in a safe range. Tested on a NanoPi Duo and
> > > OrangePi Zero.
> > 
> > How was this tested?
> 
> This is a longer story. It actually runs on hardware which is in production
> for about 2-3 years, some use H2+ with full voltage regulators and some are
> similar to the NanoPi DUO, where the voltage regulator can only switch
> between 1.1 and 1.3 volts. It runs in two ways: A fully dynamic setup where
> the ondemand scheduler is used and the second way where it is switched to
> fixed values (based on load and temperature) using the cpufrequtils. The
> devices running a 4.14.x kernel and are tested against 4.19.x kernels.
> These devices are routers running a custom tcp/ip stack and have a high I/O
> load. I also prepared devices based on a custom H3 design, which ran stable
> at 1.392 GHz (but had passive coolers attached). Do these explanations
> help?

To some extent, but not entirely. Depending on the governor / workload,
some OPPs might never be used at all.

> > cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
> > some errors that are fairly hard to spot and be quite easy to go
> > unnoticed (like caches corruptions).
> 
> Yeah, I noticed that in the beginning where I prepared the first kernels
> for these devices. But after switching to multiples of 48MHz and bigger
> steps these issues disappeared. I'm aware that this does not proof that
> these issues do not appear, but thougth I share the values which I
> consider stable.

The only really reliable test we've had so far is the one I pointed out,
so please run it on one board at least

> > The only reliable test we have is:
> > https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test
> > 
> > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@...lbox.org>
> > > ---
> > >  arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
> > >  1 file changed, 32 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > index 4e89701df91f..5517fcc02b7d 100644
> > > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > > @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
> > >  		compatible = "operating-points-v2";
> > >  		opp-shared;
> > >  
> > > -		opp-648000000 {
> > > -			opp-hz = /bits/ 64 <648000000>;
> > > +		opp-528000000 {
> > > +			opp-hz = /bits/ 64 <528000000>;
> > > +			opp-microvolt = <1020000 1020000 1300000>;
> > > +			clock-latency-ns = <244144>; /* 8 32k periods */
> > > +		};
> > > +
> > > +		opp-624000000 {
> > > +			opp-hz = /bits/ 64 <624000000>;
> > >  			opp-microvolt = <1040000 1040000 1300000>;
> > >  			clock-latency-ns = <244144>; /* 8 32k periods */
> > >  		};
> > >  
> > > +		opp-720000000 {
> > > +			opp-hz = /bits/ 64 <720000000>;
> > > +			opp-microvolt = <1060000 1060000 1300000>;
> > > +			clock-latency-ns = <244144>; /* 8 32k periods */
> > > +		};
> > > +
> > >  		opp-816000000 {
> > >  			opp-hz = /bits/ 64 <816000000>;
> > >  			opp-microvolt = <1100000 1100000 1300000>;
> > >  			clock-latency-ns = <244144>; /* 8 32k periods */
> > >  		};
> > >  
> > > +		opp-912000000 {
> > > +			opp-hz = /bits/ 64 <912000000>;
> > > +			opp-microvolt = <1140000 1140000 1300000>;
> > > +			clock-latency-ns = <244144>; /* 8 32k periods */
> > > +		};
> > > +
> > >  		opp-1008000000 {
> > >  			opp-hz = /bits/ 64 <1008000000>;
> > >  			opp-microvolt = <1200000 1200000 1300000>;
> > >  			clock-latency-ns = <244144>; /* 8 32k periods */
> > >  		};
> > > +
> > > +		opp-1104000000 {
> > > +			opp-hz = /bits/ 64 <1104000000>;
> > > +			opp-microvolt = <1240000 1240000 1300000>;
> > > +			clock-latency-ns = <244144>; /* 8 32k periods */
> > > +		};
> > > +
> > > +		opp-1200000000 {
> > > +			opp-hz = /bits/ 64 <1200000000>;
> > > +			opp-microvolt = <1300000 1300000 1300000>;
> > > +			clock-latency-ns = <244144>; /* 8 32k periods */
> > > +		};
> > >  	};
> > 
> > IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
> > situation where a board doesn't list the regulators attached to the CPU,
> > the kernel will happily use the 1104 and 1200 MHz frequencies, even
> > though it won't adjust the voltage accordingly, causing errors.
> 
> Yeah, I know that. The u-boot running on the mentioned devices also have
> these modifications, including a higher clocked DRAM (actually running at
> 624 MHz).
> 
> > The way we worked around that is to provide the extra OOPs in a separate
> > DTSI so that the boards with regulator support can opt-in.
> 
> Oh, did I overlooked something? I was working on 4.14 up to 4.19 kernels
> and may be a bit to eager to post the modifications.

It's not really my point :)

My point is that since it's in the DTSI, every board using an H2/H3 will
now get to use those OPPs, even if they don't have the regulator support
and the voltage set for the highest frequency, so you can end up on
those boards with the CPU running at 1.2GHz and the voltage associated
to 1GHz

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists