[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65ce72e1-7106-0872-88f7-bfdfcef7a24b@quicinc.com>
Date: Wed, 3 Jul 2024 16:53:36 +0530
From: Sibi Sankar <quic_sibis@...cinc.com>
To: Nikunj Kela <quic_nkela@...cinc.com>, Johan Hovold <johan@...nel.org>
CC: <sudeep.holla@....com>, <cristian.marussi@....com>, <andersson@...nel.org>,
<konrad.dybcio@...aro.org>, <jassisinghbrar@...il.com>,
<robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<dmitry.baryshkov@...aro.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<quic_rgottimu@...cinc.com>, <quic_kshivnan@...cinc.com>,
<conor+dt@...nel.org>, <quic_psodagud@...cinc.com>,
<abel.vesa@...aro.org>
Subject: Re: [PATCH V6 5/5] arm64: dts: qcom: x1e80100: Enable cpufreq
On 7/3/24 01:43, Nikunj Kela wrote:
>
> On 7/2/2024 12:59 PM, Sibi Sankar wrote:
>>
>>
>> On 7/2/24 21:25, Johan Hovold wrote:
>>> On Wed, Jun 12, 2024 at 06:10:56PM +0530, Sibi Sankar wrote:
>>>> Enable cpufreq on X1E80100 SoCs through the SCMI perf protocol node.
>>>>
>>>> Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 63
>>>> ++++++++++++++++----------
>>>> 1 file changed, 39 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>> b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>> index 7b619db07694..d134dc4c7425 100644
>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>>> @@ -69,8 +69,8 @@ CPU0: cpu@0 {
>>>> reg = <0x0 0x0>;
>>>> enable-method = "psci";
>>>> next-level-cache = <&L2_0>;
>>>> - power-domains = <&CPU_PD0>;
>>>> - power-domain-names = "psci";
>>>> + power-domains = <&CPU_PD0>, <&scmi_dvfs 0>;
>>>> + power-domain-names = "psci", "perf";
>>>> cpu-idle-states = <&CLUSTER_C4>;
>>>
>>>> + scmi {
>>>> + compatible = "arm,scmi";
>>>> + mboxes = <&cpucp_mbox 0>, <&cpucp_mbox 2>;
>>>> + mbox-names = "tx", "rx";
>>>> + shmem = <&cpu_scp_lpri0>, <&cpu_scp_lpri1>;
>>>> +
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> +
>>>> + scmi_dvfs: protocol@13 {
>>>> + reg = <0x13>;
>>>> + #power-domain-cells = <1>;
>>>> + };
>>>> + };
>>>> };
>>>
>>
>> Hey Johan,
>>
>> Thanks for trying out the series.
>>
>>> This series gives a nice performance boost on the x1e80100 CRD, but I'm
>>> seeing a bunch of warnings and errors that need to be addressed:
>>>
>>> [ 9.533053] arm-scmi firmware:scmi: Failed to get FC for protocol
>>> 13 [MSG_ID:6 / RES_ID:0] - ret:-95. Using regular messaging.
>>> [ 9.549458] arm-scmi firmware:scmi: Failed to add opps_by_lvl at
>>> 3417600 for NCC - ret:-16
>>> [ 9.563925] arm-scmi firmware:scmi: Failed to add opps_by_lvl at
>>> 3417600 for NCC - ret:-16
>>> [ 9.572835] arm-scmi firmware:scmi: Failed to get FC for protocol
>>> 13 [MSG_ID:6 / RES_ID:1] - ret:-95. Using regular messaging.
>>> [ 9.609471] arm-scmi firmware:scmi: Failed to add opps_by_lvl at
>>> 3417600 for NCC - ret:-16
>>> [ 9.633341] arm-scmi firmware:scmi: Failed to add opps_by_lvl at
>>> 3417600 for NCC - ret:-16
>>> [ 9.650000] arm-scmi firmware:scmi: Failed to get FC for protocol
>>> 13 [MSG_ID:6 / RES_ID:2] - ret:-95. Using regular messaging.
>>
>> X1E uses fast channels only for message-id: 7 (level set) and regular
>> channels for all the other messages. The spec doesn't mandate fast
>> channels for any of the supported message ids for the perf protocol.
>> So nothing to fix here.
>>
>>> [ 9.727098] cpu cpu4: _opp_is_duplicate: duplicate OPPs detected.
>>> Existing: freq: 3417600000, volt: 0, enabled: 1. New: freq:
>>> 3417600000, volt: 0, enabled: 1
>>> [ 9.737157] cpu cpu4: _opp_is_duplicate: duplicate OPPs detected.
>>> Existing: freq: 3417600000, volt: 0, enabled: 1. New: freq:
>>> 3417600000, volt: 0, enabled: 1
>>> [ 9.875039] cpu cpu8: _opp_is_duplicate: duplicate OPPs detected.
>>> Existing: freq: 3417600000, volt: 0, enabled: 1. New: freq:
>>> 3417600000, volt: 0, enabled: 1
>>> [ 9.888428] cpu cpu8: _opp_is_duplicate: duplicate OPPs detected.
>>> Existing: freq: 3417600000, volt: 0, enabled: 1. New: freq:
>>> 3417600000, volt: 0, enabled: 1
>>
>> The duplicate entries reported by the perf protocol come directly from
>> the speed bins. I was told the duplicate entry with volt 0 is meant to
>> indicate a lower power way of achieving the said frequency at a lower
>> core count. We have no way of using it in the kernel and it gets safely
>> discarded. So again nothing to fix in the kernel.
>
> Hi Sibi,
>
> Can you try increasing the max_msg_size to 256 bytes in mailbox
> transport? We saw the same issue but got resolved by increasing the
> max_msg_size for the transport(obviously, I reduced the max_msg to 10 to
> keep the total shmem size same). Even the opps_by_lvl warning went away
> with this for us.
Nikunj,
Thanks for taking time to review the series :)
Not sure if we are talking about the same things here, are you
suggesting that tweaking with the max_msg size will stop the SCMI
controller from reporting duplicate OPPs? Even if it does go away
magically wouldn't it mean you are dropping messages? Also opps_by_lvl
failing with -16 and duplicate opps detected in the opp core have the
same root cause i.e. duplicate entries reported by the controller.
>
> Thanks,
>
> -Nikunj
>
>>
>>> [ 9.913506] debugfs: Directory 'NCC' with parent 'pm_genpd'
>>> already present!
>>> [ 9.922198] debugfs: Directory 'NCC' with parent 'pm_genpd'
>>> already present!
>>
>> Yeah I did notice ^^ during dev, the series isn't the one introducing it
>> so it shouldn't block the series acceptance. Meanwhile I'll spend some
>> cycles to get this warn fixed.
Johan,
https://lore.kernel.org/lkml/20240703110741.2668800-1-quic_sibis@quicinc.com/
Posted a fix for the warn ^^
>>
>> -Sibi
>>
>>>
>>> Johan
>>>
Powered by blists - more mailing lists