[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8877e1f7-bbb6-49e9-a30c-0ae8e40bf5dd@quicinc.com>
Date: Thu, 20 Jun 2024 14:50:42 -0700
From: Mayank Rana <quic_mrana@...cinc.com>
To: Prudhvi Yarlagadda <quic_pyarlaga@...cinc.com>,
<manivannan.sadhasivam@...aro.org>, <lpieralisi@...nel.org>,
<kw@...ux.com>, <bhelgaas@...gle.com>, <robh@...nel.org>
CC: <linux-pci@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] PCI: qcom: Avoid DBI and ATU register space mirror to
BAR/MMIO region
On 6/20/2024 2:34 PM, Prudhvi Yarlagadda wrote:
> PARF hardware block which is a wrapper on top of DWC PCIe controller
> mirrors the DBI and ATU register space. It uses PARF_SLV_ADDR_SPACE_SIZE
> register to get the size of the memory block to be mirrored and uses
> PARF_DBI_BASE_ADDR, PARF_ATU_BASE_ADDR registers to determine the base
> address of DBI and ATU space inside the memory block that is being
> mirrored.
>
> When a memory region which is located above the SLV_ADDR_SPACE_SIZE
> boundary is used for BAR region then there could be an overlap of DBI and
> ATU address space that is getting mirrored and the BAR region. This
> results in DBI and ATU address space contents getting updated when a PCIe
> function driver tries updating the BAR/MMIO memory region. Reference
> memory map of the PCIe memory region with DBI and ATU address space
> overlapping BAR region is as below.
>
> |---------------|
> | |
> | |
> ------- --------|---------------|
> | | |---------------|
> | | | DBI |
> | | |---------------|---->DBI_BASE_ADDR
> | | | |
> | | | |
> | PCIe | |---->2*SLV_ADDR_SPACE_SIZE
> | BAR/MMIO|---------------|
> | Region | ATU |
> | | |---------------|---->ATU_BASE_ADDR
> | | | |
> PCIe | |---------------|
> Memory | | DBI |
> Region | |---------------|---->DBI_BASE_ADDR
> | | | |
> | --------| |
> | | |---->SLV_ADDR_SPACE_SIZE
> | |---------------|
> | | ATU |
> | |---------------|---->ATU_BASE_ADDR
> | | |
> | |---------------|
> | | DBI |
> | |---------------|---->DBI_BASE_ADDR
> | | |
> | | |
> ----------------|---------------|
> | |
> | |
> | |
> |---------------|
>
> Currently memory region beyond the SLV_ADDR_SPACE_SIZE boundary is not
> used for BAR region which is why the above mentioned issue is not
> encountered. This issue is discovered as part of internal testing when we
> tried moving the BAR region beyond the SLV_ADDR_SPACE_SIZE boundary. Hence
> we are trying to fix this.
>
> As PARF hardware block mirrors DBI and ATU register space after every
> PARF_SLV_ADDR_SPACE_SIZE (default 0x1000000) boundary multiple, write
> U64_MAX to PARF_SLV_ADDR_SPACE_SIZE register to avoid mirroring DBI and
> ATU to BAR/MMIO region. Write the physical base address of DBI and ATU
> register blocks to PARF_DBI_BASE_ADDR (default 0x0) and PARF_ATU_BASE_ADDR
> (default 0x1000) respectively to make sure DBI and ATU blocks are at
> expected memory locations.
>
> Signed-off-by: Prudhvi Yarlagadda <quic_pyarlaga@...cinc.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 40 ++++++++++++++++++++++----
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> Tested:
> - Validated NVME functionality with PCIe6a on x1e80100 platform.
> - Validated WiFi functionality with PCIe4 on x1e80100 platform.
> - Validated NVME functionality with PCIe0 and PCIe1 on SA8775p platform.
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5f9f0ff19baa..864548657551 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -49,7 +49,12 @@
> #define PARF_LTSSM 0x1b0
> #define PARF_SID_OFFSET 0x234
> #define PARF_BDF_TRANSLATE_CFG 0x24c
> +#define PARF_DBI_BASE_ADDR_V2 0x350
> +#define PARF_DBI_BASE_ADDR_V2_HI 0x354
> #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define PARF_SLV_ADDR_SPACE_SIZE_HI 0x35C
> +#define PARF_ATU_BASE_ADDR 0x634
> +#define PARF_ATU_BASE_ADDR_HI 0x638
> #define PARF_NO_SNOOP_OVERIDE 0x3d4
> #define PARF_DEVICE_TYPE 0x1000
> #define PARF_BDF_TO_SID_TABLE_N 0x2000
> @@ -319,6 +324,33 @@ static void qcom_pcie_clear_hpc(struct dw_pcie *pci)
> dw_pcie_dbi_ro_wr_dis(pci);
> }
>
> +static void qcom_pcie_avoid_dbi_atu_mirroring(struct qcom_pcie *pcie)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct platform_device *pdev;
> + struct resource *atu_res;
> + struct resource *dbi_res;
> +
> + pdev = to_platform_device(pci->dev);
> + if (!pdev)
> + return;
> +
> + dbi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> + if (dbi_res) {
> + writel(lower_32_bits(dbi_res->start), pcie->parf + PARF_DBI_BASE_ADDR_V2);
> + writel(upper_32_bits(dbi_res->start), pcie->parf + PARF_DBI_BASE_ADDR_V2_HI);
> + }
> +
> + atu_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
> + if (atu_res) {
> + writel(lower_32_bits(atu_res->start), pcie->parf + PARF_ATU_BASE_ADDR);
> + writel(upper_32_bits(atu_res->start), pcie->parf + PARF_ATU_BASE_ADDR_HI);
> + }
> +
> + writel(lower_32_bits(U64_MAX), pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> + writel(upper_32_bits(U64_MAX), pcie->parf + PARF_SLV_ADDR_SPACE_SIZE_HI);
> +}
> +
> static void qcom_pcie_2_1_0_ltssm_enable(struct qcom_pcie *pcie)
> {
> u32 val;
> @@ -623,8 +655,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
> val &= ~PHY_TEST_PWR_DOWN;
> writel(val, pcie->parf + PARF_PHY_CTRL);
>
> - /* change DBI base address */
> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR);
> + qcom_pcie_avoid_dbi_atu_mirroring(pcie);
>
> /* MAC PHY_POWERDOWN MUX DISABLE */
> val = readl(pcie->parf + PARF_SYS_CTRL);
> @@ -900,6 +931,8 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> /* Wait for reset to complete, required on SM8450 */
> usleep_range(1000, 1500);
>
> + qcom_pcie_avoid_dbi_atu_mirroring(pcie);
> +
> /* configure PCIe to RC mode */
> writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
>
> @@ -908,9 +941,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
> val &= ~PHY_TEST_PWR_DOWN;
> writel(val, pcie->parf + PARF_PHY_CTRL);
>
> - /* change DBI base address */
> - writel(0, pcie->parf + PARF_DBI_BASE_ADDR);
> -
> /* MAC PHY_POWERDOWN MUX DISABLE */
> val = readl(pcie->parf + PARF_SYS_CTRL);
> val &= ~MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN;
Reviewed-by: Mayank Rana <quic_mrana@...cinc.com>
Powered by blists - more mailing lists