Re: [PATCH 2/3] PCI: dwc: Add ECAM support with iATU configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/2/2024 10:12 PM, Manivannan Sadhasivam 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

Can you please elaborate 'config shift mode'? Quote relevant section in DWC
databook for reference.

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.

Introduce new ecam_init() function op for the clients to configure after

We use 'DWC glue drivers' to refer the 'clients' of this driver.

ecam window creation has been done.


Use 'ECAM' everywhere.

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
+	 */
+	atu.index = 0;
+	atu.type = PCIE_ATU_TYPE_CFG0;
+	atu.cpu_addr = pp->cfg0_base + SZ_1M;

You didn't mention what occupies the first 1MB.

+	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;

resource_size(bus->res)

+
+	/* 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);
+
+	return 0;
+}
+
  int dw_pcie_host_init(struct dw_pcie_rp *pp)
  {
  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -431,19 +487,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
raw_spin_lock_init(&pp->lock); - ret = dw_pcie_get_resources(pci);
-	if (ret)
-		return ret;
-
  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
-	if (res) {
-		pp->cfg0_size = resource_size(res);
-		pp->cfg0_base = res->start;
-
-		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);
-	} else {
+	if (!res) {
  		dev_err(dev, "Missing *config* reg space\n");
  		return -ENODEV;
  	}
@@ -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) {

Why can't you just use the ECAM mode when there is enough memory defined in DT?
Using this flag slightly defeats the purpose of the ECAM mode.

+		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;

'bridge->sysdata = pp->cfg'?

as we are using pci_generic_ecam_ops.pci_ops for config reads & writes
it expects cfg space as sysdata[1].

[1] https://elixir.bootlin.com/linux/v6.12.1/source/drivers/pci/ecam.c#L170

- Krishna Chaitanya.

- Mani





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux