On Thu, 2024-12-12 at 13:32 +0100, Lukas Wunner wrote: > On Thu, Dec 12, 2024 at 10:17:21AM +0100, Niklas Schnelle wrote: > > On Thu, 2024-12-12 at 10:08 +0100, Lukas Wunner wrote: > > > After re-reading the spec I'm convinced now > > > that we're doing this wrong and that we should honor the Max Link Speed > > > instead of blindly deeming all set bits in the Link Capabilities 2 > > > Register as supported speeds: > > > > > > https://lore.kernel.org/r/e3386d62a766be6d0ef7138a001dabfe563cdff8.1733991971.git.lukas@xxxxxxxxx/ > > > > > > @Niklas, could you test if this is sufficient to avoid the issue? > > > Or do we still need to stop instantiating the bandwidth controller > > > if more than one speed is supported? > > > > Yes, I will test this but will only get to do so tonight (UTC +2). > > Hey, no worries. We're not on the run! > > > If it's not sufficient I think we could use the modified > > pcie_get_supported_speeds() to check if only one link speed is > > supported, right? > > pcie_get_supported_speeds() is used to fill in the supported_speeds > field in struct pci_dev. > > And that field is used in a number of places (exposure of the max link > speed in sysfs, delay handling in pci_bridge_wait_for_secondary_bus(), > link tuning in radeon/amdgpu drivers, etc). > > So we can't use pcie_get_supported_speeds() to (exclusively) influence > the behavior of the bandwidth controller. Instead, the solution is your > patch for get_port_device_capability(), but future-proofed such that > bwctrl is only instantiated if more than one link speed is supported. > > Thanks! > > Lukas Yeah right, I was imprecise, should have said that we can use the use the updated pcie_get_supported_speeds() via the now correct dev- >supported_speeds. But first let's see if it alone already fixes things. Thanks, Niklas
