| Message ID | 20230724124711.2346886-2-quic_ipkumar@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] PCI: qcom: Add early fixup to set the max payload size for IPQ9574 | expand |
On 7/25/2023 11:36 AM, Manivannan Sadhasivam wrote: > On Tue, Jul 25, 2023 at 10:16:04AM +0530, Praveenkumar I wrote: >> On 7/24/2023 7:39 PM, Manivannan Sadhasivam wrote: >>> On Mon, Jul 24, 2023 at 06:38:55PM +0530, Manivannan Sadhasivam wrote: >>>> On Mon, Jul 24, 2023 at 02:53:37PM +0200, Konrad Dybcio wrote: >>>>> On 24.07.2023 14:47, Praveenkumar I wrote: >>>>>> Set 256 bytes as payload size for IPQ9574 via early fixup. This allows >>>>>> PCIe RC to use the max payload size when a capable link partner is >>>>>> connected. >>>>>> >>>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> >>>>>> --- >>>>> [...] >>>>> >>>>>> +static void qcom_fixup_mps_256(struct pci_dev *dev) >>>>>> +{ >>>>>> + pcie_set_mps(dev, 256); >>>>> Looks like setting "dev->pcie_mpss = 1" here would make the PCIe generic >>>>> code take care of this. >>>>> >>>> Right, also this setting should not be PCI-PCI bridge specific but rather >>>> controller specific. >>>> >>> Wait, have you tested this patch with PCIe devices having MPS < 256 i.e., >>> default 128? >>> >>> Take a look at this discussion: https://lore.kernel.org/all/20230608093652.1409485-1-vidyas@nvidia.com/ >>> >>> - Mani >> Yes, tested this patch with PCIe devices having default 128 and RC is >> falling back to 128 when pci device is added. >> This is handled inside pci_configure_mps(). >> / mpss = 128 << dev->pcie_mpss;/ >> / if (mpss < p_mps && pci_pcie_type(bridge) == >> PCI_EXP_TYPE_ROOT_PORT) {/ >> / pcie_set_mps(bridge, mpss);/ >> / pci_info(dev, "Upstream bridge's Max Payload Size set to %d >> (was %d, max %d)\n",/ >> / mpss, p_mps, 128 << bridge->pcie_mpss);/ >> / p_mps = pcie_get_mps(bridge);/ >> / }/ >> // >> Also getting the below print, >> /[ 2.011963] pci 0003:01:00.0: Upstream bridge's Max Payload Size set to >> 128 (was 256, max 256)/ > Ok. But for setting MPS, you need to change the DEVCTL register in post_init > sequence for IPQ9574. It is not a quirk, so you cannot use fixups. Sure, will add in post_init of IPQ9574. -- Thanks, Praveenkumar > > - Mani > >>>> - Mani >>>> >>>>> Konrad >>>> -- >>>> மணிவண்ணன் சதாசிவம் >> -- >> Thanks, >> Praveenkumar
On 7/25/2023 11:36 AM, Manivannan Sadhasivam wrote: > On Tue, Jul 25, 2023 at 10:16:04AM +0530, Praveenkumar I wrote: >> On 7/24/2023 7:39 PM, Manivannan Sadhasivam wrote: >>> On Mon, Jul 24, 2023 at 06:38:55PM +0530, Manivannan Sadhasivam wrote: >>>> On Mon, Jul 24, 2023 at 02:53:37PM +0200, Konrad Dybcio wrote: >>>>> On 24.07.2023 14:47, Praveenkumar I wrote: >>>>>> Set 256 bytes as payload size for IPQ9574 via early fixup. This allows >>>>>> PCIe RC to use the max payload size when a capable link partner is >>>>>> connected. >>>>>> >>>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> >>>>>> --- >>>>> [...] >>>>> >>>>>> +static void qcom_fixup_mps_256(struct pci_dev *dev) >>>>>> +{ >>>>>> + pcie_set_mps(dev, 256); >>>>> Looks like setting "dev->pcie_mpss = 1" here would make the PCIe generic >>>>> code take care of this. >>>>> >>>> Right, also this setting should not be PCI-PCI bridge specific but rather >>>> controller specific. >>>> >>> Wait, have you tested this patch with PCIe devices having MPS < 256 i.e., >>> default 128? >>> >>> Take a look at this discussion: https://lore.kernel.org/all/20230608093652.1409485-1-vidyas@nvidia.com/ >>> >>> - Mani >> Yes, tested this patch with PCIe devices having default 128 and RC is >> falling back to 128 when pci device is added. >> This is handled inside pci_configure_mps(). >> / mpss = 128 << dev->pcie_mpss;/ >> / if (mpss < p_mps && pci_pcie_type(bridge) == >> PCI_EXP_TYPE_ROOT_PORT) {/ >> / pcie_set_mps(bridge, mpss);/ >> / pci_info(dev, "Upstream bridge's Max Payload Size set to %d >> (was %d, max %d)\n",/ >> / mpss, p_mps, 128 << bridge->pcie_mpss);/ >> / p_mps = pcie_get_mps(bridge);/ >> / }/ >> // >> Also getting the below print, >> /[ 2.011963] pci 0003:01:00.0: Upstream bridge's Max Payload Size set to >> 128 (was 256, max 256)/ > Ok. But for setting MPS, you need to change the DEVCTL register in post_init > sequence for IPQ9574. It is not a quirk, so you cannot use fixups. Sorry, if I do so, then the above mentioned issue will come here as well right? This one: https://lore.kernel.org/all/20230608093652.1409485-1-vidyas@nvidia.com/ > - Mani > >>>> - Mani >>>> >>>>> Konrad >>>> -- >>>> மணிவண்ணன் சதாசிவம் >> -- >> Thanks, >> Praveenkumar
On 7/26/2023 2:52 PM, Praveenkumar I wrote: > > On 7/25/2023 11:36 AM, Manivannan Sadhasivam wrote: >> On Tue, Jul 25, 2023 at 10:16:04AM +0530, Praveenkumar I wrote: >>> On 7/24/2023 7:39 PM, Manivannan Sadhasivam wrote: >>>> On Mon, Jul 24, 2023 at 06:38:55PM +0530, Manivannan Sadhasivam wrote: >>>>> On Mon, Jul 24, 2023 at 02:53:37PM +0200, Konrad Dybcio wrote: >>>>>> On 24.07.2023 14:47, Praveenkumar I wrote: >>>>>>> Set 256 bytes as payload size for IPQ9574 via early fixup. This >>>>>>> allows >>>>>>> PCIe RC to use the max payload size when a capable link partner is >>>>>>> connected. >>>>>>> >>>>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> >>>>>>> --- >>>>>> [...] >>>>>> >>>>>>> +static void qcom_fixup_mps_256(struct pci_dev *dev) >>>>>>> +{ >>>>>>> + pcie_set_mps(dev, 256); >>>>>> Looks like setting "dev->pcie_mpss = 1" here would make the PCIe >>>>>> generic >>>>>> code take care of this. >>>>>> >>>>> Right, also this setting should not be PCI-PCI bridge specific but >>>>> rather >>>>> controller specific. >>>>> >>>> Wait, have you tested this patch with PCIe devices having MPS < 256 >>>> i.e., >>>> default 128? >>>> >>>> Take a look at this discussion: >>>> https://lore.kernel.org/all/20230608093652.1409485-1-vidyas@nvidia.com/ >>>> >>>> >>>> - Mani >>> Yes, tested this patch with PCIe devices having default 128 and RC is >>> falling back to 128 when pci device is added. >>> This is handled inside pci_configure_mps(). >>> / mpss = 128 << dev->pcie_mpss;/ >>> / if (mpss < p_mps && pci_pcie_type(bridge) == >>> PCI_EXP_TYPE_ROOT_PORT) {/ >>> / pcie_set_mps(bridge, mpss);/ >>> / pci_info(dev, "Upstream bridge's Max Payload Size >>> set to %d >>> (was %d, max %d)\n",/ >>> / mpss, p_mps, 128 << bridge->pcie_mpss);/ >>> / p_mps = pcie_get_mps(bridge);/ >>> / }/ >>> // >>> Also getting the below print, >>> /[ 2.011963] pci 0003:01:00.0: Upstream bridge's Max Payload Size >>> set to >>> 128 (was 256, max 256)/ >> Ok. But for setting MPS, you need to change the DEVCTL register in >> post_init >> sequence for IPQ9574. It is not a quirk, so you cannot use fixups. > Sorry, if I do so, then the above mentioned issue will come here as > well right? > > This one: > https://lore.kernel.org/all/20230608093652.1409485-1-vidyas@nvidia.com/ Sorry, confused a bit here. After moving the MPS setting to post_init, pci_configure_mps() is taking care if 128 byte PCIe device is connected. Posted a updated patch. - Praveenkumar > >> - Mani >> >>>>> - Mani >>>>> >>>>>> Konrad >>>>> -- >>>>> மணிவண்ணன் சதாசிவம் >>> -- >>> Thanks, >>> Praveenkumar
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index cee4e400a695..6695bc3b122f 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -1631,6 +1631,11 @@ static void qcom_fixup_class(struct pci_dev *dev) { dev->class = PCI_CLASS_BRIDGE_PCI_NORMAL; } + +static void qcom_fixup_mps_256(struct pci_dev *dev) +{ + pcie_set_mps(dev, 256); +} DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0101, qcom_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0104, qcom_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0106, qcom_fixup_class); @@ -1638,6 +1643,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0107, qcom_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x0302, qcom_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1000, qcom_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1001, qcom_fixup_class); +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_QCOM, 0x1108, qcom_fixup_mps_256); static const struct dev_pm_ops qcom_pcie_pm_ops = { NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_suspend_noirq, qcom_pcie_resume_noirq)
Set 256 bytes as payload size for IPQ9574 via early fixup. This allows PCIe RC to use the max payload size when a capable link partner is connected. Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> --- This patch depends on the below series which adds support for PCIe controllers in IPQ9574 https://lore.kernel.org/linux-arm-msm/20230519090219.15925-1-quic_devipriy@quicinc.com/ drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++ 1 file changed, 6 insertions(+)