linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: dwc: Use ATU regions to map memory regions
@ 2020-10-05 12:13 Vidya Sagar
  2020-10-19  5:51 ` Vidya Sagar
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Vidya Sagar @ 2020-10-05 12:13 UTC (permalink / raw)
  To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, bhelgaas,
	amurray, robh, treding, jonathanh
  Cc: linux-pci, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv

Use ATU region-3 and region-0 to setup mapping for prefetchable and
non-prefetchable memory regions respectively only if their respective CPU
and bus addresses are different.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
 drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
 drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
 3 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 317ff512f8df..cefde8e813e9 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
 	.write = pci_generic_config_write,
 };
 
+static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
+				  struct resource_entry *win)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
+	    win->offset) {
+		dw_pcie_prog_outbound_atu(pci,
+					  PCIE_ATU_REGION_INDEX3,
+					  PCIE_ATU_TYPE_MEM,
+					  win->res->start,
+					  win->res->start - win->offset,
+					  resource_size(win->res));
+	} else if (win->res->flags & IORESOURCE_PREFETCH &&
+		   pci->num_viewport < 4) {
+		dev_warn(pci->dev,
+			 "Insufficient ATU regions to map Prefetchable memory\n");
+	} else if (win->offset) {
+		if (upper_32_bits(resource_size(win->res)))
+			dev_warn(pci->dev,
+				 "Memory resource size exceeds max for 32 bits\n");
+		dw_pcie_prog_outbound_atu(pci,
+					  PCIE_ATU_REGION_INDEX0,
+					  PCIE_ATU_TYPE_MEM,
+					  win->res->start,
+					  win->res->start - win->offset,
+					  resource_size(win->res));
+	}
+}
+
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val, ctrl, num_ctrls;
+	struct resource_entry *win;
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 
 	/*
@@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	 * ATU, so we should not program the ATU here.
 	 */
 	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
-		struct resource_entry *entry =
-			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+		resource_list_for_each_entry(win, &pp->bridge->windows) {
+			switch (resource_type(win->res)) {
+			case IORESOURCE_MEM:
+				dw_pcie_setup_mem_atu(pp, win);
+				break;
+			}
+		}
 
-		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
-					  PCIE_ATU_TYPE_MEM, entry->res->start,
-					  entry->res->start - entry->offset,
-					  resource_size(entry->res));
 		if (pci->num_viewport > 2)
 			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
 						  PCIE_ATU_TYPE_IO, pp->io_base,
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 3c1f17c78241..6033689abb15 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
 static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 					     int index, int type,
 					     u64 cpu_addr, u64 pci_addr,
-					     u32 size)
+					     u64 size)
 {
 	u32 retries, val;
 	u64 limit_addr = cpu_addr + size - 1;
@@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 				 lower_32_bits(pci_addr));
 	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
 				 upper_32_bits(pci_addr));
-	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
-				 type | PCIE_ATU_FUNC_NUM(func_no));
+	val = type | PCIE_ATU_FUNC_NUM(func_no);
+	val = upper_32_bits(size - 1) ?
+		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
+	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
 	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
 				 PCIE_ATU_ENABLE);
 
@@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 
 static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 					int index, int type, u64 cpu_addr,
-					u64 pci_addr, u32 size)
+					u64 pci_addr, u64 size)
 {
 	u32 retries, val;
 
@@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 }
 
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
-			       u64 cpu_addr, u64 pci_addr, u32 size)
+			       u64 cpu_addr, u64 pci_addr, u64 size)
 {
 	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
 				    cpu_addr, pci_addr, size);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 97c7063b9e89..b81a1813cf9e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -80,10 +80,12 @@
 #define PCIE_ATU_VIEWPORT		0x900
 #define PCIE_ATU_REGION_INBOUND		BIT(31)
 #define PCIE_ATU_REGION_OUTBOUND	0
+#define PCIE_ATU_REGION_INDEX3		0x3
 #define PCIE_ATU_REGION_INDEX2		0x2
 #define PCIE_ATU_REGION_INDEX1		0x1
 #define PCIE_ATU_REGION_INDEX0		0x0
 #define PCIE_ATU_CR1			0x904
+#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
 #define PCIE_ATU_TYPE_MEM		0x0
 #define PCIE_ATU_TYPE_IO		0x2
 #define PCIE_ATU_TYPE_CFG0		0x4
@@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
 int dw_pcie_wait_for_link(struct dw_pcie *pci);
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
 			       int type, u64 cpu_addr, u64 pci_addr,
-			       u32 size);
+			       u64 size);
 void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
 				  int type, u64 cpu_addr, u64 pci_addr,
 				  u32 size);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: dwc: Use ATU regions to map memory regions
  2020-10-05 12:13 [PATCH] PCI: dwc: Use ATU regions to map memory regions Vidya Sagar
@ 2020-10-19  5:51 ` Vidya Sagar
  2020-10-20 13:20   ` Lorenzo Pieralisi
  2020-10-20 15:59 ` Lorenzo Pieralisi
  2020-10-20 19:59 ` [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping Vidya Sagar
  2 siblings, 1 reply; 12+ messages in thread
From: Vidya Sagar @ 2020-10-19  5:51 UTC (permalink / raw)
  To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, bhelgaas,
	amurray, robh, treding, jonathanh
  Cc: linux-pci, linux-kernel, kthota, mmaddireddy, sagar.tv

Hi Lorenzo, Rob, Gustavo,
Could you please review this change?

Thanks,
Vidya Sagar

On 10/5/2020 5:43 PM, Vidya Sagar wrote:
> Use ATU region-3 and region-0 to setup mapping for prefetchable and
> non-prefetchable memory regions respectively only if their respective CPU
> and bus addresses are different.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>   .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
>   drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
>   drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>   3 files changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 317ff512f8df..cefde8e813e9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
>   	.write = pci_generic_config_write,
>   };
>   
> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> +				  struct resource_entry *win)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
> +	    win->offset) {
> +		dw_pcie_prog_outbound_atu(pci,
> +					  PCIE_ATU_REGION_INDEX3,
> +					  PCIE_ATU_TYPE_MEM,
> +					  win->res->start,
> +					  win->res->start - win->offset,
> +					  resource_size(win->res));
> +	} else if (win->res->flags & IORESOURCE_PREFETCH &&
> +		   pci->num_viewport < 4) {
> +		dev_warn(pci->dev,
> +			 "Insufficient ATU regions to map Prefetchable memory\n");
> +	} else if (win->offset) {
> +		if (upper_32_bits(resource_size(win->res)))
> +			dev_warn(pci->dev,
> +				 "Memory resource size exceeds max for 32 bits\n");
> +		dw_pcie_prog_outbound_atu(pci,
> +					  PCIE_ATU_REGION_INDEX0,
> +					  PCIE_ATU_TYPE_MEM,
> +					  win->res->start,
> +					  win->res->start - win->offset,
> +					  resource_size(win->res));
> +	}
> +}
> +
>   void dw_pcie_setup_rc(struct pcie_port *pp)
>   {
>   	u32 val, ctrl, num_ctrls;
> +	struct resource_entry *win;
>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>   
>   	/*
> @@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>   	 * ATU, so we should not program the ATU here.
>   	 */
>   	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> -		struct resource_entry *entry =
> -			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +		resource_list_for_each_entry(win, &pp->bridge->windows) {
> +			switch (resource_type(win->res)) {
> +			case IORESOURCE_MEM:
> +				dw_pcie_setup_mem_atu(pp, win);
> +				break;
> +			}
> +		}
>   
> -		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> -					  PCIE_ATU_TYPE_MEM, entry->res->start,
> -					  entry->res->start - entry->offset,
> -					  resource_size(entry->res));
>   		if (pci->num_viewport > 2)
>   			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>   						  PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 3c1f17c78241..6033689abb15 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>   static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>   					     int index, int type,
>   					     u64 cpu_addr, u64 pci_addr,
> -					     u32 size)
> +					     u64 size)
>   {
>   	u32 retries, val;
>   	u64 limit_addr = cpu_addr + size - 1;
> @@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>   				 lower_32_bits(pci_addr));
>   	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>   				 upper_32_bits(pci_addr));
> -	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> -				 type | PCIE_ATU_FUNC_NUM(func_no));
> +	val = type | PCIE_ATU_FUNC_NUM(func_no);
> +	val = upper_32_bits(size - 1) ?
> +		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>   	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>   				 PCIE_ATU_ENABLE);
>   
> @@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>   
>   static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>   					int index, int type, u64 cpu_addr,
> -					u64 pci_addr, u32 size)
> +					u64 pci_addr, u64 size)
>   {
>   	u32 retries, val;
>   
> @@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>   }
>   
>   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> -			       u64 cpu_addr, u64 pci_addr, u32 size)
> +			       u64 cpu_addr, u64 pci_addr, u64 size)
>   {
>   	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
>   				    cpu_addr, pci_addr, size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 97c7063b9e89..b81a1813cf9e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -80,10 +80,12 @@
>   #define PCIE_ATU_VIEWPORT		0x900
>   #define PCIE_ATU_REGION_INBOUND		BIT(31)
>   #define PCIE_ATU_REGION_OUTBOUND	0
> +#define PCIE_ATU_REGION_INDEX3		0x3
>   #define PCIE_ATU_REGION_INDEX2		0x2
>   #define PCIE_ATU_REGION_INDEX1		0x1
>   #define PCIE_ATU_REGION_INDEX0		0x0
>   #define PCIE_ATU_CR1			0x904
> +#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
>   #define PCIE_ATU_TYPE_MEM		0x0
>   #define PCIE_ATU_TYPE_IO		0x2
>   #define PCIE_ATU_TYPE_CFG0		0x4
> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>   int dw_pcie_wait_for_link(struct dw_pcie *pci);
>   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>   			       int type, u64 cpu_addr, u64 pci_addr,
> -			       u32 size);
> +			       u64 size);
>   void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>   				  int type, u64 cpu_addr, u64 pci_addr,
>   				  u32 size);
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: dwc: Use ATU regions to map memory regions
  2020-10-19  5:51 ` Vidya Sagar
@ 2020-10-20 13:20   ` Lorenzo Pieralisi
  2020-10-20 13:33     ` Vidya Sagar
  0 siblings, 1 reply; 12+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-20 13:20 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: jingoohan1, gustavo.pimentel, bhelgaas, amurray, robh, treding,
	jonathanh, linux-pci, linux-kernel, kthota, mmaddireddy,
	sagar.tv

On Mon, Oct 19, 2020 at 11:21:54AM +0530, Vidya Sagar wrote:
> Hi Lorenzo, Rob, Gustavo,
> Could you please review this change?

Next cycle - we are in the middle of the merge window and I am not
queueing any more patches.

Thanks,
Lorenzo

> Thanks,
> Vidya Sagar
> 
> On 10/5/2020 5:43 PM, Vidya Sagar wrote:
> > Use ATU region-3 and region-0 to setup mapping for prefetchable and
> > non-prefetchable memory regions respectively only if their respective CPU
> > and bus addresses are different.
> > 
> > Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > ---
> >   .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
> >   drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
> >   drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
> >   3 files changed, 48 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 317ff512f8df..cefde8e813e9 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
> >   	.write = pci_generic_config_write,
> >   };
> > +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> > +				  struct resource_entry *win)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +
> > +	if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
> > +	    win->offset) {
> > +		dw_pcie_prog_outbound_atu(pci,
> > +					  PCIE_ATU_REGION_INDEX3,
> > +					  PCIE_ATU_TYPE_MEM,
> > +					  win->res->start,
> > +					  win->res->start - win->offset,
> > +					  resource_size(win->res));
> > +	} else if (win->res->flags & IORESOURCE_PREFETCH &&
> > +		   pci->num_viewport < 4) {
> > +		dev_warn(pci->dev,
> > +			 "Insufficient ATU regions to map Prefetchable memory\n");
> > +	} else if (win->offset) {
> > +		if (upper_32_bits(resource_size(win->res)))
> > +			dev_warn(pci->dev,
> > +				 "Memory resource size exceeds max for 32 bits\n");
> > +		dw_pcie_prog_outbound_atu(pci,
> > +					  PCIE_ATU_REGION_INDEX0,
> > +					  PCIE_ATU_TYPE_MEM,
> > +					  win->res->start,
> > +					  win->res->start - win->offset,
> > +					  resource_size(win->res));
> > +	}
> > +}
> > +
> >   void dw_pcie_setup_rc(struct pcie_port *pp)
> >   {
> >   	u32 val, ctrl, num_ctrls;
> > +	struct resource_entry *win;
> >   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> >   	/*
> > @@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> >   	 * ATU, so we should not program the ATU here.
> >   	 */
> >   	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> > -		struct resource_entry *entry =
> > -			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> > +		resource_list_for_each_entry(win, &pp->bridge->windows) {
> > +			switch (resource_type(win->res)) {
> > +			case IORESOURCE_MEM:
> > +				dw_pcie_setup_mem_atu(pp, win);
> > +				break;
> > +			}
> > +		}
> > -		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> > -					  PCIE_ATU_TYPE_MEM, entry->res->start,
> > -					  entry->res->start - entry->offset,
> > -					  resource_size(entry->res));
> >   		if (pci->num_viewport > 2)
> >   			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
> >   						  PCIE_ATU_TYPE_IO, pp->io_base,
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 3c1f17c78241..6033689abb15 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
> >   static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> >   					     int index, int type,
> >   					     u64 cpu_addr, u64 pci_addr,
> > -					     u32 size)
> > +					     u64 size)
> >   {
> >   	u32 retries, val;
> >   	u64 limit_addr = cpu_addr + size - 1;
> > @@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> >   				 lower_32_bits(pci_addr));
> >   	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> >   				 upper_32_bits(pci_addr));
> > -	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> > -				 type | PCIE_ATU_FUNC_NUM(func_no));
> > +	val = type | PCIE_ATU_FUNC_NUM(func_no);
> > +	val = upper_32_bits(size - 1) ?
> > +		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
> >   	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> >   				 PCIE_ATU_ENABLE);
> > @@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> >   static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >   					int index, int type, u64 cpu_addr,
> > -					u64 pci_addr, u32 size)
> > +					u64 pci_addr, u64 size)
> >   {
> >   	u32 retries, val;
> > @@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >   }
> >   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > -			       u64 cpu_addr, u64 pci_addr, u32 size)
> > +			       u64 cpu_addr, u64 pci_addr, u64 size)
> >   {
> >   	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
> >   				    cpu_addr, pci_addr, size);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 97c7063b9e89..b81a1813cf9e 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -80,10 +80,12 @@
> >   #define PCIE_ATU_VIEWPORT		0x900
> >   #define PCIE_ATU_REGION_INBOUND		BIT(31)
> >   #define PCIE_ATU_REGION_OUTBOUND	0
> > +#define PCIE_ATU_REGION_INDEX3		0x3
> >   #define PCIE_ATU_REGION_INDEX2		0x2
> >   #define PCIE_ATU_REGION_INDEX1		0x1
> >   #define PCIE_ATU_REGION_INDEX0		0x0
> >   #define PCIE_ATU_CR1			0x904
> > +#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
> >   #define PCIE_ATU_TYPE_MEM		0x0
> >   #define PCIE_ATU_TYPE_IO		0x2
> >   #define PCIE_ATU_TYPE_CFG0		0x4
> > @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> >   int dw_pcie_wait_for_link(struct dw_pcie *pci);
> >   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> >   			       int type, u64 cpu_addr, u64 pci_addr,
> > -			       u32 size);
> > +			       u64 size);
> >   void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> >   				  int type, u64 cpu_addr, u64 pci_addr,
> >   				  u32 size);
> > 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: dwc: Use ATU regions to map memory regions
  2020-10-20 13:20   ` Lorenzo Pieralisi
@ 2020-10-20 13:33     ` Vidya Sagar
  2020-10-20 13:41       ` Lorenzo Pieralisi
  0 siblings, 1 reply; 12+ messages in thread
From: Vidya Sagar @ 2020-10-20 13:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: jingoohan1, gustavo.pimentel, bhelgaas, amurray, robh, treding,
	jonathanh, linux-pci, linux-kernel, kthota, mmaddireddy,
	sagar.tv



On 10/20/2020 6:50 PM, Lorenzo Pieralisi wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Mon, Oct 19, 2020 at 11:21:54AM +0530, Vidya Sagar wrote:
>> Hi Lorenzo, Rob, Gustavo,
>> Could you please review this change?
> 
> Next cycle - we are in the middle of the merge window and I am not
> queueing any more patches.

Thanks for the update.
FWIW, PCIe is broken on Tegra194 with Rob's patches (which got accepted 
already) and without the current patch.

Thanks,
Vidya Sagar

> 
> Thanks,
> Lorenzo
> 
>> Thanks,
>> Vidya Sagar
>>
>> On 10/5/2020 5:43 PM, Vidya Sagar wrote:
>>> Use ATU region-3 and region-0 to setup mapping for prefetchable and
>>> non-prefetchable memory regions respectively only if their respective CPU
>>> and bus addresses are different.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> ---
>>>    .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
>>>    drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
>>>    drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>>>    3 files changed, 48 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> index 317ff512f8df..cefde8e813e9 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>> @@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
>>>      .write = pci_generic_config_write,
>>>    };
>>> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
>>> +                             struct resource_entry *win)
>>> +{
>>> +   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> +
>>> +   if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
>>> +       win->offset) {
>>> +           dw_pcie_prog_outbound_atu(pci,
>>> +                                     PCIE_ATU_REGION_INDEX3,
>>> +                                     PCIE_ATU_TYPE_MEM,
>>> +                                     win->res->start,
>>> +                                     win->res->start - win->offset,
>>> +                                     resource_size(win->res));
>>> +   } else if (win->res->flags & IORESOURCE_PREFETCH &&
>>> +              pci->num_viewport < 4) {
>>> +           dev_warn(pci->dev,
>>> +                    "Insufficient ATU regions to map Prefetchable memory\n");
>>> +   } else if (win->offset) {
>>> +           if (upper_32_bits(resource_size(win->res)))
>>> +                   dev_warn(pci->dev,
>>> +                            "Memory resource size exceeds max for 32 bits\n");
>>> +           dw_pcie_prog_outbound_atu(pci,
>>> +                                     PCIE_ATU_REGION_INDEX0,
>>> +                                     PCIE_ATU_TYPE_MEM,
>>> +                                     win->res->start,
>>> +                                     win->res->start - win->offset,
>>> +                                     resource_size(win->res));
>>> +   }
>>> +}
>>> +
>>>    void dw_pcie_setup_rc(struct pcie_port *pp)
>>>    {
>>>      u32 val, ctrl, num_ctrls;
>>> +   struct resource_entry *win;
>>>      struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>      /*
>>> @@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>>       * ATU, so we should not program the ATU here.
>>>       */
>>>      if (pp->bridge->child_ops == &dw_child_pcie_ops) {
>>> -           struct resource_entry *entry =
>>> -                   resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
>>> +           resource_list_for_each_entry(win, &pp->bridge->windows) {
>>> +                   switch (resource_type(win->res)) {
>>> +                   case IORESOURCE_MEM:
>>> +                           dw_pcie_setup_mem_atu(pp, win);
>>> +                           break;
>>> +                   }
>>> +           }
>>> -           dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
>>> -                                     PCIE_ATU_TYPE_MEM, entry->res->start,
>>> -                                     entry->res->start - entry->offset,
>>> -                                     resource_size(entry->res));
>>>              if (pci->num_viewport > 2)
>>>                      dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>>>                                                PCIE_ATU_TYPE_IO, pp->io_base,
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>> index 3c1f17c78241..6033689abb15 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>> @@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>>>    static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>>                                           int index, int type,
>>>                                           u64 cpu_addr, u64 pci_addr,
>>> -                                        u32 size)
>>> +                                        u64 size)
>>>    {
>>>      u32 retries, val;
>>>      u64 limit_addr = cpu_addr + size - 1;
>>> @@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>>                               lower_32_bits(pci_addr));
>>>      dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>>>                               upper_32_bits(pci_addr));
>>> -   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
>>> -                            type | PCIE_ATU_FUNC_NUM(func_no));
>>> +   val = type | PCIE_ATU_FUNC_NUM(func_no);
>>> +   val = upper_32_bits(size - 1) ?
>>> +           val | PCIE_ATU_INCREASE_REGION_SIZE : val;
>>> +   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>>>      dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>>>                               PCIE_ATU_ENABLE);
>>> @@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>>    static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>>>                                      int index, int type, u64 cpu_addr,
>>> -                                   u64 pci_addr, u32 size)
>>> +                                   u64 pci_addr, u64 size)
>>>    {
>>>      u32 retries, val;
>>> @@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>>>    }
>>>    void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>> -                          u64 cpu_addr, u64 pci_addr, u32 size)
>>> +                          u64 cpu_addr, u64 pci_addr, u64 size)
>>>    {
>>>      __dw_pcie_prog_outbound_atu(pci, 0, index, type,
>>>                                  cpu_addr, pci_addr, size);
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>>> index 97c7063b9e89..b81a1813cf9e 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>>> @@ -80,10 +80,12 @@
>>>    #define PCIE_ATU_VIEWPORT         0x900
>>>    #define PCIE_ATU_REGION_INBOUND           BIT(31)
>>>    #define PCIE_ATU_REGION_OUTBOUND  0
>>> +#define PCIE_ATU_REGION_INDEX3             0x3
>>>    #define PCIE_ATU_REGION_INDEX2            0x2
>>>    #define PCIE_ATU_REGION_INDEX1            0x1
>>>    #define PCIE_ATU_REGION_INDEX0            0x0
>>>    #define PCIE_ATU_CR1                      0x904
>>> +#define PCIE_ATU_INCREASE_REGION_SIZE      BIT(13)
>>>    #define PCIE_ATU_TYPE_MEM         0x0
>>>    #define PCIE_ATU_TYPE_IO          0x2
>>>    #define PCIE_ATU_TYPE_CFG0                0x4
>>> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>>>    int dw_pcie_wait_for_link(struct dw_pcie *pci);
>>>    void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>                             int type, u64 cpu_addr, u64 pci_addr,
>>> -                          u32 size);
>>> +                          u64 size);
>>>    void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>>>                                int type, u64 cpu_addr, u64 pci_addr,
>>>                                u32 size);
>>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: dwc: Use ATU regions to map memory regions
  2020-10-20 13:33     ` Vidya Sagar
@ 2020-10-20 13:41       ` Lorenzo Pieralisi
  0 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-20 13:41 UTC (permalink / raw)
  To: Vidya Sagar, robh
  Cc: jingoohan1, gustavo.pimentel, bhelgaas, amurray, treding,
	jonathanh, linux-pci, linux-kernel, kthota, mmaddireddy,
	sagar.tv

On Tue, Oct 20, 2020 at 07:03:59PM +0530, Vidya Sagar wrote:
> 
> 
> On 10/20/2020 6:50 PM, Lorenzo Pieralisi wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Mon, Oct 19, 2020 at 11:21:54AM +0530, Vidya Sagar wrote:
> > > Hi Lorenzo, Rob, Gustavo,
> > > Could you please review this change?
> > 
> > Next cycle - we are in the middle of the merge window and I am not
> > queueing any more patches.
> 
> Thanks for the update.
> FWIW, PCIe is broken on Tegra194 with Rob's patches (which got accepted
> already) and without the current patch.

Ah, that changes the picture then, it was not clear, this requires
immediate attention then.

Thanks,
Lorenzo

> Thanks,
> Vidya Sagar
> 
> > 
> > Thanks,
> > Lorenzo
> > 
> > > Thanks,
> > > Vidya Sagar
> > > 
> > > On 10/5/2020 5:43 PM, Vidya Sagar wrote:
> > > > Use ATU region-3 and region-0 to setup mapping for prefetchable and
> > > > non-prefetchable memory regions respectively only if their respective CPU
> > > > and bus addresses are different.
> > > > 
> > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > > > ---
> > > >    .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
> > > >    drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
> > > >    drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
> > > >    3 files changed, 48 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > index 317ff512f8df..cefde8e813e9 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > @@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
> > > >      .write = pci_generic_config_write,
> > > >    };
> > > > +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> > > > +                             struct resource_entry *win)
> > > > +{
> > > > +   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > +
> > > > +   if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
> > > > +       win->offset) {
> > > > +           dw_pcie_prog_outbound_atu(pci,
> > > > +                                     PCIE_ATU_REGION_INDEX3,
> > > > +                                     PCIE_ATU_TYPE_MEM,
> > > > +                                     win->res->start,
> > > > +                                     win->res->start - win->offset,
> > > > +                                     resource_size(win->res));
> > > > +   } else if (win->res->flags & IORESOURCE_PREFETCH &&
> > > > +              pci->num_viewport < 4) {
> > > > +           dev_warn(pci->dev,
> > > > +                    "Insufficient ATU regions to map Prefetchable memory\n");
> > > > +   } else if (win->offset) {
> > > > +           if (upper_32_bits(resource_size(win->res)))
> > > > +                   dev_warn(pci->dev,
> > > > +                            "Memory resource size exceeds max for 32 bits\n");
> > > > +           dw_pcie_prog_outbound_atu(pci,
> > > > +                                     PCIE_ATU_REGION_INDEX0,
> > > > +                                     PCIE_ATU_TYPE_MEM,
> > > > +                                     win->res->start,
> > > > +                                     win->res->start - win->offset,
> > > > +                                     resource_size(win->res));
> > > > +   }
> > > > +}
> > > > +
> > > >    void dw_pcie_setup_rc(struct pcie_port *pp)
> > > >    {
> > > >      u32 val, ctrl, num_ctrls;
> > > > +   struct resource_entry *win;
> > > >      struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > >      /*
> > > > @@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> > > >       * ATU, so we should not program the ATU here.
> > > >       */
> > > >      if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> > > > -           struct resource_entry *entry =
> > > > -                   resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> > > > +           resource_list_for_each_entry(win, &pp->bridge->windows) {
> > > > +                   switch (resource_type(win->res)) {
> > > > +                   case IORESOURCE_MEM:
> > > > +                           dw_pcie_setup_mem_atu(pp, win);
> > > > +                           break;
> > > > +                   }
> > > > +           }
> > > > -           dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> > > > -                                     PCIE_ATU_TYPE_MEM, entry->res->start,
> > > > -                                     entry->res->start - entry->offset,
> > > > -                                     resource_size(entry->res));
> > > >              if (pci->num_viewport > 2)
> > > >                      dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
> > > >                                                PCIE_ATU_TYPE_IO, pp->io_base,
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > > index 3c1f17c78241..6033689abb15 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > @@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
> > > >    static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> > > >                                           int index, int type,
> > > >                                           u64 cpu_addr, u64 pci_addr,
> > > > -                                        u32 size)
> > > > +                                        u64 size)
> > > >    {
> > > >      u32 retries, val;
> > > >      u64 limit_addr = cpu_addr + size - 1;
> > > > @@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> > > >                               lower_32_bits(pci_addr));
> > > >      dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> > > >                               upper_32_bits(pci_addr));
> > > > -   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> > > > -                            type | PCIE_ATU_FUNC_NUM(func_no));
> > > > +   val = type | PCIE_ATU_FUNC_NUM(func_no);
> > > > +   val = upper_32_bits(size - 1) ?
> > > > +           val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> > > > +   dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
> > > >      dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> > > >                               PCIE_ATU_ENABLE);
> > > > @@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> > > >    static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > > >                                      int index, int type, u64 cpu_addr,
> > > > -                                   u64 pci_addr, u32 size)
> > > > +                                   u64 pci_addr, u64 size)
> > > >    {
> > > >      u32 retries, val;
> > > > @@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > > >    }
> > > >    void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> > > > -                          u64 cpu_addr, u64 pci_addr, u32 size)
> > > > +                          u64 cpu_addr, u64 pci_addr, u64 size)
> > > >    {
> > > >      __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> > > >                                  cpu_addr, pci_addr, size);
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index 97c7063b9e89..b81a1813cf9e 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -80,10 +80,12 @@
> > > >    #define PCIE_ATU_VIEWPORT         0x900
> > > >    #define PCIE_ATU_REGION_INBOUND           BIT(31)
> > > >    #define PCIE_ATU_REGION_OUTBOUND  0
> > > > +#define PCIE_ATU_REGION_INDEX3             0x3
> > > >    #define PCIE_ATU_REGION_INDEX2            0x2
> > > >    #define PCIE_ATU_REGION_INDEX1            0x1
> > > >    #define PCIE_ATU_REGION_INDEX0            0x0
> > > >    #define PCIE_ATU_CR1                      0x904
> > > > +#define PCIE_ATU_INCREASE_REGION_SIZE      BIT(13)
> > > >    #define PCIE_ATU_TYPE_MEM         0x0
> > > >    #define PCIE_ATU_TYPE_IO          0x2
> > > >    #define PCIE_ATU_TYPE_CFG0                0x4
> > > > @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> > > >    int dw_pcie_wait_for_link(struct dw_pcie *pci);
> > > >    void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> > > >                             int type, u64 cpu_addr, u64 pci_addr,
> > > > -                          u32 size);
> > > > +                          u64 size);
> > > >    void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > > >                                int type, u64 cpu_addr, u64 pci_addr,
> > > >                                u32 size);
> > > > 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: dwc: Use ATU regions to map memory regions
  2020-10-05 12:13 [PATCH] PCI: dwc: Use ATU regions to map memory regions Vidya Sagar
  2020-10-19  5:51 ` Vidya Sagar
@ 2020-10-20 15:59 ` Lorenzo Pieralisi
  2020-10-20 19:59 ` [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping Vidya Sagar
  2 siblings, 0 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-20 15:59 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: jingoohan1, gustavo.pimentel, bhelgaas, amurray, robh, treding,
	jonathanh, linux-pci, linux-kernel, kthota, mmaddireddy,
	sagar.tv

On Mon, Oct 05, 2020 at 05:43:51PM +0530, Vidya Sagar wrote:
> Use ATU region-3 and region-0 to setup mapping for prefetchable and
> non-prefetchable memory regions respectively only if their respective CPU
> and bus addresses are different.
> 

The commit subject and log must be rewritten. You should describe
why you are making this change, add a Link: tag to the discussion
we had about this change and provide a reason why we are making it.

Also, please add a Fixes: tag reference to the commit you are fixing.

I think this is related to prefetchable handling in DT and dwc/tegra,
we do need a link to those discussions here please.

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 44 ++++++++++++++++---
>  drivers/pci/controller/dwc/pcie-designware.c  | 12 ++---
>  drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>  3 files changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 317ff512f8df..cefde8e813e9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -515,9 +515,40 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> +				  struct resource_entry *win)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	if (win->res->flags & IORESOURCE_PREFETCH && pci->num_viewport >= 4 &&
> +	    win->offset) {
> +		dw_pcie_prog_outbound_atu(pci,
> +					  PCIE_ATU_REGION_INDEX3,
> +					  PCIE_ATU_TYPE_MEM,
> +					  win->res->start,
> +					  win->res->start - win->offset,
> +					  resource_size(win->res));
> +	} else if (win->res->flags & IORESOURCE_PREFETCH &&
> +		   pci->num_viewport < 4) {
> +		dev_warn(pci->dev,
> +			 "Insufficient ATU regions to map Prefetchable memory\n");
> +	} else if (win->offset) {
> +		if (upper_32_bits(resource_size(win->res)))
> +			dev_warn(pci->dev,
> +				 "Memory resource size exceeds max for 32 bits\n");
> +		dw_pcie_prog_outbound_atu(pci,
> +					  PCIE_ATU_REGION_INDEX0,
> +					  PCIE_ATU_TYPE_MEM,
> +					  win->res->start,
> +					  win->res->start - win->offset,
> +					  resource_size(win->res));
> +	}

This function logic must be explained - anyone else reading it should
be able to understand it, I think it really deserves a comment.

> +}
> +
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>  	u32 val, ctrl, num_ctrls;
> +	struct resource_entry *win;
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  
>  	/*
> @@ -572,13 +603,14 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  	 * ATU, so we should not program the ATU here.
>  	 */
>  	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> -		struct resource_entry *entry =
> -			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +		resource_list_for_each_entry(win, &pp->bridge->windows) {
> +			switch (resource_type(win->res)) {
> +			case IORESOURCE_MEM:
> +				dw_pcie_setup_mem_atu(pp, win);
> +				break;
> +			}

Nit: an if statement would do but that's not where the problem lies.

> +		}
>  
> -		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> -					  PCIE_ATU_TYPE_MEM, entry->res->start,
> -					  entry->res->start - entry->offset,
> -					  resource_size(entry->res));
>  		if (pci->num_viewport > 2)
>  			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>  						  PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 3c1f17c78241..6033689abb15 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -227,7 +227,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  					     int index, int type,
>  					     u64 cpu_addr, u64 pci_addr,
> -					     u32 size)
> +					     u64 size)
>  {
>  	u32 retries, val;
>  	u64 limit_addr = cpu_addr + size - 1;
> @@ -244,8 +244,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  				 lower_32_bits(pci_addr));
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>  				 upper_32_bits(pci_addr));
> -	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> -				 type | PCIE_ATU_FUNC_NUM(func_no));
> +	val = type | PCIE_ATU_FUNC_NUM(func_no);
> +	val = upper_32_bits(size - 1) ?
> +		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>  				 PCIE_ATU_ENABLE);
>  
> @@ -266,7 +268,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  
>  static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  					int index, int type, u64 cpu_addr,
> -					u64 pci_addr, u32 size)
> +					u64 pci_addr, u64 size)
>  {
>  	u32 retries, val;
>  
> @@ -310,7 +312,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  }
>  
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> -			       u64 cpu_addr, u64 pci_addr, u32 size)
> +			       u64 cpu_addr, u64 pci_addr, u64 size)
>  {
>  	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
>  				    cpu_addr, pci_addr, size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 97c7063b9e89..b81a1813cf9e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -80,10 +80,12 @@
>  #define PCIE_ATU_VIEWPORT		0x900
>  #define PCIE_ATU_REGION_INBOUND		BIT(31)
>  #define PCIE_ATU_REGION_OUTBOUND	0
> +#define PCIE_ATU_REGION_INDEX3		0x3
>  #define PCIE_ATU_REGION_INDEX2		0x2
>  #define PCIE_ATU_REGION_INDEX1		0x1
>  #define PCIE_ATU_REGION_INDEX0		0x0
>  #define PCIE_ATU_CR1			0x904
> +#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
>  #define PCIE_ATU_TYPE_MEM		0x0
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  			       int type, u64 cpu_addr, u64 pci_addr,
> -			       u32 size);
> +			       u64 size);
>  void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>  				  int type, u64 cpu_addr, u64 pci_addr,
>  				  u32 size);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-05 12:13 [PATCH] PCI: dwc: Use ATU regions to map memory regions Vidya Sagar
  2020-10-19  5:51 ` Vidya Sagar
  2020-10-20 15:59 ` Lorenzo Pieralisi
@ 2020-10-20 19:59 ` Vidya Sagar
  2020-10-21 10:08   ` Lorenzo Pieralisi
  2020-10-22 19:08   ` Rob Herring
  2 siblings, 2 replies; 12+ messages in thread
From: Vidya Sagar @ 2020-10-20 19:59 UTC (permalink / raw)
  To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, bhelgaas,
	amurray, robh, treding, jonathanh
  Cc: linux-pci, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv

DWC sub-system currently doesn't differentiate between prefetchable and
non-prefetchable memory aperture entries in the 'ranges' property and
provides ATU mapping only for the first memory aperture entry out of all
the entries present. This was introduced by the
commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
Mapping for a memory apreture is required if its CPU address and the bus
address are different and the current mechanism works only if the memory
aperture which needs mapping happens to be the first entry. It doesn't
work either if the memory aperture that needs mapping is not the first
entry or if both prefetchable and non-prefetchable apertures need mapping.

This patch fixes this issue by differentiating between prefetchable and
non-prefetchable apertures in the 'ranges' property there by removing the
dependency on the order in which they are specified and adds support for
mapping prefetchable aperture using ATU region-3 if required.

Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* Rewrote commit subject and description
* Addressed review comments from Lorenzo

 .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
 drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
 drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
 3 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index db547ee6ff3a..dae6da39bb90 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
 	.write = pci_generic_config_write,
 };
 
+static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
+				  struct resource_entry *win)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	/* Check for prefetchable memory aperture */
+	if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
+		/* Number of view ports must at least be 4 to enable mapping */
+		if (pci->num_viewport < 4) {
+			dev_warn(pci->dev,
+				 "Insufficient ATU regions to map Prefetchable memory\n");
+		} else {
+			dw_pcie_prog_outbound_atu(pci,
+						  PCIE_ATU_REGION_INDEX3,
+						  PCIE_ATU_TYPE_MEM,
+						  win->res->start,
+						  win->res->start - win->offset,
+						  resource_size(win->res));
+		}
+	} else if (win->offset) { /* Non-prefetchable memory aperture */
+		if (upper_32_bits(resource_size(win->res)))
+			dev_warn(pci->dev,
+				 "Memory resource size exceeds max for 32 bits\n");
+		dw_pcie_prog_outbound_atu(pci,
+					  PCIE_ATU_REGION_INDEX0,
+					  PCIE_ATU_TYPE_MEM,
+					  win->res->start,
+					  win->res->start - win->offset,
+					  resource_size(win->res));
+	}
+}
+
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val, ctrl, num_ctrls;
+	struct resource_entry *win;
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 
 	/*
@@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	 * ATU, so we should not program the ATU here.
 	 */
 	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
-		struct resource_entry *entry =
-			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+		resource_list_for_each_entry(win, &pp->bridge->windows)
+			if (resource_type(win->res) == IORESOURCE_MEM)
+				dw_pcie_setup_mem_atu(pp, win);
 
-		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
-					  PCIE_ATU_TYPE_MEM, entry->res->start,
-					  entry->res->start - entry->offset,
-					  resource_size(entry->res));
 		if (pci->num_viewport > 2)
 			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
 						  PCIE_ATU_TYPE_IO, pp->io_base,
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c2dea8fc97c8..b5e438b70cd5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
 static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 					     int index, int type,
 					     u64 cpu_addr, u64 pci_addr,
-					     u32 size)
+					     u64 size)
 {
 	u32 retries, val;
 	u64 limit_addr = cpu_addr + size - 1;
@@ -245,8 +245,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 				 lower_32_bits(pci_addr));
 	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
 				 upper_32_bits(pci_addr));
-	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
-				 type | PCIE_ATU_FUNC_NUM(func_no));
+	val = type | PCIE_ATU_FUNC_NUM(func_no);
+	val = upper_32_bits(size - 1) ?
+		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
+	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
 	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
 				 PCIE_ATU_ENABLE);
 
@@ -267,7 +269,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
 
 static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 					int index, int type, u64 cpu_addr,
-					u64 pci_addr, u32 size)
+					u64 pci_addr, u64 size)
 {
 	u32 retries, val;
 
@@ -311,7 +313,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 }
 
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
-			       u64 cpu_addr, u64 pci_addr, u32 size)
+			       u64 cpu_addr, u64 pci_addr, u64 size)
 {
 	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
 				    cpu_addr, pci_addr, size);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 9d2f511f13fa..21dd06831b50 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -80,10 +80,12 @@
 #define PCIE_ATU_VIEWPORT		0x900
 #define PCIE_ATU_REGION_INBOUND		BIT(31)
 #define PCIE_ATU_REGION_OUTBOUND	0
+#define PCIE_ATU_REGION_INDEX3		0x3
 #define PCIE_ATU_REGION_INDEX2		0x2
 #define PCIE_ATU_REGION_INDEX1		0x1
 #define PCIE_ATU_REGION_INDEX0		0x0
 #define PCIE_ATU_CR1			0x904
+#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
 #define PCIE_ATU_TYPE_MEM		0x0
 #define PCIE_ATU_TYPE_IO		0x2
 #define PCIE_ATU_TYPE_CFG0		0x4
@@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
 int dw_pcie_wait_for_link(struct dw_pcie *pci);
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
 			       int type, u64 cpu_addr, u64 pci_addr,
-			       u32 size);
+			       u64 size);
 void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
 				  int type, u64 cpu_addr, u64 pci_addr,
 				  u32 size);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-20 19:59 ` [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping Vidya Sagar
@ 2020-10-21 10:08   ` Lorenzo Pieralisi
  2020-10-22 19:08   ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-21 10:08 UTC (permalink / raw)
  To: Vidya Sagar, jingoohan1, gustavo.pimentel
  Cc: bhelgaas, amurray, robh, treding, jonathanh, linux-pci,
	linux-kernel, kthota, mmaddireddy, sagar.tv

Jingoo, Gustavo,

please review this patch, thanks.

Lorenzo

On Wed, Oct 21, 2020 at 01:29:31AM +0530, Vidya Sagar wrote:
> DWC sub-system currently doesn't differentiate between prefetchable and
> non-prefetchable memory aperture entries in the 'ranges' property and
> provides ATU mapping only for the first memory aperture entry out of all
> the entries present. This was introduced by the
> commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
> Mapping for a memory apreture is required if its CPU address and the bus
> address are different and the current mechanism works only if the memory
> aperture which needs mapping happens to be the first entry. It doesn't
> work either if the memory aperture that needs mapping is not the first
> entry or if both prefetchable and non-prefetchable apertures need mapping.
> 
> This patch fixes this issue by differentiating between prefetchable and
> non-prefetchable apertures in the 'ranges' property there by removing the
> dependency on the order in which they are specified and adds support for
> mapping prefetchable aperture using ATU region-3 if required.
> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * Rewrote commit subject and description
> * Addressed review comments from Lorenzo
> 
>  .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
>  drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
>  drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>  3 files changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index db547ee6ff3a..dae6da39bb90 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> +				  struct resource_entry *win)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	/* Check for prefetchable memory aperture */
> +	if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
> +		/* Number of view ports must at least be 4 to enable mapping */
> +		if (pci->num_viewport < 4) {
> +			dev_warn(pci->dev,
> +				 "Insufficient ATU regions to map Prefetchable memory\n");
> +		} else {
> +			dw_pcie_prog_outbound_atu(pci,
> +						  PCIE_ATU_REGION_INDEX3,
> +						  PCIE_ATU_TYPE_MEM,
> +						  win->res->start,
> +						  win->res->start - win->offset,
> +						  resource_size(win->res));
> +		}
> +	} else if (win->offset) { /* Non-prefetchable memory aperture */
> +		if (upper_32_bits(resource_size(win->res)))
> +			dev_warn(pci->dev,
> +				 "Memory resource size exceeds max for 32 bits\n");
> +		dw_pcie_prog_outbound_atu(pci,
> +					  PCIE_ATU_REGION_INDEX0,
> +					  PCIE_ATU_TYPE_MEM,
> +					  win->res->start,
> +					  win->res->start - win->offset,
> +					  resource_size(win->res));
> +	}
> +}
> +
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>  	u32 val, ctrl, num_ctrls;
> +	struct resource_entry *win;
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  
>  	/*
> @@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  	 * ATU, so we should not program the ATU here.
>  	 */
>  	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> -		struct resource_entry *entry =
> -			resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +		resource_list_for_each_entry(win, &pp->bridge->windows)
> +			if (resource_type(win->res) == IORESOURCE_MEM)
> +				dw_pcie_setup_mem_atu(pp, win);
>  
> -		dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> -					  PCIE_ATU_TYPE_MEM, entry->res->start,
> -					  entry->res->start - entry->offset,
> -					  resource_size(entry->res));
>  		if (pci->num_viewport > 2)
>  			dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>  						  PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c2dea8fc97c8..b5e438b70cd5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  					     int index, int type,
>  					     u64 cpu_addr, u64 pci_addr,
> -					     u32 size)
> +					     u64 size)
>  {
>  	u32 retries, val;
>  	u64 limit_addr = cpu_addr + size - 1;
> @@ -245,8 +245,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  				 lower_32_bits(pci_addr));
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>  				 upper_32_bits(pci_addr));
> -	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> -				 type | PCIE_ATU_FUNC_NUM(func_no));
> +	val = type | PCIE_ATU_FUNC_NUM(func_no);
> +	val = upper_32_bits(size - 1) ?
> +		val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>  	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>  				 PCIE_ATU_ENABLE);
>  
> @@ -267,7 +269,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>  
>  static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  					int index, int type, u64 cpu_addr,
> -					u64 pci_addr, u32 size)
> +					u64 pci_addr, u64 size)
>  {
>  	u32 retries, val;
>  
> @@ -311,7 +313,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  }
>  
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> -			       u64 cpu_addr, u64 pci_addr, u32 size)
> +			       u64 cpu_addr, u64 pci_addr, u64 size)
>  {
>  	__dw_pcie_prog_outbound_atu(pci, 0, index, type,
>  				    cpu_addr, pci_addr, size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 9d2f511f13fa..21dd06831b50 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -80,10 +80,12 @@
>  #define PCIE_ATU_VIEWPORT		0x900
>  #define PCIE_ATU_REGION_INBOUND		BIT(31)
>  #define PCIE_ATU_REGION_OUTBOUND	0
> +#define PCIE_ATU_REGION_INDEX3		0x3
>  #define PCIE_ATU_REGION_INDEX2		0x2
>  #define PCIE_ATU_REGION_INDEX1		0x1
>  #define PCIE_ATU_REGION_INDEX0		0x0
>  #define PCIE_ATU_CR1			0x904
> +#define PCIE_ATU_INCREASE_REGION_SIZE	BIT(13)
>  #define PCIE_ATU_TYPE_MEM		0x0
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  			       int type, u64 cpu_addr, u64 pci_addr,
> -			       u32 size);
> +			       u64 size);
>  void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>  				  int type, u64 cpu_addr, u64 pci_addr,
>  				  u32 size);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-20 19:59 ` [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping Vidya Sagar
  2020-10-21 10:08   ` Lorenzo Pieralisi
@ 2020-10-22 19:08   ` Rob Herring
  2020-10-23  7:38     ` Vidya Sagar
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-10-22 19:08 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas,
	Andrew Murray, Thierry Reding, Jon Hunter, PCI, linux-kernel,
	kthota, Manikanta Maddireddy, sagar.tv

On Tue, Oct 20, 2020 at 2:59 PM Vidya Sagar <vidyas@nvidia.com> wrote:
>
> DWC sub-system currently doesn't differentiate between prefetchable and
> non-prefetchable memory aperture entries in the 'ranges' property and
> provides ATU mapping only for the first memory aperture entry out of all
> the entries present. This was introduced by the
> commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
> Mapping for a memory apreture is required if its CPU address and the bus
> address are different and the current mechanism works only if the memory
> aperture which needs mapping happens to be the first entry. It doesn't
> work either if the memory aperture that needs mapping is not the first
> entry or if both prefetchable and non-prefetchable apertures need mapping.

Well that's subtle...

> This patch fixes this issue by differentiating between prefetchable and
> non-prefetchable apertures in the 'ranges' property there by removing the
> dependency on the order in which they are specified and adds support for
> mapping prefetchable aperture using ATU region-3 if required.

Now you don't do any iATU entry for a 1:1 memory range which is a
change for pretty much every other platform. That means we leave the
PCI transaction config to the whims of how h/w designers hooked up the
sideband signals. I guess this is how Uniphier works as it only has 1
viewport...

I think the assignment should be in this order:
- config space
- non-prefetchable (IIRC, it's required)
- prefetchable
- i/o

Stopping assignment and warning if you run out of viewports. Looking
at the platforms, I think that would always work. There's only
uniphier and ls1012a where we run out. Those would still behave the
same.


>
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * Rewrote commit subject and description
> * Addressed review comments from Lorenzo
>
>  .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
>  drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
>  drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>  3 files changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index db547ee6ff3a..dae6da39bb90 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
>         .write = pci_generic_config_write,
>  };
>
> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> +                                 struct resource_entry *win)
> +{
> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +       /* Check for prefetchable memory aperture */
> +       if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
> +               /* Number of view ports must at least be 4 to enable mapping */
> +               if (pci->num_viewport < 4) {
> +                       dev_warn(pci->dev,
> +                                "Insufficient ATU regions to map Prefetchable memory\n");
> +               } else {
> +                       dw_pcie_prog_outbound_atu(pci,
> +                                                 PCIE_ATU_REGION_INDEX3,
> +                                                 PCIE_ATU_TYPE_MEM,
> +                                                 win->res->start,
> +                                                 win->res->start - win->offset,
> +                                                 resource_size(win->res));
> +               }
> +       } else if (win->offset) { /* Non-prefetchable memory aperture */
> +               if (upper_32_bits(resource_size(win->res)))
> +                       dev_warn(pci->dev,
> +                                "Memory resource size exceeds max for 32 bits\n");

This is not designware specific. Either core code should check this or
write a DT schema to check it.

> +               dw_pcie_prog_outbound_atu(pci,
> +                                         PCIE_ATU_REGION_INDEX0,
> +                                         PCIE_ATU_TYPE_MEM,
> +                                         win->res->start,
> +                                         win->res->start - win->offset,
> +                                         resource_size(win->res));
> +       }
> +}
> +
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>         u32 val, ctrl, num_ctrls;
> +       struct resource_entry *win;
>         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>
>         /*
> @@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>          * ATU, so we should not program the ATU here.
>          */
>         if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> -               struct resource_entry *entry =
> -                       resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +               resource_list_for_each_entry(win, &pp->bridge->windows)
> +                       if (resource_type(win->res) == IORESOURCE_MEM)
> +                               dw_pcie_setup_mem_atu(pp, win);
>
> -               dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> -                                         PCIE_ATU_TYPE_MEM, entry->res->start,
> -                                         entry->res->start - entry->offset,
> -                                         resource_size(entry->res));
>                 if (pci->num_viewport > 2)
>                         dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>                                                   PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c2dea8fc97c8..b5e438b70cd5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>                                              int index, int type,
>                                              u64 cpu_addr, u64 pci_addr,
> -                                            u32 size)
> +                                            u64 size)

How was this working for you before? Looks like a different change
from what I broke.

>  {
>         u32 retries, val;
>         u64 limit_addr = cpu_addr + size - 1;
> @@ -245,8 +245,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>                                  lower_32_bits(pci_addr));
>         dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>                                  upper_32_bits(pci_addr));
> -       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> -                                type | PCIE_ATU_FUNC_NUM(func_no));
> +       val = type | PCIE_ATU_FUNC_NUM(func_no);
> +       val = upper_32_bits(size - 1) ?
> +               val | PCIE_ATU_INCREASE_REGION_SIZE : val;
> +       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>         dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>                                  PCIE_ATU_ENABLE);
>
> @@ -267,7 +269,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>
>  static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>                                         int index, int type, u64 cpu_addr,
> -                                       u64 pci_addr, u32 size)
> +                                       u64 pci_addr, u64 size)
>  {
>         u32 retries, val;
>
> @@ -311,7 +313,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  }
>
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> -                              u64 cpu_addr, u64 pci_addr, u32 size)
> +                              u64 cpu_addr, u64 pci_addr, u64 size)
>  {
>         __dw_pcie_prog_outbound_atu(pci, 0, index, type,
>                                     cpu_addr, pci_addr, size);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 9d2f511f13fa..21dd06831b50 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -80,10 +80,12 @@
>  #define PCIE_ATU_VIEWPORT              0x900
>  #define PCIE_ATU_REGION_INBOUND                BIT(31)
>  #define PCIE_ATU_REGION_OUTBOUND       0
> +#define PCIE_ATU_REGION_INDEX3         0x3
>  #define PCIE_ATU_REGION_INDEX2         0x2
>  #define PCIE_ATU_REGION_INDEX1         0x1
>  #define PCIE_ATU_REGION_INDEX0         0x0
>  #define PCIE_ATU_CR1                   0x904
> +#define PCIE_ATU_INCREASE_REGION_SIZE  BIT(13)
>  #define PCIE_ATU_TYPE_MEM              0x0
>  #define PCIE_ATU_TYPE_IO               0x2
>  #define PCIE_ATU_TYPE_CFG0             0x4
> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>                                int type, u64 cpu_addr, u64 pci_addr,
> -                              u32 size);
> +                              u64 size);
>  void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>                                   int type, u64 cpu_addr, u64 pci_addr,
>                                   u32 size);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-22 19:08   ` Rob Herring
@ 2020-10-23  7:38     ` Vidya Sagar
  2020-10-23 15:37       ` Rob Herring
  0 siblings, 1 reply; 12+ messages in thread
From: Vidya Sagar @ 2020-10-23  7:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas,
	Andrew Murray, Thierry Reding, Jon Hunter, PCI, linux-kernel,
	kthota, Manikanta Maddireddy, sagar.tv



On 10/23/2020 12:38 AM, Rob Herring wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Oct 20, 2020 at 2:59 PM Vidya Sagar <vidyas@nvidia.com> wrote:
>>
>> DWC sub-system currently doesn't differentiate between prefetchable and
>> non-prefetchable memory aperture entries in the 'ranges' property and
>> provides ATU mapping only for the first memory aperture entry out of all
>> the entries present. This was introduced by the
>> commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
>> Mapping for a memory apreture is required if its CPU address and the bus
>> address are different and the current mechanism works only if the memory
>> aperture which needs mapping happens to be the first entry. It doesn't
>> work either if the memory aperture that needs mapping is not the first
>> entry or if both prefetchable and non-prefetchable apertures need mapping.
> 
> Well that's subtle...
> 
>> This patch fixes this issue by differentiating between prefetchable and
>> non-prefetchable apertures in the 'ranges' property there by removing the
>> dependency on the order in which they are specified and adds support for
>> mapping prefetchable aperture using ATU region-3 if required.
> 
> Now you don't do any iATU entry for a 1:1 memory range which is a
> change for pretty much every other platform. That means we leave the
> PCI transaction config to the whims of how h/w designers hooked up the
> sideband signals. I guess this is how Uniphier works as it only has 1
> viewport...
> 
> I think the assignment should be in this order:
> - config space
> - non-prefetchable (IIRC, it's required)
> - prefetchable
> - i/o
> 
> Stopping assignment and warning if you run out of viewports. Looking
> at the platforms, I think that would always work. There's only
> uniphier and ls1012a where we run out. Those would still behave the
> same.
As I see from the code this is how the current mapping is done

Region-0: [Fixed] Non-Prefetchable memory mapping

Region-1: [Shared] if the num of view ports <= 2
         Used for I/O by default but whenever config space is accessed, 
it is programmed to generate config space, and once done, will program 
it back for I/O generation. I'm not sure how they are synchonized when 
two different entities are trying to access the config space as well as 
the I/O at the same time.  I don't see any locking mechanism (or am I 
missing something here??)
           [Fixed] if the num of view ports > 2
         Used to generate configuration space accesses

Region-2: [Fixed] I/O accesses

Region-3: [Fixed] Prefetchable memory mapping (This patch is adding it)

I honestly think that an attempt to re-assing what region is used for 
what purpose should go into a different patch.

I agree that I'm removing configuring an iATU region if it is for 1:1 
memory mapping. What I heard from our HW designers is that it is the 
default behavior of the Synopsys IP that if the CPU access falls in the 
aperture owned by Synopsys IP and there is no iATU region mapped to 
capture and generate any specific transaction for that address, then, by 
default it generates a memory transaction over the PCIe bus.
It is also possible that some implementors might have chosen to alter 
this behavior. In any case, I can continue to have the iATU programming 
done irrespective of whether it is for 1:1 mapping or not.

> 
> 
>>
>> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
>> Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * Rewrote commit subject and description
>> * Addressed review comments from Lorenzo
>>
>>   .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
>>   drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
>>   drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>>   3 files changed, 46 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index db547ee6ff3a..dae6da39bb90 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
>>          .write = pci_generic_config_write,
>>   };
>>
>> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
>> +                                 struct resource_entry *win)
>> +{
>> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> +
>> +       /* Check for prefetchable memory aperture */
>> +       if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
>> +               /* Number of view ports must at least be 4 to enable mapping */
>> +               if (pci->num_viewport < 4) {
>> +                       dev_warn(pci->dev,
>> +                                "Insufficient ATU regions to map Prefetchable memory\n");
>> +               } else {
>> +                       dw_pcie_prog_outbound_atu(pci,
>> +                                                 PCIE_ATU_REGION_INDEX3,
>> +                                                 PCIE_ATU_TYPE_MEM,
>> +                                                 win->res->start,
>> +                                                 win->res->start - win->offset,
>> +                                                 resource_size(win->res));
>> +               }
>> +       } else if (win->offset) { /* Non-prefetchable memory aperture */
>> +               if (upper_32_bits(resource_size(win->res)))
>> +                       dev_warn(pci->dev,
>> +                                "Memory resource size exceeds max for 32 bits\n");
> 
> This is not designware specific. Either core code should check this or
> write a DT schema to check it.
Ok. I'll move it to pci_parse_request_of_pci_ranges() API. A change like 
below would do right?

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index ac24cd5439a9..6d872458196c 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -556,6 +556,11 @@ static int pci_parse_request_of_pci_ranges(struct 
device *dev,
                         break;
                 case IORESOURCE_MEM:
                         res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+
+                       if (!(res->flags & IORESOURCE_PREFETCH))
+                               if (upper_32_bits(resource_size(res)))
+                                       dev_warn(dev,
+                                                "Memory resource size 
exceeds max for 32 bits\n");
                         break;
                 }
         }

I hope that is the right place for it.

> 
>> +               dw_pcie_prog_outbound_atu(pci,
>> +                                         PCIE_ATU_REGION_INDEX0,
>> +                                         PCIE_ATU_TYPE_MEM,
>> +                                         win->res->start,
>> +                                         win->res->start - win->offset,
>> +                                         resource_size(win->res));
>> +       }
>> +}
>> +
>>   void dw_pcie_setup_rc(struct pcie_port *pp)
>>   {
>>          u32 val, ctrl, num_ctrls;
>> +       struct resource_entry *win;
>>          struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>
>>          /*
>> @@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>           * ATU, so we should not program the ATU here.
>>           */
>>          if (pp->bridge->child_ops == &dw_child_pcie_ops) {
>> -               struct resource_entry *entry =
>> -                       resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
>> +               resource_list_for_each_entry(win, &pp->bridge->windows)
>> +                       if (resource_type(win->res) == IORESOURCE_MEM)
>> +                               dw_pcie_setup_mem_atu(pp, win);
>>
>> -               dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
>> -                                         PCIE_ATU_TYPE_MEM, entry->res->start,
>> -                                         entry->res->start - entry->offset,
>> -                                         resource_size(entry->res));
>>                  if (pci->num_viewport > 2)
>>                          dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>>                                                    PCIE_ATU_TYPE_IO, pp->io_base,
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index c2dea8fc97c8..b5e438b70cd5 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>>   static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>                                               int index, int type,
>>                                               u64 cpu_addr, u64 pci_addr,
>> -                                            u32 size)
>> +                                            u64 size)
> 
> How was this working for you before? Looks like a different change
> from what I broke.
Tegra194 has a 1:1 mapping for prefetchable memory as of today. But, 
when it is changed to a non 1:1 mapping, we need this support.

> 
>>   {
>>          u32 retries, val;
>>          u64 limit_addr = cpu_addr + size - 1;
>> @@ -245,8 +245,10 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>                                   lower_32_bits(pci_addr));
>>          dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
>>                                   upper_32_bits(pci_addr));
>> -       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
>> -                                type | PCIE_ATU_FUNC_NUM(func_no));
>> +       val = type | PCIE_ATU_FUNC_NUM(func_no);
>> +       val = upper_32_bits(size - 1) ?
>> +               val | PCIE_ATU_INCREASE_REGION_SIZE : val;
>> +       dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, val);
>>          dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
>>                                   PCIE_ATU_ENABLE);
>>
>> @@ -267,7 +269,7 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>
>>   static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>>                                          int index, int type, u64 cpu_addr,
>> -                                       u64 pci_addr, u32 size)
>> +                                       u64 pci_addr, u64 size)
>>   {
>>          u32 retries, val;
>>
>> @@ -311,7 +313,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>>   }
>>
>>   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>> -                              u64 cpu_addr, u64 pci_addr, u32 size)
>> +                              u64 cpu_addr, u64 pci_addr, u64 size)
>>   {
>>          __dw_pcie_prog_outbound_atu(pci, 0, index, type,
>>                                      cpu_addr, pci_addr, size);
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 9d2f511f13fa..21dd06831b50 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -80,10 +80,12 @@
>>   #define PCIE_ATU_VIEWPORT              0x900
>>   #define PCIE_ATU_REGION_INBOUND                BIT(31)
>>   #define PCIE_ATU_REGION_OUTBOUND       0
>> +#define PCIE_ATU_REGION_INDEX3         0x3
>>   #define PCIE_ATU_REGION_INDEX2         0x2
>>   #define PCIE_ATU_REGION_INDEX1         0x1
>>   #define PCIE_ATU_REGION_INDEX0         0x0
>>   #define PCIE_ATU_CR1                   0x904
>> +#define PCIE_ATU_INCREASE_REGION_SIZE  BIT(13)
>>   #define PCIE_ATU_TYPE_MEM              0x0
>>   #define PCIE_ATU_TYPE_IO               0x2
>>   #define PCIE_ATU_TYPE_CFG0             0x4
>> @@ -295,7 +297,7 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>>   int dw_pcie_wait_for_link(struct dw_pcie *pci);
>>   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>                                 int type, u64 cpu_addr, u64 pci_addr,
>> -                              u32 size);
>> +                              u64 size);
>>   void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
>>                                    int type, u64 cpu_addr, u64 pci_addr,
>>                                    u32 size);
>> --
>> 2.17.1
>>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-23  7:38     ` Vidya Sagar
@ 2020-10-23 15:37       ` Rob Herring
  2020-10-23 19:55         ` Vidya Sagar
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-10-23 15:37 UTC (permalink / raw)
  To: Vidya Sagar
  Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Bjorn Helgaas,
	Andrew Murray, Thierry Reding, Jon Hunter, PCI, linux-kernel,
	kthota, Manikanta Maddireddy, sagar.tv

On Fri, Oct 23, 2020 at 2:38 AM Vidya Sagar <vidyas@nvidia.com> wrote:
>
>
>
> On 10/23/2020 12:38 AM, Rob Herring wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Tue, Oct 20, 2020 at 2:59 PM Vidya Sagar <vidyas@nvidia.com> wrote:
> >>
> >> DWC sub-system currently doesn't differentiate between prefetchable and
> >> non-prefetchable memory aperture entries in the 'ranges' property and
> >> provides ATU mapping only for the first memory aperture entry out of all
> >> the entries present. This was introduced by the
> >> commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
> >> Mapping for a memory apreture is required if its CPU address and the bus
> >> address are different and the current mechanism works only if the memory
> >> aperture which needs mapping happens to be the first entry. It doesn't
> >> work either if the memory aperture that needs mapping is not the first
> >> entry or if both prefetchable and non-prefetchable apertures need mapping.
> >
> > Well that's subtle...
> >
> >> This patch fixes this issue by differentiating between prefetchable and
> >> non-prefetchable apertures in the 'ranges' property there by removing the
> >> dependency on the order in which they are specified and adds support for
> >> mapping prefetchable aperture using ATU region-3 if required.
> >
> > Now you don't do any iATU entry for a 1:1 memory range which is a
> > change for pretty much every other platform. That means we leave the
> > PCI transaction config to the whims of how h/w designers hooked up the
> > sideband signals. I guess this is how Uniphier works as it only has 1
> > viewport...
> >
> > I think the assignment should be in this order:
> > - config space
> > - non-prefetchable (IIRC, it's required)
> > - prefetchable
> > - i/o
> >
> > Stopping assignment and warning if you run out of viewports. Looking
> > at the platforms, I think that would always work. There's only
> > uniphier and ls1012a where we run out. Those would still behave the
> > same.
> As I see from the code this is how the current mapping is done
>
> Region-0: [Fixed] Non-Prefetchable memory mapping
>
> Region-1: [Shared] if the num of view ports <= 2
>          Used for I/O by default but whenever config space is accessed,
> it is programmed to generate config space, and once done, will program
> it back for I/O generation. I'm not sure how they are synchonized when
> two different entities are trying to access the config space as well as
> the I/O at the same time.  I don't see any locking mechanism (or am I
> missing something here??)

They aren't synchronized. We just get lucky that I/O and config
accesses aren't interleaved frequently. IMO, we should just not
support I/O space on those platforms. AIUI, almost everything doesn't
use I/O nowadays.

>            [Fixed] if the num of view ports > 2
>          Used to generate configuration space accesses
>
> Region-2: [Fixed] I/O accesses
>
> Region-3: [Fixed] Prefetchable memory mapping (This patch is adding it)
>
> I honestly think that an attempt to re-assing what region is used for
> what purpose should go into a different patch.

Normally, I'd agree for a fix. If you want to fix it in a minimal way
such that we just setup the last memory region instead of the first,
then that's fine. But to implement the review feedback, you will
simply be adding support for multiple memory regions. The ATU doesn't
care about prefetchable or not. I think it will end up being a more
simple implementation if you just do the I/O region last and only if
you have an available.

>
> I agree that I'm removing configuring an iATU region if it is for 1:1
> memory mapping. What I heard from our HW designers is that it is the
> default behavior of the Synopsys IP that if the CPU access falls in the
> aperture owned by Synopsys IP and there is no iATU region mapped to
> capture and generate any specific transaction for that address, then, by
> default it generates a memory transaction over the PCIe bus.
> It is also possible that some implementors might have chosen to alter
> this behavior.

If we assume they haven't, that pretty much guarantees they did. :)

> In any case, I can continue to have the iATU programming
> done irrespective of whether it is for 1:1 mapping or not.
>
> >
> >
> >>
> >> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> >> Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/
> >>
> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> >> ---
> >> V2:
> >> * Rewrote commit subject and description
> >> * Addressed review comments from Lorenzo
> >>
> >>   .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
> >>   drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
> >>   drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
> >>   3 files changed, 46 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> >> index db547ee6ff3a..dae6da39bb90 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> >> @@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
> >>          .write = pci_generic_config_write,
> >>   };
> >>
> >> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
> >> +                                 struct resource_entry *win)
> >> +{
> >> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> >> +
> >> +       /* Check for prefetchable memory aperture */
> >> +       if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
> >> +               /* Number of view ports must at least be 4 to enable mapping */
> >> +               if (pci->num_viewport < 4) {
> >> +                       dev_warn(pci->dev,
> >> +                                "Insufficient ATU regions to map Prefetchable memory\n");
> >> +               } else {
> >> +                       dw_pcie_prog_outbound_atu(pci,
> >> +                                                 PCIE_ATU_REGION_INDEX3,
> >> +                                                 PCIE_ATU_TYPE_MEM,
> >> +                                                 win->res->start,
> >> +                                                 win->res->start - win->offset,
> >> +                                                 resource_size(win->res));
> >> +               }
> >> +       } else if (win->offset) { /* Non-prefetchable memory aperture */
> >> +               if (upper_32_bits(resource_size(win->res)))
> >> +                       dev_warn(pci->dev,
> >> +                                "Memory resource size exceeds max for 32 bits\n");
> >
> > This is not designware specific. Either core code should check this or
> > write a DT schema to check it.
> Ok. I'll move it to pci_parse_request_of_pci_ranges() API. A change like
> below would do right?

Yes.

>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index ac24cd5439a9..6d872458196c 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -556,6 +556,11 @@ static int pci_parse_request_of_pci_ranges(struct
> device *dev,
>                          break;
>                  case IORESOURCE_MEM:
>                          res_valid |= !(res->flags & IORESOURCE_PREFETCH);
> +
> +                       if (!(res->flags & IORESOURCE_PREFETCH))
> +                               if (upper_32_bits(resource_size(res)))
> +                                       dev_warn(dev,
> +                                                "Memory resource size
> exceeds max for 32 bits\n");
>                          break;
>                  }
>          }
>
> I hope that is the right place for it.
>
> >
> >> +               dw_pcie_prog_outbound_atu(pci,
> >> +                                         PCIE_ATU_REGION_INDEX0,
> >> +                                         PCIE_ATU_TYPE_MEM,
> >> +                                         win->res->start,
> >> +                                         win->res->start - win->offset,
> >> +                                         resource_size(win->res));
> >> +       }
> >> +}
> >> +
> >>   void dw_pcie_setup_rc(struct pcie_port *pp)
> >>   {
> >>          u32 val, ctrl, num_ctrls;
> >> +       struct resource_entry *win;
> >>          struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> >>
> >>          /*
> >> @@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> >>           * ATU, so we should not program the ATU here.
> >>           */
> >>          if (pp->bridge->child_ops == &dw_child_pcie_ops) {
> >> -               struct resource_entry *entry =
> >> -                       resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> >> +               resource_list_for_each_entry(win, &pp->bridge->windows)
> >> +                       if (resource_type(win->res) == IORESOURCE_MEM)
> >> +                               dw_pcie_setup_mem_atu(pp, win);
> >>
> >> -               dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
> >> -                                         PCIE_ATU_TYPE_MEM, entry->res->start,
> >> -                                         entry->res->start - entry->offset,
> >> -                                         resource_size(entry->res));
> >>                  if (pci->num_viewport > 2)
> >>                          dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
> >>                                                    PCIE_ATU_TYPE_IO, pp->io_base,
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> >> index c2dea8fc97c8..b5e438b70cd5 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware.c
> >> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> >> @@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
> >>   static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> >>                                               int index, int type,
> >>                                               u64 cpu_addr, u64 pci_addr,
> >> -                                            u32 size)
> >> +                                            u64 size)
> >
> > How was this working for you before? Looks like a different change
> > from what I broke.
> Tegra194 has a 1:1 mapping for prefetchable memory as of today. But,
> when it is changed to a non 1:1 mapping, we need this support.

It's not about a non 1:1 map, but the size. In any case, it's a
separate change from fixing the regression.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping
  2020-10-23 15:37       ` Rob Herring
@ 2020-10-23 19:55         ` Vidya Sagar
  0 siblings, 0 replies; 12+ messages in thread
From: Vidya Sagar @ 2020-10-23 19:55 UTC (permalink / raw)
  To: Rob Herring, Jingoo Han, Gustavo Pimentel
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Andrew Murray, Thierry Reding,
	Jon Hunter, PCI, linux-kernel, kthota, Manikanta Maddireddy,
	sagar.tv



On 10/23/2020 9:07 PM, Rob Herring wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Oct 23, 2020 at 2:38 AM Vidya Sagar <vidyas@nvidia.com> wrote:
>>
>>
>>
>> On 10/23/2020 12:38 AM, Rob Herring wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Tue, Oct 20, 2020 at 2:59 PM Vidya Sagar <vidyas@nvidia.com> wrote:
>>>>
>>>> DWC sub-system currently doesn't differentiate between prefetchable and
>>>> non-prefetchable memory aperture entries in the 'ranges' property and
>>>> provides ATU mapping only for the first memory aperture entry out of all
>>>> the entries present. This was introduced by the
>>>> commit 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources").
>>>> Mapping for a memory apreture is required if its CPU address and the bus
>>>> address are different and the current mechanism works only if the memory
>>>> aperture which needs mapping happens to be the first entry. It doesn't
>>>> work either if the memory aperture that needs mapping is not the first
>>>> entry or if both prefetchable and non-prefetchable apertures need mapping.
>>>
>>> Well that's subtle...
>>>
>>>> This patch fixes this issue by differentiating between prefetchable and
>>>> non-prefetchable apertures in the 'ranges' property there by removing the
>>>> dependency on the order in which they are specified and adds support for
>>>> mapping prefetchable aperture using ATU region-3 if required.
>>>
>>> Now you don't do any iATU entry for a 1:1 memory range which is a
>>> change for pretty much every other platform. That means we leave the
>>> PCI transaction config to the whims of how h/w designers hooked up the
>>> sideband signals. I guess this is how Uniphier works as it only has 1
>>> viewport...
>>>
>>> I think the assignment should be in this order:
>>> - config space
>>> - non-prefetchable (IIRC, it's required)
>>> - prefetchable
>>> - i/o
>>>
>>> Stopping assignment and warning if you run out of viewports. Looking
>>> at the platforms, I think that would always work. There's only
>>> uniphier and ls1012a where we run out. Those would still behave the
>>> same.
>> As I see from the code this is how the current mapping is done
>>
>> Region-0: [Fixed] Non-Prefetchable memory mapping
>>
>> Region-1: [Shared] if the num of view ports <= 2
>>           Used for I/O by default but whenever config space is accessed,
>> it is programmed to generate config space, and once done, will program
>> it back for I/O generation. I'm not sure how they are synchonized when
>> two different entities are trying to access the config space as well as
>> the I/O at the same time.  I don't see any locking mechanism (or am I
>> missing something here??)
> 
> They aren't synchronized. We just get lucky that I/O and config
> accesses aren't interleaved frequently. IMO, we should just not
> support I/O space on those platforms. AIUI, almost everything doesn't
> use I/O nowadays.
> 
>>             [Fixed] if the num of view ports > 2
>>           Used to generate configuration space accesses
>>
>> Region-2: [Fixed] I/O accesses
>>
>> Region-3: [Fixed] Prefetchable memory mapping (This patch is adding it)
>>
>> I honestly think that an attempt to re-assing what region is used for
>> what purpose should go into a different patch.
> 
> Normally, I'd agree for a fix. If you want to fix it in a minimal way
> such that we just setup the last memory region instead of the first,
> then that's fine. But to implement the review feedback, you will
> simply be adding support for multiple memory regions. The ATU doesn't
> care about prefetchable or not. I think it will end up being a more
> simple implementation if you just do the I/O region last and only if
> you have an available.

I'll surely take it up next but would really like to hear from Jingoo 
and Gustavo if this breaks any legacy implementations. For now, I'll 
just program the ATU irrespective of 1:1 mapping as I want to get ToT 
working again on Tegra194.

> 
>>
>> I agree that I'm removing configuring an iATU region if it is for 1:1
>> memory mapping. What I heard from our HW designers is that it is the
>> default behavior of the Synopsys IP that if the CPU access falls in the
>> aperture owned by Synopsys IP and there is no iATU region mapped to
>> capture and generate any specific transaction for that address, then, by
>> default it generates a memory transaction over the PCIe bus.
>> It is also possible that some implementors might have chosen to alter
>> this behavior.
> 
> If we assume they haven't, that pretty much guarantees they did. :)

Ok. I'll address it in the next version

> 
>> In any case, I can continue to have the iATU programming
>> done irrespective of whether it is for 1:1 mapping or not.
>>
>>>
>>>
>>>>
>>>> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
>>>> Link: http://patchwork.ozlabs.org/project/linux-pci/patch/20200513190855.23318-1-vidyas@nvidia.com/
>>>>
>>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>>> ---
>>>> V2:
>>>> * Rewrote commit subject and description
>>>> * Addressed review comments from Lorenzo
>>>>
>>>>    .../pci/controller/dwc/pcie-designware-host.c | 42 ++++++++++++++++---
>>>>    drivers/pci/controller/dwc/pcie-designware.c  | 12 +++---
>>>>    drivers/pci/controller/dwc/pcie-designware.h  |  4 +-
>>>>    3 files changed, 46 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>>>> index db547ee6ff3a..dae6da39bb90 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>>>> @@ -521,9 +521,42 @@ static struct pci_ops dw_pcie_ops = {
>>>>           .write = pci_generic_config_write,
>>>>    };
>>>>
>>>> +static void dw_pcie_setup_mem_atu(struct pcie_port *pp,
>>>> +                                 struct resource_entry *win)
>>>> +{
>>>> +       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>> +
>>>> +       /* Check for prefetchable memory aperture */
>>>> +       if (win->res->flags & IORESOURCE_PREFETCH && win->offset) {
>>>> +               /* Number of view ports must at least be 4 to enable mapping */
>>>> +               if (pci->num_viewport < 4) {
>>>> +                       dev_warn(pci->dev,
>>>> +                                "Insufficient ATU regions to map Prefetchable memory\n");
>>>> +               } else {
>>>> +                       dw_pcie_prog_outbound_atu(pci,
>>>> +                                                 PCIE_ATU_REGION_INDEX3,
>>>> +                                                 PCIE_ATU_TYPE_MEM,
>>>> +                                                 win->res->start,
>>>> +                                                 win->res->start - win->offset,
>>>> +                                                 resource_size(win->res));
>>>> +               }
>>>> +       } else if (win->offset) { /* Non-prefetchable memory aperture */
>>>> +               if (upper_32_bits(resource_size(win->res)))
>>>> +                       dev_warn(pci->dev,
>>>> +                                "Memory resource size exceeds max for 32 bits\n");
>>>
>>> This is not designware specific. Either core code should check this or
>>> write a DT schema to check it.
>> Ok. I'll move it to pci_parse_request_of_pci_ranges() API. A change like
>> below would do right?
> 
> Yes.
> 
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index ac24cd5439a9..6d872458196c 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -556,6 +556,11 @@ static int pci_parse_request_of_pci_ranges(struct
>> device *dev,
>>                           break;
>>                   case IORESOURCE_MEM:
>>                           res_valid |= !(res->flags & IORESOURCE_PREFETCH);
>> +
>> +                       if (!(res->flags & IORESOURCE_PREFETCH))
>> +                               if (upper_32_bits(resource_size(res)))
>> +                                       dev_warn(dev,
>> +                                                "Memory resource size
>> exceeds max for 32 bits\n");
>>                           break;
>>                   }
>>           }
>>
>> I hope that is the right place for it.
>>
>>>
>>>> +               dw_pcie_prog_outbound_atu(pci,
>>>> +                                         PCIE_ATU_REGION_INDEX0,
>>>> +                                         PCIE_ATU_TYPE_MEM,
>>>> +                                         win->res->start,
>>>> +                                         win->res->start - win->offset,
>>>> +                                         resource_size(win->res));
>>>> +       }
>>>> +}
>>>> +
>>>>    void dw_pcie_setup_rc(struct pcie_port *pp)
>>>>    {
>>>>           u32 val, ctrl, num_ctrls;
>>>> +       struct resource_entry *win;
>>>>           struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>>
>>>>           /*
>>>> @@ -578,13 +611,10 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>>>>            * ATU, so we should not program the ATU here.
>>>>            */
>>>>           if (pp->bridge->child_ops == &dw_child_pcie_ops) {
>>>> -               struct resource_entry *entry =
>>>> -                       resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
>>>> +               resource_list_for_each_entry(win, &pp->bridge->windows)
>>>> +                       if (resource_type(win->res) == IORESOURCE_MEM)
>>>> +                               dw_pcie_setup_mem_atu(pp, win);
>>>>
>>>> -               dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX0,
>>>> -                                         PCIE_ATU_TYPE_MEM, entry->res->start,
>>>> -                                         entry->res->start - entry->offset,
>>>> -                                         resource_size(entry->res));
>>>>                   if (pci->num_viewport > 2)
>>>>                           dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX2,
>>>>                                                     PCIE_ATU_TYPE_IO, pp->io_base,
>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>>> index c2dea8fc97c8..b5e438b70cd5 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>>> @@ -228,7 +228,7 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>>>>    static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
>>>>                                                int index, int type,
>>>>                                                u64 cpu_addr, u64 pci_addr,
>>>> -                                            u32 size)
>>>> +                                            u64 size)
>>>
>>> How was this working for you before? Looks like a different change
>>> from what I broke.
>> Tegra194 has a 1:1 mapping for prefetchable memory as of today. But,
>> when it is changed to a non 1:1 mapping, we need this support.
> 
> It's not about a non 1:1 map, but the size. In any case, it's a
> separate change from fixing the regression.

Ok. I'm going to push this part as a separate change in the next version.

> 
> Rob
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-10-23 19:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05 12:13 [PATCH] PCI: dwc: Use ATU regions to map memory regions Vidya Sagar
2020-10-19  5:51 ` Vidya Sagar
2020-10-20 13:20   ` Lorenzo Pieralisi
2020-10-20 13:33     ` Vidya Sagar
2020-10-20 13:41       ` Lorenzo Pieralisi
2020-10-20 15:59 ` Lorenzo Pieralisi
2020-10-20 19:59 ` [PATCH V2] PCI: dwc: Add support to handle prefetchable memory mapping Vidya Sagar
2020-10-21 10:08   ` Lorenzo Pieralisi
2020-10-22 19:08   ` Rob Herring
2020-10-23  7:38     ` Vidya Sagar
2020-10-23 15:37       ` Rob Herring
2020-10-23 19:55         ` Vidya Sagar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).