| Message ID | 1407338300-9146-1-git-send-email-m-karicheri2@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 06, 2014 at 11:18:20AM -0400, Murali Karicheri wrote: > Keystone PCI controller has a limitation that memory read request > size must not exceed 256 bytes. This is a hardware limitation and > add a quirk to force this limit on all downstream devices by > updating mrrs. Does this still work if the tuning is enabled, or does the tuning run after this? > + if (pcie_get_readrq(dev) > 256) { > + pr_info("limiting mrrs to 256\n"); > + pcie_set_readrq(dev, 256); The pr_info should either go away, or at least print the PCI bdf.. Jason
On 08/06/2014 12:30 PM, Jason Gunthorpe wrote: > On Wed, Aug 06, 2014 at 11:18:20AM -0400, Murali Karicheri wrote: >> Keystone PCI controller has a limitation that memory read request >> size must not exceed 256 bytes. This is a hardware limitation and >> add a quirk to force this limit on all downstream devices by >> updating mrrs. > > Does this still work if the tuning is enabled, or does the tuning run > after this? Yes it works with tuning enabled. Tuning happens afterwards. The 'limiting mrrs to 256' below is from my patch. [ 2.267670] limiting mrrs to 256 [ 2.267698] limiting mrrs to 256 [ 2.267808] pcieport 0000:00:00.0: BAR 8: assigned [mem 0x50000000-0x500fffff] [ 2.267818] pcieport 0000:00:00.0: BAR 9: assigned [mem 0x50100000-0x501fffff pref] [ 2.267827] pcieport 0000:00:00.0: BAR 7: assigned [io 0x1000-0x1fff] [ 2.267840] pci 0000:01:00.0: BAR 0: assigned [mem 0x50000000-0x5001ffff] [ 2.267855] pci 0000:01:00.0: BAR 1: assigned [mem 0x50020000-0x5003ffff] [ 2.267869] pci 0000:01:00.0: BAR 6: assigned [mem 0x50100000-0x5011ffff pref] [ 2.267877] pci 0000:01:00.1: BAR 0: assigned [mem 0x50040000-0x5005ffff] [ 2.267891] pci 0000:01:00.1: BAR 1: assigned [mem 0x50060000-0x5007ffff] [ 2.267904] pci 0000:01:00.1: BAR 6: assigned [mem 0x50120000-0x5013ffff pref] [ 2.267913] pci 0000:01:00.0: BAR 2: assigned [io 0x1000-0x101f] [ 2.267926] pci 0000:01:00.1: BAR 2: assigned [io 0x1020-0x103f] [ 2.267946] pcieport 0000:00:00.0: Max Payload Size set to 256/ 256 (was 128), Max Read Rq 256 [ 2.267980] pci 0000:01:00.0: Max Payload Size set to 256/ 256 (was 128), Max Read Rq 256 [ 2.268013] pci 0000:01:00.1: Max Payload Size set to 256/ 256 (was 128), Max Read Rq 256 > >> + if (pcie_get_readrq(dev)> 256) { >> + pr_info("limiting mrrs to 256\n"); >> + pcie_set_readrq(dev, 256); > > The pr_info should either go away, or at least print the PCI bdf.. I will change this to a dev_info and print the PCI bdf as part of it. > > Jason
On Wed, Aug 06, 2014 at 12:56:04PM -0400, Murali Karicheri wrote: > On 08/06/2014 12:30 PM, Jason Gunthorpe wrote: > >On Wed, Aug 06, 2014 at 11:18:20AM -0400, Murali Karicheri wrote: > >>Keystone PCI controller has a limitation that memory read request > >>size must not exceed 256 bytes. This is a hardware limitation and > >>add a quirk to force this limit on all downstream devices by > >>updating mrrs. > > > >Does this still work if the tuning is enabled, or does the tuning run > >after this? > > Yes it works with tuning enabled. Tuning happens afterwards. The > 'limiting mrrs to 256' below is from my patch. That seems backwards to me... Jason
On 08/06/2014 12:58 PM, Jason Gunthorpe wrote: > On Wed, Aug 06, 2014 at 12:56:04PM -0400, Murali Karicheri wrote: >> On 08/06/2014 12:30 PM, Jason Gunthorpe wrote: >>> On Wed, Aug 06, 2014 at 11:18:20AM -0400, Murali Karicheri wrote: >>>> Keystone PCI controller has a limitation that memory read request >>>> size must not exceed 256 bytes. This is a hardware limitation and >>>> add a quirk to force this limit on all downstream devices by >>>> updating mrrs. >>> >>> Does this still work if the tuning is enabled, or does the tuning run >>> after this? >> >> Yes it works with tuning enabled. Tuning happens afterwards. The >> 'limiting mrrs to 256' below is from my patch. > > That seems backwards to me... Rational? The tuning is reading mrrs and set mps to less than or equal to mrss. So adding this before make sure mrrs used is below keystones's limit. Murali > > Jason
On Wed, Aug 06, 2014 at 01:09:41PM -0400, Murali Karicheri wrote: > >>Yes it works with tuning enabled. Tuning happens afterwards. The > >>'limiting mrrs to 256' below is from my patch. > > > >That seems backwards to me... > Rational? The tuning is reading mrrs and set mps to less than or > equal to mrss. So adding this before make sure mrrs used is below > keystones's limit. The tuning process adjusts the parameters however it sees fit, todays algorithm might not increase a BIOS set MRRS, but tomorrow's could. The quirk should be after the tuning to ensure the MRRS is limited.. Jason
On 08/06/2014 01:30 PM, Jason Gunthorpe wrote: > On Wed, Aug 06, 2014 at 01:09:41PM -0400, Murali Karicheri wrote: > >>>> Yes it works with tuning enabled. Tuning happens afterwards. The >>>> 'limiting mrrs to 256' below is from my patch. >>> >>> That seems backwards to me... >> Rational? The tuning is reading mrrs and set mps to less than or >> equal to mrss. So adding this before make sure mrrs used is below >> keystones's limit. > > The tuning process adjusts the parameters however it sees fit, todays > algorithm might not increase a BIOS set MRRS, but tomorrow's could. > > The quirk should be after the tuning to ensure the MRRS is limited.. > > Jason Ok. I see it. The latest possible quirk application point seems to be before enabling the ep device. A change in macro as below will achieve this and it works. -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_mrrs); +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_mrrs); If this looks reasonable, I will repost my patch with this change. regards, Murali
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c index c1cfaef..541ab24 100644 --- a/drivers/pci/host/pci-keystone.c +++ b/drivers/pci/host/pci-keystone.c @@ -42,8 +42,48 @@ /* DEV_STAT_CTRL */ #define PCIE_CAP_BASE 0x70 +/* PCIE controller device IDs */ +#define PCIE_RC_K2HK 0xb008 +#define PCIE_RC_K2E 0xb009 +#define PCIE_RC_K2L 0xb00a + #define to_keystone_pcie(x) container_of(x, struct keystone_pcie, pp) +static void quirk_limit_mrrs(struct pci_dev *dev) +{ + struct pci_bus *bus = dev->bus; + struct pci_dev *bridge = bus->self; + + if (pci_is_root_bus(bus)) + return; + + /* look for the host bridge */ + while (!pci_is_root_bus(bus)) { + bridge = bus->self; + bus = bus->parent; + } + + if (bridge) { + u16 id; + + /* + * Keystone PCI controller has a h/w limitation of + * 256 bytes maximum read request size. It can't handle + * anything higher than this. So force this limit on + * all downstream devices + */ + pci_read_config_word(bridge, PCI_DEVICE_ID, &id); + if ((id == PCIE_RC_K2HK) || (id == PCIE_RC_K2E) || + (id == PCIE_RC_K2L)) { + if (pcie_get_readrq(dev) > 256) { + pr_info("limiting mrrs to 256\n"); + pcie_set_readrq(dev, 256); + } + } + } +} +DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_mrrs); + static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie) { struct pcie_port *pp = &ks_pcie->pp;
Keystone PCI controller has a limitation that memory read request size must not exceed 256 bytes. This is a hardware limitation and add a quirk to force this limit on all downstream devices by updating mrrs. Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> --- This applies on top of the Keystone PCI controller patch series at http://thread.gmane.org/gmane.linux.kernel.pci/33523 drivers/pci/host/pci-keystone.c | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)