Use new pci interfaces to set read request tuning values
Untested because of lack of hardware.
Signed-off-by: Stephen Hemminger <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_main.c | 38 +++++++------------------------
1 file changed, 9 insertions(+), 29 deletions(-)
--- pci-x.orig/drivers/infiniband/hw/mthca/mthca_main.c
+++ pci-x/drivers/infiniband/hw/mthca/mthca_main.c
@@ -100,45 +100,25 @@ static struct mthca_profile default_prof
static int mthca_tune_pci(struct mthca_dev *mdev)
{
- int cap;
- u16 val;
-
if (!tune_pci)
return 0;
/* First try to max out Read Byte Count */
- cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX);
- if (cap) {
- if (pci_read_config_word(mdev->pdev, cap + PCI_X_CMD, &val)) {
- mthca_err(mdev, "Couldn't read PCI-X command register, "
+ if (pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX)) {
+ if (pcix_set_mmrbc(mdev->pdev, 4096)) {
+ mthca_err(mdev, "Couldn't set PCI-X max read count, "
"aborting.\n");
return -ENODEV;
}
- val = (val & ~PCI_X_CMD_MAX_READ) | (3 << 2);
- if (pci_write_config_word(mdev->pdev, cap + PCI_X_CMD, val)) {
- mthca_err(mdev, "Couldn't write PCI-X command register, "
- "aborting.\n");
- return -ENODEV;
- }
- } else if (!(mdev->mthca_flags & MTHCA_FLAG_PCIE))
- mthca_info(mdev, "No PCI-X capability, not setting RBC.\n");
+ }
- cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP);
- if (cap) {
- if (pci_read_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, &val)) {
- mthca_err(mdev, "Couldn't read PCI Express device control "
- "register, aborting.\n");
- return -ENODEV;
- }
- val = (val & ~PCI_EXP_DEVCTL_READRQ) | (5 << 12);
- if (pci_write_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, val)) {
- mthca_err(mdev, "Couldn't write PCI Express device control "
- "register, aborting.\n");
+ if (pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP)) {
+ if (pcie_set_readrq(mdev->pdev, 4096)) {
+ mthca_err(mdev, "Couldn't write PCI Express read request, "
+ "aborting.\n");
return -ENODEV;
}
- } else if (mdev->mthca_flags & MTHCA_FLAG_PCIE)
- mthca_info(mdev, "No PCI Express capability, "
- "not setting Max Read Request Size.\n");
+ }
return 0;
}
--
Looks fine (assuming the PCI core interfaces it uses go in).
Acked-by: Roland Dreier <[email protected]>
On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> plain text document attachment (mthca-rbc.patch)
> Use new pci interfaces to set read request tuning values
> Untested because of lack of hardware.
I'm worried by this... At no point do you check the host bridge
capabilities, and thus will happily set the max read req size to some
value larger than the max the host bridge can cope...
I've been having exactly that problem on a number of setups, for
example, the sky2 cards tend to start with a value of 512 while the G5's
host bridge can't cope with more than 256 (iirc). The firmware fixes
that up properly on the G5 at least (but not on all machines), but if
you allow drivers to go tweak the value without a way to go check what
are the host bridge capabilities, you are toast.
Of course, on PCI-X, this is moot, there is no clear definition on how
to get to a host bridge config space (if any), but on PCI-E, we should
be more careful.
So for PCI-X, if we want tat, we need a pcibios hook for the platform
to validate the size requested. For PCI-E, we can use standard code to
look for the root complex (and bridges on the path to it) and get the
proper max value.
Ben.
On Mon, Dec 11, 2006 at 02:55:39PM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> > plain text document attachment (mthca-rbc.patch)
> > Use new pci interfaces to set read request tuning values
> > Untested because of lack of hardware.
Sorry...I missed that. I have mthca HW on publicly available IA64 machines.
I'll contact Steve off list to check if he is interested/time.
> I'm worried by this... At no point do you check the host bridge
> capabilities, and thus will happily set the max read req size to some
> value larger than the max the host bridge can cope...
>
> I've been having exactly that problem on a number of setups, for
> example, the sky2 cards tend to start with a value of 512 while the G5's
> host bridge can't cope with more than 256 (iirc). The firmware fixes
> that up properly on the G5 at least (but not on all machines), but if
> you allow drivers to go tweak the value without a way to go check what
> are the host bridge capabilities, you are toast.
>
> Of course, on PCI-X, this is moot, there is no clear definition on how
> to get to a host bridge config space (if any)
...
> So for PCI-X, if we want tat, we need a pcibios hook for the platform
> to validate the size requested.
Yes, agreed.
grant
> I'm worried by this... At no point do you check the host bridge
> capabilities, and thus will happily set the max read req size to some
> value larger than the max the host bridge can cope...
Well, it's disabled by default... the option is there as a quick way
to fix "why is my bandwidth so low" when a broken BIOS sets these to
minimum values. Maybe we should just strip out that code and point
people who want to tweak this at setpci instead.
> So for PCI-X, if we want tat, we need a pcibios hook for the platform
> to validate the size requested. For PCI-E, we can use standard code to
> look for the root complex (and bridges on the path to it) and get the
> proper max value.
Actually even PCIe might not be that easy. For example with current
kernels on PowerPC 440SPe (SoC with PCIe), I just get:
# lspci
00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0)
ie no host bridge / root complex.
- R.
> Actually even PCIe might not be that easy. For example with current
> kernels on PowerPC 440SPe (SoC with PCIe), I just get:
>
> # lspci
> 00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0)
>
> ie no host bridge / root complex.
Did somebody used the spec as toilet paper again ? Or is it just the
kernel that isn't properly showing the root complex ?
Ben.