[v7,00/22] Restructure RPM SMD ICC

Message ID 20230526-topic-smd_icc-v7-0-09c78c175546@linaro.org
Headers
Series Restructure RPM SMD ICC |

Message

Konrad Dybcio June 19, 2023, 1:04 p.m. UTC
  This series reshuffles things around, moving the management of SMD RPM
bus clocks to the interconnect framework where they belong. This helps
us solve a couple of issues:

1. We can work towards unused clk cleanup of RPMCC without worrying
   about it killing some NoC bus, resulting in the SoC dying.
   Deasserting actually unused RPM clocks (among other things) will
   let us achieve "true SoC-wide power collapse states", also known as
   VDD_LOW and VDD_MIN.

2. We no longer have to keep tons of quirky bus clock ifs in the icc
   driver. You either have a RPM clock and call "rpm set rate" or you
   have a single non-RPM clock (like AHB_CLK_SRC) or you don't have any.

3. There's less overhead - instead of going through layers and layers of
   the CCF, ratesetting comes down to calling max() and sending a single
   RPM message. ICC is very very dynamic so that's a big plus.

The clocks still need to be vaguely described in the clk-smd-rpm driver,
as it gives them an initial kickoff, before actually telling RPM to
enable DVFS scaling.  After RPM receives that command, all clocks that
have not been assigned a rate are considered unused and are shut down
in hardware, leading to the same issue as described in point 1.

We can consider marking them __initconst in the future, but this series
is very fat even without that..

Apart from that, it squashes a couple of bugs that really need fixing..

--- MERGING STRATEGY ---
If Stephen and Georgi agree, it would be best to take all of this through
the qcom tree, as it touches on heavily intertwined components and
introduces compile-time dependencies between icc and clk drivers.

Tested on SM6375 (OOT), MSM8998 (OOT), MSM8996.

MSM8974 conversion to common code and modernization will be handled separately.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Changes in v7:
- Rebase on next-20230619
- Throw some consts at "Separate out interconnect bus clocks"
- Link to v6: https://lore.kernel.org/r/20230526-topic-smd_icc-v6-0-263283111e66@linaro.org

Changes in v6:
- Fix argument naming in "Add rpmcc handling skeleton code"
- Fix missing clk.h and reorder patch "Add missing headers in icc-rpm.h",
  drop Dmitry's rb
- Pick up tags
- Link to v5: https://lore.kernel.org/r/20230526-topic-smd_icc-v5-0-eeaa09d0082e@linaro.org

Changes in v5:
- Pass RPM context id to qcom_icc_rpm_set_bus_rate()
- Fix min_t call cutting off bits 32-63 in set() path
- Pick up tags
- Link to v4: https://lore.kernel.org/r/20230526-topic-smd_icc-v4-0-5ba82b6fbba2@linaro.org

Changes in v4:
- Only set clk rate on a context if necessary
- Mention qcom,icc.h is not the correct header in "Control bus rpmcc form icc"
- Fix the bindings (BIT vs 1<<)
- Fix one more wrong use of qcom,icc.h in "Fix bucket numer" and uninclude it
- Drop "Allow negative QoS offset" (will be handled separately)
- Export icc clocks descriptions to unbreak =m builds
- Pick up tags
- Link to v3: https://lore.kernel.org/r/20230526-topic-smd_icc-v3-0-5fb7d39b874f@linaro.org

Changes in v3:
- Use devm_clk_get_optional and only get() the clock once
- Drop unnecessary NULL-checks for qp->bus_clk
- Handle ARM32 CCF limitations, add an explicit comment about them
- Use Stephan's alternative rpmcc readiness check
- Fix one more wrong usage of QCOM_ICC_NUM_BUCKETS in icc-rpm.h
- Introduce new dt-bindings for icc rpm tags
- Mention the rpm tags situation in the commit message of
  "Fix bucket number"
- Pick up tags
- Link to v2: https://lore.kernel.org/r/20230526-topic-smd_icc-v2-0-e5934b07d813@linaro.org

Changes in v2:
- Sort entries properly in "Add missing headers in icc-rpm.h"
- Fix the check for no clocks on a given provider
- Replace "Divide clk rate by src node bus width" with a proper fix
- Add "Set correct bandwidth through RPM bw req"
- Split "Add QCOM_SMD_RPM_STATE_NUM" into 2 logical changes
- Move "Separate out interconnect bus clocks" a bit later in the series
- Link to v1: https://lore.kernel.org/r/20230526-topic-smd_icc-v1-0-1bf8e6663c4e@linaro.org

---
Konrad Dybcio (21):
      dt-bindings: interconnect: Add Qcom RPM ICC bindings
      soc: qcom: smd-rpm: Add QCOM_SMD_RPM_STATE_NUM
      soc: qcom: smd-rpm: Use tabs for defines
      clk: qcom: smd-rpm: Move some RPM resources to the common header
      interconnect: qcom: icc-rpm: Introduce keep_alive
      interconnect: qcom: Add missing headers in icc-rpm.h
      interconnect: qcom: Fold smd-rpm.h into icc-rpm.h
      interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code
      interconnect: qcom: Define RPM bus clocks
      interconnect: qcom: sdm660: Hook up RPM bus clk definitions
      interconnect: qcom: msm8996: Hook up RPM bus clk definitions
      interconnect: qcom: qcs404: Hook up RPM bus clk definitions
      interconnect: qcom: msm8939: Hook up RPM bus clk definitions
      interconnect: qcom: msm8916: Hook up RPM bus clk definitions
      interconnect: qcom: qcm2290: Hook up RPM bus clk definitions
      interconnect: qcom: icc-rpm: Control bus rpmcc from icc
      clk: qcom: smd-rpm: Separate out interconnect bus clocks
      interconnect: qcom: icc-rpm: Fix bucket number
      interconnect: qcom: icc-rpm: Set bandwidth on both contexts
      interconnect: qcom: icc-rpm: Set correct bandwidth through RPM bw req
      interconnect: qcom: icc-rpm: Fix bandwidth calculations

Stephan Gerhold (1):
      soc: qcom: smd-rpm: Move icc_smd_rpm registration to clk-smd-rpm

 drivers/clk/qcom/clk-smd-rpm.c                  | 314 +++++++++++-------------
 drivers/interconnect/qcom/Makefile              |   2 +-
 drivers/interconnect/qcom/icc-rpm-clocks.c      |  77 ++++++
 drivers/interconnect/qcom/icc-rpm.c             | 220 +++++++++--------
 drivers/interconnect/qcom/icc-rpm.h             |  56 ++++-
 drivers/interconnect/qcom/msm8916.c             |   5 +-
 drivers/interconnect/qcom/msm8939.c             |   6 +-
 drivers/interconnect/qcom/msm8974.c             |   2 +-
 drivers/interconnect/qcom/msm8996.c             |  10 +-
 drivers/interconnect/qcom/qcm2290.c             |   8 +-
 drivers/interconnect/qcom/qcs404.c              |   5 +-
 drivers/interconnect/qcom/sdm660.c              |   8 +-
 drivers/interconnect/qcom/smd-rpm.c             |  23 +-
 drivers/interconnect/qcom/smd-rpm.h             |  15 --
 drivers/soc/qcom/smd-rpm.c                      |  17 +-
 include/dt-bindings/interconnect/qcom,rpm-icc.h |  13 +
 include/linux/soc/qcom/smd-rpm.h                |  20 +-
 17 files changed, 456 insertions(+), 345 deletions(-)
---
base-commit: 47045630bc409ce6606d97b790895210dd1d517d
change-id: 20230526-topic-smd_icc-b8213948a5ed

Best regards,
  

Comments

Georgi Djakov July 13, 2023, 5:37 p.m. UTC | #1
On 19.06.23 16:04, Konrad Dybcio wrote:
> This series reshuffles things around, moving the management of SMD RPM
> bus clocks to the interconnect framework where they belong. This helps
> us solve a couple of issues:
> 
> 1. We can work towards unused clk cleanup of RPMCC without worrying
>     about it killing some NoC bus, resulting in the SoC dying.
>     Deasserting actually unused RPM clocks (among other things) will
>     let us achieve "true SoC-wide power collapse states", also known as
>     VDD_LOW and VDD_MIN.
> 
> 2. We no longer have to keep tons of quirky bus clock ifs in the icc
>     driver. You either have a RPM clock and call "rpm set rate" or you
>     have a single non-RPM clock (like AHB_CLK_SRC) or you don't have any.
> 
> 3. There's less overhead - instead of going through layers and layers of
>     the CCF, ratesetting comes down to calling max() and sending a single
>     RPM message. ICC is very very dynamic so that's a big plus.
> 
> The clocks still need to be vaguely described in the clk-smd-rpm driver,
> as it gives them an initial kickoff, before actually telling RPM to
> enable DVFS scaling.  After RPM receives that command, all clocks that
> have not been assigned a rate are considered unused and are shut down
> in hardware, leading to the same issue as described in point 1.
> 
> We can consider marking them __initconst in the future, but this series
> is very fat even without that..
> 
> Apart from that, it squashes a couple of bugs that really need fixing..
> 
> --- MERGING STRATEGY ---
> If Stephen and Georgi agree, it would be best to take all of this through
> the qcom tree, as it touches on heavily intertwined components and
> introduces compile-time dependencies between icc and clk drivers.
> 
> Tested on SM6375 (OOT), MSM8998 (OOT), MSM8996.
> 
> MSM8974 conversion to common code and modernization will be handled separately.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Changes in v7:
> - Rebase on next-20230619
> - Throw some consts at "Separate out interconnect bus clocks"
> - Link to v6: https://lore.kernel.org/r/20230526-topic-smd_icc-v6-0-263283111e66@linaro.org
> 
> Changes in v6:
> - Fix argument naming in "Add rpmcc handling skeleton code"
> - Fix missing clk.h and reorder patch "Add missing headers in icc-rpm.h",
>    drop Dmitry's rb
> - Pick up tags
> - Link to v5: https://lore.kernel.org/r/20230526-topic-smd_icc-v5-0-eeaa09d0082e@linaro.org
> 
> Changes in v5:
> - Pass RPM context id to qcom_icc_rpm_set_bus_rate()
> - Fix min_t call cutting off bits 32-63 in set() path
> - Pick up tags
> - Link to v4: https://lore.kernel.org/r/20230526-topic-smd_icc-v4-0-5ba82b6fbba2@linaro.org
> 
> Changes in v4:
> - Only set clk rate on a context if necessary
> - Mention qcom,icc.h is not the correct header in "Control bus rpmcc form icc"
> - Fix the bindings (BIT vs 1<<)
> - Fix one more wrong use of qcom,icc.h in "Fix bucket numer" and uninclude it
> - Drop "Allow negative QoS offset" (will be handled separately)
> - Export icc clocks descriptions to unbreak =m builds
> - Pick up tags
> - Link to v3: https://lore.kernel.org/r/20230526-topic-smd_icc-v3-0-5fb7d39b874f@linaro.org
> 
> Changes in v3:
> - Use devm_clk_get_optional and only get() the clock once
> - Drop unnecessary NULL-checks for qp->bus_clk
> - Handle ARM32 CCF limitations, add an explicit comment about them
> - Use Stephan's alternative rpmcc readiness check
> - Fix one more wrong usage of QCOM_ICC_NUM_BUCKETS in icc-rpm.h
> - Introduce new dt-bindings for icc rpm tags
> - Mention the rpm tags situation in the commit message of
>    "Fix bucket number"
> - Pick up tags
> - Link to v2: https://lore.kernel.org/r/20230526-topic-smd_icc-v2-0-e5934b07d813@linaro.org
> 
> Changes in v2:
> - Sort entries properly in "Add missing headers in icc-rpm.h"
> - Fix the check for no clocks on a given provider
> - Replace "Divide clk rate by src node bus width" with a proper fix
> - Add "Set correct bandwidth through RPM bw req"
> - Split "Add QCOM_SMD_RPM_STATE_NUM" into 2 logical changes
> - Move "Separate out interconnect bus clocks" a bit later in the series
> - Link to v1: https://lore.kernel.org/r/20230526-topic-smd_icc-v1-0-1bf8e6663c4e@linaro.org
> 
> ---
> Konrad Dybcio (21):
>        dt-bindings: interconnect: Add Qcom RPM ICC bindings
>        soc: qcom: smd-rpm: Add QCOM_SMD_RPM_STATE_NUM
>        soc: qcom: smd-rpm: Use tabs for defines
>        clk: qcom: smd-rpm: Move some RPM resources to the common header
>        interconnect: qcom: icc-rpm: Introduce keep_alive
>        interconnect: qcom: Add missing headers in icc-rpm.h
>        interconnect: qcom: Fold smd-rpm.h into icc-rpm.h
>        interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code
>        interconnect: qcom: Define RPM bus clocks
>        interconnect: qcom: sdm660: Hook up RPM bus clk definitions
>        interconnect: qcom: msm8996: Hook up RPM bus clk definitions
>        interconnect: qcom: qcs404: Hook up RPM bus clk definitions
>        interconnect: qcom: msm8939: Hook up RPM bus clk definitions
>        interconnect: qcom: msm8916: Hook up RPM bus clk definitions
>        interconnect: qcom: qcm2290: Hook up RPM bus clk definitions
>        interconnect: qcom: icc-rpm: Control bus rpmcc from icc
>        clk: qcom: smd-rpm: Separate out interconnect bus clocks
>        interconnect: qcom: icc-rpm: Fix bucket number
>        interconnect: qcom: icc-rpm: Set bandwidth on both contexts
>        interconnect: qcom: icc-rpm: Set correct bandwidth through RPM bw req
>        interconnect: qcom: icc-rpm: Fix bandwidth calculations
> 
> Stephan Gerhold (1):
>        soc: qcom: smd-rpm: Move icc_smd_rpm registration to clk-smd-rpm
> 
>   drivers/clk/qcom/clk-smd-rpm.c                  | 314 +++++++++++-------------
>   drivers/interconnect/qcom/Makefile              |   2 +-
>   drivers/interconnect/qcom/icc-rpm-clocks.c      |  77 ++++++
>   drivers/interconnect/qcom/icc-rpm.c             | 220 +++++++++--------
>   drivers/interconnect/qcom/icc-rpm.h             |  56 ++++-
>   drivers/interconnect/qcom/msm8916.c             |   5 +-
>   drivers/interconnect/qcom/msm8939.c             |   6 +-
>   drivers/interconnect/qcom/msm8974.c             |   2 +-
>   drivers/interconnect/qcom/msm8996.c             |  10 +-
>   drivers/interconnect/qcom/qcm2290.c             |   8 +-
>   drivers/interconnect/qcom/qcs404.c              |   5 +-
>   drivers/interconnect/qcom/sdm660.c              |   8 +-
>   drivers/interconnect/qcom/smd-rpm.c             |  23 +-
>   drivers/interconnect/qcom/smd-rpm.h             |  15 --
>   drivers/soc/qcom/smd-rpm.c                      |  17 +-
>   include/dt-bindings/interconnect/qcom,rpm-icc.h |  13 +
>   include/linux/soc/qcom/smd-rpm.h                |  20 +-
>   17 files changed, 456 insertions(+), 345 deletions(-)

Thank you, Konrad!

Acked-by: Georgi Djakov <djakov@kernel.org>
  
Bjorn Andersson July 15, 2023, 5:09 p.m. UTC | #2
On Thu, Jul 13, 2023 at 08:37:57PM +0300, Georgi Djakov wrote:
> On 19.06.23 16:04, Konrad Dybcio wrote:
> > This series reshuffles things around, moving the management of SMD RPM
> > bus clocks to the interconnect framework where they belong. This helps
> > us solve a couple of issues:
> > 
[..]
> 
> Thank you, Konrad!
> 
> Acked-by: Georgi Djakov <djakov@kernel.org>

Thank you, Georgi.

I've applied the patches to a branch based on v6.5-rc1, merged that into
the qcom-tree and pushed the branch.

Regards,
Bjorn

The following changes since commit 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5:

  Linux 6.5-rc1 (2023-07-09 13:53:13 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git 20230526-topic-smd_icc-v7-0-09c78c175546@linaro.org

for you to fetch changes up to 17fc623e0f728c35b82a0e79cd4f3b8aab495663:

  interconnect: qcom: icc-rpm: Fix bandwidth calculations (2023-07-15 09:54:45 -0700)

----------------------------------------------------------------
Konrad Dybcio (21):
      dt-bindings: interconnect: Add Qcom RPM ICC bindings
      soc: qcom: smd-rpm: Add QCOM_SMD_RPM_STATE_NUM
      soc: qcom: smd-rpm: Use tabs for defines
      clk: qcom: smd-rpm: Move some RPM resources to the common header
      interconnect: qcom: icc-rpm: Introduce keep_alive
      interconnect: qcom: Add missing headers in icc-rpm.h
      interconnect: qcom: Fold smd-rpm.h into icc-rpm.h
      interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code
      interconnect: qcom: Define RPM bus clocks
      interconnect: qcom: sdm660: Hook up RPM bus clk definitions
      interconnect: qcom: msm8996: Hook up RPM bus clk definitions
      interconnect: qcom: qcs404: Hook up RPM bus clk definitions
      interconnect: qcom: msm8939: Hook up RPM bus clk definitions
      interconnect: qcom: msm8916: Hook up RPM bus clk definitions
      interconnect: qcom: qcm2290: Hook up RPM bus clk definitions
      interconnect: qcom: icc-rpm: Control bus rpmcc from icc
      clk: qcom: smd-rpm: Separate out interconnect bus clocks
      interconnect: qcom: icc-rpm: Fix bucket number
      interconnect: qcom: icc-rpm: Set bandwidth on both contexts
      interconnect: qcom: icc-rpm: Set correct bandwidth through RPM bw req
      interconnect: qcom: icc-rpm: Fix bandwidth calculations

Stephan Gerhold (1):
      soc: qcom: smd-rpm: Move icc_smd_rpm registration to clk-smd-rpm

 drivers/clk/qcom/clk-smd-rpm.c                  | 314 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 drivers/interconnect/qcom/Makefile              |   2 +-
 drivers/interconnect/qcom/icc-rpm-clocks.c      |  77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/interconnect/qcom/icc-rpm.c             | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------
 drivers/interconnect/qcom/icc-rpm.h             |  56 +++++++++++++++++++++++++++++++++++++++++++++-----------
 drivers/interconnect/qcom/msm8916.c             |   5 +++--
 drivers/interconnect/qcom/msm8939.c             |   6 ++++--
 drivers/interconnect/qcom/msm8974.c             |   2 +-
 drivers/interconnect/qcom/msm8996.c             |  10 +++++++---
 drivers/interconnect/qcom/qcm2290.c             |   8 ++++++--
 drivers/interconnect/qcom/qcs404.c              |   5 +++--
 drivers/interconnect/qcom/sdm660.c              |   8 +++++---
 drivers/interconnect/qcom/smd-rpm.c             |  23 ++++++++++++++++++++++-
 drivers/interconnect/qcom/smd-rpm.h             |  15 ---------------
 drivers/soc/qcom/smd-rpm.c                      |  17 +----------------
 include/dt-bindings/interconnect/qcom,rpm-icc.h |  13 +++++++++++++
 include/linux/soc/qcom/smd-rpm.h                |  20 ++++++++++++++++++--
 17 files changed, 456 insertions(+), 345 deletions(-)
 create mode 100644 drivers/interconnect/qcom/icc-rpm-clocks.c
 delete mode 100644 drivers/interconnect/qcom/smd-rpm.h
 create mode 100644 include/dt-bindings/interconnect/qcom,rpm-icc.h
  
Bjorn Andersson July 15, 2023, 5:09 p.m. UTC | #3
On Mon, 19 Jun 2023 15:04:25 +0200, Konrad Dybcio wrote:
> This series reshuffles things around, moving the management of SMD RPM
> bus clocks to the interconnect framework where they belong. This helps
> us solve a couple of issues:
> 
> 1. We can work towards unused clk cleanup of RPMCC without worrying
>    about it killing some NoC bus, resulting in the SoC dying.
>    Deasserting actually unused RPM clocks (among other things) will
>    let us achieve "true SoC-wide power collapse states", also known as
>    VDD_LOW and VDD_MIN.
> 
> [...]

Applied, thanks!

[01/22] dt-bindings: interconnect: Add Qcom RPM ICC bindings
        commit: 7296bd3f0046f87e089499865345ab337598f5ab
[02/22] soc: qcom: smd-rpm: Add QCOM_SMD_RPM_STATE_NUM
        commit: cb0116090e4cff6da2e9abd1c29b8e16491af176
[03/22] soc: qcom: smd-rpm: Use tabs for defines
        commit: 82a793e2d3e3da748f23a0fbe0b4615292625fe8
[04/22] clk: qcom: smd-rpm: Move some RPM resources to the common header
        commit: e1e1267413d2e9fbe3a34c5a5f701b0f5fb0bf2c
[05/22] soc: qcom: smd-rpm: Move icc_smd_rpm registration to clk-smd-rpm
        commit: 410da7e344a69faf65af8a1e80d9963648a9e6a3
[06/22] interconnect: qcom: icc-rpm: Introduce keep_alive
        commit: b979049c38e170286158e97290c892957c836903
[07/22] interconnect: qcom: Add missing headers in icc-rpm.h
        commit: 40cdeed66933abc80a9b133f062770af616ce50a
[08/22] interconnect: qcom: Fold smd-rpm.h into icc-rpm.h
        commit: 19ced2aec6cdff7d227e012f7ac63d28dae07288
[09/22] interconnect: qcom: smd-rpm: Add rpmcc handling skeleton code
        commit: a927eb09d49c73f5250d61d5b7a0b5de09ab6a28
[10/22] interconnect: qcom: Define RPM bus clocks
        commit: 918b86042b48da5ea86d0a12495dff95241dedd3
[11/22] interconnect: qcom: sdm660: Hook up RPM bus clk definitions
        commit: 70ab8dee97d66560cacf0c58ac6adfcdcab43922
[12/22] interconnect: qcom: msm8996: Hook up RPM bus clk definitions
        commit: 9258801ef94f166b519c3f4364f72c7766b9e7e4
[13/22] interconnect: qcom: qcs404: Hook up RPM bus clk definitions
        commit: a923b8bbeb145ba192b3f1402de1582eab5adc50
[14/22] interconnect: qcom: msm8939: Hook up RPM bus clk definitions
        commit: 258a64f26679523dadb8fd60e9dd1538cab811f1
[15/22] interconnect: qcom: msm8916: Hook up RPM bus clk definitions
        commit: 6424985edee17772a83c8e8d1313f511e458f649
[16/22] interconnect: qcom: qcm2290: Hook up RPM bus clk definitions
        commit: 9d0a282c3e26c59c5a90d5fb4c167fe103f53bc8
[17/22] interconnect: qcom: icc-rpm: Control bus rpmcc from icc
        commit: 11f63efe58a052da3f4d179ba6fdccd128bc95ee
[18/22] clk: qcom: smd-rpm: Separate out interconnect bus clocks
        commit: d6edc31f3a68d8d0636e0cfcd9eced7460ad32f4
[19/22] interconnect: qcom: icc-rpm: Fix bucket number
        commit: 6ed0e5e60e444c135ce56169784695b5f5fcd1f9
[20/22] interconnect: qcom: icc-rpm: Set bandwidth on both contexts
        commit: 32846c4a8f2a80c0138aee07543e7c305d75abea
[21/22] interconnect: qcom: icc-rpm: Set correct bandwidth through RPM bw req
        commit: 3b7480106bf8ac2666d901e42bf9835959f79e7f
[22/22] interconnect: qcom: icc-rpm: Fix bandwidth calculations
        commit: 17fc623e0f728c35b82a0e79cd4f3b8aab495663

Best regards,