| 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
| ||
|
Message-ID: <62901afd-0944-4ecb-8f3c-90904410a0f6@cherry.de>
Date: Mon, 17 Feb 2025 17:24:32 +0100
From: Quentin Schulz <quentin.schulz@...rry.de>
To: Alexey Charkov <alchark@...il.com>, Heiko Stübner
<heiko@...ech.de>
Cc: Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Dragan Simic
<dsimic@...jaro.org>, Viresh Kumar <viresh.kumar@...aro.org>,
Chen-Yu Tsai <wens@...nel.org>, Diederik de Haas <didi.debian@...ow.org>,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
Kever Yang <kever.yang@...k-chips.com>
Subject: Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores
on RK3588j
Hi all,
On 2/16/25 1:32 PM, Alexey Charkov wrote:
> Hi Heiko,
>
> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@...ech.de> wrote:
>>
>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@...rry.de> wrote:
>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
>>>>> RK3588j is the 'industrial' variant of RK3588, and it uses a different
>>>>> set of OPPs both in terms of allowed frequencies and in terms of
>>>>> applicable voltages at each frequency setpoint.
>>>>>
>>>>> Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
>>>>> enable dynamic CPU frequency scaling.
>>>>>
>>>>> OPP values are derived from Rockchip downstream sources [1] by taking
>>>>> only those OPPs which have the highest frequency for a given voltage
>>>>> level and dropping the rest (if they are included, the kernel complains
>>>>> at boot time about them being inefficient)
>>>>>
>>>>> [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>>>>>
>>>>
>>>> Funny stuff actually. Rockchip have their own downstream cpufreq driver
>>>> which autodetects at runtime the SoC variant and filter out OPPs based
>>>> on that info. And this patch is based on those values and filters.
>>>>
>>>> However, they also decided that maybe this isn't the best way to do it
>>>> and they decided to have an rk3588j.dtsi where they remove some of those
>>>> OPPs instead of just updating the mask/filter in their base dtsi
>>>> (rk3588s.dtsi downstream). See
>>>> https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>
>>> Funny stuff indeed! Judging by the comments in the file you
>>> referenced, those higher OPP values constitute an 'overdrive' mode,
>>> and apparently the chip shouldn't stay in those for prolonged periods
>>> of time. However, I couldn't locate any dts file inside Rockchip
>>> sources that would include this rk3588j.dtsi - so not sure if we
>>> should follow what it says too zealously.
>>>
That was a clear oversight on my side.
The commit adding support for the J/M OPPs in rk3588s.dtsi downstream
just precedes the one adding the removal of the J OPPs in rk3588j.dtsi,
so not sure what was the intended effect there. I'll open a ticket with
them to see if I can get some answer until/unless Kever chimes in.
>>>> So...
>>>>
>>>>> Signed-off-by: Alexey Charkov <alchark@...il.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
>>>>> 1 file changed, 108 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> index 0bbeee399a63..b7e69553857b 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> @@ -5,3 +5,111 @@
>>>>> */
>>>>>
>>>>> #include "rk3588-extra.dtsi"
>>>>> +
>>>>> +/ {
>>>>> + cluster0_opp_table: opp-table-cluster0 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + opp-suspend;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <887500 887500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1704000000 {
>>>>> + opp-hz = /bits/ 64 <1704000000>;
>>>>> + opp-microvolt = <937500 937500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> None of those are valid according to Rockchip, we should have
>>>
>>> Well, valid but more taxing on the hardware apparently.
>>>
>>>> opp-1296000000 {
>>>> opp-hz = /bits/ 64 <1296000000>;
>>>> opp-microvolt = <750000 750000 950000>;
>>>> clock-latency-ns = <40000>;
>>>> opp-suspend;
>>>> };
>>>>
>>>> instead?
>>>
>>> I dropped this one because it uses a lower frequency but same voltage
>>> as the 1.416 GHz one, thus being 'inefficient' as the thermal
>>> subsystem says in the logs. It can be added back if we decide to
>>> remove opp-1416000000.
That was exactly my point, the 1.296GHz OPP would then be the highest
frequency at that voltage.
>>>
>>>> and...
>>>>
>>>>> + };
>>>>> +
>>>>> + cluster1_opp_table: opp-table-cluster1 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1800000000 {
>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-2016000000 {
>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> opp-1800000000 and opp-2016000000 should be removed.
>>>>
>>>> and...
>>>>
>>>>> + };
>>>>> +
>>>>> + cluster2_opp_table: opp-table-cluster2 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1800000000 {
>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-2016000000 {
>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>>>>
>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
>>>> for awareness on Rockchip side :)
>>>>
>>>> I guess another option could be to mark those above as
>>>>
>>>> turbo-mode;
>>>>
>>>> though no clue what this would entail. From a glance at cpufreq, it
>>>> seems that for Rockchip since we use the default cpufreq-dt, it would
>>>> mark those as unusable, so essentially a no-op, so better remove them
>>>> entirely?
>>>
>>> I believe the opp core just marks 'turbo-mode' opps as
>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
>>> cpufreq core. But then to actually use those boost frequencies I would
>>> guess the governor needs to somehow know the power/thermal constraints
>>> of the chip?.. Don't know where that is defined.
>>
>> personally I don't believe in "boost" frequencies - except when they are
>> declared officially.
>>
>> Rockchip for the longest time maintains that running the chip outside
>> the declared power/rate envelope can reduce its overall lifetime.
>>
>> So for Rockchip in mainline a "it runs stable for me" has never been a
>> suitable reasoning ;-) .
>
> My key concern here was perhaps that we are looking at a file inside
> the Rockchip source tree which looks relevant by the name of it, but
> doesn't actually get included anywhere for any of the boards. This may
> or may not constitute an endorsement by Rockchip of any OPPs listed
> there :-D
>
Will ask for confirmation through their ticket system.
> I haven't seen a TRM for the J variant, nor do I have a board with
> RK3588J to run tests, so it would be great if Kever could chime in
> with how these OPPs are different from the others (or not).
>
I do have access to some J variant product but that won't help if you
need to run the boards for months/years at the highest freq in a
temperature-controlled chamber at the highest supported temperature for
the SoC. I don't think there's a TRM for the J variant, that wouldn't
make sense as it's the exact same SoC, just a different binning.
Similarly to what was done for RK3588S, RK3588S2, RK3582, RK3583, etc..
I don't think there's a TRM for those, but there's a datasheet, c.f.
https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
but that says 1.6GHz for the A76 cores and 1.3GHz for the A55 while the
the j-m OPP in rk3558s.dtsi downstream list the highest as 1.7GHz (but
rk3588j.dtsi removes it).
Will report if Rockchip has answered on their ticket system.
Cheers,
Quentin
Powered by blists - more mailing lists