Re: [PATCH V6 5/5] arm64: dts: qcom: x1e80100: Enable cpufreq

[Nikunj Kela]

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@xxxxxxxxxxx>
>>> ---
>>>   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.

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.
>
> -Sibi
>
>>
>> Johan
>>