[RFT,v2,00/14] SMD RPMCC sleep preparations

Message ID 20230303-topic-rpmcc_sleep-v2-0-ae80a325fe94@linaro.org
Headers
Series SMD RPMCC sleep preparations |

Message

Konrad Dybcio March 8, 2023, 9:35 p.m. UTC
  v1 -> v2:
- Use CLK_IS_CRITICAL instead of leaving a clk enable vote, expand macros
  to do so
- Fix the keepalive clocks for 8998 & 660 (CNoC -> PNoC, it was
  confusingly named cnoc_periph downstream)
- Introduce .determinte_rate to ensure we don't set keepalive clocks'
  rates below 19.2 MHz
- Add a (!conditional!) way to test the ultimate goal of all these changes
  by essentially enabling unused clk cleanup through a dt property (for
  legacy reasons)

v2 was tested on:

- MSM8996 Sony Kagura (can disable unused)
- MSM8998 Sony Maple (can disable unused with OOT icc)
- SM6375 Sony PDX225 (can disable unused with OOT icc)

v1: https://lore.kernel.org/r/20230303-topic-rpmcc_sleep-v1-0-d9cfaf9b27a7@linaro.org

This series brings support for a couple of things necessary for the full
system idle on SMD RPM SoCs, namely unused clk shutdown and keepalive
votes (permanent active votes that are required on certain clocks for the
platform to function).

Tested on MSM8996 and SM6375, does not seem to introduce any additional
regressions.

Keepalive clocks for other platforms were gathered by digging in old
downstream kernels, please give them a test.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (11):
      dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup
      clk: qcom: smd-rpm_ Make __DEFINE_CLK_SMD_RPM_BRANCH_PREFIX accept flags
      clk: qcom: smd-rpm: Make DEFINE_CLK_SMD_RPM_BRANCH_A accept flags
      clk: qcom: smd-rpm: Make BI_TCXO_AO critical
      clk: qcom: smd-rpm: Make __DEFINE_CLK_SMD_RPM_PREFIX accept flags
      clk: qcom: smd-rpm: Separate out a macro for defining an AO clock
      clk: qcom: smd-rpm: Add support for keepalive votes
      clk: qcom: smd-rpm: Introduce DEFINE_CLK_SMD_RPM_BUS_KEEPALIVE
      clk: qcom: smd-rpm: Hook up PCNoC_0 keep_alive
      clk: qcom: smd-rpm: Hook up CNoC_1 and SNoC_2 keep_alive
      arm64: dts: qcom: msm8996: Enable rpmcc unused clk disablement

Shawn Guo (3):
      clk: qcom: smd-rpm: Add .is_enabled hook
      clk: qcom: smd-rpm: Add .is_prepared hook
      clk: qcom: smd-rpm: Mark clock enabled in clk_smd_rpm_handoff()

 .../devicetree/bindings/clock/qcom,rpmcc.yaml      |   6 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |   1 +
 drivers/clk/qcom/clk-smd-rpm.c                     | 133 +++++++++++++++------
 3 files changed, 106 insertions(+), 34 deletions(-)
---
base-commit: fc31900c948610e7b5c2f15fb7795832c8325327
change-id: 20230303-topic-rpmcc_sleep-d67aad9f3012

Best regards,
  

Comments

Konrad Dybcio April 20, 2023, 1:50 a.m. UTC | #1
On 8.03.2023 22:35, Konrad Dybcio wrote:
> v1 -> v2:
> - Use CLK_IS_CRITICAL instead of leaving a clk enable vote, expand macros
>   to do so
> - Fix the keepalive clocks for 8998 & 660 (CNoC -> PNoC, it was
>   confusingly named cnoc_periph downstream)
> - Introduce .determinte_rate to ensure we don't set keepalive clocks'
>   rates below 19.2 MHz
> - Add a (!conditional!) way to test the ultimate goal of all these changes
>   by essentially enabling unused clk cleanup through a dt property (for
>   legacy reasons)
> 
> v2 was tested on:
> 
> - MSM8996 Sony Kagura (can disable unused)
> - MSM8998 Sony Maple (can disable unused with OOT icc)
> - SM6375 Sony PDX225 (can disable unused with OOT icc)
> 
> v1: https://lore.kernel.org/r/20230303-topic-rpmcc_sleep-v1-0-d9cfaf9b27a7@linaro.org
> 
> This series brings support for a couple of things necessary for the full
> system idle on SMD RPM SoCs, namely unused clk shutdown and keepalive
> votes (permanent active votes that are required on certain clocks for the
> platform to function).
> 
> Tested on MSM8996 and SM6375, does not seem to introduce any additional
> regressions.
> 
> Keepalive clocks for other platforms were gathered by digging in old
> downstream kernels, please give them a test.
I have an implementation of rpmcc-within-icc ready(ish) locally. Turns out
some SoCs need a keepalive (19.2MHz, active-only) vote on clocks that
are NOT governed by interconnect.. So before we can disable clocks,
both will need to be implemented.. ugh... I was hoping we could avoid
having it in rpmcc..

Konrad
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Konrad Dybcio (11):
>       dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup
>       clk: qcom: smd-rpm_ Make __DEFINE_CLK_SMD_RPM_BRANCH_PREFIX accept flags
>       clk: qcom: smd-rpm: Make DEFINE_CLK_SMD_RPM_BRANCH_A accept flags
>       clk: qcom: smd-rpm: Make BI_TCXO_AO critical
>       clk: qcom: smd-rpm: Make __DEFINE_CLK_SMD_RPM_PREFIX accept flags
>       clk: qcom: smd-rpm: Separate out a macro for defining an AO clock
>       clk: qcom: smd-rpm: Add support for keepalive votes
>       clk: qcom: smd-rpm: Introduce DEFINE_CLK_SMD_RPM_BUS_KEEPALIVE
>       clk: qcom: smd-rpm: Hook up PCNoC_0 keep_alive
>       clk: qcom: smd-rpm: Hook up CNoC_1 and SNoC_2 keep_alive
>       arm64: dts: qcom: msm8996: Enable rpmcc unused clk disablement
> 
> Shawn Guo (3):
>       clk: qcom: smd-rpm: Add .is_enabled hook
>       clk: qcom: smd-rpm: Add .is_prepared hook
>       clk: qcom: smd-rpm: Mark clock enabled in clk_smd_rpm_handoff()
> 
>  .../devicetree/bindings/clock/qcom,rpmcc.yaml      |   6 +
>  arch/arm64/boot/dts/qcom/msm8996.dtsi              |   1 +
>  drivers/clk/qcom/clk-smd-rpm.c                     | 133 +++++++++++++++------
>  3 files changed, 106 insertions(+), 34 deletions(-)
> ---
> base-commit: fc31900c948610e7b5c2f15fb7795832c8325327
> change-id: 20230303-topic-rpmcc_sleep-d67aad9f3012
> 
> Best regards,
  
Stephan Gerhold April 20, 2023, 7:56 a.m. UTC | #2
On Thu, Apr 20, 2023 at 03:50:16AM +0200, Konrad Dybcio wrote:
> On 8.03.2023 22:35, Konrad Dybcio wrote:
> > Keepalive clocks for other platforms were gathered by digging in old
> > downstream kernels, please give them a test.
> I have an implementation of rpmcc-within-icc ready(ish) locally. Turns out
> some SoCs need a keepalive (19.2MHz, active-only) vote on clocks that
> are NOT governed by interconnect.. So before we can disable clocks,
> both will need to be implemented.. ugh... I was hoping we could avoid
> having it in rpmcc..
> 

Can you give an example? Which clocks are affected on which SoC?

Thanks,
Stephan
  
Konrad Dybcio April 20, 2023, 9:36 a.m. UTC | #3
On 20.04.2023 09:56, Stephan Gerhold wrote:
> On Thu, Apr 20, 2023 at 03:50:16AM +0200, Konrad Dybcio wrote:
>> On 8.03.2023 22:35, Konrad Dybcio wrote:
>>> Keepalive clocks for other platforms were gathered by digging in old
>>> downstream kernels, please give them a test.
>> I have an implementation of rpmcc-within-icc ready(ish) locally. Turns out
>> some SoCs need a keepalive (19.2MHz, active-only) vote on clocks that
>> are NOT governed by interconnect.. So before we can disable clocks,
>> both will need to be implemented.. ugh... I was hoping we could avoid
>> having it in rpmcc..
>>
> 
> Can you give an example? Which clocks are affected on which SoC?
msm8998/sdm660 and PNoC

Konrad
> 
> Thanks,
> Stephan
  
Stephan Gerhold April 20, 2023, 10:04 a.m. UTC | #4
On Thu, Apr 20, 2023 at 11:36:24AM +0200, Konrad Dybcio wrote:
> On 20.04.2023 09:56, Stephan Gerhold wrote:
> > On Thu, Apr 20, 2023 at 03:50:16AM +0200, Konrad Dybcio wrote:
> >> On 8.03.2023 22:35, Konrad Dybcio wrote:
> >>> Keepalive clocks for other platforms were gathered by digging in old
> >>> downstream kernels, please give them a test.
> >> I have an implementation of rpmcc-within-icc ready(ish) locally. Turns out
> >> some SoCs need a keepalive (19.2MHz, active-only) vote on clocks that
> >> are NOT governed by interconnect.. So before we can disable clocks,
> >> both will need to be implemented.. ugh... I was hoping we could avoid
> >> having it in rpmcc..
> > Can you give an example? Which clocks are affected on which SoC?
> msm8998/sdm660 and PNoC

I don't see a PNoC for 8998/660, do you mean the "cnoc_periph_clk"
downstream? Like the other NoCs it seems to be a RPM_BUS_CLK_TYPE, which
means it does fit best into interconnect in my opinion. From a quick
grep I don't see any usage of it in msm-4.4 downstream other than the
active-only keepalive vote. So maybe you could just send that vote once
in icc_rpm_smd and then ignore that clock (don't expose it at all)?

Thanks,
Stephan
  
Konrad Dybcio April 20, 2023, 10:20 a.m. UTC | #5
On 20.04.2023 12:04, Stephan Gerhold wrote:
> On Thu, Apr 20, 2023 at 11:36:24AM +0200, Konrad Dybcio wrote:
>> On 20.04.2023 09:56, Stephan Gerhold wrote:
>>> On Thu, Apr 20, 2023 at 03:50:16AM +0200, Konrad Dybcio wrote:
>>>> On 8.03.2023 22:35, Konrad Dybcio wrote:
>>>>> Keepalive clocks for other platforms were gathered by digging in old
>>>>> downstream kernels, please give them a test.
>>>> I have an implementation of rpmcc-within-icc ready(ish) locally. Turns out
>>>> some SoCs need a keepalive (19.2MHz, active-only) vote on clocks that
>>>> are NOT governed by interconnect.. So before we can disable clocks,
>>>> both will need to be implemented.. ugh... I was hoping we could avoid
>>>> having it in rpmcc..
>>> Can you give an example? Which clocks are affected on which SoC?
>> msm8998/sdm660 and PNoC
> 
> I don't see a PNoC for 8998/660, do you mean the "cnoc_periph_clk"
It's the same, but Qualcomm kept changing the name every kernel
release, so that's why we have 50 defines for the same thing
upstream :(


> downstream? Like the other NoCs it seems to be a RPM_BUS_CLK_TYPE, which
> means it does fit best into interconnect in my opinion. From a quick
> grep I don't see any usage of it in msm-4.4 downstream other than the
> active-only keepalive vote. So maybe you could just send that vote once
> in icc_rpm_smd and then ignore that clock (don't expose it at all)?
Hm, perhaps that does sound like a good idea! As far as I understand,
it's governed internally.. Older SoCs had a separate PNoC fabric
exposed.

Konrad

> 
> Thanks,
> Stephan
  
Konrad Dybcio April 20, 2023, 3:57 p.m. UTC | #6
On 8.03.2023 22:35, Konrad Dybcio wrote:
> v1 -> v2:
> - Use CLK_IS_CRITICAL instead of leaving a clk enable vote, expand macros
>   to do so
> - Fix the keepalive clocks for 8998 & 660 (CNoC -> PNoC, it was
>   confusingly named cnoc_periph downstream)
> - Introduce .determinte_rate to ensure we don't set keepalive clocks'
>   rates below 19.2 MHz
> - Add a (!conditional!) way to test the ultimate goal of all these changes
>   by essentially enabling unused clk cleanup through a dt property (for
>   legacy reasons)
> 
> v2 was tested on:
> 
> - MSM8996 Sony Kagura (can disable unused)
> - MSM8998 Sony Maple (can disable unused with OOT icc)
> - SM6375 Sony PDX225 (can disable unused with OOT icc)
> 
> v1: https://lore.kernel.org/r/20230303-topic-rpmcc_sleep-v1-0-d9cfaf9b27a7@linaro.org
> 
> This series brings support for a couple of things necessary for the full
> system idle on SMD RPM SoCs, namely unused clk shutdown and keepalive
> votes (permanent active votes that are required on certain clocks for the
> platform to function).
> 
> Tested on MSM8996 and SM6375, does not seem to introduce any additional
> regressions.
> 
> Keepalive clocks for other platforms were gathered by digging in old
> downstream kernels, please give them a test.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Konrad Dybcio (11):
>       dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup

>       clk: qcom: smd-rpm_ Make __DEFINE_CLK_SMD_RPM_BRANCH_PREFIX accept flags
>       clk: qcom: smd-rpm: Make DEFINE_CLK_SMD_RPM_BRANCH_A accept flags
>       clk: qcom: smd-rpm: Make BI_TCXO_AO critical
Stephen, parallel to all of the discussions, would you be willing to
take patches 4-6 as they are? XO_A being critical is something that
won't hurt without the rest.

Konrad

>       clk: qcom: smd-rpm: Make __DEFINE_CLK_SMD_RPM_PREFIX accept flags
>       clk: qcom: smd-rpm: Separate out a macro for defining an AO clock
>       clk: qcom: smd-rpm: Add support for keepalive votes
>       clk: qcom: smd-rpm: Introduce DEFINE_CLK_SMD_RPM_BUS_KEEPALIVE
>       clk: qcom: smd-rpm: Hook up PCNoC_0 keep_alive
>       clk: qcom: smd-rpm: Hook up CNoC_1 and SNoC_2 keep_alive
>       arm64: dts: qcom: msm8996: Enable rpmcc unused clk disablement
> 
> Shawn Guo (3):
>       clk: qcom: smd-rpm: Add .is_enabled hook
>       clk: qcom: smd-rpm: Add .is_prepared hook
>       clk: qcom: smd-rpm: Mark clock enabled in clk_smd_rpm_handoff()
> 
>  .../devicetree/bindings/clock/qcom,rpmcc.yaml      |   6 +
>  arch/arm64/boot/dts/qcom/msm8996.dtsi              |   1 +
>  drivers/clk/qcom/clk-smd-rpm.c                     | 133 +++++++++++++++------
>  3 files changed, 106 insertions(+), 34 deletions(-)
> ---
> base-commit: fc31900c948610e7b5c2f15fb7795832c8325327
> change-id: 20230303-topic-rpmcc_sleep-d67aad9f3012
> 
> Best regards,
  
Stephen Boyd April 25, 2023, 7:35 p.m. UTC | #7
Quoting Konrad Dybcio (2023-04-20 08:57:53)
> 
> 
> > Konrad Dybcio (11):
> >       dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup
> 
> >       clk: qcom: smd-rpm_ Make __DEFINE_CLK_SMD_RPM_BRANCH_PREFIX accept flags
> >       clk: qcom: smd-rpm: Make DEFINE_CLK_SMD_RPM_BRANCH_A accept flags
> >       clk: qcom: smd-rpm: Make BI_TCXO_AO critical
> Stephen, parallel to all of the discussions, would you be willing to
> take patches 4-6 as they are? XO_A being critical is something that
> won't hurt without the rest.

Sure, can you resend just those in a series?
  
Konrad Dybcio April 26, 2023, 9:40 a.m. UTC | #8
On 4/25/23 20:35, Stephen Boyd wrote:
> Quoting Konrad Dybcio (2023-04-20 08:57:53)
>>
>>> Konrad Dybcio (11):
>>>        dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup
>>>        clk: qcom: smd-rpm_ Make __DEFINE_CLK_SMD_RPM_BRANCH_PREFIX accept flags
>>>        clk: qcom: smd-rpm: Make DEFINE_CLK_SMD_RPM_BRANCH_A accept flags
>>>        clk: qcom: smd-rpm: Make BI_TCXO_AO critical
>> Stephen, parallel to all of the discussions, would you be willing to
>> take patches 4-6 as they are? XO_A being critical is something that
>> won't hurt without the rest.
> Sure, can you resend just those in a series?

Thanks, I'll do that after Connect!


Konrad