On Sat, Apr 19, 2025 at 01:21:54AM +0800, Hans Zhang wrote:
>
>
> On 2025/4/18 22:55, Niklas Cassel wrote:
> >
> >
> > On 18 April 2025 14:33:08 CEST, Hans Zhang <18255117159@xxxxxxx> wrote:
> > >
> > > Dear Bjorn,
> > >
> > > Thanks your for reply. Niklas and I attempted to modify the relevant logic in drivers/pci/probe.c and found that there was a lot of code judging the global variable pcie_bus_config. At present, there is no good method. I will keep trying.
> > >
> > > I wonder if you have any good suggestions? It seems that the code logic regarding pcie_bus_config is a little complicated and cannot be modified for the time being?
> >
> >
> > Hans,
> >
> > If:
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 364fa2a514f8..2e1c92fdd577 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -2983,6 +2983,13 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
> > if (!pci_is_pcie(bus->self))
> > return;
> > + /*
> > + * Start off with DevCtl.MPS == DevCap.MPS, unless PCIE_BUS_TUNE_OFF.
> > + * This might get overriden by a MPS strategy below.
> > + */
> > + if (pcie_bus_config != PCIE_BUS_TUNE_OFF)
> > + smpss = pcie_get_mps(bus->self);
> > +
>
> Dear Niklas,
>
> Thank you very much for your reply and thoughts.
>
> pcie_get_mps: Returns maximum payload size in bytes
>
> I guess you want to obtain the DevCap MPS. But the purpose of the smpss
> variable is to save the DevCtl MPS.
>
> > /*
> > * FIXME - Peer to peer DMA is possible, though the endpoint would need
> > * to be aware of the MPS of the destination. To work around this,
> >
> >
> >
> > does not work, can't you modify the code slightly so that it works?
> >
> > I haven't tried myself, but considering that it works when walking the bus, it seems that it should be possible to get something working.
> >
>
>
> After making the following modifications, my test shows that it is normal.
> If the consideration is not comprehensive. Could Bjorn and Niklas please
> review my revisions?
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 364fa2a514f8..5b54f1b0a91d 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2951,8 +2951,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev,
> void *data)
> if (!pci_is_pcie(dev))
> return 0;
>
> - if (pcie_bus_config == PCIE_BUS_TUNE_OFF ||
> - pcie_bus_config == PCIE_BUS_DEFAULT)
> + if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
> return 0;
>
> mps = 128 << *(u8 *)data;
> @@ -2991,7 +2990,8 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
> if (pcie_bus_config == PCIE_BUS_PEER2PEER)
> smpss = 0;
>
> - if (pcie_bus_config == PCIE_BUS_SAFE) {
> + if ((pcie_bus_config == PCIE_BUS_SAFE) ||
> + (pcie_bus_config != PCIE_BUS_TUNE_OFF)) {
> smpss = bus->self->pcie_mpss;
>
> pcie_find_smpss(bus->self, &smpss);
>
>
As I replied to Niklas, I'd like to limit the changes to platforms having
controller drivers. So here is my proposal:
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 364fa2a514f8..d32a0f90beb1 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3228,6 +3228,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
*/
pci_assign_unassigned_root_bus_resources(bus);
+ if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
+ /* Configure root ports MPS to be MPSS by default */
+ for_each_pci_bridge(dev, bus) {
+ if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+ continue;
+
+ pcie_write_mps(dev, 128 << dev->pcie_mpss);
+ pcie_write_mrrs(dev);
+ }
+ }
+
list_for_each_entry(child, &bus->children, node)
pcie_bus_configure_settings(child);
- Mani
--
மணிவண்ணன் சதாசிவம்
