* [PATCH v1 0/3] PCI: qcom: Add support for OPP
@ 2023-08-15 12:26 Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2 Krishna chaitanya chundru
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Krishna chaitanya chundru @ 2023-08-15 12:26 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
krzysztof.kozlowski, Krishna chaitanya chundru
This patch adds support for OPP to vote for the performance state of RPMH
power domain based upon GEN speed it PCIe got enumerated.
Before link up PCIe driver will vote for the maximum performance state.
Krishna chaitanya chundru (3):
dt-bindings: pci: qcom: Add binding for operating-points-v2
arm64: dts: qcom: sm8450: Add opp table support to PCIe
PCI: qcom: Add OPP suuport for speed based performance state of RPMH
.../devicetree/bindings/pci/qcom,pcie.yaml | 2 +
arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 +++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++
3 files changed, 110 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2
2023-08-15 12:26 [PATCH v1 0/3] PCI: qcom: Add support for OPP Krishna chaitanya chundru
@ 2023-08-15 12:26 ` Krishna chaitanya chundru
2023-08-15 12:30 ` Krzysztof Kozlowski
2023-08-15 12:26 ` [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
2 siblings, 1 reply; 15+ messages in thread
From: Krishna chaitanya chundru @ 2023-08-15 12:26 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
krzysztof.kozlowski, Krishna chaitanya chundru, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
This adds a binding documenting operating-points-v2.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 81971be4..6bc99c5 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -121,6 +121,8 @@ properties:
description: GPIO controlled connection to WAKE# signal
maxItems: 1
+ operating-points-v2: true
+
required:
- compatible
- reg
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-15 12:26 [PATCH v1 0/3] PCI: qcom: Add support for OPP Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2 Krishna chaitanya chundru
@ 2023-08-15 12:26 ` Krishna chaitanya chundru
2023-08-15 12:31 ` Krzysztof Kozlowski
2023-08-16 7:05 ` Pavan Kondeti
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
2 siblings, 2 replies; 15+ messages in thread
From: Krishna chaitanya chundru @ 2023-08-15 12:26 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
krzysztof.kozlowski, Krishna chaitanya chundru, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
PCIe needs to choose the appropriate performance state of RPMH power
domain based upon the PCIe gen speed.
So, let's add the OPP table support to specify RPMH performance states.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 595533a..681ea9c 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -381,6 +381,49 @@
};
};
+ pcie0_opp_table: opp-table-pcie0 {
+ compatible = "operating-points-v2";
+
+ opp-2500000 {
+ opp-hz = /bits/ 64 <2500000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+
+ opp-5000000 {
+ opp-hz = /bits/ 64 <5000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+
+ opp-8000000 {
+ opp-hz = /bits/ 64 <8000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+ };
+
+ pcie1_opp_table: opp-table-pcie1 {
+ compatible = "operating-points-v2";
+
+ opp-2500000 {
+ opp-hz = /bits/ 64 <2500000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+
+ opp-5000000 {
+ opp-hz = /bits/ 64 <5000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+
+ opp-8000000 {
+ opp-hz = /bits/ 64 <8000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ };
+
+ opp-16000000 {
+ opp-hz = /bits/ 64 <16000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
+ };
+ };
+
reserved_memory: reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
@@ -1803,6 +1846,8 @@
pinctrl-names = "default";
pinctrl-0 = <&pcie0_default_state>;
+ operating-points-v2 = <&pcie0_opp_table>;
+
status = "disabled";
};
@@ -1915,6 +1960,8 @@
pinctrl-names = "default";
pinctrl-0 = <&pcie1_default_state>;
+ operating-points-v2 = <&pcie1_opp_table>;
+
status = "disabled";
};
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
2023-08-15 12:26 [PATCH v1 0/3] PCI: qcom: Add support for OPP Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2 Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe Krishna chaitanya chundru
@ 2023-08-15 12:26 ` Krishna chaitanya chundru
2023-08-15 14:03 ` kernel test robot
` (2 more replies)
2 siblings, 3 replies; 15+ messages in thread
From: Krishna chaitanya chundru @ 2023-08-15 12:26 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
krzysztof.kozlowski, Krishna chaitanya chundru, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas
Before link training vote for the maximum performance state of RPMH
and once the link is up, vote for the performance state based upon
the link speed.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 7a87a47..e29a986 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -22,6 +22,7 @@
#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pci.h>
+#include <linux/pm_opp.h>
#include <linux/pm_runtime.h>
#include <linux/platform_device.h>
#include <linux/phy/pcie.h>
@@ -1357,6 +1358,51 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
return 0;
}
+static void qcom_pcie_opp_update(struct qcom_pcie *pcie)
+{
+ struct dw_pcie *pci = pcie->pci;
+ struct dev_pm_opp *opp;
+ u32 offset, status;
+ uint32_t freq;
+ int speed;
+ int ret = 0;
+
+ offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+
+ /* Only update constraints if link is up. */
+ if (!(status & PCI_EXP_LNKSTA_DLLLA))
+ return;
+
+ speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
+
+ switch (speed) {
+ case 1:
+ freq = 2500000;
+ break;
+ case 2:
+ freq = 5000000;
+ break;
+ case 3:
+ freq = 8000000;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ fallthrough;
+ case 4:
+ freq = 16000000;
+ break;
+ }
+
+ opp = dev_pm_opp_find_freq_exact(pci->dev, freq, true);
+
+ if (!IS_ERR(opp)) {
+ ret = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+ }
+
+}
+
static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
{
struct dw_pcie *pci = pcie->pci;
@@ -1439,8 +1485,10 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
static int qcom_pcie_probe(struct platform_device *pdev)
{
const struct qcom_pcie_cfg *pcie_cfg;
+ unsigned long max_freq = INT_MAX;
struct device *dev = &pdev->dev;
struct qcom_pcie *pcie;
+ struct dev_pm_opp *opp;
struct dw_pcie_rp *pp;
struct resource *res;
struct dw_pcie *pci;
@@ -1511,6 +1559,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
if (ret)
goto err_pm_runtime_put;
+ /* OPP table is optional */
+ ret = devm_pm_opp_of_add_table(dev);
+ if (ret && ret != -ENODEV) {
+ dev_err(dev, "Invalid OPP table in Device tree\n");
+ goto err_pm_runtime_put;
+ }
+
+ opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
+ if (!IS_ERR(opp))
+ dev_pm_opp_put(opp);
+
ret = pcie->cfg->ops->get_resources(pcie);
if (ret)
goto err_pm_runtime_put;
@@ -1531,6 +1590,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
qcom_pcie_icc_update(pcie);
+ qcom_pcie_opp_update(pcie);
+
if (pcie->mhi)
qcom_pcie_init_debugfs(pcie);
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2
2023-08-15 12:26 ` [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2 Krishna chaitanya chundru
@ 2023-08-15 12:30 ` Krzysztof Kozlowski
2023-08-16 8:49 ` Krishna Chaitanya Chundru
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 12:30 UTC (permalink / raw)
To: Krishna chaitanya chundru, manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 15/08/2023 14:26, Krishna chaitanya chundru wrote:
> This adds a binding documenting operating-points-v2.
1. Missing blank line. Don't write patches by yourself, but use tools
which create proper commit automatically. Every decent editor does it.
2. Please do not use "This commit/patch", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
3. A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 81971be4..6bc99c5 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -121,6 +121,8 @@ properties:
> description: GPIO controlled connection to WAKE# signal
> maxItems: 1
>
> + operating-points-v2: true
phandle without actual table (opp-table) is rather meaningless.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-15 12:26 ` [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe Krishna chaitanya chundru
@ 2023-08-15 12:31 ` Krzysztof Kozlowski
2023-08-16 8:50 ` Krishna Chaitanya Chundru
2023-08-16 7:05 ` Pavan Kondeti
1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 12:31 UTC (permalink / raw)
To: Krishna chaitanya chundru, manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 15/08/2023 14:26, Krishna chaitanya chundru wrote:
> PCIe needs to choose the appropriate performance state of RPMH power
> domain based upon the PCIe gen speed.
This explanation should be also in bindings patch, otherwise why would
we consider the bindings patch?
>
> So, let's add the OPP table support to specify RPMH performance states.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 595533a..681ea9c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -381,6 +381,49 @@
> };
> };
>
> + pcie0_opp_table: opp-table-pcie0 {
> + compatible = "operating-points-v2";
> +
> + opp-2500000 {
> + opp-hz = /bits/ 64 <2500000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-5000000 {
> + opp-hz = /bits/ 64 <5000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-8000000 {
> + opp-hz = /bits/ 64 <8000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> + };
> +
> + pcie1_opp_table: opp-table-pcie1 {
> + compatible = "operating-points-v2";
> +
> + opp-2500000 {
> + opp-hz = /bits/ 64 <2500000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-5000000 {
> + opp-hz = /bits/ 64 <5000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-8000000 {
> + opp-hz = /bits/ 64 <8000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-16000000 {
> + opp-hz = /bits/ 64 <16000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
> + };
> + };
> +
> reserved_memory: reserved-memory {
> #address-cells = <2>;
> #size-cells = <2>;
> @@ -1803,6 +1846,8 @@
> pinctrl-names = "default";
> pinctrl-0 = <&pcie0_default_state>;
>
> + operating-points-v2 = <&pcie0_opp_table>;
Why the table is not here? Is it shared with multiple devices?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
@ 2023-08-15 14:03 ` kernel test robot
2023-08-16 6:24 ` Pavan Kondeti
2023-08-16 12:25 ` Konrad Dybcio
2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-08-15 14:03 UTC (permalink / raw)
To: Krishna chaitanya chundru, manivannan.sadhasivam
Cc: oe-kbuild-all, helgaas, linux-pci, linux-arm-msm, linux-kernel,
quic_vbadigan, quic_nitegupt, quic_skananth, quic_ramkri,
quic_parass, krzysztof.kozlowski, Krishna chaitanya chundru,
Andy Gross, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring
Hi Krishna,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/for-linus]
[also build test WARNING on robh/for-next linus/master v6.5-rc6 next-20230809]
[cannot apply to pci/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Krishna-chaitanya-chundru/dt-bindings-pci-qcom-Add-binding-for-operating-points-v2/20230815-203103
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link: https://lore.kernel.org/r/1692102408-7010-4-git-send-email-quic_krichai%40quicinc.com
patch subject: [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230815/202308152125.sU1aQfAd-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230815/202308152125.sU1aQfAd-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308152125.sU1aQfAd-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/pci/controller/dwc/pcie-qcom.c: In function 'qcom_pcie_opp_update':
>> drivers/pci/controller/dwc/pcie-qcom.c:1368:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
1368 | int ret = 0;
| ^~~
vim +/ret +1368 drivers/pci/controller/dwc/pcie-qcom.c
1360
1361 static void qcom_pcie_opp_update(struct qcom_pcie *pcie)
1362 {
1363 struct dw_pcie *pci = pcie->pci;
1364 struct dev_pm_opp *opp;
1365 u32 offset, status;
1366 uint32_t freq;
1367 int speed;
> 1368 int ret = 0;
1369
1370 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
1371 status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
1372
1373 /* Only update constraints if link is up. */
1374 if (!(status & PCI_EXP_LNKSTA_DLLLA))
1375 return;
1376
1377 speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
1378
1379 switch (speed) {
1380 case 1:
1381 freq = 2500000;
1382 break;
1383 case 2:
1384 freq = 5000000;
1385 break;
1386 case 3:
1387 freq = 8000000;
1388 break;
1389 default:
1390 WARN_ON_ONCE(1);
1391 fallthrough;
1392 case 4:
1393 freq = 16000000;
1394 break;
1395 }
1396
1397 opp = dev_pm_opp_find_freq_exact(pci->dev, freq, true);
1398
1399 if (!IS_ERR(opp)) {
1400 ret = dev_pm_opp_get_voltage(opp);
1401 dev_pm_opp_put(opp);
1402 }
1403
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
2023-08-15 14:03 ` kernel test robot
@ 2023-08-16 6:24 ` Pavan Kondeti
2023-08-16 8:53 ` Krishna Chaitanya Chundru
2023-08-16 12:25 ` Konrad Dybcio
2 siblings, 1 reply; 15+ messages in thread
From: Pavan Kondeti @ 2023-08-16 6:24 UTC (permalink / raw)
To: Krishna chaitanya chundru
Cc: manivannan.sadhasivam, helgaas, linux-pci, linux-arm-msm,
linux-kernel, quic_vbadigan, quic_nitegupt, quic_skananth,
quic_ramkri, quic_parass, krzysztof.kozlowski, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas
On Tue, Aug 15, 2023 at 05:56:48PM +0530, Krishna chaitanya chundru wrote:
> Before link training vote for the maximum performance state of RPMH
> and once the link is up, vote for the performance state based upon
> the link speed.
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 7a87a47..e29a986 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -22,6 +22,7 @@
> #include <linux/of_device.h>
> #include <linux/of_gpio.h>
> #include <linux/pci.h>
> +#include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> #include <linux/platform_device.h>
> #include <linux/phy/pcie.h>
> @@ -1357,6 +1358,51 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
> return 0;
> }
>
> +static void qcom_pcie_opp_update(struct qcom_pcie *pcie)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct dev_pm_opp *opp;
> + u32 offset, status;
> + uint32_t freq;
> + int speed;
> + int ret = 0;
> +
> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> +
> + /* Only update constraints if link is up. */
> + if (!(status & PCI_EXP_LNKSTA_DLLLA))
> + return;
> +
> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
> +
> + switch (speed) {
> + case 1:
> + freq = 2500000;
> + break;
> + case 2:
> + freq = 5000000;
> + break;
> + case 3:
> + freq = 8000000;
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + fallthrough;
> + case 4:
> + freq = 16000000;
> + break;
> + }
> +
> + opp = dev_pm_opp_find_freq_exact(pci->dev, freq, true);
> +
> + if (!IS_ERR(opp)) {
> + ret = dev_pm_opp_get_voltage(opp);
> + dev_pm_opp_put(opp);
> + }
> +
Where are we setting the OPP here?
> +}
> +
> static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
> {
> struct dw_pcie *pci = pcie->pci;
> @@ -1439,8 +1485,10 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
> static int qcom_pcie_probe(struct platform_device *pdev)
> {
> const struct qcom_pcie_cfg *pcie_cfg;
> + unsigned long max_freq = INT_MAX;
> struct device *dev = &pdev->dev;
> struct qcom_pcie *pcie;
> + struct dev_pm_opp *opp;
> struct dw_pcie_rp *pp;
> struct resource *res;
> struct dw_pcie *pci;
> @@ -1511,6 +1559,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> if (ret)
> goto err_pm_runtime_put;
>
> + /* OPP table is optional */
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret && ret != -ENODEV) {
> + dev_err(dev, "Invalid OPP table in Device tree\n");
> + goto err_pm_runtime_put;
> + }
> +
> + opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> + if (!IS_ERR(opp))
> + dev_pm_opp_put(opp);
> +
This OPP (corresponding to max freq) is not used, so how are we voting
for max perf state during probe?
> ret = pcie->cfg->ops->get_resources(pcie);
> if (ret)
> goto err_pm_runtime_put;
> @@ -1531,6 +1590,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>
> qcom_pcie_icc_update(pcie);
>
> + qcom_pcie_opp_update(pcie);
> +
commit description says, OPP voting is done as per the link speed after
probe? I don't see any calls to qcom_pcie_opp_update() outside probe.
> if (pcie->mhi)
> qcom_pcie_init_debugfs(pcie);
>
>
Thanks,
Pavan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-15 12:26 ` [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe Krishna chaitanya chundru
2023-08-15 12:31 ` Krzysztof Kozlowski
@ 2023-08-16 7:05 ` Pavan Kondeti
2023-08-16 8:51 ` Krishna Chaitanya Chundru
2023-08-16 12:22 ` Konrad Dybcio
1 sibling, 2 replies; 15+ messages in thread
From: Pavan Kondeti @ 2023-08-16 7:05 UTC (permalink / raw)
To: Krishna chaitanya chundru
Cc: manivannan.sadhasivam, helgaas, linux-pci, linux-arm-msm,
linux-kernel, quic_vbadigan, quic_nitegupt, quic_skananth,
quic_ramkri, quic_parass, krzysztof.kozlowski, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Tue, Aug 15, 2023 at 05:56:47PM +0530, Krishna chaitanya chundru wrote:
> PCIe needs to choose the appropriate performance state of RPMH power
> domain based upon the PCIe gen speed.
>
> So, let's add the OPP table support to specify RPMH performance states.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 595533a..681ea9c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -381,6 +381,49 @@
> };
> };
>
> + pcie0_opp_table: opp-table-pcie0 {
> + compatible = "operating-points-v2";
> +
> + opp-2500000 {
> + opp-hz = /bits/ 64 <2500000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-5000000 {
> + opp-hz = /bits/ 64 <5000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-8000000 {
> + opp-hz = /bits/ 64 <8000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> + };
> +
> + pcie1_opp_table: opp-table-pcie1 {
> + compatible = "operating-points-v2";
> +
> + opp-2500000 {
> + opp-hz = /bits/ 64 <2500000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-5000000 {
> + opp-hz = /bits/ 64 <5000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-8000000 {
> + opp-hz = /bits/ 64 <8000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> + };
> +
> + opp-16000000 {
> + opp-hz = /bits/ 64 <16000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
> + };
> + };
> +
Should not we using required-opps property to pass the
rpmhpd_opp_xxx phandle so that when this OPP is selected based on your
clock rate, the appropriate OPP (voltage) would be selected on the RPMH side?
Please see SDHCI/MMC voting (sdhc2_opp_table) as an example.
Thanks,
Pavan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2
2023-08-15 12:30 ` Krzysztof Kozlowski
@ 2023-08-16 8:49 ` Krishna Chaitanya Chundru
0 siblings, 0 replies; 15+ messages in thread
From: Krishna Chaitanya Chundru @ 2023-08-16 8:49 UTC (permalink / raw)
To: Krzysztof Kozlowski, manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Manivannan Sadhasivam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 8/15/2023 6:00 PM, Krzysztof Kozlowski wrote:
> On 15/08/2023 14:26, Krishna chaitanya chundru wrote:
>> This adds a binding documenting operating-points-v2.
> 1. Missing blank line. Don't write patches by yourself, but use tools
> which create proper commit automatically. Every decent editor does it.
>
> 2. Please do not use "This commit/patch", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> 3. A nit, subject: drop second/last, redundant "binding for". The
> "dt-bindings" prefix is already stating that these are bindings.
>
>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 81971be4..6bc99c5 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -121,6 +121,8 @@ properties:
>> description: GPIO controlled connection to WAKE# signal
>> maxItems: 1
>>
>> + operating-points-v2: true
> phandle without actual table (opp-table) is rather meaningless.
>
> Best regards,
> Krzysztof
I will take all your comments and will send next patch
- KC
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-15 12:31 ` Krzysztof Kozlowski
@ 2023-08-16 8:50 ` Krishna Chaitanya Chundru
0 siblings, 0 replies; 15+ messages in thread
From: Krishna Chaitanya Chundru @ 2023-08-16 8:50 UTC (permalink / raw)
To: Krzysztof Kozlowski, manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 8/15/2023 6:01 PM, Krzysztof Kozlowski wrote:
> On 15/08/2023 14:26, Krishna chaitanya chundru wrote:
>> PCIe needs to choose the appropriate performance state of RPMH power
>> domain based upon the PCIe gen speed.
> This explanation should be also in bindings patch, otherwise why would
> we consider the bindings patch?
I will update binding patch with this information.
>
>> So, let's add the OPP table support to specify RPMH performance states.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 595533a..681ea9c 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -381,6 +381,49 @@
>> };
>> };
>>
>> + pcie0_opp_table: opp-table-pcie0 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> + };
>> +
>> + pcie1_opp_table: opp-table-pcie1 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-16000000 {
>> + opp-hz = /bits/ 64 <16000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
>> + };
>> + };
>> +
>> reserved_memory: reserved-memory {
>> #address-cells = <2>;
>> #size-cells = <2>;
>> @@ -1803,6 +1846,8 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&pcie0_default_state>;
>>
>> + operating-points-v2 = <&pcie0_opp_table>;
> Why the table is not here? Is it shared with multiple devices?
I will move the table to here in the next patch.
- KC
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-16 7:05 ` Pavan Kondeti
@ 2023-08-16 8:51 ` Krishna Chaitanya Chundru
2023-08-16 12:22 ` Konrad Dybcio
1 sibling, 0 replies; 15+ messages in thread
From: Krishna Chaitanya Chundru @ 2023-08-16 8:51 UTC (permalink / raw)
To: Pavan Kondeti
Cc: manivannan.sadhasivam, helgaas, linux-pci, linux-arm-msm,
linux-kernel, quic_vbadigan, quic_nitegupt, quic_skananth,
quic_ramkri, quic_parass, krzysztof.kozlowski, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 8/16/2023 12:35 PM, Pavan Kondeti wrote:
> On Tue, Aug 15, 2023 at 05:56:47PM +0530, Krishna chaitanya chundru wrote:
>> PCIe needs to choose the appropriate performance state of RPMH power
>> domain based upon the PCIe gen speed.
>>
>> So, let's add the OPP table support to specify RPMH performance states.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 595533a..681ea9c 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -381,6 +381,49 @@
>> };
>> };
>>
>> + pcie0_opp_table: opp-table-pcie0 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> + };
>> +
>> + pcie1_opp_table: opp-table-pcie1 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-16000000 {
>> + opp-hz = /bits/ 64 <16000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
>> + };
>> + };
>> +
> Should not we using required-opps property to pass the
> rpmhpd_opp_xxx phandle so that when this OPP is selected based on your
> clock rate, the appropriate OPP (voltage) would be selected on the RPMH side?
>
> Please see SDHCI/MMC voting (sdhc2_opp_table) as an example.
Sure I will try to use rpmhpd_opp_xxx phandle in next patch
- KC
>
> Thanks,
> Pavan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
2023-08-16 6:24 ` Pavan Kondeti
@ 2023-08-16 8:53 ` Krishna Chaitanya Chundru
0 siblings, 0 replies; 15+ messages in thread
From: Krishna Chaitanya Chundru @ 2023-08-16 8:53 UTC (permalink / raw)
To: Pavan Kondeti
Cc: manivannan.sadhasivam, helgaas, linux-pci, linux-arm-msm,
linux-kernel, quic_vbadigan, quic_nitegupt, quic_skananth,
quic_ramkri, quic_parass, krzysztof.kozlowski, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas
On 8/16/2023 11:54 AM, Pavan Kondeti wrote:
> On Tue, Aug 15, 2023 at 05:56:48PM +0530, Krishna chaitanya chundru wrote:
>> Before link training vote for the maximum performance state of RPMH
>> and once the link is up, vote for the performance state based upon
>> the link speed.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 7a87a47..e29a986 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -22,6 +22,7 @@
>> #include <linux/of_device.h>
>> #include <linux/of_gpio.h>
>> #include <linux/pci.h>
>> +#include <linux/pm_opp.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/platform_device.h>
>> #include <linux/phy/pcie.h>
>> @@ -1357,6 +1358,51 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
>> return 0;
>> }
>>
>> +static void qcom_pcie_opp_update(struct qcom_pcie *pcie)
>> +{
>> + struct dw_pcie *pci = pcie->pci;
>> + struct dev_pm_opp *opp;
>> + u32 offset, status;
>> + uint32_t freq;
>> + int speed;
>> + int ret = 0;
>> +
>> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
>> +
>> + /* Only update constraints if link is up. */
>> + if (!(status & PCI_EXP_LNKSTA_DLLLA))
>> + return;
>> +
>> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
>> +
>> + switch (speed) {
>> + case 1:
>> + freq = 2500000;
>> + break;
>> + case 2:
>> + freq = 5000000;
>> + break;
>> + case 3:
>> + freq = 8000000;
>> + break;
>> + default:
>> + WARN_ON_ONCE(1);
>> + fallthrough;
>> + case 4:
>> + freq = 16000000;
>> + break;
>> + }
>> +
>> + opp = dev_pm_opp_find_freq_exact(pci->dev, freq, true);
>> +
>> + if (!IS_ERR(opp)) {
>> + ret = dev_pm_opp_get_voltage(opp);
>> + dev_pm_opp_put(opp);
>> + }
>> +
> Where are we setting the OPP here?
>
>> +}
>> +
>> static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
>> {
>> struct dw_pcie *pci = pcie->pci;
>> @@ -1439,8 +1485,10 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
>> static int qcom_pcie_probe(struct platform_device *pdev)
>> {
>> const struct qcom_pcie_cfg *pcie_cfg;
>> + unsigned long max_freq = INT_MAX;
>> struct device *dev = &pdev->dev;
>> struct qcom_pcie *pcie;
>> + struct dev_pm_opp *opp;
>> struct dw_pcie_rp *pp;
>> struct resource *res;
>> struct dw_pcie *pci;
>> @@ -1511,6 +1559,17 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_pm_runtime_put;
>>
>> + /* OPP table is optional */
>> + ret = devm_pm_opp_of_add_table(dev);
>> + if (ret && ret != -ENODEV) {
>> + dev_err(dev, "Invalid OPP table in Device tree\n");
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
>> + if (!IS_ERR(opp))
>> + dev_pm_opp_put(opp);
>> +
> This OPP (corresponding to max freq) is not used, so how are we voting
> for max perf state during probe?
>
>> ret = pcie->cfg->ops->get_resources(pcie);
>> if (ret)
>> goto err_pm_runtime_put;
>> @@ -1531,6 +1590,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>
>> qcom_pcie_icc_update(pcie);
>>
>> + qcom_pcie_opp_update(pcie);
>> +
> commit description says, OPP voting is done as per the link speed after
> probe? I don't see any calls to qcom_pcie_opp_update() outside probe.
my mistake dev_pm_opp_set_opp somehow missed here I will update in next
patch.
- KC
>> if (pcie->mhi)
>> qcom_pcie_init_debugfs(pcie);
>>
>>
> Thanks,
> Pavan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe
2023-08-16 7:05 ` Pavan Kondeti
2023-08-16 8:51 ` Krishna Chaitanya Chundru
@ 2023-08-16 12:22 ` Konrad Dybcio
1 sibling, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2023-08-16 12:22 UTC (permalink / raw)
To: Pavan Kondeti, Krishna chaitanya chundru
Cc: manivannan.sadhasivam, helgaas, linux-pci, linux-arm-msm,
linux-kernel, quic_vbadigan, quic_nitegupt, quic_skananth,
quic_ramkri, quic_parass, krzysztof.kozlowski, Andy Gross,
Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On 16.08.2023 09:05, Pavan Kondeti wrote:
> On Tue, Aug 15, 2023 at 05:56:47PM +0530, Krishna chaitanya chundru wrote:
>> PCIe needs to choose the appropriate performance state of RPMH power
>> domain based upon the PCIe gen speed.
>>
>> So, let's add the OPP table support to specify RPMH performance states.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 47 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 595533a..681ea9c 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -381,6 +381,49 @@
>> };
>> };
>>
>> + pcie0_opp_table: opp-table-pcie0 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> + };
>> +
>> + pcie1_opp_table: opp-table-pcie1 {
>> + compatible = "operating-points-v2";
>> +
>> + opp-2500000 {
>> + opp-hz = /bits/ 64 <2500000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-5000000 {
>> + opp-hz = /bits/ 64 <5000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-8000000 {
>> + opp-hz = /bits/ 64 <8000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>> + };
>> +
>> + opp-16000000 {
>> + opp-hz = /bits/ 64 <16000000>;
>> + opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
>> + };
>> + };
>> +
>
> Should not we using required-opps property to pass the
> rpmhpd_opp_xxx phandle so that when this OPP is selected based on your
> clock rate, the appropriate OPP (voltage) would be selected on the RPMH side?
Yes, opp-level is for opp providers.
Konrad
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
2023-08-15 14:03 ` kernel test robot
2023-08-16 6:24 ` Pavan Kondeti
@ 2023-08-16 12:25 ` Konrad Dybcio
2 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2023-08-16 12:25 UTC (permalink / raw)
To: Krishna chaitanya chundru, manivannan.sadhasivam
Cc: helgaas, linux-pci, linux-arm-msm, linux-kernel, quic_vbadigan,
quic_nitegupt, quic_skananth, quic_ramkri, quic_parass,
krzysztof.kozlowski, Andy Gross, Bjorn Andersson,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
On 15.08.2023 14:26, Krishna chaitanya chundru wrote:
> Before link training vote for the maximum performance state of RPMH
> and once the link is up, vote for the performance state based upon
> the link speed.
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 7a87a47..e29a986 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -22,6 +22,7 @@
> #include <linux/of_device.h>
> #include <linux/of_gpio.h>
> #include <linux/pci.h>
> +#include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> #include <linux/platform_device.h>
> #include <linux/phy/pcie.h>
> @@ -1357,6 +1358,51 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
> return 0;
> }
>
> +static void qcom_pcie_opp_update(struct qcom_pcie *pcie)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct dev_pm_opp *opp;
> + u32 offset, status;
> + uint32_t freq;
> + int speed;
> + int ret = 0;
On top of Krzysztof's comments:
ret is effectively unused
[...]
> + opp = dev_pm_opp_find_freq_exact(pci->dev, freq, true);
> +
> + if (!IS_ERR(opp)) {
Unnecessary newline
Konrad
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-08-16 12:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-15 12:26 [PATCH v1 0/3] PCI: qcom: Add support for OPP Krishna chaitanya chundru
2023-08-15 12:26 ` [PATCH v1 1/3] dt-bindings: pci: qcom: Add binding for operating-points-v2 Krishna chaitanya chundru
2023-08-15 12:30 ` Krzysztof Kozlowski
2023-08-16 8:49 ` Krishna Chaitanya Chundru
2023-08-15 12:26 ` [PATCH v1 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe Krishna chaitanya chundru
2023-08-15 12:31 ` Krzysztof Kozlowski
2023-08-16 8:50 ` Krishna Chaitanya Chundru
2023-08-16 7:05 ` Pavan Kondeti
2023-08-16 8:51 ` Krishna Chaitanya Chundru
2023-08-16 12:22 ` Konrad Dybcio
2023-08-15 12:26 ` [PATCH v1 3/3] PCI: qcom: Add OPP suuport for speed based performance state of RPMH Krishna chaitanya chundru
2023-08-15 14:03 ` kernel test robot
2023-08-16 6:24 ` Pavan Kondeti
2023-08-16 8:53 ` Krishna Chaitanya Chundru
2023-08-16 12:25 ` Konrad Dybcio
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).