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 <[email protected]>
> > ---
> [...]
>
> >
> > +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.
- Mani
> Konrad
--
மணிவண்ணன் சதாசிவம்
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 <[email protected]>
> > > ---
> > [...]
> >
> > >
> > > +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/[email protected]/
- Mani
> - Mani
>
> > Konrad
>
> --
> மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
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 <[email protected]>
>>>> ---
>>> [...]
>>>
>>>> +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/[email protected]/
>
> - 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)/
>> - Mani
>>
>>> Konrad
>> --
>> மணிவண்ணன் சதாசிவம்
--
Thanks,
Praveenkumar
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 <[email protected]>
> > > > > ---
> > > > [...]
> > > >
> > > > > +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/[email protected]/
> >
> > - 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.
- 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 <[email protected]>
>>>>>> ---
>>>>> [...]
>>>>>
>>>>>> +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/[email protected]/
>>>
>>> - 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 <[email protected]>
>>>>>> ---
>>>>> [...]
>>>>>
>>>>>> +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/[email protected]/
>>>
>>> - 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/[email protected]/
> - 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 <[email protected]>
>>>>>>> ---
>>>>>> [...]
>>>>>>
>>>>>>> +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/[email protected]/
>>>>
>>>>
>>>> - 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/[email protected]/
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