* [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears @ 2022-04-01 14:58 Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain Krzysztof Kozlowski ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-01 14:58 UTC (permalink / raw) To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi Cc: Krzysztof Kozlowski Hi, This is a proof-of-concept/RFC for changing the performance state of power rails when scaling gears. So far I added it as a parallel method to scaling clocks, thus freq-table-hz stays in DTS, however OPP table should be probably replace freq-table-hz entirely. Best regards, Krzysztof Krzysztof Kozlowski (4): dt-bindings: clock: qcom,gcc-sdm845: add parent power domain dt-bindings: ufs: common: allow OPP table arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS ufs: set power domain performance state when scaling gears .../bindings/clock/qcom,gcc-sdm845.yaml | 3 ++ .../devicetree/bindings/ufs/ufs-common.yaml | 4 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 17 +++++++- drivers/scsi/ufs/ufshcd-pltfrm.c | 6 +++ drivers/scsi/ufs/ufshcd.c | 42 +++++++++++++++---- drivers/scsi/ufs/ufshcd.h | 3 ++ 6 files changed, 65 insertions(+), 10 deletions(-) -- 2.32.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain 2022-04-01 14:58 [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski @ 2022-04-01 14:58 ` Krzysztof Kozlowski [not found] ` <20220401232451.1B7A9C340F3@smtp.kernel.org> 2022-04-01 14:58 ` [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table Krzysztof Kozlowski ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-01 14:58 UTC (permalink / raw) To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi Cc: Krzysztof Kozlowski Allow Qualcomm GCC to register its parent power domain (e.g. RPMHPD) to properly pass performance state from children. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml index d902f137ab17..5fe1b2c42d5a 100644 --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml @@ -43,6 +43,9 @@ properties: '#reset-cells': const: 1 + powert-domains: + maxItems: 1 + '#power-domain-cells': const: 1 -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <20220401232451.1B7A9C340F3@smtp.kernel.org>]
* Re: [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain [not found] ` <20220401232451.1B7A9C340F3@smtp.kernel.org> @ 2022-04-02 9:09 ` Krzysztof Kozlowski 0 siblings, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-02 9:09 UTC (permalink / raw) To: Stephen Boyd, Alim Akhtar, Andy Gross, Avri Altman, Bart Van Assche, Bean Huo, Bjorn Andersson, James E.J. Bottomley, Krzysztof Kozlowski, Martin K. Petersen, Michael Turquette, Rob Herring, Srinivas Kandagatla, Taniya Das, devicetree, linux-arm-msm, linux-clk, linux-kernel, linux-scsi On 02/04/2022 01:24, Stephen Boyd wrote: > Quoting Krzysztof Kozlowski (2022-04-01 07:58:17) >> Allow Qualcomm GCC to register its parent power domain (e.g. RPMHPD) to >> properly pass performance state from children. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml >> index d902f137ab17..5fe1b2c42d5a 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml >> @@ -43,6 +43,9 @@ properties: >> '#reset-cells': >> const: 1 >> >> + powert-domains: > > s/powert/power/ Thanks. This actually points to the fact I did not test this bindings change :( Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table 2022-04-01 14:58 [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain Krzysztof Kozlowski @ 2022-04-01 14:58 ` Krzysztof Kozlowski 2022-04-04 21:39 ` Rob Herring 2022-04-01 14:58 ` [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 3 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-01 14:58 UTC (permalink / raw) To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi Cc: Krzysztof Kozlowski Except scaling UFS and bus clocks, it's necessary to scale also the voltages of regulators or power domain performance state levels. Adding Operating Performance Points table allows to adjust power domain performance state, depending on the UFS clock speed. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Documentation/devicetree/bindings/ufs/ufs-common.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml index 47a4e9e1a775..ce767bfbf05a 100644 --- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml +++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml @@ -26,6 +26,9 @@ properties: array is "0" then it is assumed that the frequency is set by the parent clock or a fixed rate clock source. + operating-points-v2: true + opp-table: true + interrupts: maxItems: 1 @@ -75,6 +78,7 @@ properties: dependencies: freq-table-hz: [ 'clocks' ] + operating-points-v2: [ 'freq-table-hz' ] required: - interrupts -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table 2022-04-01 14:58 ` [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table Krzysztof Kozlowski @ 2022-04-04 21:39 ` Rob Herring 0 siblings, 0 replies; 11+ messages in thread From: Rob Herring @ 2022-04-04 21:39 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bean Huo, linux-kernel, Bjorn Andersson, Rob Herring, linux-arm-msm, Srinivas Kandagatla, Alim Akhtar, Andy Gross, Bart Van Assche, Avri Altman, linux-scsi, Martin K. Petersen, Taniya Das, linux-clk, Krzysztof Kozlowski, Stephen Boyd, devicetree, Michael Turquette, James E.J. Bottomley On Fri, 01 Apr 2022 16:58:18 +0200, Krzysztof Kozlowski wrote: > Except scaling UFS and bus clocks, it's necessary to scale also the > voltages of regulators or power domain performance state levels. Adding > Operating Performance Points table allows to adjust power domain > performance state, depending on the UFS clock speed. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > Documentation/devicetree/bindings/ufs/ufs-common.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS 2022-04-01 14:58 [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table Krzysztof Kozlowski @ 2022-04-01 14:58 ` Krzysztof Kozlowski 2022-04-04 0:02 ` Dmitry Baryshkov 2022-04-01 14:58 ` [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 3 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-01 14:58 UTC (permalink / raw) To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi Cc: Krzysztof Kozlowski UFS, when scaling gears, should choose appropriate performance state of RPMHPD power domain controller. Since UFS belongs to UFS_PHY_GDSC power domain, add necessary parent power domain to GCC. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index b31bf62e8680..c999b41c2605 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -1078,6 +1078,7 @@ gcc: clock-controller@100000 { #clock-cells = <1>; #reset-cells = <1>; #power-domain-cells = <1>; + power-domains = <&rpmhpd SDM845_CX>; }; qfprom@784000 { @@ -2336,8 +2337,22 @@ ufs_mem_hc: ufshc@1d84000 { <0 0>, <0 0>, <0 300000000>; - + operating-points-v2 = <&ufs_opp_table>; status = "disabled"; + + ufs_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-50000000 { + opp-hz = /bits/ 64 <50000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-200000000 { + opp-hz = /bits/ 64 <200000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; }; ufs_mem_phy: phy@1d87000 { -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS 2022-04-01 14:58 ` [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS Krzysztof Kozlowski @ 2022-04-04 0:02 ` Dmitry Baryshkov 2022-04-05 17:13 ` Bjorn Andersson 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Baryshkov @ 2022-04-04 0:02 UTC (permalink / raw) To: Krzysztof Kozlowski, Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi On 01/04/2022 17:58, Krzysztof Kozlowski wrote: > UFS, when scaling gears, should choose appropriate performance state of > RPMHPD power domain controller. Since UFS belongs to UFS_PHY_GDSC power > domain, add necessary parent power domain to GCC. This will cause all gcc GDSCs to be rooted in the CX. Are we sure that this is an expected (and correct) change? > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index b31bf62e8680..c999b41c2605 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -1078,6 +1078,7 @@ gcc: clock-controller@100000 { > #clock-cells = <1>; > #reset-cells = <1>; > #power-domain-cells = <1>; > + power-domains = <&rpmhpd SDM845_CX>; > }; > > qfprom@784000 { > @@ -2336,8 +2337,22 @@ ufs_mem_hc: ufshc@1d84000 { > <0 0>, > <0 0>, > <0 300000000>; > - > + operating-points-v2 = <&ufs_opp_table>; > status = "disabled"; > + > + ufs_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-50000000 { > + opp-hz = /bits/ 64 <50000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-200000000 { > + opp-hz = /bits/ 64 <200000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > }; > > ufs_mem_phy: phy@1d87000 { -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS 2022-04-04 0:02 ` Dmitry Baryshkov @ 2022-04-05 17:13 ` Bjorn Andersson 0 siblings, 0 replies; 11+ messages in thread From: Bjorn Andersson @ 2022-04-05 17:13 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Krzysztof Kozlowski, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi On Sun 03 Apr 17:02 PDT 2022, Dmitry Baryshkov wrote: > On 01/04/2022 17:58, Krzysztof Kozlowski wrote: > > UFS, when scaling gears, should choose appropriate performance state of > > RPMHPD power domain controller. Since UFS belongs to UFS_PHY_GDSC power > > domain, add necessary parent power domain to GCC. > > This will cause all gcc GDSCs to be rooted in the CX. Are we sure that this > is an expected (and correct) change? > Per the last part of Rajendra's reply in [1], this should be fine. Naturally we might have to come up with some way to bind gdscs to one of multiple power-domains if that changes. [1] https://lore.kernel.org/all/5e572c50-d6fe-5a21-d09f-f11a072538c5@codeaurora.org/ Regards, Bjorn > > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index b31bf62e8680..c999b41c2605 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -1078,6 +1078,7 @@ gcc: clock-controller@100000 { > > #clock-cells = <1>; > > #reset-cells = <1>; > > #power-domain-cells = <1>; > > + power-domains = <&rpmhpd SDM845_CX>; > > }; > > qfprom@784000 { > > @@ -2336,8 +2337,22 @@ ufs_mem_hc: ufshc@1d84000 { > > <0 0>, > > <0 0>, > > <0 300000000>; > > - > > + operating-points-v2 = <&ufs_opp_table>; > > status = "disabled"; > > + > > + ufs_opp_table: opp-table { > > + compatible = "operating-points-v2"; > > + > > + opp-50000000 { > > + opp-hz = /bits/ 64 <50000000>; > > + required-opps = <&rpmhpd_opp_svs>; > > + }; > > + > > + opp-200000000 { > > + opp-hz = /bits/ 64 <200000000>; > > + required-opps = <&rpmhpd_opp_nom>; > > + }; > > + }; > > }; > > ufs_mem_phy: phy@1d87000 { > > > -- > With best wishes > Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears 2022-04-01 14:58 [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski ` (2 preceding siblings ...) 2022-04-01 14:58 ` [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS Krzysztof Kozlowski @ 2022-04-01 14:58 ` Krzysztof Kozlowski 2022-04-01 19:47 ` Bjorn Andersson 3 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-01 14:58 UTC (permalink / raw) To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi Cc: Krzysztof Kozlowski Scaling gears requires not only scaling clocks, but also voltage levels, e.g. via performance states. USe the provided OPP table, to set proper OPP frequency which through required-opps will trigger performance state change. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/scsi/ufs/ufshcd-pltfrm.c | 6 +++++ drivers/scsi/ufs/ufshcd.c | 42 +++++++++++++++++++++++++------- drivers/scsi/ufs/ufshcd.h | 3 +++ 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index cca4b2181a81..c8f19b54be92 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -360,6 +360,12 @@ int ufshcd_pltfrm_init(struct platform_device *pdev, goto dealloc_host; } + if (devm_pm_opp_of_add_table(dev)) + dev_dbg(dev, "no OPP table (%d), no performance state control\n", + err); + else + hba->use_pm_opp = true; + ufshcd_init_lanes_per_dir(hba); err = ufshcd_init(hba, mmio_base, irq); diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 3f9caafa91bf..84912db86da8 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1164,11 +1164,16 @@ static int ufshcd_wait_for_doorbell_clr(struct ufs_hba *hba, static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) { int ret = 0; + struct ufs_clk_info *clki; + unsigned long pm_opp_target_rate; struct ufs_pa_layer_attr new_pwr_info; + clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list); + if (scale_up) { memcpy(&new_pwr_info, &hba->clk_scaling.saved_pwr_info.info, sizeof(struct ufs_pa_layer_attr)); + pm_opp_target_rate = clki->max_freq; } else { memcpy(&new_pwr_info, &hba->pwr_info, sizeof(struct ufs_pa_layer_attr)); @@ -1184,6 +1189,13 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) new_pwr_info.gear_tx = hba->clk_scaling.min_gear; new_pwr_info.gear_rx = hba->clk_scaling.min_gear; } + pm_opp_target_rate = clki->min_freq; + } + + if (hba->use_pm_opp && scale_up) { + ret = dev_pm_opp_set_rate(hba->dev, pm_opp_target_rate); + if (ret) + return ret; } /* check if the power mode needs to be changed or not? */ @@ -1194,6 +1206,11 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) hba->pwr_info.gear_tx, hba->pwr_info.gear_rx, new_pwr_info.gear_tx, new_pwr_info.gear_rx); + if (ret && hba->use_pm_opp && scale_up) + dev_pm_opp_set_rate(hba->dev, hba->devfreq->previous_freq); + else if (hba->use_pm_opp && !scale_up) + ret = dev_pm_opp_set_rate(hba->dev, pm_opp_target_rate); + return ret; } @@ -1435,9 +1452,11 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) if (list_empty(clk_list)) return 0; - clki = list_first_entry(clk_list, struct ufs_clk_info, list); - dev_pm_opp_add(hba->dev, clki->min_freq, 0); - dev_pm_opp_add(hba->dev, clki->max_freq, 0); + if (!hba->use_pm_opp) { + clki = list_first_entry(clk_list, struct ufs_clk_info, list); + dev_pm_opp_add(hba->dev, clki->min_freq, 0); + dev_pm_opp_add(hba->dev, clki->max_freq, 0); + } ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile, &hba->vps->ondemand_data); @@ -1449,8 +1468,10 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) ret = PTR_ERR(devfreq); dev_err(hba->dev, "Unable to register with devfreq %d\n", ret); - dev_pm_opp_remove(hba->dev, clki->min_freq); - dev_pm_opp_remove(hba->dev, clki->max_freq); + if (!hba->use_pm_opp) { + dev_pm_opp_remove(hba->dev, clki->min_freq); + dev_pm_opp_remove(hba->dev, clki->max_freq); + } return ret; } @@ -1462,7 +1483,6 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) static void ufshcd_devfreq_remove(struct ufs_hba *hba) { struct list_head *clk_list = &hba->clk_list_head; - struct ufs_clk_info *clki; if (!hba->devfreq) return; @@ -1470,9 +1490,13 @@ static void ufshcd_devfreq_remove(struct ufs_hba *hba) devfreq_remove_device(hba->devfreq); hba->devfreq = NULL; - clki = list_first_entry(clk_list, struct ufs_clk_info, list); - dev_pm_opp_remove(hba->dev, clki->min_freq); - dev_pm_opp_remove(hba->dev, clki->max_freq); + if (!hba->use_pm_opp) { + struct ufs_clk_info *clki; + + clki = list_first_entry(clk_list, struct ufs_clk_info, list); + dev_pm_opp_remove(hba->dev, clki->min_freq); + dev_pm_opp_remove(hba->dev, clki->max_freq); + } } static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba) diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 88c20f3608c2..3bd02095897f 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -776,6 +776,8 @@ struct ufs_hba_monitor { * @auto_bkops_enabled: to track whether bkops is enabled in device * @vreg_info: UFS device voltage regulator information * @clk_list_head: UFS host controller clocks list node head + * @use_pm_opp: whether OPP table is provided and scaling gears should trigger + * setting OPP * @pwr_info: holds current power mode * @max_pwr_info: keeps the device max valid pwm * @clk_scaling_lock: used to serialize device commands and clock scaling @@ -894,6 +896,7 @@ struct ufs_hba { bool auto_bkops_enabled; struct ufs_vreg_info vreg_info; struct list_head clk_list_head; + bool use_pm_opp; /* Number of requests aborts */ int req_abort_count; -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears 2022-04-01 14:58 ` [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski @ 2022-04-01 19:47 ` Bjorn Andersson 2022-04-03 17:46 ` Krzysztof Kozlowski 0 siblings, 1 reply; 11+ messages in thread From: Bjorn Andersson @ 2022-04-01 19:47 UTC (permalink / raw) To: Krzysztof Kozlowski, Nitin Rawat, Asutosh Das Cc: Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi On Fri 01 Apr 07:58 PDT 2022, Krzysztof Kozlowski wrote: > Scaling gears requires not only scaling clocks, but also voltage levels, > e.g. via performance states. > > USe the provided OPP table, to set proper OPP frequency which through > required-opps will trigger performance state change. > This looks quite nice! Just two questions about the path looking forward. If we where to extend the opp core to allow specifying the clock rate for some N first clocks (similar to how e.g. regulators are handled) it seems possible to extend this to replace the freq-table property as well. Would you agree? The other missing required feature (in this area) from the upstream UFS driver is the ability of voting for interconnect bandwidth. Based on your path it would be trivial to specify different values for the votes for each speed, but looking at downstream [1] (each row represents the vote for the two paths in KB/s) indicates a more complex relationship between gear and voted bandwidth. This was the reason I suggested that perhaps we need to key the opp-table based on the gear? But I don't think there would be any issue detecting this in runtime... [1] https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio.dtsi#L1982 Regards, Bjorn > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/scsi/ufs/ufshcd-pltfrm.c | 6 +++++ > drivers/scsi/ufs/ufshcd.c | 42 +++++++++++++++++++++++++------- > drivers/scsi/ufs/ufshcd.h | 3 +++ > 3 files changed, 42 insertions(+), 9 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c > index cca4b2181a81..c8f19b54be92 100644 > --- a/drivers/scsi/ufs/ufshcd-pltfrm.c > +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c > @@ -360,6 +360,12 @@ int ufshcd_pltfrm_init(struct platform_device *pdev, > goto dealloc_host; > } > > + if (devm_pm_opp_of_add_table(dev)) > + dev_dbg(dev, "no OPP table (%d), no performance state control\n", > + err); > + else > + hba->use_pm_opp = true; > + > ufshcd_init_lanes_per_dir(hba); > > err = ufshcd_init(hba, mmio_base, irq); > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 3f9caafa91bf..84912db86da8 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -1164,11 +1164,16 @@ static int ufshcd_wait_for_doorbell_clr(struct ufs_hba *hba, > static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) > { > int ret = 0; > + struct ufs_clk_info *clki; > + unsigned long pm_opp_target_rate; > struct ufs_pa_layer_attr new_pwr_info; > > + clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list); > + > if (scale_up) { > memcpy(&new_pwr_info, &hba->clk_scaling.saved_pwr_info.info, > sizeof(struct ufs_pa_layer_attr)); > + pm_opp_target_rate = clki->max_freq; > } else { > memcpy(&new_pwr_info, &hba->pwr_info, > sizeof(struct ufs_pa_layer_attr)); > @@ -1184,6 +1189,13 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) > new_pwr_info.gear_tx = hba->clk_scaling.min_gear; > new_pwr_info.gear_rx = hba->clk_scaling.min_gear; > } > + pm_opp_target_rate = clki->min_freq; > + } > + > + if (hba->use_pm_opp && scale_up) { > + ret = dev_pm_opp_set_rate(hba->dev, pm_opp_target_rate); > + if (ret) > + return ret; > } > > /* check if the power mode needs to be changed or not? */ > @@ -1194,6 +1206,11 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) > hba->pwr_info.gear_tx, hba->pwr_info.gear_rx, > new_pwr_info.gear_tx, new_pwr_info.gear_rx); > > + if (ret && hba->use_pm_opp && scale_up) > + dev_pm_opp_set_rate(hba->dev, hba->devfreq->previous_freq); > + else if (hba->use_pm_opp && !scale_up) > + ret = dev_pm_opp_set_rate(hba->dev, pm_opp_target_rate); > + > return ret; > } > > @@ -1435,9 +1452,11 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) > if (list_empty(clk_list)) > return 0; > > - clki = list_first_entry(clk_list, struct ufs_clk_info, list); > - dev_pm_opp_add(hba->dev, clki->min_freq, 0); > - dev_pm_opp_add(hba->dev, clki->max_freq, 0); > + if (!hba->use_pm_opp) { > + clki = list_first_entry(clk_list, struct ufs_clk_info, list); > + dev_pm_opp_add(hba->dev, clki->min_freq, 0); > + dev_pm_opp_add(hba->dev, clki->max_freq, 0); > + } > > ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile, > &hba->vps->ondemand_data); > @@ -1449,8 +1468,10 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) > ret = PTR_ERR(devfreq); > dev_err(hba->dev, "Unable to register with devfreq %d\n", ret); > > - dev_pm_opp_remove(hba->dev, clki->min_freq); > - dev_pm_opp_remove(hba->dev, clki->max_freq); > + if (!hba->use_pm_opp) { > + dev_pm_opp_remove(hba->dev, clki->min_freq); > + dev_pm_opp_remove(hba->dev, clki->max_freq); > + } > return ret; > } > > @@ -1462,7 +1483,6 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) > static void ufshcd_devfreq_remove(struct ufs_hba *hba) > { > struct list_head *clk_list = &hba->clk_list_head; > - struct ufs_clk_info *clki; > > if (!hba->devfreq) > return; > @@ -1470,9 +1490,13 @@ static void ufshcd_devfreq_remove(struct ufs_hba *hba) > devfreq_remove_device(hba->devfreq); > hba->devfreq = NULL; > > - clki = list_first_entry(clk_list, struct ufs_clk_info, list); > - dev_pm_opp_remove(hba->dev, clki->min_freq); > - dev_pm_opp_remove(hba->dev, clki->max_freq); > + if (!hba->use_pm_opp) { > + struct ufs_clk_info *clki; > + > + clki = list_first_entry(clk_list, struct ufs_clk_info, list); > + dev_pm_opp_remove(hba->dev, clki->min_freq); > + dev_pm_opp_remove(hba->dev, clki->max_freq); > + } > } > > static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba) > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 88c20f3608c2..3bd02095897f 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -776,6 +776,8 @@ struct ufs_hba_monitor { > * @auto_bkops_enabled: to track whether bkops is enabled in device > * @vreg_info: UFS device voltage regulator information > * @clk_list_head: UFS host controller clocks list node head > + * @use_pm_opp: whether OPP table is provided and scaling gears should trigger > + * setting OPP > * @pwr_info: holds current power mode > * @max_pwr_info: keeps the device max valid pwm > * @clk_scaling_lock: used to serialize device commands and clock scaling > @@ -894,6 +896,7 @@ struct ufs_hba { > bool auto_bkops_enabled; > struct ufs_vreg_info vreg_info; > struct list_head clk_list_head; > + bool use_pm_opp; > > /* Number of requests aborts */ > int req_abort_count; > -- > 2.32.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears 2022-04-01 19:47 ` Bjorn Andersson @ 2022-04-03 17:46 ` Krzysztof Kozlowski 0 siblings, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-04-03 17:46 UTC (permalink / raw) To: Bjorn Andersson, Nitin Rawat, Asutosh Das Cc: Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo, Bart Van Assche, Srinivas Kandagatla, Taniya Das, linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-scsi On 01/04/2022 21:47, Bjorn Andersson wrote: > On Fri 01 Apr 07:58 PDT 2022, Krzysztof Kozlowski wrote: > >> Scaling gears requires not only scaling clocks, but also voltage levels, >> e.g. via performance states. >> >> USe the provided OPP table, to set proper OPP frequency which through >> required-opps will trigger performance state change. >> > > This looks quite nice! Just two questions about the path looking forward. > > If we where to extend the opp core to allow specifying the clock rate > for some N first clocks (similar to how e.g. regulators are handled) it > seems possible to extend this to replace the freq-table property as > well. Would you agree? Yes, although that might be trickier. The frequency is a key. I'll take a look whether it could be changed to multiple values like the voltage. > > > The other missing required feature (in this area) from the upstream UFS > driver is the ability of voting for interconnect bandwidth. Based on > your path it would be trivial to specify different values for the votes > for each speed, but looking at downstream [1] (each row represents the > vote for the two paths in KB/s) indicates a more complex relationship > between gear and voted bandwidth. > > This was the reason I suggested that perhaps we need to key the > opp-table based on the gear? But I don't think there would be any issue > detecting this in runtime... > > [1] https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/waipio.dtsi#L1982 It should be doable with current bindings, assuming that gear is some imaginary frequency. We have two interconnects for UFS (the DDR and CPU) and OPP bindings allow to specify opp-peak-kBps and opp-avg-kBps for all of interconnects. IOW, the opp-peak-kBps will have two values and opp-avg-kBps as well. What would be still missing is scaling clocks. interconnects = <&ddr>, <&cpu> interconnect-names = "ufs-ddr", "cpu-ufs"; opp-table { // gear 1 or some core clock frequency? opp-1 { opp-hz = /bits/ 64 <75000000>, <0>, <0>, <75000000> ....; opp-avg-kBps = <922 1000>; opp-peak-kBps = <0 0>; required-opps = <&rpmpd_opp_low_svs>; } } arch/arm64/boot/dts/qcom/sdm630.dtsi already uses it. I think still the problem is scaling of multiple clocks, depending on the gear. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-04-05 23:02 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-01 14:58 [RFC PATCH 0/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 1/4] dt-bindings: clock: qcom,gcc-sdm845: add parent power domain Krzysztof Kozlowski [not found] ` <20220401232451.1B7A9C340F3@smtp.kernel.org> 2022-04-02 9:09 ` Krzysztof Kozlowski 2022-04-01 14:58 ` [RFC PATCH 2/4] dt-bindings: ufs: common: allow OPP table Krzysztof Kozlowski 2022-04-04 21:39 ` Rob Herring 2022-04-01 14:58 ` [RFC PATCH 3/4] arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS Krzysztof Kozlowski 2022-04-04 0:02 ` Dmitry Baryshkov 2022-04-05 17:13 ` Bjorn Andersson 2022-04-01 14:58 ` [RFC PATCH 4/4] ufs: set power domain performance state when scaling gears Krzysztof Kozlowski 2022-04-01 19:47 ` Bjorn Andersson 2022-04-03 17:46 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).