| Message ID | 1434405656-10465-5-git-send-email-jacob.e.keller@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Kirsher |
| Headers | show |
On 06/15/2015 03:00 PM, Jacob Keller wrote: > This is useful in cases where we connect to a slot at Gen3, but the slot > is behind a bus which only connected at Gen2. This generally only > happens when a PCIe switch is in the sequence of devices, and can be > very confusing when you see slow performance with no obvious cause. > > I am aware this patch has a few lines that break 80 characters, but > there does not seem to be a readable way to format them to less than 80 > characters. Suggestions welcome. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > --- > drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++-------- > 1 file changed, 76 insertions(+), 29 deletions(-) > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > index 37cd78f3ce24..d8ab6cdca456 100644 > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > @@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc *interface, > > static void fm10k_slot_warn(struct fm10k_intfc *interface) > { > - struct device *dev = &interface->pdev->dev; > + enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN; > + enum pci_bus_speed speed = PCI_SPEED_UNKNOWN; > struct fm10k_hw *hw = &interface->hw; > + int max_gts = 0, expected_gts = 0; > > - if (hw->mac.ops.is_slot_appropriate(hw)) If I am not mistaken I believe this is the only spot that uses the is_slot_appropriate function in the upstream driver. You could just drop it from the code if that is the case. I'll try to get to it in a couple of weeks when I get back from vacation otherwise. - Alex
On Thu, 2015-06-18 at 19:22 -0700, Alexander Duyck wrote: > On 06/15/2015 03:00 PM, Jacob Keller wrote: > > This is useful in cases where we connect to a slot at Gen3, but the > > slot > > is behind a bus which only connected at Gen2. This generally only > > happens when a PCIe switch is in the sequence of devices, and can > > be > > very confusing when you see slow performance with no obvious cause. > > > > I am aware this patch has a few lines that break 80 characters, but > > there does not seem to be a readable way to format them to less > > than 80 > > characters. Suggestions welcome. > > > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > > --- > > drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 > > +++++++++++++++++++-------- > > 1 file changed, 76 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > > b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > > index 37cd78f3ce24..d8ab6cdca456 100644 > > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c > > @@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc > > *interface, > > > > static void fm10k_slot_warn(struct fm10k_intfc *interface) > > { > > - struct device *dev = &interface->pdev->dev; > > + enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN; > > + enum pci_bus_speed speed = PCI_SPEED_UNKNOWN; > > struct fm10k_hw *hw = &interface->hw; > > + int max_gts = 0, expected_gts = 0; > > > > - if (hw->mac.ops.is_slot_appropriate(hw)) > > If I am not mistaken I believe this is the only spot that uses the > is_slot_appropriate function in the upstream driver. You could just > drop it from the code if that is the case. I'll try to get to it in > a > couple of weeks when I get back from vacation otherwise. > > - Alex Makes sense. Regards, Jake
-----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Jacob Keller Sent: Monday, June 15, 2015 3:01 PM To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org> Subject: [Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link This is useful in cases where we connect to a slot at Gen3, but the slot is behind a bus which only connected at Gen2. This generally only happens when a PCIe switch is in the sequence of devices, and can be very confusing when you see slow performance with no obvious cause. I am aware this patch has a few lines that break 80 characters, but there does not seem to be a readable way to format them to less than 80 characters. Suggestions welcome. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++-------- 1 file changed, 76 insertions(+), 29 deletions(-) Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>
-----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Jacob Keller Sent: Monday, June 15, 2015 3:01 PM To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org> Subject: [Intel-wired-lan] [PATCH 5/6] fm10k: update fm10k_slot_warn to use pcie_get_minimum link This is useful in cases where we connect to a slot at Gen3, but the slot is behind a bus which only connected at Gen2. This generally only happens when a PCIe switch is in the sequence of devices, and can be very confusing when you see slow performance with no obvious cause. I am aware this patch has a few lines that break 80 characters, but there does not seem to be a readable way to format them to less than 80 characters. Suggestions welcome. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- Tested-By: Krishneil Singh <krishneil.k.singh@intel.com>
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c index 37cd78f3ce24..d8ab6cdca456 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c @@ -1703,22 +1703,86 @@ static int fm10k_sw_init(struct fm10k_intfc *interface, static void fm10k_slot_warn(struct fm10k_intfc *interface) { - struct device *dev = &interface->pdev->dev; + enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN; + enum pci_bus_speed speed = PCI_SPEED_UNKNOWN; struct fm10k_hw *hw = &interface->hw; + int max_gts = 0, expected_gts = 0; - if (hw->mac.ops.is_slot_appropriate(hw)) + if (pcie_get_minimum_link(interface->pdev, &speed, &width) || + speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN) { + dev_warn(&interface->pdev->dev, + "Unable to determine PCI Express bandwidth.\n"); return; + } - dev_warn(dev, - "For optimal performance, a %s %s slot is recommended.\n", - (hw->bus_caps.width == fm10k_bus_width_pcie_x1 ? "x1" : - hw->bus_caps.width == fm10k_bus_width_pcie_x4 ? "x4" : - "x8"), - (hw->bus_caps.speed == fm10k_bus_speed_2500 ? "2.5GT/s" : - hw->bus_caps.speed == fm10k_bus_speed_5000 ? "5.0GT/s" : - "8.0GT/s")); - dev_warn(dev, - "A slot with more lanes and/or higher speed is suggested.\n"); + switch (speed) { + case PCIE_SPEED_2_5GT: + /* 8b/10b encoding reduces max throughput by 20% */ + max_gts = 2 * width; + break; + case PCIE_SPEED_5_0GT: + /* 8b/10b encoding reduces max throughput by 20% */ + max_gts = 4 * width; + break; + case PCIE_SPEED_8_0GT: + /* 128b/130b encoding has less than 2% impact on throughput */ + max_gts = 8 * width; + break; + default: + dev_warn(&interface->pdev->dev, + "Unable to determine PCI Express bandwidth.\n"); + return; + } + + dev_info(&interface->pdev->dev, + "PCI Express bandwidth of %dGT/s available\n", + max_gts); + dev_info(&interface->pdev->dev, + "(Speed:%s, Width: x%d, Encoding Loss:%s, Payload:%s)\n", + (speed == PCIE_SPEED_8_0GT ? "8.0GT/s" : + speed == PCIE_SPEED_5_0GT ? "5.0GT/s" : + speed == PCIE_SPEED_2_5GT ? "2.5GT/s" : + "Unknown"), + hw->bus.width, + (speed == PCIE_SPEED_2_5GT ? "20%" : + speed == PCIE_SPEED_5_0GT ? "20%" : + speed == PCIE_SPEED_8_0GT ? "<2%" : + "Unknown"), + (hw->bus.payload == fm10k_bus_payload_128 ? "128B" : + hw->bus.payload == fm10k_bus_payload_256 ? "256B" : + hw->bus.payload == fm10k_bus_payload_512 ? "512B" : + "Unknown")); + + switch (hw->bus_caps.speed) { + case fm10k_bus_speed_2500: + /* 8b/10b encoding reduces max throughput by 20% */ + expected_gts = 2 * hw->bus_caps.width; + break; + case fm10k_bus_speed_5000: + /* 8b/10b encoding reduces max throughput by 20% */ + expected_gts = 4 * hw->bus_caps.width; + break; + case fm10k_bus_speed_8000: + /* 128b/130b encoding has less than 2% impact on throughput */ + expected_gts = 8 * hw->bus_caps.width; + break; + default: + dev_warn(&interface->pdev->dev, + "Unable to determine expected PCI Express bandwidth.\n"); + return; + } + + if (max_gts < expected_gts) { + dev_warn(&interface->pdev->dev, + "This device requires %dGT/s of bandwidth for optimal performance.\n", + expected_gts); + dev_warn(&interface->pdev->dev, + "A %sslot with x%d lanes is suggested.\n", + (hw->bus_caps.speed == fm10k_bus_speed_2500 ? "2.5GT/s " : + hw->bus_caps.speed == fm10k_bus_speed_5000 ? "5.0GT/s " : + hw->bus_caps.speed == fm10k_bus_speed_8000 ? "8.0GT/s " : ""), + hw->bus_caps.width); + } } /** @@ -1737,7 +1801,6 @@ static int fm10k_probe(struct pci_dev *pdev, { struct net_device *netdev; struct fm10k_intfc *interface; - struct fm10k_hw *hw; int err; err = pci_enable_device_mem(pdev); @@ -1781,7 +1844,6 @@ static int fm10k_probe(struct pci_dev *pdev, interface->netdev = netdev; interface->pdev = pdev; - hw = &interface->hw; interface->uc_addr = ioremap(pci_resource_start(pdev, 0), FM10K_UC_ADDR_SIZE); @@ -1823,21 +1885,6 @@ static int fm10k_probe(struct pci_dev *pdev, /* Register PTP interface */ fm10k_ptp_register(interface); - /* print bus type/speed/width info */ - dev_info(&pdev->dev, "(PCI Express:%s Width: %s Payload: %s)\n", - (hw->bus.speed == fm10k_bus_speed_8000 ? "8.0GT/s" : - hw->bus.speed == fm10k_bus_speed_5000 ? "5.0GT/s" : - hw->bus.speed == fm10k_bus_speed_2500 ? "2.5GT/s" : - "Unknown"), - (hw->bus.width == fm10k_bus_width_pcie_x8 ? "x8" : - hw->bus.width == fm10k_bus_width_pcie_x4 ? "x4" : - hw->bus.width == fm10k_bus_width_pcie_x1 ? "x1" : - "Unknown"), - (hw->bus.payload == fm10k_bus_payload_128 ? "128B" : - hw->bus.payload == fm10k_bus_payload_256 ? "256B" : - hw->bus.payload == fm10k_bus_payload_512 ? "512B" : - "Unknown")); - /* print warning for non-optimal configurations */ fm10k_slot_warn(interface);
This is useful in cases where we connect to a slot at Gen3, but the slot is behind a bus which only connected at Gen2. This generally only happens when a PCIe switch is in the sequence of devices, and can be very confusing when you see slow performance with no obvious cause. I am aware this patch has a few lines that break 80 characters, but there does not seem to be a readable way to format them to less than 80 characters. Suggestions welcome. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 105 +++++++++++++++++++-------- 1 file changed, 76 insertions(+), 29 deletions(-)