Hi Kever, On 2023-06-29 12:25, Kever Yang wrote:
Hi Jonas,
Sorry for reply late for this patch.
I have check internally again for this BAR issue, the BAR alloce fail for RC would not affect other process, and it always works on vendor U-Boot tree.
The issue is not that the RC BAR allocation fails and that it affect other PCI devices BAR allocation after that, the issue is that with updated ranges the RC BAR allocation is successful and the entire memory region gets allocated and that in turn affect other PCI devices from a successful probe.
There are multiple parts in play here:
1. Two BARs of 1 GiB each is reported for RC on RK3568, 0xC0000000 is read back instead of 0x0000000. As seen by "PCI Autoconfig: BAR X, Mem, size=0x40000000" below.
2. With the ranges from "rockchip: rk356x: Update PCIe config, IO and memory regions", now also merged in linux, the first BAR for RC does allocate the entire memory region. As seen by "With a memory region of the entire 1 GiB this leads to".
PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000
3. Any other BAR processed will fail with "No room in resource" and the PCI autoconfig fails and no device is discovered. As seen by "With a memory region of the entire 1 GiB this leads to".
PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000 PCI: Failed autoconfig bar 10
On downstream vendor U-Boot this is not an issue because the start address is 0x0 instead of 0x40000000. U-Boot skips first 0x1000 of the range when the start address is 0x0, meaning the available range is less then 1 GiB, and the BAR allocation for RC fails.
As seen by "With a memory region less than 1 GiB this was not a real issue" this also happen until memory ranges are adjusted in device tree.
PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000
The issue with RC allocating the entire 1 GiB region would probably affect vendor U-Boot if it would use 0x40000000 as start address instead of 0x0.
There has been reports on linux ML that the use of 0x0 as start address caused issues for some PCI devices and that the use of 0x40000000 as start address fixed such issue.
And if we have to handle this, we can disable the BAR instead of this workaround.
Please provide details on how we can configure the HW to not report any BARs when in RC mode.
Using a very similar approach that other pci drivers have used, e.g. the Intel FPGA PCIe host controller driver, is the only way that I found that worked consistently. See https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_intel_fp...
Regards, Jonas
Thanks,
- Kever
On 2023/5/18 06:53, Jonas Karlman wrote:
PCI Autoconfig read the Root Complex BARs and try to claim the entire 1 GiB memory region on RK3568, leaving no space for any attached device.
With a memory region less than 1 GiB this was not a real issue:
PCI Autoconfig: Bus Memory region: [0-3eefffff], PCI Autoconfig: Bus I/O region: [3ef00000-3effffff], PCI Autoconfig: Found P2P bridge, device 0 PCI Autoconfig: BAR 0, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000 PCI: Failed autoconfig bar 10 PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=1000 / size=3ef00000, need=40000000 PCI: Failed autoconfig bar 14 PCI Autoconfig: ROM, size=0x10000, address=0x10000 bus_lower=0x20000
PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x100000 bus_lower=0x104000
With a memory region of the entire 1 GiB this leads to:
PCI Autoconfig: Bus Memory region: [40000000-7fffffff], PCI Autoconfig: Bus I/O region: [f0100000-f01fffff], PCI Autoconfig: Found P2P bridge, device 0 PCI Autoconfig: BAR 0, Mem, size=0x40000000, address=0x40000000 bus_lower=0x80000000 PCI Autoconfig: BAR 1, Mem, size=0x40000000, No room in resource, avail start=80000000 / size=40000000, need=40000000 PCI: Failed autoconfig bar 14 PCI Autoconfig: ROM, size=0x10000, No room in resource, avail start=80000000 / size=40000000, need=10000
PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail start=80000000 / size=40000000, need=4000 PCI: Failed autoconfig bar 10
After this change with a memory region of the entire 1 GiB:
PCI Autoconfig: Bus Memory region: [40000000-7fffffff], PCI Autoconfig: Bus I/O region: [f0100000-f01fffff], PCI Autoconfig: Found P2P bridge, device 0 PCI Autoconfig: ROM, size=0x10000, address=0x40000000 bus_lower=0x40010000
PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x40100000 bus_lower=0x40104000
Return an invalid value during config read of Root Complex BARs during autoconfig to work around such issue.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
v2:
Update commit message
drivers/pci/pcie_dw_rockchip.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c index 82a8b9c96e2b..f56773c2e58c 100644 --- a/drivers/pci/pcie_dw_rockchip.c +++ b/drivers/pci/pcie_dw_rockchip.c @@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg, __rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val); }
+/**
- The BARs of bridge should be hidden during enumeration to avoid
- allocation of the entire memory region by PCIe core on RK3568.
- */
+static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
uint offset)+{
- int bus = PCI_BUS(bdf) - pcie->first_busno;
- return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;+}
+static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
uint offset, ulong *valuep,enum pci_size_t size)+{
- struct pcie_dw *pcie = dev_get_priv(bus);
- int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
- if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
*valuep = pci_get_ff(size);- return ret;
+}
- /**
- rk_pcie_configure() - Configure link capabilities and speed
@@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port: }
static const struct dm_pci_ops rockchip_pcie_ops = {
- .read_config = pcie_dw_read_config,
- .read_config = rk_pcie_read_config, .write_config = pcie_dw_write_config, };