On 12/4/2024 12:25 AM, Bjorn Helgaas wrote:
On Sun, Nov 17, 2024 at 03:30:19AM +0530, Krishna chaitanya chundru wrote:
The current implementation requires iATU for every configuration
space access which increases latency & cpu utilization.
Configuring iATU in config shift mode enables ECAM feature to access the
config space, which avoids iATU configuration for every config access.
Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift mode.
As DBI comes under config space, this avoids remapping of DBI space
separately. Instead, it uses the mapped config space address returned from
ECAM initialization. Change the order of dw_pcie_get_resources() execution
to acheive this.
s/acheive/achieve/
Introduce new ecam_init() function op for the clients to configure after
ecam window creation has been done.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@xxxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 114 ++++++++++++++++++----
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 6 ++
3 files changed, 102 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3e41865c7290..e98cc841a2a9 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -418,6 +418,62 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
}
}
+static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct dw_pcie_ob_atu_cfg atu = {0};
+ struct resource_entry *bus;
+ int ret, bus_range_max;
+
+ bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
+
+ /*
+ * Bus 1 config space needs type 0 atu configuration
+ * Remaining buses need type 1 atu configuration
s/atu/ATU/ (initialism, looks like "iATU" might be appropriate here?)
I'm confused about the bus numbering; you refer to "bus 1" and "bus
2". Is bus 1 the root bus, i.e., the primary bus of a Root Port?
The root bus number would typically be 0, not 1, and is sometimes
programmable. I don't know how the DesignWare core works, but since
you have "bus" here, referring to "bus 1" and "bus 2" here seems
overly specific.
root bus is bus 0 and we don't need any iATU configuration for it as
its config space is accessible from the system memory, for usp port of
the switch or the direct the endpoint i.e bus 1 we need to send
Configuration Type 0 requests and for other buses we need to send
Configuration Type 1 requests this is as per PCIe spec, I will try to
include PCIe spec details in next patch.
+ */
+ atu.index = 0;
+ atu.type = PCIE_ATU_TYPE_CFG0;
+ atu.cpu_addr = pp->cfg0_base + SZ_1M;
+ atu.size = SZ_1M;
+ atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
+ ret = dw_pcie_prog_outbound_atu(pci, &atu);
+ if (ret)
+ return ret;
+
+ bus_range_max = bus->res->end - bus->res->start + 1;
+
+ /* Configure for bus 2 - bus_range_max in type 1 */
+ atu.index = 1;
+ atu.type = PCIE_ATU_TYPE_CFG1;
+ atu.cpu_addr = pp->cfg0_base + SZ_2M;
+ atu.size = (SZ_1M * (bus_range_max - 2));
+ atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
+ return dw_pcie_prog_outbound_atu(pci, &atu);
+}
+
+static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct device *dev = pci->dev;
+ struct resource_entry *bus;
+
+ bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
+ if (!bus)
+ return -ENODEV;
+
+ pp->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
+ if (IS_ERR(pp->cfg))
+ return PTR_ERR(pp->cfg);
+
+ pci->dbi_base = pp->cfg->win;
+ pci->dbi_phys_addr = res->start;
+
+ if (pp->ops->ecam_init)
+ pp->ops->ecam_init(pci, pp->cfg);
.ecam_init() is defined to return int, but you ignore the return value.
If it's practical, I think it would be nicer if you could manage to:
- Drop .enable_ecam.
- Have .ecam_init() return failure if there's not enough ECAM space
or whatever, i.e., move the qcom_pcie_check_ecam_support() code
there.
- Handle .ecam_init() failure here by doing whatever we did before.
If there's no useful return value from .ecam_init(), make it void.
In controller driver we need to skip few thing if we want to enable
this feature before calling dw_pcie_host_init, better to have this way
+ return 0;
+}
@@ -454,6 +499,30 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
pp->bridge = bridge;
+ pp->cfg0_size = resource_size(res);
+ pp->cfg0_base = res->start;
+
+ if (!pp->enable_ecam) {
If you can't get rid of .enable_ecam, reverse order so this uses
positive logic:
if (pp->enable_ecam) {
...
} else {
...
}
ack.
- Krishna Chaitanya.
+ pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
+ if (IS_ERR(pp->va_cfg0_base))
+ return PTR_ERR(pp->va_cfg0_base);
+
+ /* Set default bus ops */
+ bridge->ops = &dw_pcie_ops;
+ bridge->child_ops = &dw_child_pcie_ops;
+ bridge->sysdata = pp;
+ } else {
+ ret = dw_pcie_create_ecam_window(pp, res);
+ if (ret)
+ return ret;
+ bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
+ pp->bridge->sysdata = pp->cfg;
+ }
