[PATCH v2 0/3] PCI: qcom: Add support for OPP

[PATCH v2 0/3] PCI: qcom: Add support for OPP

[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.

Changes from v1:
	- Addressed comments from Krzysztof Kozlowski.
	- Added the rpmhpd_opp_xxx phandle as suggested by pavan.
	- Added dev_pm_opp_set_opp API call which was missed on previous patch.

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 support for speed based performance state of RPMH

 .../devicetree/bindings/pci/qcom,pcie.yaml         |  4 ++
 arch/arm64/boot/dts/qcom/sm8450.dtsi               | 47 +++++++++++++++
 drivers/pci/controller/dwc/pcie-qcom.c             | 67 ++++++++++++++++++++++
 3 files changed, 118 insertions(+)

-- 
2.7.4

[PATCH v2 1/3] dt-bindings: pci: qcom: Add opp table

[Krishna chaitanya chundru]

PCIe needs to choose the appropriate performance state of RPMH power
domain based upon the PCIe gen speed.

Adding the Operating Performance Points table allows to adjust power domain
performance state, depending on the PCIe gen speed.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 81971be4..779339c 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -121,6 +121,10 @@ properties:
     description: GPIO controlled connection to WAKE# signal
     maxItems: 1
 
+  operating-points-v2: true
+  opp-table:
+    type: object
+
 required:
   - compatible
   - reg
-- 
2.7.4

[PATCH v2 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe

[Krishna chaitanya chundru]

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..c77a683 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -1803,7 +1803,28 @@
 			pinctrl-names = "default";
 			pinctrl-0 = <&pcie0_default_state>;
 
+			operating-points-v2 = <&pcie0_opp_table>;
+
 			status = "disabled";
+
+			pcie0_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+				};
+			};
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1915,7 +1936,33 @@
 			pinctrl-names = "default";
 			pinctrl-0 = <&pcie1_default_state>;
 
+			operating-points-v2 = <&pcie1_opp_table>;
+
 			status = "disabled";
+
+			pcie1_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-16000000 {
+					opp-hz = /bits/ 64 <16000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+				};
+			};
 		};
 
 		pcie1_phy: phy@1c0f000 {
-- 
2.7.4

[PATCH v2 3/3] PCI: qcom: Add OPP support for speed based performance state of RPMH

[Krishna chaitanya chundru]

Before link training vote for the maximum performance state of RPMH
and once 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 | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 7a87a47..831d158 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;
+	int speed, ret = 0;
+	uint32_t freq;
+
+	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_set_opp(pci->dev, opp);
+		if (ret)
+			dev_err(pci->dev, "Failed to set opp: %d\n", ret);
+		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,21 @@ 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)) {
+		ret = dev_pm_opp_set_opp(dev, opp);
+		if (ret)
+			dev_err(pci->dev, "Failed to set opp: %d\n", ret);
+		dev_pm_opp_put(opp);
+	}
+
 	ret = pcie->cfg->ops->get_resources(pcie);
 	if (ret)
 		goto err_pm_runtime_put;
@@ -1531,6 +1594,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

Re: [PATCH v2 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe

[Dmitry Baryshkov]

On Wed, 16 Aug 2023 at 16:25, Krishna chaitanya chundru
<quic_krichai@quicinc.com> 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..c77a683 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -1803,7 +1803,28 @@
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&pcie0_default_state>;
>
> +                       operating-points-v2 = <&pcie0_opp_table>;
> +
>                         status = "disabled";
> +
> +                       pcie0_opp_table: opp-table {
> +                               compatible = "operating-points-v2";
> +
> +                               opp-2500000 {

As a random suggestion: these frequencies are calculated by the
driver. It might be easier to use opp-level for the PCIe generation
instead.

This way this OPP entry can become:

opp-1 {
    opp-level = <1>;
    required-opps = <&rpmhpd_opp_low_svs>;
};

> +                                       opp-hz = /bits/ 64 <2500000>;
> +                                       required-opps = <&rpmhpd_opp_low_svs>;
> +                               };
> +
> +                               opp-5000000 {
> +                                       opp-hz = /bits/ 64 <5000000>;
> +                                       required-opps = <&rpmhpd_opp_low_svs>;
> +                               };
> +
> +                               opp-8000000 {
> +                                       opp-hz = /bits/ 64 <8000000>;
> +                                       required-opps = <&rpmhpd_opp_nom>;
> +                               };
> +                       };
>                 };
>
>                 pcie0_phy: phy@1c06000 {
> @@ -1915,7 +1936,33 @@
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&pcie1_default_state>;
>
> +                       operating-points-v2 = <&pcie1_opp_table>;
> +
>                         status = "disabled";
> +
> +                       pcie1_opp_table: opp-table {
> +                               compatible = "operating-points-v2";
> +
> +                               opp-2500000 {
> +                                       opp-hz = /bits/ 64 <2500000>;
> +                                       required-opps = <&rpmhpd_opp_low_svs>;
> +                               };
> +
> +                               opp-5000000 {
> +                                       opp-hz = /bits/ 64 <5000000>;
> +                                       required-opps = <&rpmhpd_opp_low_svs>;
> +                               };
> +
> +                               opp-8000000 {
> +                                       opp-hz = /bits/ 64 <8000000>;
> +                                       required-opps = <&rpmhpd_opp_low_svs>;
> +                               };
> +
> +                               opp-16000000 {
> +                                       opp-hz = /bits/ 64 <16000000>;
> +                                       required-opps = <&rpmhpd_opp_nom>;
> +                               };
> +                       };
>                 };
>
>                 pcie1_phy: phy@1c0f000 {
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] arm64: dts: qcom: sm8450: Add opp table support to PCIe

[Krishna Chaitanya Chundru]

On 8/16/2023 7:10 PM, Dmitry Baryshkov wrote:
> On Wed, 16 Aug 2023 at 16:25, Krishna chaitanya chundru
> <quic_krichai@quicinc.com> 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..c77a683 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -1803,7 +1803,28 @@
>>                          pinctrl-names = "default";
>>                          pinctrl-0 = <&pcie0_default_state>;
>>
>> +                       operating-points-v2 = <&pcie0_opp_table>;
>> +
>>                          status = "disabled";
>> +
>> +                       pcie0_opp_table: opp-table {
>> +                               compatible = "operating-points-v2";
>> +
>> +                               opp-2500000 {
> As a random suggestion: these frequencies are calculated by the
> driver. It might be easier to use opp-level for the PCIe generation
> instead.
>
> This way this OPP entry can become:
>
> opp-1 {
>      opp-level = <1>;
>      required-opps = <&rpmhpd_opp_low_svs>;
> };

This looks good I will try to do in this approach in the next patch if 
there are no other comments.

- KC

>
>> +                                       opp-hz = /bits/ 64 <2500000>;
>> +                                       required-opps = <&rpmhpd_opp_low_svs>;
>> +                               };
>> +
>> +                               opp-5000000 {
>> +                                       opp-hz = /bits/ 64 <5000000>;
>> +                                       required-opps = <&rpmhpd_opp_low_svs>;
>> +                               };
>> +
>> +                               opp-8000000 {
>> +                                       opp-hz = /bits/ 64 <8000000>;
>> +                                       required-opps = <&rpmhpd_opp_nom>;
>> +                               };
>> +                       };
>>                  };
>>
>>                  pcie0_phy: phy@1c06000 {
>> @@ -1915,7 +1936,33 @@
>>                          pinctrl-names = "default";
>>                          pinctrl-0 = <&pcie1_default_state>;
>>
>> +                       operating-points-v2 = <&pcie1_opp_table>;
>> +
>>                          status = "disabled";
>> +
>> +                       pcie1_opp_table: opp-table {
>> +                               compatible = "operating-points-v2";
>> +
>> +                               opp-2500000 {
>> +                                       opp-hz = /bits/ 64 <2500000>;
>> +                                       required-opps = <&rpmhpd_opp_low_svs>;
>> +                               };
>> +
>> +                               opp-5000000 {
>> +                                       opp-hz = /bits/ 64 <5000000>;
>> +                                       required-opps = <&rpmhpd_opp_low_svs>;
>> +                               };
>> +
>> +                               opp-8000000 {
>> +                                       opp-hz = /bits/ 64 <8000000>;
>> +                                       required-opps = <&rpmhpd_opp_low_svs>;
>> +                               };
>> +
>> +                               opp-16000000 {
>> +                                       opp-hz = /bits/ 64 <16000000>;
>> +                                       required-opps = <&rpmhpd_opp_nom>;
>> +                               };
>> +                       };
>>                  };
>>
>>                  pcie1_phy: phy@1c0f000 {
>> --
>> 2.7.4
>>
>

Re: [PATCH v2 1/3] dt-bindings: pci: qcom: Add opp table

[Krzysztof Kozlowski]

On 16/08/2023 15:24, Krishna chaitanya chundru wrote:
> PCIe needs to choose the appropriate performance state of RPMH power
> domain based upon the PCIe gen speed.
> 
> Adding the Operating Performance Points table allows to adjust power domain
> performance state, depending on the PCIe gen speed.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof