On 7/3/2024 4:23 AM, Sibi Sankar wrote:
>
>
> 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@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.
>
> 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.
Sibi,
My observation was that only 12 OPPs could fit it 128bytes msg_size and
our platform was sending 16 OPPs in one go. OPPs above 12 were getting
clobbered so the duplicate warning/error were not genuine. You may need
to tweak platform to send only 12(or less) OPPs in one go.
>
>>
>> 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@xxxxxxxxxxx/
>
>
> Posted a fix for the warn ^^
>
>>>
>>> -Sibi
>>>
>>>>
>>>> Johan
>>>>
