diff mbox series

[v10,4/6] arm64: dts: qcom: sm8450: Add OPP table support to PCIe

Message ID 20240409-opp_support-v10-4-1956e6be343f@quicinc.com (mailing list archive)
State Superseded
Headers show
Series PCI: qcom: Add support for OPP | expand

Commit Message

Krishna Chaitanya Chundru April 9, 2024, 10:13 a.m. UTC
PCIe needs to choose the appropriate performance state of RPMh power
domain and interconnect bandwidth based up on the PCIe data rate.

Add the OPP table support to specify RPMh performance states and
interconnect peak bandwidth.

Different link configurations may share the same aggregate bandwidth,
e.g., a 2.5 GT/s x2 link and a 5.0 GT/s x1 link have the same bandwidth
and share the same OPP entry.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 77 ++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

Comments

Manivannan Sadhasivam April 22, 2024, 2:44 p.m. UTC | #1
On Tue, Apr 09, 2024 at 03:43:22PM +0530, Krishna chaitanya chundru wrote:
> PCIe needs to choose the appropriate performance state of RPMh power

'PCIe host controller driver'

> domain and interconnect bandwidth based up on the PCIe data rate.

'based on the PCIe data rate'

> 
> Add the OPP table support to specify RPMh performance states and

'Hence, add...'

> interconnect peak bandwidth.
> 
> Different link configurations may share the same aggregate bandwidth,

'It should be noted that the different...'

> e.g., a 2.5 GT/s x2 link and a 5.0 GT/s x1 link have the same bandwidth
> and share the same OPP entry.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 77 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 615296e13c43..9dfe16012726 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -1855,7 +1855,35 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  			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";
> +
> +				/* GEN 1 x1 */
> +				opp-2500000 {
> +					opp-hz = /bits/ 64 <2500000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <250000 1>;
> +				};
> +
> +				/* GEN 2 x1 */
> +				opp-5000000 {
> +					opp-hz = /bits/ 64 <5000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <500000 1>;
> +				};
> +
> +				/* GEN 3 x1 */
> +				opp-8000000 {
> +					opp-hz = /bits/ 64 <8000000>;

I doubt this value. See below...

> +					required-opps = <&rpmhpd_opp_nom>;
> +					opp-peak-kBps = <984500 1>;
> +				};
> +			};
> +
>  		};
>  
>  		pcie0_phy: phy@1c06000 {
> @@ -1982,7 +2010,56 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  			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";
> +
> +				/* GEN 1 x1 */
> +				opp-2500000 {
> +					opp-hz = /bits/ 64 <2500000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <250000 1>;
> +				};
> +
> +				/* GEN 1 x2 GEN 2 x1 */
> +				opp-5000000 {
> +					opp-hz = /bits/ 64 <5000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <500000 1>;
> +				};
> +
> +				/* GEN 2 x2 */
> +				opp-10000000 {
> +					opp-hz = /bits/ 64 <10000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <1000000 1>;
> +				};
> +
> +				/* GEN 3 x1 */
> +				opp-8000000 {
> +					opp-hz = /bits/ 64 <8000000>;

GEN 3 x1 frequency is lower than GEN 2 x2? This looks strange. Both should be of
same frequency.

> +					required-opps = <&rpmhpd_opp_nom>;
> +					opp-peak-kBps = <984500 1>;
> +				};
> +
> +				/* GEN 3 x2 GEN 4 x1 */

'GEN 3 x2 and GEN 4 x1'

- Mani
Krishna Chaitanya Chundru April 22, 2024, 4:55 p.m. UTC | #2
On 4/22/2024 8:14 PM, Manivannan Sadhasivam wrote:
> On Tue, Apr 09, 2024 at 03:43:22PM +0530, Krishna chaitanya chundru wrote:
>> PCIe needs to choose the appropriate performance state of RPMh power
> 
> 'PCIe host controller driver'
> 
>> domain and interconnect bandwidth based up on the PCIe data rate.
> 
> 'based on the PCIe data rate'
> 
>>
>> Add the OPP table support to specify RPMh performance states and
> 
> 'Hence, add...'
> 
>> interconnect peak bandwidth.
>>
>> Different link configurations may share the same aggregate bandwidth,
> 
> 'It should be noted that the different...'
> 
>> e.g., a 2.5 GT/s x2 link and a 5.0 GT/s x1 link have the same bandwidth
>> and share the same OPP entry.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8450.dtsi | 77 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 77 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 615296e13c43..9dfe16012726 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -1855,7 +1855,35 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>   			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";
>> +
>> +				/* GEN 1 x1 */
>> +				opp-2500000 {
>> +					opp-hz = /bits/ 64 <2500000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <250000 1>;
>> +				};
>> +
>> +				/* GEN 2 x1 */
>> +				opp-5000000 {
>> +					opp-hz = /bits/ 64 <5000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <500000 1>;
>> +				};
>> +
>> +				/* GEN 3 x1 */
>> +				opp-8000000 {
>> +					opp-hz = /bits/ 64 <8000000>;
> 
> I doubt this value. See below...
> 
>> +					required-opps = <&rpmhpd_opp_nom>;
>> +					opp-peak-kBps = <984500 1>;
>> +				};
>> +			};
>> +
>>   		};
>>   
>>   		pcie0_phy: phy@1c06000 {
>> @@ -1982,7 +2010,56 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>   			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";
>> +
>> +				/* GEN 1 x1 */
>> +				opp-2500000 {
>> +					opp-hz = /bits/ 64 <2500000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <250000 1>;
>> +				};
>> +
>> +				/* GEN 1 x2 GEN 2 x1 */
>> +				opp-5000000 {
>> +					opp-hz = /bits/ 64 <5000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <500000 1>;
>> +				};
>> +
>> +				/* GEN 2 x2 */
>> +				opp-10000000 {
>> +					opp-hz = /bits/ 64 <10000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <1000000 1>;
>> +				};
>> +
>> +				/* GEN 3 x1 */
>> +				opp-8000000 {
>> +					opp-hz = /bits/ 64 <8000000>;
> 
> GEN 3 x1 frequency is lower than GEN 2 x2? This looks strange. Both should be of
> same frequency.
> 
Gen2 is 5GT/s where as GEN3 is 8GT/s. so the freq for 3 x1(8 x1 GT/s) is
less than Gen2 x2(5 x2 GT/s)

- Krishna Chaitanya.
>> +					required-opps = <&rpmhpd_opp_nom>;
>> +					opp-peak-kBps = <984500 1>;
>> +				};
>> +
>> +				/* GEN 3 x2 GEN 4 x1 */
> 
> 'GEN 3 x2 and GEN 4 x1'
> 
> - Mani
>
Manivannan Sadhasivam April 22, 2024, 5:08 p.m. UTC | #3
On Mon, Apr 22, 2024 at 10:25:06PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 4/22/2024 8:14 PM, Manivannan Sadhasivam wrote:
> > On Tue, Apr 09, 2024 at 03:43:22PM +0530, Krishna chaitanya chundru wrote:
> > > PCIe needs to choose the appropriate performance state of RPMh power
> > 
> > 'PCIe host controller driver'
> > 
> > > domain and interconnect bandwidth based up on the PCIe data rate.
> > 
> > 'based on the PCIe data rate'
> > 
> > > 
> > > Add the OPP table support to specify RPMh performance states and
> > 
> > 'Hence, add...'
> > 
> > > interconnect peak bandwidth.
> > > 
> > > Different link configurations may share the same aggregate bandwidth,
> > 
> > 'It should be noted that the different...'
> > 
> > > e.g., a 2.5 GT/s x2 link and a 5.0 GT/s x1 link have the same bandwidth
> > > and share the same OPP entry.
> > > 
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > > ---
> > >   arch/arm64/boot/dts/qcom/sm8450.dtsi | 77 ++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 77 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > index 615296e13c43..9dfe16012726 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > @@ -1855,7 +1855,35 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> > >   			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";
> > > +
> > > +				/* GEN 1 x1 */
> > > +				opp-2500000 {
> > > +					opp-hz = /bits/ 64 <2500000>;
> > > +					required-opps = <&rpmhpd_opp_low_svs>;
> > > +					opp-peak-kBps = <250000 1>;
> > > +				};
> > > +
> > > +				/* GEN 2 x1 */
> > > +				opp-5000000 {
> > > +					opp-hz = /bits/ 64 <5000000>;
> > > +					required-opps = <&rpmhpd_opp_low_svs>;
> > > +					opp-peak-kBps = <500000 1>;
> > > +				};
> > > +
> > > +				/* GEN 3 x1 */
> > > +				opp-8000000 {
> > > +					opp-hz = /bits/ 64 <8000000>;
> > 
> > I doubt this value. See below...
> > 
> > > +					required-opps = <&rpmhpd_opp_nom>;
> > > +					opp-peak-kBps = <984500 1>;
> > > +				};
> > > +			};
> > > +
> > >   		};
> > >   		pcie0_phy: phy@1c06000 {
> > > @@ -1982,7 +2010,56 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> > >   			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";
> > > +
> > > +				/* GEN 1 x1 */
> > > +				opp-2500000 {
> > > +					opp-hz = /bits/ 64 <2500000>;
> > > +					required-opps = <&rpmhpd_opp_low_svs>;
> > > +					opp-peak-kBps = <250000 1>;
> > > +				};
> > > +
> > > +				/* GEN 1 x2 GEN 2 x1 */
> > > +				opp-5000000 {
> > > +					opp-hz = /bits/ 64 <5000000>;
> > > +					required-opps = <&rpmhpd_opp_low_svs>;
> > > +					opp-peak-kBps = <500000 1>;
> > > +				};
> > > +
> > > +				/* GEN 2 x2 */
> > > +				opp-10000000 {
> > > +					opp-hz = /bits/ 64 <10000000>;
> > > +					required-opps = <&rpmhpd_opp_low_svs>;
> > > +					opp-peak-kBps = <1000000 1>;
> > > +				};
> > > +
> > > +				/* GEN 3 x1 */
> > > +				opp-8000000 {
> > > +					opp-hz = /bits/ 64 <8000000>;
> > 
> > GEN 3 x1 frequency is lower than GEN 2 x2? This looks strange. Both should be of
> > same frequency.
> > 
> Gen2 is 5GT/s where as GEN3 is 8GT/s. so the freq for 3 x1(8 x1 GT/s) is
> less than Gen2 x2(5 x2 GT/s)
> 

Sorry, that's my bad. I missed the fact that the spec doubled the data rate
starting from GEN 3 only.

- Mani
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 615296e13c43..9dfe16012726 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -1855,7 +1855,35 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			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";
+
+				/* GEN 1 x1 */
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <250000 1>;
+				};
+
+				/* GEN 2 x1 */
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <500000 1>;
+				};
+
+				/* GEN 3 x1 */
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <984500 1>;
+				};
+			};
+
 		};
 
 		pcie0_phy: phy@1c06000 {
@@ -1982,7 +2010,56 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			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";
+
+				/* GEN 1 x1 */
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <250000 1>;
+				};
+
+				/* GEN 1 x2 GEN 2 x1 */
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <500000 1>;
+				};
+
+				/* GEN 2 x2 */
+				opp-10000000 {
+					opp-hz = /bits/ 64 <10000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <1000000 1>;
+				};
+
+				/* GEN 3 x1 */
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <984500 1>;
+				};
+
+				/* GEN 3 x2 GEN 4 x1 */
+				opp-16000000 {
+					opp-hz = /bits/ 64 <16000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <1969000 1>;
+				};
+
+				/* GEN 4 x2 */
+				opp-32000000 {
+					opp-hz = /bits/ 64 <32000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <3938000 1>;
+				};
+			};
+
 		};
 
 		pcie1_phy: phy@1c0e000 {