[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <de875511-7356-32cd-514a-ac532d75a8fd@kernel.org>
Date: Thu, 13 Jul 2023 20:37:57 +0300
From: Georgi Djakov <djakov@...nel.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Leo Yan <leo.yan@...aro.org>,
Evan Green <evgreen@...omium.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-clk@...r.kernel.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Stephan Gerhold <stephan@...hold.net>
Subject: Re: [PATCH v7 00/22] Restructure RPM SMD ICC
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@...aro.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@...nel.org>
Powered by blists - more mailing lists