This needs to say whether this is a functional or a performance issue.

Looks like not a performance issue , this is a functional issue.

We detect just one problem(may be exist another i dont know ) for
writing(reading works fine) any data to NVME devices with MRRS != 256
we have scrambled HDMI display

Does this problem actually affect *all* DesignWare-based controllers?
So why is this limited to PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3?

i can say only for VIM3 device which use this controller and have this problem

On Wed, Jul 28, 2021 at 3:43 AM Bjorn Helgaas [off-list ref] wrote:

On Tue, Jul 27, 2021 at 10:30:00AM +0800, Artem Lapkin wrote:

quoted

256 bytes maximum read request size. They can't handle
anything larger than this. So force this limit on
any devices attached under these ports.

This needs to say whether this is a functional or a performance issue.

If it's a functional issue, i.e., if meson signals an error or abort
when it receives a read request for > 256 bytes, we need to explain
exactly what happens.

If it's a performance issue, we need to explain why MRRS affects
performance and that this is an optimization.

quoted

Come-from: https://lkml.org/lkml/2021/6/18/160
Come-from: https://lkml.org/lkml/2021/6/19/19

Please use lore.kernel.org URLs instead.  The lore URLs are a little
uglier, but are more functional, more likely to continue working, and
avoid the ads.  These are:

  https://lore.kernel.org/r/20210618230132.GA3228427@bjorn-Precision-5520 (local)
  https://lore.kernel.org/r/20210619063952.2008746-1-art@khadas.com (local)

quoted

It only affects PCIe in P2P, in non-P2P is will certainly affect
transfers on the internal SoC/Processor/Chip internal bus/fabric.

This needs to explain how a field in a PCIe TLP affects transfers on
these non-PCIe fabrics.

quoted

These quirks are currently implemented in the
controller driver and only applies when the controller has been probed
and to each endpoint detected on this particular controller.

Continue having separate quirks for each controller if the core
isn't the right place to handle MPS/MRRS.

I see similar code in dwc/pci-keystone.c.  Does this problem actually
affect *all* DesignWare-based controllers?

If so, we should put the workaround in the common dwc code, e.g.,
pcie-designware.c or similar.

It also seems to affect pci-loongson.c (not DesignWare-based).  Is
there some reason it has the same problem, e.g., does loongson contain
DesignWare IP, or does it use the same non-PCIe fabric?

quoted

quoted

quoted

Neil

Signed-off-by: Artem Lapkin <redacted>
---
 drivers/pci/controller/dwc/pci-meson.c | 31 ++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 686ded034..1498950de 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c

@@ -466,6 +466,37 @@ static int meson_pcie_probe(struct platform_device *pdev)
      return ret;
 }

+static void meson_mrrs_limit_quirk(struct pci_dev *dev)
+{
+     struct pci_bus *bus = dev->bus;
+     int mrrs, mrrs_limit = 256;
+     static const struct pci_device_id bridge_devids[] = {
+             { PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3) },

I don't really believe that PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 is the
only device affected here.  Is this related to the Meson root port, or
is it related to a PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 on a plug-in card?
I guess the former, since you're searching upward for a root port.

So why is this limited to PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3?

quoted

+             { 0, },
+     };
+
+     /* look for the matching bridge */
+     while (!pci_is_root_bus(bus)) {
+             /*
+              * 256 bytes maximum read request size. They can't handle
+              * anything larger than this. So force this limit on
+              * any devices attached under these ports.
+              */
+             if (!pci_match_id(bridge_devids, bus->self)) {
+                     bus = bus->parent;
+                     continue;
+             }
+
+             mrrs = pcie_get_readrq(dev);
+             if (mrrs > mrrs_limit) {
+                     pci_info(dev, "limiting MRRS %d to %d\n", mrrs, mrrs_limit);
+                     pcie_set_readrq(dev, mrrs_limit);
+             }
+             break;
+     }
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, meson_mrrs_limit_quirk);
+
 static const struct of_device_id meson_pcie_of_match[] = {
      {
              .compatible = "amlogic,axg-pcie",
--
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel