mbox series

[v3,0/2] PCI: qcom: Avoid DBI and ATU register space mirroring

Message ID 20240724022719.2868490-1-quic_pyarlaga@quicinc.com
Headers show
Series PCI: qcom: Avoid DBI and ATU register space mirroring | expand

Message

Prudhvi Yarlagadda July 24, 2024, 2:27 a.m. UTC
Qualcomm PCIe controller has a wrapper called PARF which supports the
relocation of DBI and ATU address space within the system memory. PARF
gets the location of DBI and ATU from PARF_DBI_BASE_ADDR and
PARF_ATU_BASE_ADDR registers. PARF also mirrors the memory block
containing DBI and ATU registers within the system memory. And the size
of this memory block is programmed in PARF_SLV_ADDR_SPACE_SIZE register.

Power on reset of values of the above mentioned registers are good enough
on platforms which require smaller size (less than 16MB) BAR memory. For
platforms that need bigger BAR memory size, this mirroring of DBI and ATU
address space by PARF conflicts with BAR memory.

So to allow usage of bigger size of BAR, it is required to program
PARF registers to prevent mirroring of DBI and ATU blocks and provide
the physical addresses of DBI and ATU to PARF.

This patch series stores physical addresses of DBI and ATU address space
in 'struct dw_pcie' and programs the required PARF registers in the
pcie_qcom.c driver.

Changes in v3:
- Updated the functions qcom_pcie_configure_dbi_atu_base() and
  qcom_pcie_configure_dbi_base() to make having 'dbi_phys_addr' mandatory
  before programming PARF_SLV_ADDR_SPACE_SIZE register as suggested by
  Mayank Rana.
- Link to v2: https://lore.kernel.org/linux-pci/20240718051258.1115271-1-quic_pyarlaga@quicinc.com/T/

Changes in v2:
- Updated commit message as suggested by Bjorn Helgaas.
- Updated function name from qcom_pcie_avoid_dbi_atu_mirroring()
  to qcom_pcie_configure_dbi_atu_base() as suggested by Bjorn Helgaas.
- Removed check for pdev in qcom_pcie_configure_dbi_atu_base() as
  suggested by Bjorn Helgaas.
- Moved the qcom_pcie_configure_dbi_atu_base() call in the
  qcom_pcie_init_2_7_0() to the same place where PARF_DBI_BASE_ADDR
  register is being programmed as suggested by Bjorn Helgaas.
- Added 'dbi_phys_addr', 'atu_phys_addr' in the 'struct dw_pcie' to store
  the physical addresses of dbi, atu base registers in
  dw_pcie_get_resources() as suggested by Manivannan Sadhasivam.
- Added separate functions qcom_pcie_configure_dbi_atu_base() and
  qcom_pcie_configure_dbi_base() to program PARF register of different
  PARF versions. This is to disable DBI mirroring in all Qualcomm PCIe
  controllers as suggested by Manivannan Sadhasivam.
- Link to v1: https://lore.kernel.org/linux-pci/a01404d2-2f4d-4fb8-af9d-3db66d39acf7@quicinc.com/T/

Tested:
- Validated NVME functionality with PCIe6a on x1e80100 platform.
- Validated WiFi functionality with PCIe4 on x1e80100 platform.

Prudhvi Yarlagadda (2):
  PCI: dwc: Add dbi_phys_addr and atu_phys_addr to struct dw_pcie
  PCI: qcom: Avoid DBI and ATU register space mirror to BAR/MMIO region

 drivers/pci/controller/dwc/pcie-designware.c |  2 +
 drivers/pci/controller/dwc/pcie-designware.h |  2 +
 drivers/pci/controller/dwc/pcie-qcom.c       | 62 ++++++++++++++------
 3 files changed, 49 insertions(+), 17 deletions(-)

Comments

Manivannan Sadhasivam July 24, 2024, 1:53 p.m. UTC | #1
On Tue, Jul 23, 2024 at 07:27:18PM -0700, Prudhvi Yarlagadda wrote:

Subject could be modified as below:

PCI: dwc: Cache DBI and iATU physical addresses in 'struct dw_pcie_ops'

> Both DBI and ATU physical base addresses are needed by pcie_qcom.c
> driver to program the location of DBI and ATU blocks in Qualcomm
> PCIe Controller specific PARF hardware block.
> 
> Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com>

Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> Reviewed-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 2 ++
>  drivers/pci/controller/dwc/pcie-designware.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 1b5aba1f0c92..bc3a5d6b0177 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -112,6 +112,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>  		pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
>  		if (IS_ERR(pci->dbi_base))
>  			return PTR_ERR(pci->dbi_base);
> +		pci->dbi_phys_addr = res->start;
>  	}
>  
>  	/* DBI2 is mainly useful for the endpoint controller */
> @@ -134,6 +135,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>  			pci->atu_base = devm_ioremap_resource(pci->dev, res);
>  			if (IS_ERR(pci->atu_base))
>  				return PTR_ERR(pci->atu_base);
> +			pci->atu_phys_addr = res->start;
>  		} else {
>  			pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
>  		}
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 53c4c8f399c8..efc72989330c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -407,8 +407,10 @@ struct dw_pcie_ops {
>  struct dw_pcie {
>  	struct device		*dev;
>  	void __iomem		*dbi_base;
> +	phys_addr_t		dbi_phys_addr;
>  	void __iomem		*dbi_base2;
>  	void __iomem		*atu_base;
> +	phys_addr_t		atu_phys_addr;
>  	size_t			atu_size;
>  	u32			num_ib_windows;
>  	u32			num_ob_windows;
> -- 
> 2.25.1
>
Prudhvi Yarlagadda July 24, 2024, 6:28 p.m. UTC | #2
Hi Manivannan,

Thanks for the review comments.

On 7/24/2024 6:53 AM, Manivannan Sadhasivam wrote:
> On Tue, Jul 23, 2024 at 07:27:18PM -0700, Prudhvi Yarlagadda wrote:
> 
> Subject could be modified as below:
> 
> PCI: dwc: Cache DBI and iATU physical addresses in 'struct dw_pcie_ops'
> 

ACK. I will update it in the next patch.

>> Both DBI and ATU physical base addresses are needed by pcie_qcom.c
>> driver to program the location of DBI and ATU blocks in Qualcomm
>> PCIe Controller specific PARF hardware block.
>>
>> Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@quicinc.com>
> 
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> - Mani
> 

ACK. I will add it in the next patch.

Thanks,
Prudhvi
>> Reviewed-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>>  drivers/pci/controller/dwc/pcie-designware.c | 2 ++
>>  drivers/pci/controller/dwc/pcie-designware.h | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 1b5aba1f0c92..bc3a5d6b0177 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -112,6 +112,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>>  		pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
>>  		if (IS_ERR(pci->dbi_base))
>>  			return PTR_ERR(pci->dbi_base);
>> +		pci->dbi_phys_addr = res->start;
>>  	}
>>  
>>  	/* DBI2 is mainly useful for the endpoint controller */
>> @@ -134,6 +135,7 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>>  			pci->atu_base = devm_ioremap_resource(pci->dev, res);
>>  			if (IS_ERR(pci->atu_base))
>>  				return PTR_ERR(pci->atu_base);
>> +			pci->atu_phys_addr = res->start;
>>  		} else {
>>  			pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
>>  		}
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 53c4c8f399c8..efc72989330c 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -407,8 +407,10 @@ struct dw_pcie_ops {
>>  struct dw_pcie {
>>  	struct device		*dev;
>>  	void __iomem		*dbi_base;
>> +	phys_addr_t		dbi_phys_addr;
>>  	void __iomem		*dbi_base2;
>>  	void __iomem		*atu_base;
>> +	phys_addr_t		atu_phys_addr;
>>  	size_t			atu_size;
>>  	u32			num_ib_windows;
>>  	u32			num_ob_windows;
>> -- 
>> 2.25.1
>>
>