[PATCH v2 0/3] Qcom: Add GIC-ITS support to SM8450 PCIe controllers

[PATCH v2 0/3] Qcom: Add GIC-ITS support to SM8450 PCIe controllers

[Manivannan Sadhasivam]

Hello,

This series adds GIC-ITS support to SM8450 PCIe controllers for receiving
the MSIs from endpoint devices.

The GIC-ITS MSI implementation provides an advantage over internal MSI
implementation using Locality-specific Peripheral Interrupts (LPI) that
would allow MSIs to be targeted for each CPU core.

This series has been tested on SM8450 based dev board that works using an
out-of-tree dts where the MSIs from endpoint devices are distributed across
the CPU cores.

Thanks,
Mani

Changes in v2:

* Swapped the Device ID for PCIe0 as it causes same issue as PCIe1
* Removed the definition of msi-map and msi-map-mask from binding
* Added Ack from Krzysztof

Manivannan Sadhasivam (3):
  dt-bindings: PCI: qcom: Update maintainers
  dt-bindings: PCI: qcom: Document msi-map and msi-map-mask properties
  arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

 .../devicetree/bindings/pci/qcom,pcie.yaml    | 14 +++++++++----
 arch/arm64/boot/dts/qcom/sm8450.dtsi          | 20 +++++++++++++------
 2 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] dt-bindings: PCI: qcom: Update maintainers

[Manivannan Sadhasivam]

Stanimir has left mm-sol and already expressed his wish to not continue
maintaining the PCIe RC driver. So his entry can be removed.

Adding myself as the co-maintainer since I took over the PCIe RC driver
maintainership from Stanimir.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 54f07852d279..02450fb26bb9 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -8,7 +8,7 @@ title: Qualcomm PCI express root complex
 
 maintainers:
   - Bjorn Andersson <bjorn.andersson@linaro.org>
-  - Stanimir Varbanov <svarbanov@mm-sol.com>
+  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
 
 description: |
   Qualcomm PCIe root complex controller is based on the Synopsys DesignWare
-- 
2.25.1

[PATCH v2 2/3] dt-bindings: PCI: qcom: Document msi-map and msi-map-mask properties

[Manivannan Sadhasivam]

The Qcom PCIe controller is capable of using either internal MSI controller
or the external GIC-ITS for receiving the MSIs from endpoint devices.
Currently, the binding only documents the internal MSI implementation.

Let's document the GIC-ITS imeplementation by making use of msi-map and
msi-map-mask properties. Only one of the implementation should be used
at a time.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 02450fb26bb9..10fec6a7abfc 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -104,14 +104,20 @@ required:
   - compatible
   - reg
   - reg-names
-  - interrupts
-  - interrupt-names
-  - "#interrupt-cells"
   - interrupt-map-mask
   - interrupt-map
   - clocks
   - clock-names
 
+oneOf:
+  - required:
+      - interrupts
+      - interrupt-names
+      - "#interrupt-cells"
+  - required:
+      - msi-map
+      - msi-map-mask
+
 allOf:
   - $ref: /schemas/pci/pci-bus.yaml#
   - if:
-- 
2.25.1

[PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Manivannan Sadhasivam]

Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
endpoint devices using GIC-ITS MSI controller. Add support for it.

Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
msi-map-mask of 0xff00, all the 32 devices under these two busses can
share the same Device ID.

The GIC-ITS MSI implementation provides an advantage over internal MSI
implementation using Locality-specific Peripheral Interrupts (LPI) that
would allow MSIs to be targeted for each CPU core.

It should be noted that the MSIs for BDF (1:0.0) only works with Device
ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 570475040d95..c4dd5838fac6 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
 			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
 				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
 
-			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi";
-			#interrupt-cells = <1>;
+			/*
+			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
+			 * Hence, the IDs are swapped.
+			 */
+			msi-map = <0x0 &gic_its 0x5981 0x1>,
+				  <0x100 &gic_its 0x5980 0x1>;
+			msi-map-mask = <0xff00>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
 					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
@@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
 			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
 				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
 
-			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi";
-			#interrupt-cells = <1>;
+			/*
+			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
+			 * Hence, the IDs are swapped.
+			 */
+			msi-map = <0x0 &gic_its 0x5a01 0x1>,
+				  <0x100 &gic_its 0x5a00 0x1>;
+			msi-map-mask = <0xff00>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
 					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
-- 
2.25.1

Re: [PATCH v2 2/3] dt-bindings: PCI: qcom: Document msi-map and msi-map-mask properties

[Rob Herring]

On Thu, Dec 22, 2022 at 07:01:22PM +0530, Manivannan Sadhasivam wrote:
> The Qcom PCIe controller is capable of using either internal MSI controller
> or the external GIC-ITS for receiving the MSIs from endpoint devices.
> Currently, the binding only documents the internal MSI implementation.
> 
> Let's document the GIC-ITS imeplementation by making use of msi-map and
> msi-map-mask properties. Only one of the implementation should be used
> at a time.

Isn't that up to the OS to decide? Some versions may not support MSIs.

What about legacy interrupts? Don't you need to keep the interrupt 
properties for them?

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 02450fb26bb9..10fec6a7abfc 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -104,14 +104,20 @@ required:
>    - compatible
>    - reg
>    - reg-names
> -  - interrupts
> -  - interrupt-names
> -  - "#interrupt-cells"
>    - interrupt-map-mask
>    - interrupt-map
>    - clocks
>    - clock-names
>  
> +oneOf:
> +  - required:
> +      - interrupts
> +      - interrupt-names
> +      - "#interrupt-cells"
> +  - required:
> +      - msi-map
> +      - msi-map-mask
> +
>  allOf:
>    - $ref: /schemas/pci/pci-bus.yaml#
>    - if:
> -- 
> 2.25.1
> 
> 

Re: [PATCH v2 2/3] dt-bindings: PCI: qcom: Document msi-map and msi-map-mask properties

[Manivannan Sadhasivam]

On Thu, Dec 22, 2022 at 01:01:41PM -0600, Rob Herring wrote:
> On Thu, Dec 22, 2022 at 07:01:22PM +0530, Manivannan Sadhasivam wrote:
> > The Qcom PCIe controller is capable of using either internal MSI controller
> > or the external GIC-ITS for receiving the MSIs from endpoint devices.
> > Currently, the binding only documents the internal MSI implementation.
> > 
> > Let's document the GIC-ITS imeplementation by making use of msi-map and
> > msi-map-mask properties. Only one of the implementation should be used
> > at a time.
> 
> Isn't that up to the OS to decide? Some versions may not support MSIs.
> 

Yes, OS may choose either of them but the controller supports both and only one
implementation can be used at a time.

AFAIK, all of the SoCs supported in upstream support both MSI and legacy
interrupts.

> What about legacy interrupts? Don't you need to keep the interrupt 
> properties for them?
> 

We have "interrupt-map-mask" and "interrupt-map" properties for legacy
interrupts.

Thanks,
Mani

> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index 02450fb26bb9..10fec6a7abfc 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -104,14 +104,20 @@ required:
> >    - compatible
> >    - reg
> >    - reg-names
> > -  - interrupts
> > -  - interrupt-names
> > -  - "#interrupt-cells"
> >    - interrupt-map-mask
> >    - interrupt-map
> >    - clocks
> >    - clock-names
> >  
> > +oneOf:
> > +  - required:
> > +      - interrupts
> > +      - interrupt-names
> > +      - "#interrupt-cells"
> > +  - required:
> > +      - msi-map
> > +      - msi-map-mask
> > +
> >  allOf:
> >    - $ref: /schemas/pci/pci-bus.yaml#
> >    - if:
> > -- 
> > 2.25.1
> > 
> > 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Dmitry Baryshkov]

On 22/12/2022 15:31, Manivannan Sadhasivam wrote:
> Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
> endpoint devices using GIC-ITS MSI controller. Add support for it.
> 
> Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
> msi-map-mask of 0xff00, all the 32 devices under these two busses can
> share the same Device ID.
> 
> The GIC-ITS MSI implementation provides an advantage over internal MSI
> implementation using Locality-specific Peripheral Interrupts (LPI) that
> would allow MSIs to be targeted for each CPU core.
> 
> It should be noted that the MSIs for BDF (1:0.0) only works with Device
> ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 570475040d95..c4dd5838fac6 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
>   			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
>   				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
>   
> -			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
> -			interrupt-names = "msi";
> -			#interrupt-cells = <1>;
> +			/*
> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
> +			 * Hence, the IDs are swapped.
> +			 */
> +			msi-map = <0x0 &gic_its 0x5981 0x1>,
> +				  <0x100 &gic_its 0x5980 0x1>;

This definitely doesn't match what has been used in the downstream.

Also if I understand correctly this change would prevent us from using 
multiple MSI interrupts for the connected device, as the last value of 
the 0x100 mapping is 0x1, while the vendor kernel uses <0x100 &its 
0x5981 0x20>.

Do you know by chance, why do we differ from the vendor dtsi?

> +			msi-map-mask = <0xff00>;
>   			interrupt-map-mask = <0 0 0 0x7>;
>   			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>   					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> @@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
>   			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
>   				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
>   
> -			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
> -			interrupt-names = "msi";
> -			#interrupt-cells = <1>;
> +			/*
> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
> +			 * Hence, the IDs are swapped.
> +			 */
> +			msi-map = <0x0 &gic_its 0x5a01 0x1>,
> +				  <0x100 &gic_its 0x5a00 0x1>;
> +			msi-map-mask = <0xff00>;
>   			interrupt-map-mask = <0 0 0 0x7>;
>   			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>   					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Manivannan Sadhasivam]

On Fri, Dec 23, 2022 at 07:18:32PM +0200, Dmitry Baryshkov wrote:
> On 22/12/2022 15:31, Manivannan Sadhasivam wrote:
> > Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
> > endpoint devices using GIC-ITS MSI controller. Add support for it.
> > 
> > Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
> > msi-map-mask of 0xff00, all the 32 devices under these two busses can
> > share the same Device ID.
> > 
> > The GIC-ITS MSI implementation provides an advantage over internal MSI
> > implementation using Locality-specific Peripheral Interrupts (LPI) that
> > would allow MSIs to be targeted for each CPU core.
> > 
> > It should be noted that the MSIs for BDF (1:0.0) only works with Device
> > ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
> >   1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > index 570475040d95..c4dd5838fac6 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > @@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
> >   			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
> >   				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
> > -			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
> > -			interrupt-names = "msi";
> > -			#interrupt-cells = <1>;
> > +			/*
> > +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
> > +			 * Hence, the IDs are swapped.
> > +			 */
> > +			msi-map = <0x0 &gic_its 0x5981 0x1>,
> > +				  <0x100 &gic_its 0x5980 0x1>;
> 
> This definitely doesn't match what has been used in the downstream.
> 

Yes, I do not know why the downstream Device ID doesn't work. I tried finding
the answer within Qcom but didn't get any answer so far :/ So I just went with
the value that works on multiple boards.

> Also if I understand correctly this change would prevent us from using
> multiple MSI interrupts for the connected device, as the last value of the
> 0x100 mapping is 0x1, while the vendor kernel uses <0x100 &its 0x5981 0x20>.
> 

Not true. The controller can still support multiple MSIs for the endpoint
devices but the only difference is, it would use the same Device ID for all.

The Qcom GIC-ITS implementation could only support 32 Device IDs. By specifying
the size of 0x20, a separate Device ID would be used for each devices of bus 1.
But if a PCIe switch is connected and the bus count becomes > 1, then the MSI
allocation would fail because Device IDs are exhausted.

The downstream implementation just assumes that there will be only bus 1 and I
do not want to follow that assumption.

That's why I used "msi-map-mask" property of value "0xff00" here, as that will
allow all the devices under the bus 1 to share the same Device ID. For now I
only mapped bus 1, but extending that in the future for other busses is simple.

Thanks,
Mani

> Do you know by chance, why do we differ from the vendor dtsi?
> 
> > +			msi-map-mask = <0xff00>;
> >   			interrupt-map-mask = <0 0 0 0x7>;
> >   			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> >   					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> > @@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
> >   			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
> >   				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
> > -			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
> > -			interrupt-names = "msi";
> > -			#interrupt-cells = <1>;
> > +			/*
> > +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
> > +			 * Hence, the IDs are swapped.
> > +			 */
> > +			msi-map = <0x0 &gic_its 0x5a01 0x1>,
> > +				  <0x100 &gic_its 0x5a00 0x1>;
> > +			msi-map-mask = <0xff00>;
> >   			interrupt-map-mask = <0 0 0 0x7>;
> >   			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> >   					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> 
> -- 
> With best wishes
> Dmitry
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Dmitry Baryshkov]

On 23/12/2022 19:45, Manivannan Sadhasivam wrote:
> On Fri, Dec 23, 2022 at 07:18:32PM +0200, Dmitry Baryshkov wrote:
>> On 22/12/2022 15:31, Manivannan Sadhasivam wrote:
>>> Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
>>> endpoint devices using GIC-ITS MSI controller. Add support for it.
>>>
>>> Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
>>> msi-map-mask of 0xff00, all the 32 devices under these two busses can
>>> share the same Device ID.
>>>
>>> The GIC-ITS MSI implementation provides an advantage over internal MSI
>>> implementation using Locality-specific Peripheral Interrupts (LPI) that
>>> would allow MSIs to be targeted for each CPU core.
>>>
>>> It should be noted that the MSIs for BDF (1:0.0) only works with Device
>>> ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.
>>>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>> ---
>>>    arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
>>>    1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> index 570475040d95..c4dd5838fac6 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> @@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
>>>    			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
>>>    				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
>>> -			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
>>> -			interrupt-names = "msi";
>>> -			#interrupt-cells = <1>;
>>> +			/*
>>> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
>>> +			 * Hence, the IDs are swapped.
>>> +			 */
>>> +			msi-map = <0x0 &gic_its 0x5981 0x1>,
>>> +				  <0x100 &gic_its 0x5980 0x1>;
>>
>> This definitely doesn't match what has been used in the downstream.
>>
> 
> Yes, I do not know why the downstream Device ID doesn't work. I tried finding
> the answer within Qcom but didn't get any answer so far :/ So I just went with
> the value that works on multiple boards.

Ugh :-(

> 
>> Also if I understand correctly this change would prevent us from using
>> multiple MSI interrupts for the connected device, as the last value of the
>> 0x100 mapping is 0x1, while the vendor kernel uses <0x100 &its 0x5981 0x20>.
>>
> 
> Not true. The controller can still support multiple MSIs for the endpoint
> devices but the only difference is, it would use the same Device ID for all.

I see, please excuse me then. But don't we have to define multiple MSI 
vectors here too?

> 
> The Qcom GIC-ITS implementation could only support 32 Device IDs. By specifying
> the size of 0x20, a separate Device ID would be used for each devices of bus 1.
> But if a PCIe switch is connected and the bus count becomes > 1, then the MSI
> allocation would fail because Device IDs are exhausted.
> 
> The downstream implementation just assumes that there will be only bus 1 and I
> do not want to follow that assumption.
> 
> That's why I used "msi-map-mask" property of value "0xff00" here, as that will
> allow all the devices under the bus 1 to share the same Device ID. For now I
> only mapped bus 1, but extending that in the future for other busses is simple.
> 
> Thanks,
> Mani
> 
>> Do you know by chance, why do we differ from the vendor dtsi?
>>
>>> +			msi-map-mask = <0xff00>;
>>>    			interrupt-map-mask = <0 0 0 0x7>;
>>>    			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>>    					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>> @@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
>>>    			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
>>>    				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
>>> -			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
>>> -			interrupt-names = "msi";
>>> -			#interrupt-cells = <1>;
>>> +			/*
>>> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
>>> +			 * Hence, the IDs are swapped.
>>> +			 */
>>> +			msi-map = <0x0 &gic_its 0x5a01 0x1>,
>>> +				  <0x100 &gic_its 0x5a00 0x1>;
>>> +			msi-map-mask = <0xff00>;
>>>    			interrupt-map-mask = <0 0 0 0x7>;
>>>    			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>>    					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>
>> -- 
>> With best wishes
>> Dmitry
>>
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Manivannan Sadhasivam]

On Fri, Dec 23, 2022 at 08:15:32PM +0200, Dmitry Baryshkov wrote:
> On 23/12/2022 19:45, Manivannan Sadhasivam wrote:
> > On Fri, Dec 23, 2022 at 07:18:32PM +0200, Dmitry Baryshkov wrote:
> > > On 22/12/2022 15:31, Manivannan Sadhasivam wrote:
> > > > Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
> > > > endpoint devices using GIC-ITS MSI controller. Add support for it.
> > > > 
> > > > Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
> > > > msi-map-mask of 0xff00, all the 32 devices under these two busses can
> > > > share the same Device ID.
> > > > 
> > > > The GIC-ITS MSI implementation provides an advantage over internal MSI
> > > > implementation using Locality-specific Peripheral Interrupts (LPI) that
> > > > would allow MSIs to be targeted for each CPU core.
> > > > 
> > > > It should be noted that the MSIs for BDF (1:0.0) only works with Device
> > > > ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.
> > > > 
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > ---
> > > >    arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
> > > >    1 file changed, 14 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > > index 570475040d95..c4dd5838fac6 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > > @@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
> > > >    			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
> > > >    				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
> > > > -			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
> > > > -			interrupt-names = "msi";
> > > > -			#interrupt-cells = <1>;
> > > > +			/*
> > > > +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
> > > > +			 * Hence, the IDs are swapped.
> > > > +			 */
> > > > +			msi-map = <0x0 &gic_its 0x5981 0x1>,
> > > > +				  <0x100 &gic_its 0x5980 0x1>;
> > > 
> > > This definitely doesn't match what has been used in the downstream.
> > > 
> > 
> > Yes, I do not know why the downstream Device ID doesn't work. I tried finding
> > the answer within Qcom but didn't get any answer so far :/ So I just went with
> > the value that works on multiple boards.
> 
> Ugh :-(
> 
> > 
> > > Also if I understand correctly this change would prevent us from using
> > > multiple MSI interrupts for the connected device, as the last value of the
> > > 0x100 mapping is 0x1, while the vendor kernel uses <0x100 &its 0x5981 0x20>.
> > > 
> > 
> > Not true. The controller can still support multiple MSIs for the endpoint
> > devices but the only difference is, it would use the same Device ID for all.
> 
> I see, please excuse me then. But don't we have to define multiple MSI
> vectors here too?
> 

No, it is not required. GIC-ITS driver will handle the MSI mapping internally
and devicetree only needs to specify the Device ID for each PCIe device.

Thanks,
Mani

> > 
> > The Qcom GIC-ITS implementation could only support 32 Device IDs. By specifying
> > the size of 0x20, a separate Device ID would be used for each devices of bus 1.
> > But if a PCIe switch is connected and the bus count becomes > 1, then the MSI
> > allocation would fail because Device IDs are exhausted.
> > 
> > The downstream implementation just assumes that there will be only bus 1 and I
> > do not want to follow that assumption.
> > 
> > That's why I used "msi-map-mask" property of value "0xff00" here, as that will
> > allow all the devices under the bus 1 to share the same Device ID. For now I
> > only mapped bus 1, but extending that in the future for other busses is simple.
> > 
> > Thanks,
> > Mani
> > 
> > > Do you know by chance, why do we differ from the vendor dtsi?
> > > 
> > > > +			msi-map-mask = <0xff00>;
> > > >    			interrupt-map-mask = <0 0 0 0x7>;
> > > >    			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> > > >    					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> > > > @@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
> > > >    			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
> > > >    				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
> > > > -			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
> > > > -			interrupt-names = "msi";
> > > > -			#interrupt-cells = <1>;
> > > > +			/*
> > > > +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
> > > > +			 * Hence, the IDs are swapped.
> > > > +			 */
> > > > +			msi-map = <0x0 &gic_its 0x5a01 0x1>,
> > > > +				  <0x100 &gic_its 0x5a00 0x1>;
> > > > +			msi-map-mask = <0xff00>;
> > > >    			interrupt-map-mask = <0 0 0 0x7>;
> > > >    			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> > > >    					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> > > 
> > > -- 
> > > With best wishes
> > > Dmitry
> > > 
> > 
> 
> -- 
> With best wishes
> Dmitry
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 2/3] dt-bindings: PCI: qcom: Document msi-map and msi-map-mask properties

[Lorenzo Pieralisi]

On Thu, Dec 22, 2022 at 07:01:22PM +0530, Manivannan Sadhasivam wrote:
> The Qcom PCIe controller is capable of using either internal MSI controller
> or the external GIC-ITS for receiving the MSIs from endpoint devices.

"For signaling MSIs sent by endpoint devices"

> Currently, the binding only documents the internal MSI implementation.
> 
> Let's document the GIC-ITS imeplementation by making use of msi-map and
> msi-map-mask properties. Only one of the implementation should be used
> at a time.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 02450fb26bb9..10fec6a7abfc 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -104,14 +104,20 @@ required:
>    - compatible
>    - reg
>    - reg-names
> -  - interrupts
> -  - interrupt-names
> -  - "#interrupt-cells"
>    - interrupt-map-mask
>    - interrupt-map
>    - clocks
>    - clock-names
>  
> +oneOf:
> +  - required:
> +      - interrupts
> +      - interrupt-names
> +      - "#interrupt-cells"
> +  - required:
> +      - msi-map
> +      - msi-map-mask
> +
>  allOf:
>    - $ref: /schemas/pci/pci-bus.yaml#
>    - if:
> -- 
> 2.25.1
> 

Re: [PATCH v2 3/3] arm64: dts: qcom: sm8450: Use GIC-ITS for PCIe0 and PCIe1

[Lorenzo Pieralisi]

On Thu, Dec 22, 2022 at 07:01:23PM +0530, Manivannan Sadhasivam wrote:
> Both PCIe0 and PCIe1 controllers are capable of receiving MSIs from
> endpoint devices using GIC-ITS MSI controller. Add support for it.

Nit: the PCI controllers don't receive MSIs using the GIC-ITS MSI
controller, they signal MSIs interrupts using the GIC-ITS controller.

> 
> Currently, BDF (0:0.0) and BDF (1:0.0) are enabled and with the
> msi-map-mask of 0xff00, all the 32 devices under these two busses can
> share the same Device ID.
> 
> The GIC-ITS MSI implementation provides an advantage over internal MSI
> implementation using Locality-specific Peripheral Interrupts (LPI) that
> would allow MSIs to be targeted for each CPU core.
> 
> It should be noted that the MSIs for BDF (1:0.0) only works with Device
> ID of 0x5980 and 0x5a00. Hence, the IDs are swapped.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 570475040d95..c4dd5838fac6 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -1733,9 +1733,13 @@ pcie0: pci@1c00000 {
>  			ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>,
>  				 <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>;
>  
> -			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
> -			interrupt-names = "msi";
> -			#interrupt-cells = <1>;
> +			/*
> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5980.
> +			 * Hence, the IDs are swapped.
> +			 */
> +			msi-map = <0x0 &gic_its 0x5981 0x1>,
> +				  <0x100 &gic_its 0x5980 0x1>;
> +			msi-map-mask = <0xff00>;
>  			interrupt-map-mask = <0 0 0 0x7>;
>  			interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>  					<0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> @@ -1842,9 +1846,13 @@ pcie1: pci@1c08000 {
>  			ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>,
>  				 <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>;
>  
> -			interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
> -			interrupt-names = "msi";
> -			#interrupt-cells = <1>;
> +			/*
> +			 * MSIs for BDF (1:0.0) only works with Device ID 0x5a00.
> +			 * Hence, the IDs are swapped.
> +			 */
> +			msi-map = <0x0 &gic_its 0x5a01 0x1>,
> +				  <0x100 &gic_its 0x5a00 0x1>;
> +			msi-map-mask = <0xff00>;
>  			interrupt-map-mask = <0 0 0 0x7>;
>  			interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>  					<0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> -- 
> 2.25.1
>