| Message ID | 20181107100854.28389-5-Zhiqiang.Hou@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: add prefetchable memory range support | expand |
On 07/11/2018 10:09, Z.q. Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > The current code only support non-prefetchable memory range, > as the non-prefetchable memory range must not be greater than > 4GiB, one viewport can cover it, which supports upto 4GiB. > > To support prefetchable memory range, which is upto 64-bit > memory space and can be greater than 4GiB, so we need multiple > viewports. And added separate vars to store prefetchable memory > range info to prevent overriding the non-prefetchable memory > range info. > > And this patch explicitly assigned the last (if there are only > 2 viewports) or last 2 viewports for CFG and I/O windows and the > rests for MEM windows. > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > V2: > - Reworded the subject and commit description. > - Fix the prefetchable memory range overriding non-perfetchable > memory range issue by adding vars to store prefetchable memory > range info. > > .../pci/controller/dwc/pcie-designware-host.c | 107 ++++++++++++++---- > drivers/pci/controller/dwc/pcie-designware.h | 7 ++ > 2 files changed, 95 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index ecacce016489..328aa40a6609 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -346,6 +346,35 @@ int dw_pcie_host_init(struct pcie_port *pp) > dev_err(dev, "Missing *config* reg space\n"); > } > > + /* > + * If vendor's platform driver has set the num_viewport and it is > + * not less than 2, skip getting the num_viewport from DT here. > + */ > + if (pci->num_viewport < 2) { > + ret = of_property_read_u32(np, "num-viewport", > + &pci->num_viewport); > + if (ret || pci->num_viewport < 2) > + pci->num_viewport = 2; > + } > + > + /* > + * if there are only 2 viewports, assign the last viewport for > + * both CFG and IO window, otherwise assign the last 2 viewport > + * for CFG and IO window specific. And the rest viewports are > + * assigned to MEM windows. > + */ > + if (pci->num_viewport == 2) { > + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; > + pp->mem_wins = 1; > + } else { > + pp->cfg_idx = pci->num_viewport - 1; > + pp->io_idx = pci->num_viewport - 2; > + pp->mem_wins = pci->num_viewport - 2; > + } > + > + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", > + pp->cfg_idx, pp->io_idx, pp->mem_wins); > + > bridge = pci_alloc_host_bridge(0); > if (!bridge) > return -ENOMEM; > @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) > } > break; > case IORESOURCE_MEM: > - pp->mem = win->res; > - pp->mem->name = "MEM"; > - pp->mem_size = resource_size(pp->mem); > - pp->mem_bus_addr = pp->mem->start - win->offset; > + if (win->res->flags & IORESOURCE_PREFETCH) { > + pp->mem_perf = win->res; > + pp->mem_perf->name = "MEM perf"; > + pp->mem_perf_size = resource_size(pp->mem_perf); > + pp->mem_perf_bus_addr = pp->mem_perf->start - > + win->offset; > + pp->mem_perf_base = pp->mem_perf->start; > + } else { > + pp->mem = win->res; > + pp->mem->name = "MEM"; > + pp->mem_size = resource_size(pp->mem); > + pp->mem_bus_addr = pp->mem->start - win->offset; > + pp->mem_base = pp->mem->start; > + } > break; > case 0: > pp->cfg = win->res; > @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > } > } > > - pp->mem_base = pp->mem->start; > - > if (!pp->va_cfg0_base) { > pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, > pp->cfg0_base, pp->cfg0_size); > @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, > va_cfg_base = pp->va_cfg1_base; > } > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > type, cpu_addr, > busdev, cfg_size); > ret = dw_pcie_read(va_cfg_base + where, size, val); > - if (pci->num_viewport <= 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + if (pp->cfg_idx == pp->io_idx) > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > PCIE_ATU_TYPE_IO, pp->io_base, > pp->io_bus_addr, pp->io_size); > > @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > va_cfg_base = pp->va_cfg1_base; > } > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > type, cpu_addr, > busdev, cfg_size); > ret = dw_pcie_write(va_cfg_base + where, size, val); > - if (pci->num_viewport <= 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + if (pp->cfg_idx == pp->io_idx) > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > PCIE_ATU_TYPE_IO, pp->io_base, > pp->io_bus_addr, pp->io_size); > > @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci) > void dw_pcie_setup_rc(struct pcie_port *pp) > { > u32 val, ctrl, num_ctrls; > + u64 remain_size, base, win_size; > + phys_addr_t bus_addr; > + int i; > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > dw_pcie_setup(pci); > @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > dev_dbg(pci->dev, "iATU unroll: %s\n", > pci->iatu_unroll_enabled ? "enabled" : "disabled"); > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, > - PCIE_ATU_TYPE_MEM, pp->mem_base, > - pp->mem_bus_addr, pp->mem_size); > - if (pci->num_viewport > 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, > - PCIE_ATU_TYPE_IO, pp->io_base, > - pp->io_bus_addr, pp->io_size); > + /* > + * The maximum region size is 4 GB, and a region > + * must not cross a 4 GB boundary. > + */ > + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); > + win_size = min(win_size, pp->mem_size); > + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, > + pp->mem_base, pp->mem_bus_addr, > + win_size); > + dev_dbg(pci->dev, > + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", > + 0, pp->mem_base, &pp->mem_bus_addr, win_size); > + > + /* Prefetchable range can be 64bit space */ > + remain_size = pp->mem_perf_size; > + base = pp->mem_perf_base; > + bus_addr = pp->mem_perf_bus_addr; > + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { > + win_size = SZ_4G - (base & (SZ_4G - 1)); > + win_size = min(win_size, remain_size); > + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, > + base, bus_addr, win_size); > + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", > + i, base, &bus_addr, win_size); > + > + base += win_size; > + bus_addr += win_size; > + remain_size -= win_size; > + } > + > + if (remain_size > 0) > + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); > + > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, > + pp->io_base, pp->io_bus_addr, > + pp->io_size); > } > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index a438c3879aa9..0197f67f82b7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -148,15 +148,22 @@ struct pcie_port { > u64 cfg1_base; > void __iomem *va_cfg1_base; > u32 cfg1_size; > + u32 cfg_idx; > resource_size_t io_base; > phys_addr_t io_bus_addr; > u32 io_size; > + u32 io_idx; > u64 mem_base; > phys_addr_t mem_bus_addr; > u64 mem_size; > + u64 mem_perf_base; > + phys_addr_t mem_perf_bus_addr; > + u64 mem_perf_size; > + u32 mem_wins; > struct resource *cfg; > struct resource *io; > struct resource *mem; > + struct resource *mem_perf; > struct resource *busn; > int irq; > const struct dw_pcie_host_ops *ops; > Hi, It worked perfectly in my setup, however my current configuration have non-prefetchable memory less than 4Gb. In overall the code seems good. Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> Regards, Gustavo
Hi Gustavo, Thanks a lot for your testing and ACK! Regards, Zhiqiang > -----Original Message----- > From: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > Sent: 2018年11月22日 1:37 > To: Z.q. Hou <zhiqiang.hou@nxp.com>; linux-pci@vger.kernel.org; > linux-kernel@vger.kernel.org; bhelgaas@google.com; > lorenzo.pieralisi@arm.com; jingoohan1@gmail.com; > gustavo.pimentel@synopsys.com > Cc: Roy Zang <roy.zang@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; M.h. > Lian <minghuan.lian@nxp.com> > Subject: Re: [PATCHv2 4/4] PCI: dwc: add prefetchable memory range support > > On 07/11/2018 10:09, Z.q. Hou wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > The current code only support non-prefetchable memory range, as the > > non-prefetchable memory range must not be greater than 4GiB, one > > viewport can cover it, which supports upto 4GiB. > > > > To support prefetchable memory range, which is upto 64-bit memory > > space and can be greater than 4GiB, so we need multiple viewports. And > > added separate vars to store prefetchable memory range info to prevent > > overriding the non-prefetchable memory range info. > > > > And this patch explicitly assigned the last (if there are only > > 2 viewports) or last 2 viewports for CFG and I/O windows and the rests > > for MEM windows. > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > --- > > V2: > > - Reworded the subject and commit description. > > - Fix the prefetchable memory range overriding non-perfetchable > > memory range issue by adding vars to store prefetchable memory > > range info. > > > > .../pci/controller/dwc/pcie-designware-host.c | 107 ++++++++++++++---- > > drivers/pci/controller/dwc/pcie-designware.h | 7 ++ > > 2 files changed, 95 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > > b/drivers/pci/controller/dwc/pcie-designware-host.c > > index ecacce016489..328aa40a6609 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -346,6 +346,35 @@ int dw_pcie_host_init(struct pcie_port *pp) > > dev_err(dev, "Missing *config* reg space\n"); > > } > > > > + /* > > + * If vendor's platform driver has set the num_viewport and it is > > + * not less than 2, skip getting the num_viewport from DT here. > > + */ > > + if (pci->num_viewport < 2) { > > + ret = of_property_read_u32(np, "num-viewport", > > + &pci->num_viewport); > > + if (ret || pci->num_viewport < 2) > > + pci->num_viewport = 2; > > + } > > + > > + /* > > + * if there are only 2 viewports, assign the last viewport for > > + * both CFG and IO window, otherwise assign the last 2 viewport > > + * for CFG and IO window specific. And the rest viewports are > > + * assigned to MEM windows. > > + */ > > + if (pci->num_viewport == 2) { > > + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; > > + pp->mem_wins = 1; > > + } else { > > + pp->cfg_idx = pci->num_viewport - 1; > > + pp->io_idx = pci->num_viewport - 2; > > + pp->mem_wins = pci->num_viewport - 2; > > + } > > + > > + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", > > + pp->cfg_idx, pp->io_idx, pp->mem_wins); > > + > > bridge = pci_alloc_host_bridge(0); > > if (!bridge) > > return -ENOMEM; > > @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) > > } > > break; > > case IORESOURCE_MEM: > > - pp->mem = win->res; > > - pp->mem->name = "MEM"; > > - pp->mem_size = resource_size(pp->mem); > > - pp->mem_bus_addr = pp->mem->start - win->offset; > > + if (win->res->flags & IORESOURCE_PREFETCH) { > > + pp->mem_perf = win->res; > > + pp->mem_perf->name = "MEM perf"; > > + pp->mem_perf_size = resource_size(pp->mem_perf); > > + pp->mem_perf_bus_addr = pp->mem_perf->start - > > + win->offset; > > + pp->mem_perf_base = pp->mem_perf->start; > > + } else { > > + pp->mem = win->res; > > + pp->mem->name = "MEM"; > > + pp->mem_size = resource_size(pp->mem); > > + pp->mem_bus_addr = pp->mem->start - win->offset; > > + pp->mem_base = pp->mem->start; > > + } > > break; > > case 0: > > pp->cfg = win->res; > > @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > > } > > } > > > > - pp->mem_base = pp->mem->start; > > - > > if (!pp->va_cfg0_base) { > > pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, > > pp->cfg0_base, pp->cfg0_size); > > @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > va_cfg_base = pp->va_cfg1_base; > > } > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > type, cpu_addr, > > busdev, cfg_size); > > ret = dw_pcie_read(va_cfg_base + where, size, val); > > - if (pci->num_viewport <= 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + if (pp->cfg_idx == pp->io_idx) > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > PCIE_ATU_TYPE_IO, pp->io_base, > > pp->io_bus_addr, pp->io_size); > > > > @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > va_cfg_base = pp->va_cfg1_base; > > } > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > type, cpu_addr, > > busdev, cfg_size); > > ret = dw_pcie_write(va_cfg_base + where, size, val); > > - if (pci->num_viewport <= 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + if (pp->cfg_idx == pp->io_idx) > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > PCIE_ATU_TYPE_IO, pp->io_base, > > pp->io_bus_addr, pp->io_size); > > > > @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct > > dw_pcie *pci) void dw_pcie_setup_rc(struct pcie_port *pp) { > > u32 val, ctrl, num_ctrls; > > + u64 remain_size, base, win_size; > > + phys_addr_t bus_addr; > > + int i; > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > > > dw_pcie_setup(pci); > > @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > > dev_dbg(pci->dev, "iATU unroll: %s\n", > > pci->iatu_unroll_enabled ? "enabled" : "disabled"); > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, > > - PCIE_ATU_TYPE_MEM, pp->mem_base, > > - pp->mem_bus_addr, pp->mem_size); > > - if (pci->num_viewport > 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, > > - PCIE_ATU_TYPE_IO, pp->io_base, > > - pp->io_bus_addr, pp->io_size); > > + /* > > + * The maximum region size is 4 GB, and a region > > + * must not cross a 4 GB boundary. > > + */ > > + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); > > + win_size = min(win_size, pp->mem_size); > > + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, > > + pp->mem_base, pp->mem_bus_addr, > > + win_size); > > + dev_dbg(pci->dev, > > + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, > size = %llx\n", > > + 0, pp->mem_base, &pp->mem_bus_addr, win_size); > > + > > + /* Prefetchable range can be 64bit space */ > > + remain_size = pp->mem_perf_size; > > + base = pp->mem_perf_base; > > + bus_addr = pp->mem_perf_bus_addr; > > + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { > > + win_size = SZ_4G - (base & (SZ_4G - 1)); > > + win_size = min(win_size, remain_size); > > + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, > > + base, bus_addr, win_size); > > + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, > bus_addr = %pa, size = %llx\n", > > + i, base, &bus_addr, win_size); > > + > > + base += win_size; > > + bus_addr += win_size; > > + remain_size -= win_size; > > + } > > + > > + if (remain_size > 0) > > + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); > > + > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, > > + pp->io_base, pp->io_bus_addr, > > + pp->io_size); > > } > > > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); diff --git > > a/drivers/pci/controller/dwc/pcie-designware.h > > b/drivers/pci/controller/dwc/pcie-designware.h > > index a438c3879aa9..0197f67f82b7 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -148,15 +148,22 @@ struct pcie_port { > > u64 cfg1_base; > > void __iomem *va_cfg1_base; > > u32 cfg1_size; > > + u32 cfg_idx; > > resource_size_t io_base; > > phys_addr_t io_bus_addr; > > u32 io_size; > > + u32 io_idx; > > u64 mem_base; > > phys_addr_t mem_bus_addr; > > u64 mem_size; > > + u64 mem_perf_base; > > + phys_addr_t mem_perf_bus_addr; > > + u64 mem_perf_size; > > + u32 mem_wins; > > struct resource *cfg; > > struct resource *io; > > struct resource *mem; > > + struct resource *mem_perf; > > struct resource *busn; > > int irq; > > const struct dw_pcie_host_ops *ops; > > > > Hi, > > It worked perfectly in my setup, however my current configuration have > non-prefetchable memory less than 4Gb. In overall the code seems good. > > Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > Regards, > Gustavo
On Wed, Nov 07, 2018 at 10:09:21AM +0000, Z.q. Hou wrote: > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > The current code only support non-prefetchable memory range, > as the non-prefetchable memory range must not be greater than > 4GiB, one viewport can cover it, which supports upto 4GiB. > > To support prefetchable memory range, which is upto 64-bit > memory space and can be greater than 4GiB, so we need multiple > viewports. And added separate vars to store prefetchable memory > range info to prevent overriding the non-prefetchable memory > range info. > > And this patch explicitly assigned the last (if there are only > 2 viewports) or last 2 viewports for CFG and I/O windows and the > rests for MEM windows. > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > --- > V2: > - Reworded the subject and commit description. > - Fix the prefetchable memory range overriding non-perfetchable > memory range issue by adding vars to store prefetchable memory > range info. > > .../pci/controller/dwc/pcie-designware-host.c | 107 ++++++++++++++---- > drivers/pci/controller/dwc/pcie-designware.h | 7 ++ > 2 files changed, 95 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index ecacce016489..328aa40a6609 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -346,6 +346,35 @@ int dw_pcie_host_init(struct pcie_port *pp) > dev_err(dev, "Missing *config* reg space\n"); > } > > + /* > + * If vendor's platform driver has set the num_viewport and it is > + * not less than 2, skip getting the num_viewport from DT here. That's not what the device tree bindings specify. If DT does contain the property you *must* read it. > + */ > + if (pci->num_viewport < 2) { > + ret = of_property_read_u32(np, "num-viewport", > + &pci->num_viewport); > + if (ret || pci->num_viewport < 2) > + pci->num_viewport = 2; > + } > + > + /* > + * if there are only 2 viewports, assign the last viewport for > + * both CFG and IO window, otherwise assign the last 2 viewport Gah. Can anyone explain to me how this driver works if only two viewports are available ? What happens if an IO access happens at the same time of a config access (that fiddles with the outbound memory windows) ? > + * for CFG and IO window specific. And the rest viewports are > + * assigned to MEM windows. > + */ > + if (pci->num_viewport == 2) { > + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; > + pp->mem_wins = 1; > + } else { > + pp->cfg_idx = pci->num_viewport - 1; > + pp->io_idx = pci->num_viewport - 2; > + pp->mem_wins = pci->num_viewport - 2; > + } > + > + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", > + pp->cfg_idx, pp->io_idx, pp->mem_wins); > + > bridge = pci_alloc_host_bridge(0); > if (!bridge) > return -ENOMEM; > @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) > } > break; > case IORESOURCE_MEM: > - pp->mem = win->res; > - pp->mem->name = "MEM"; > - pp->mem_size = resource_size(pp->mem); > - pp->mem_bus_addr = pp->mem->start - win->offset; > + if (win->res->flags & IORESOURCE_PREFETCH) { > + pp->mem_perf = win->res; > + pp->mem_perf->name = "MEM perf"; Nit: Why "perf" and not "pref" ? It is confusing but that's the least of this patch problems. > + pp->mem_perf_size = resource_size(pp->mem_perf); > + pp->mem_perf_bus_addr = pp->mem_perf->start - > + win->offset; > + pp->mem_perf_base = pp->mem_perf->start; > + } else { > + pp->mem = win->res; > + pp->mem->name = "MEM"; > + pp->mem_size = resource_size(pp->mem); > + pp->mem_bus_addr = pp->mem->start - win->offset; > + pp->mem_base = pp->mem->start; > + } > break; > case 0: > pp->cfg = win->res; > @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > } > } > > - pp->mem_base = pp->mem->start; > - > if (!pp->va_cfg0_base) { > pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, > pp->cfg0_base, pp->cfg0_size); > @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, > va_cfg_base = pp->va_cfg1_base; > } > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > type, cpu_addr, > busdev, cfg_size); > ret = dw_pcie_read(va_cfg_base + where, size, val); > - if (pci->num_viewport <= 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + if (pp->cfg_idx == pp->io_idx) > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > PCIE_ATU_TYPE_IO, pp->io_base, > pp->io_bus_addr, pp->io_size); See above, even though this is not related to this patch. > > @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > va_cfg_base = pp->va_cfg1_base; > } > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > type, cpu_addr, > busdev, cfg_size); > ret = dw_pcie_write(va_cfg_base + where, size, val); > - if (pci->num_viewport <= 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > + if (pp->cfg_idx == pp->io_idx) > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > PCIE_ATU_TYPE_IO, pp->io_base, > pp->io_bus_addr, pp->io_size); > > @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci) > void dw_pcie_setup_rc(struct pcie_port *pp) > { > u32 val, ctrl, num_ctrls; > + u64 remain_size, base, win_size; > + phys_addr_t bus_addr; > + int i; > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > dw_pcie_setup(pci); > @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > dev_dbg(pci->dev, "iATU unroll: %s\n", > pci->iatu_unroll_enabled ? "enabled" : "disabled"); > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, > - PCIE_ATU_TYPE_MEM, pp->mem_base, > - pp->mem_bus_addr, pp->mem_size); > - if (pci->num_viewport > 2) > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, > - PCIE_ATU_TYPE_IO, pp->io_base, > - pp->io_bus_addr, pp->io_size); > + /* > + * The maximum region size is 4 GB, and a region > + * must not cross a 4 GB boundary. > + */ > + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); > + win_size = min(win_size, pp->mem_size); > + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, > + pp->mem_base, pp->mem_bus_addr, > + win_size); > + dev_dbg(pci->dev, > + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", > + 0, pp->mem_base, &pp->mem_bus_addr, win_size); > + > + /* Prefetchable range can be 64bit space */ > + remain_size = pp->mem_perf_size; unallocated/free_size ? > + base = pp->mem_perf_base; > + bus_addr = pp->mem_perf_bus_addr; > + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { > + win_size = SZ_4G - (base & (SZ_4G - 1)); > + win_size = min(win_size, remain_size); > + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, > + base, bus_addr, win_size); > + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", > + i, base, &bus_addr, win_size); > + > + base += win_size; > + bus_addr += win_size; > + remain_size -= win_size; > + } > + > + if (remain_size > 0) > + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); > + > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, > + pp->io_base, pp->io_bus_addr, > + pp->io_size); > } > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index a438c3879aa9..0197f67f82b7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -148,15 +148,22 @@ struct pcie_port { > u64 cfg1_base; > void __iomem *va_cfg1_base; > u32 cfg1_size; > + u32 cfg_idx; > resource_size_t io_base; > phys_addr_t io_bus_addr; > u32 io_size; > + u32 io_idx; > u64 mem_base; > phys_addr_t mem_bus_addr; > u64 mem_size; > + u64 mem_perf_base; This is a phys_addr_t > + phys_addr_t mem_perf_bus_addr; pci_bus_addr_t ? Lorenzo > + u64 mem_perf_size; > + u32 mem_wins; > struct resource *cfg; > struct resource *io; > struct resource *mem; > + struct resource *mem_perf; > struct resource *busn; > int irq; > const struct dw_pcie_host_ops *ops; > -- > 2.17.1 >
Hi Lorenzo, Thanks a lot for your comments! > -----Original Message----- > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Sent: 2018年11月29日 2:00 > To: Z.q. Hou <zhiqiang.hou@nxp.com> > Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; > bhelgaas@google.com; jingoohan1@gmail.com; > gustavo.pimentel@synopsys.com; Roy Zang <roy.zang@nxp.com>; Mingkai Hu > <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@nxp.com> > Subject: Re: [PATCHv2 4/4] PCI: dwc: add prefetchable memory range support > > On Wed, Nov 07, 2018 at 10:09:21AM +0000, Z.q. Hou wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > The current code only support non-prefetchable memory range, as the > > non-prefetchable memory range must not be greater than 4GiB, one > > viewport can cover it, which supports upto 4GiB. > > > > To support prefetchable memory range, which is upto 64-bit memory > > space and can be greater than 4GiB, so we need multiple viewports. And > > added separate vars to store prefetchable memory range info to prevent > > overriding the non-prefetchable memory range info. > > > > And this patch explicitly assigned the last (if there are only > > 2 viewports) or last 2 viewports for CFG and I/O windows and the rests > > for MEM windows. > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > --- > > V2: > > - Reworded the subject and commit description. > > - Fix the prefetchable memory range overriding non-perfetchable > > memory range issue by adding vars to store prefetchable memory > > range info. > > > > .../pci/controller/dwc/pcie-designware-host.c | 107 ++++++++++++++---- > > drivers/pci/controller/dwc/pcie-designware.h | 7 ++ > > 2 files changed, 95 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > > b/drivers/pci/controller/dwc/pcie-designware-host.c > > index ecacce016489..328aa40a6609 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -346,6 +346,35 @@ int dw_pcie_host_init(struct pcie_port *pp) > > dev_err(dev, "Missing *config* reg space\n"); > > } > > > > + /* > > + * If vendor's platform driver has set the num_viewport and it is > > + * not less than 2, skip getting the num_viewport from DT here. > > That's not what the device tree bindings specify. If DT does contain the > property you *must* read it. Yes, will change back to *must* read. > > > + */ > > + if (pci->num_viewport < 2) { > > + ret = of_property_read_u32(np, "num-viewport", > > + &pci->num_viewport); > > + if (ret || pci->num_viewport < 2) > > + pci->num_viewport = 2; > > + } > > + > > + /* > > + * if there are only 2 viewports, assign the last viewport for > > + * both CFG and IO window, otherwise assign the last 2 viewport > > Gah. Can anyone explain to me how this driver works if only two viewports > are available ? What happens if an IO access happens at the same time of a > config access (that fiddles with the outbound memory windows) ? You are right it has potential IO transaction drop issue, but DWC PCIe databook does give a default value 2 of viewport number. > > + * for CFG and IO window specific. And the rest viewports are > > + * assigned to MEM windows. > > + */ > > + if (pci->num_viewport == 2) { > > + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; > > + pp->mem_wins = 1; > > + } else { > > + pp->cfg_idx = pci->num_viewport - 1; > > + pp->io_idx = pci->num_viewport - 2; > > + pp->mem_wins = pci->num_viewport - 2; > > + } > > + > > + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", > > + pp->cfg_idx, pp->io_idx, pp->mem_wins); > > + > > bridge = pci_alloc_host_bridge(0); > > if (!bridge) > > return -ENOMEM; > > @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) > > } > > break; > > case IORESOURCE_MEM: > > - pp->mem = win->res; > > - pp->mem->name = "MEM"; > > - pp->mem_size = resource_size(pp->mem); > > - pp->mem_bus_addr = pp->mem->start - win->offset; > > + if (win->res->flags & IORESOURCE_PREFETCH) { > > + pp->mem_perf = win->res; > > + pp->mem_perf->name = "MEM perf"; > > Nit: Why "perf" and not "pref" ? > > It is confusing but that's the least of this patch problems. My bad, it is typo, will fix in next version. > > > + pp->mem_perf_size = resource_size(pp->mem_perf); > > + pp->mem_perf_bus_addr = pp->mem_perf->start - > > + win->offset; > > + pp->mem_perf_base = pp->mem_perf->start; > > + } else { > > + pp->mem = win->res; > > + pp->mem->name = "MEM"; > > + pp->mem_size = resource_size(pp->mem); > > + pp->mem_bus_addr = pp->mem->start - win->offset; > > + pp->mem_base = pp->mem->start; > > + } > > break; > > case 0: > > pp->cfg = win->res; > > @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > > } > > } > > > > - pp->mem_base = pp->mem->start; > > - > > if (!pp->va_cfg0_base) { > > pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, > > pp->cfg0_base, pp->cfg0_size); > > @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > va_cfg_base = pp->va_cfg1_base; > > } > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > type, cpu_addr, > > busdev, cfg_size); > > ret = dw_pcie_read(va_cfg_base + where, size, val); > > - if (pci->num_viewport <= 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + if (pp->cfg_idx == pp->io_idx) > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > PCIE_ATU_TYPE_IO, pp->io_base, > > pp->io_bus_addr, pp->io_size); > > See above, even though this is not related to this patch. I think it is more reasonable to check if the CFG and IO shared viewport than the viewport number. > > > > > @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > va_cfg_base = pp->va_cfg1_base; > > } > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > type, cpu_addr, > > busdev, cfg_size); > > ret = dw_pcie_write(va_cfg_base + where, size, val); > > - if (pci->num_viewport <= 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > + if (pp->cfg_idx == pp->io_idx) > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > PCIE_ATU_TYPE_IO, pp->io_base, > > pp->io_bus_addr, pp->io_size); > > > > @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct > > dw_pcie *pci) void dw_pcie_setup_rc(struct pcie_port *pp) { > > u32 val, ctrl, num_ctrls; > > + u64 remain_size, base, win_size; > > + phys_addr_t bus_addr; > > + int i; > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > > > dw_pcie_setup(pci); > > @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > > dev_dbg(pci->dev, "iATU unroll: %s\n", > > pci->iatu_unroll_enabled ? "enabled" : "disabled"); > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, > > - PCIE_ATU_TYPE_MEM, pp->mem_base, > > - pp->mem_bus_addr, pp->mem_size); > > - if (pci->num_viewport > 2) > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, > > - PCIE_ATU_TYPE_IO, pp->io_base, > > - pp->io_bus_addr, pp->io_size); > > + /* > > + * The maximum region size is 4 GB, and a region > > + * must not cross a 4 GB boundary. > > + */ > > + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); > > + win_size = min(win_size, pp->mem_size); > > + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, > > + pp->mem_base, pp->mem_bus_addr, > > + win_size); > > + dev_dbg(pci->dev, > > + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, > size = %llx\n", > > + 0, pp->mem_base, &pp->mem_bus_addr, win_size); > > + > > + /* Prefetchable range can be 64bit space */ > > + remain_size = pp->mem_perf_size; > > unallocated/free_size ? Sorry, I don't understand your question. > > > + base = pp->mem_perf_base; > > + bus_addr = pp->mem_perf_bus_addr; > > + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { > > + win_size = SZ_4G - (base & (SZ_4G - 1)); > > + win_size = min(win_size, remain_size); > > + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, > > + base, bus_addr, win_size); > > + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, > bus_addr = %pa, size = %llx\n", > > + i, base, &bus_addr, win_size); > > + > > + base += win_size; > > + bus_addr += win_size; > > + remain_size -= win_size; > > + } > > + > > + if (remain_size > 0) > > + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); > > + > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, > > + pp->io_base, pp->io_bus_addr, > > + pp->io_size); > > } > > > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); diff --git > > a/drivers/pci/controller/dwc/pcie-designware.h > > b/drivers/pci/controller/dwc/pcie-designware.h > > index a438c3879aa9..0197f67f82b7 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -148,15 +148,22 @@ struct pcie_port { > > u64 cfg1_base; > > void __iomem *va_cfg1_base; > > u32 cfg1_size; > > + u32 cfg_idx; > > resource_size_t io_base; > > phys_addr_t io_bus_addr; > > u32 io_size; > > + u32 io_idx; > > u64 mem_base; > > phys_addr_t mem_bus_addr; > > u64 mem_size; > > + u64 mem_perf_base; > > This is a phys_addr_t > > > + phys_addr_t mem_perf_bus_addr; > > pci_bus_addr_t ? Will fix in next version. > > Lorenzo > > > + u64 mem_perf_size; > > + u32 mem_wins; > > struct resource *cfg; > > struct resource *io; > > struct resource *mem; > > + struct resource *mem_perf; > > struct resource *busn; > > int irq; > > const struct dw_pcie_host_ops *ops; > > -- > > 2.17.1 > > Thanks, Zhiqiang
On Tue, Dec 11, 2018 at 10:21:08AM +0000, Z.q. Hou wrote: [...] > > > + */ > > > + if (pci->num_viewport < 2) { > > > + ret = of_property_read_u32(np, "num-viewport", > > > + &pci->num_viewport); > > > + if (ret || pci->num_viewport < 2) > > > + pci->num_viewport = 2; > > > + } > > > + > > > + /* > > > + * if there are only 2 viewports, assign the last viewport for > > > + * both CFG and IO window, otherwise assign the last 2 viewport > > > > Gah. Can anyone explain to me how this driver works if only two > > viewports are available ? What happens if an IO access happens at > > the same time of a config access (that fiddles with the outbound > > memory windows) ? Guys, this is utterly broken. If the IP does not have enough address decoders it is better not to support IO at all rather implementing it with this broken code that can fail anytime. This ought to be fixed and it is not a problem with this patch it is a *bug* in the mainline kernel. Lorenzo > You are right it has potential IO transaction drop issue, but DWC PCIe databook does give a default value 2 of viewport number. > > > > > + * for CFG and IO window specific. And the rest viewports are > > > + * assigned to MEM windows. > > > + */ > > > + if (pci->num_viewport == 2) { > > > + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; > > > + pp->mem_wins = 1; > > > + } else { > > > + pp->cfg_idx = pci->num_viewport - 1; > > > + pp->io_idx = pci->num_viewport - 2; > > > + pp->mem_wins = pci->num_viewport - 2; > > > + } > > > + > > > + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", > > > + pp->cfg_idx, pp->io_idx, pp->mem_wins); > > > + > > > bridge = pci_alloc_host_bridge(0); > > > if (!bridge) > > > return -ENOMEM; > > > @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) > > > } > > > break; > > > case IORESOURCE_MEM: > > > - pp->mem = win->res; > > > - pp->mem->name = "MEM"; > > > - pp->mem_size = resource_size(pp->mem); > > > - pp->mem_bus_addr = pp->mem->start - win->offset; > > > + if (win->res->flags & IORESOURCE_PREFETCH) { > > > + pp->mem_perf = win->res; > > > + pp->mem_perf->name = "MEM perf"; > > > > Nit: Why "perf" and not "pref" ? > > > > It is confusing but that's the least of this patch problems. > > My bad, it is typo, will fix in next version. > > > > > > + pp->mem_perf_size = resource_size(pp->mem_perf); > > > + pp->mem_perf_bus_addr = pp->mem_perf->start - > > > + win->offset; > > > + pp->mem_perf_base = pp->mem_perf->start; > > > + } else { > > > + pp->mem = win->res; > > > + pp->mem->name = "MEM"; > > > + pp->mem_size = resource_size(pp->mem); > > > + pp->mem_bus_addr = pp->mem->start - win->offset; > > > + pp->mem_base = pp->mem->start; > > > + } > > > break; > > > case 0: > > > pp->cfg = win->res; > > > @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) > > > } > > > } > > > > > > - pp->mem_base = pp->mem->start; > > > - > > > if (!pp->va_cfg0_base) { > > > pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, > > > pp->cfg0_base, pp->cfg0_size); > > > @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct > > pcie_port *pp, struct pci_bus *bus, > > > va_cfg_base = pp->va_cfg1_base; > > > } > > > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > > type, cpu_addr, > > > busdev, cfg_size); > > > ret = dw_pcie_read(va_cfg_base + where, size, val); > > > - if (pci->num_viewport <= 2) > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > > + if (pp->cfg_idx == pp->io_idx) > > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > > PCIE_ATU_TYPE_IO, pp->io_base, > > > pp->io_bus_addr, pp->io_size); > > > > See above, even though this is not related to this patch. > > I think it is more reasonable to check if the CFG and IO shared viewport than the viewport number. > > > > > > > > > @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct > > pcie_port *pp, struct pci_bus *bus, > > > va_cfg_base = pp->va_cfg1_base; > > > } > > > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > > + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, > > > type, cpu_addr, > > > busdev, cfg_size); > > > ret = dw_pcie_write(va_cfg_base + where, size, val); > > > - if (pci->num_viewport <= 2) > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, > > > + if (pp->cfg_idx == pp->io_idx) > > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, > > > PCIE_ATU_TYPE_IO, pp->io_base, > > > pp->io_bus_addr, pp->io_size); > > > > > > @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct > > > dw_pcie *pci) void dw_pcie_setup_rc(struct pcie_port *pp) { > > > u32 val, ctrl, num_ctrls; > > > + u64 remain_size, base, win_size; > > > + phys_addr_t bus_addr; > > > + int i; > > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > > > > > dw_pcie_setup(pci); > > > @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) > > > dev_dbg(pci->dev, "iATU unroll: %s\n", > > > pci->iatu_unroll_enabled ? "enabled" : "disabled"); > > > > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, > > > - PCIE_ATU_TYPE_MEM, pp->mem_base, > > > - pp->mem_bus_addr, pp->mem_size); > > > - if (pci->num_viewport > 2) > > > - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, > > > - PCIE_ATU_TYPE_IO, pp->io_base, > > > - pp->io_bus_addr, pp->io_size); > > > + /* > > > + * The maximum region size is 4 GB, and a region > > > + * must not cross a 4 GB boundary. > > > + */ > > > + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); > > > + win_size = min(win_size, pp->mem_size); > > > + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, > > > + pp->mem_base, pp->mem_bus_addr, > > > + win_size); > > > + dev_dbg(pci->dev, > > > + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, > > size = %llx\n", > > > + 0, pp->mem_base, &pp->mem_bus_addr, win_size); > > > + > > > + /* Prefetchable range can be 64bit space */ > > > + remain_size = pp->mem_perf_size; > > > > unallocated/free_size ? > > Sorry, I don't understand your question. > > > > > > + base = pp->mem_perf_base; > > > + bus_addr = pp->mem_perf_bus_addr; > > > + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { > > > + win_size = SZ_4G - (base & (SZ_4G - 1)); > > > + win_size = min(win_size, remain_size); > > > + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, > > > + base, bus_addr, win_size); > > > + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, > > bus_addr = %pa, size = %llx\n", > > > + i, base, &bus_addr, win_size); > > > + > > > + base += win_size; > > > + bus_addr += win_size; > > > + remain_size -= win_size; > > > + } > > > + > > > + if (remain_size > 0) > > > + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); > > > + > > > + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, > > > + pp->io_base, pp->io_bus_addr, > > > + pp->io_size); > > > } > > > > > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); diff --git > > > a/drivers/pci/controller/dwc/pcie-designware.h > > > b/drivers/pci/controller/dwc/pcie-designware.h > > > index a438c3879aa9..0197f67f82b7 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > @@ -148,15 +148,22 @@ struct pcie_port { > > > u64 cfg1_base; > > > void __iomem *va_cfg1_base; > > > u32 cfg1_size; > > > + u32 cfg_idx; > > > resource_size_t io_base; > > > phys_addr_t io_bus_addr; > > > u32 io_size; > > > + u32 io_idx; > > > u64 mem_base; > > > phys_addr_t mem_bus_addr; > > > u64 mem_size; > > > + u64 mem_perf_base; > > > > This is a phys_addr_t > > > > > + phys_addr_t mem_perf_bus_addr; > > > > pci_bus_addr_t ? > > Will fix in next version. > > > > > Lorenzo > > > > > + u64 mem_perf_size; > > > + u32 mem_wins; > > > struct resource *cfg; > > > struct resource *io; > > > struct resource *mem; > > > + struct resource *mem_perf; > > > struct resource *busn; > > > int irq; > > > const struct dw_pcie_host_ops *ops; > > > -- > > > 2.17.1 > > > > > Thanks, > Zhiqiang
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index ecacce016489..328aa40a6609 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -346,6 +346,35 @@ int dw_pcie_host_init(struct pcie_port *pp) dev_err(dev, "Missing *config* reg space\n"); } + /* + * If vendor's platform driver has set the num_viewport and it is + * not less than 2, skip getting the num_viewport from DT here. + */ + if (pci->num_viewport < 2) { + ret = of_property_read_u32(np, "num-viewport", + &pci->num_viewport); + if (ret || pci->num_viewport < 2) + pci->num_viewport = 2; + } + + /* + * if there are only 2 viewports, assign the last viewport for + * both CFG and IO window, otherwise assign the last 2 viewport + * for CFG and IO window specific. And the rest viewports are + * assigned to MEM windows. + */ + if (pci->num_viewport == 2) { + pp->cfg_idx = pp->io_idx = PCIE_ATU_REGION_INDEX1; + pp->mem_wins = 1; + } else { + pp->cfg_idx = pci->num_viewport - 1; + pp->io_idx = pci->num_viewport - 2; + pp->mem_wins = pci->num_viewport - 2; + } + + dev_dbg(dev, "CFG win id: %d, I/O win id: %d, Total MEM win: %d\n", + pp->cfg_idx, pp->io_idx, pp->mem_wins); + bridge = pci_alloc_host_bridge(0); if (!bridge) return -ENOMEM; @@ -377,10 +406,20 @@ int dw_pcie_host_init(struct pcie_port *pp) } break; case IORESOURCE_MEM: - pp->mem = win->res; - pp->mem->name = "MEM"; - pp->mem_size = resource_size(pp->mem); - pp->mem_bus_addr = pp->mem->start - win->offset; + if (win->res->flags & IORESOURCE_PREFETCH) { + pp->mem_perf = win->res; + pp->mem_perf->name = "MEM perf"; + pp->mem_perf_size = resource_size(pp->mem_perf); + pp->mem_perf_bus_addr = pp->mem_perf->start - + win->offset; + pp->mem_perf_base = pp->mem_perf->start; + } else { + pp->mem = win->res; + pp->mem->name = "MEM"; + pp->mem_size = resource_size(pp->mem); + pp->mem_bus_addr = pp->mem->start - win->offset; + pp->mem_base = pp->mem->start; + } break; case 0: pp->cfg = win->res; @@ -406,8 +445,6 @@ int dw_pcie_host_init(struct pcie_port *pp) } } - pp->mem_base = pp->mem->start; - if (!pp->va_cfg0_base) { pp->va_cfg0_base = devm_pci_remap_cfgspace(dev, pp->cfg0_base, pp->cfg0_size); @@ -534,12 +571,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, va_cfg_base = pp->va_cfg1_base; } - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, type, cpu_addr, busdev, cfg_size); ret = dw_pcie_read(va_cfg_base + where, size, val); - if (pci->num_viewport <= 2) - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, + if (pp->cfg_idx == pp->io_idx) + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, pp->io_base, pp->io_bus_addr, pp->io_size); @@ -573,12 +610,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, va_cfg_base = pp->va_cfg1_base; } - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, + dw_pcie_prog_outbound_atu(pci, pp->cfg_idx, type, cpu_addr, busdev, cfg_size); ret = dw_pcie_write(va_cfg_base + where, size, val); - if (pci->num_viewport <= 2) - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1, + if (pp->cfg_idx == pp->io_idx) + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, pp->io_base, pp->io_bus_addr, pp->io_size); @@ -652,6 +689,9 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci) void dw_pcie_setup_rc(struct pcie_port *pp) { u32 val, ctrl, num_ctrls; + u64 remain_size, base, win_size; + phys_addr_t bus_addr; + int i; struct dw_pcie *pci = to_dw_pcie_from_pp(pp); dw_pcie_setup(pci); @@ -700,13 +740,42 @@ void dw_pcie_setup_rc(struct pcie_port *pp) dev_dbg(pci->dev, "iATU unroll: %s\n", pci->iatu_unroll_enabled ? "enabled" : "disabled"); - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0, - PCIE_ATU_TYPE_MEM, pp->mem_base, - pp->mem_bus_addr, pp->mem_size); - if (pci->num_viewport > 2) - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2, - PCIE_ATU_TYPE_IO, pp->io_base, - pp->io_bus_addr, pp->io_size); + /* + * The maximum region size is 4 GB, and a region + * must not cross a 4 GB boundary. + */ + win_size = SZ_4G - (pp->mem_base & (SZ_4G - 1)); + win_size = min(win_size, pp->mem_size); + dw_pcie_prog_outbound_atu(pci, 0, PCIE_ATU_TYPE_MEM, + pp->mem_base, pp->mem_bus_addr, + win_size); + dev_dbg(pci->dev, + "iATU: non-pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", + 0, pp->mem_base, &pp->mem_bus_addr, win_size); + + /* Prefetchable range can be 64bit space */ + remain_size = pp->mem_perf_size; + base = pp->mem_perf_base; + bus_addr = pp->mem_perf_bus_addr; + for (i = 1; remain_size > 0 && i < pp->mem_wins; i++) { + win_size = SZ_4G - (base & (SZ_4G - 1)); + win_size = min(win_size, remain_size); + dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM, + base, bus_addr, win_size); + dev_dbg(pci->dev, "iATU: pref MEM: win = %d: base = %llx, bus_addr = %pa, size = %llx\n", + i, base, &bus_addr, win_size); + + base += win_size; + bus_addr += win_size; + remain_size -= win_size; + } + + if (remain_size > 0) + dev_info(pci->dev, "iATU: MEM window isn't enough\n"); + + dw_pcie_prog_outbound_atu(pci, pp->io_idx, PCIE_ATU_TYPE_IO, + pp->io_base, pp->io_bus_addr, + pp->io_size); } dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index a438c3879aa9..0197f67f82b7 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -148,15 +148,22 @@ struct pcie_port { u64 cfg1_base; void __iomem *va_cfg1_base; u32 cfg1_size; + u32 cfg_idx; resource_size_t io_base; phys_addr_t io_bus_addr; u32 io_size; + u32 io_idx; u64 mem_base; phys_addr_t mem_bus_addr; u64 mem_size; + u64 mem_perf_base; + phys_addr_t mem_perf_bus_addr; + u64 mem_perf_size; + u32 mem_wins; struct resource *cfg; struct resource *io; struct resource *mem; + struct resource *mem_perf; struct resource *busn; int irq; const struct dw_pcie_host_ops *ops;