Re: [PATCH] PCI: qcom: Disable halting feature

[Manivannan Sadhasivam]

> The PARF write, read and BDF halt features make the controller wait
> for earlier AXI transactions to complete before issuing new ones. This
> is needed on SoCs whose NoC cannot preserve the ordering of inbound
> transactions from the controller.
> 
> On newer SoCs, the NoC preserves this ordering on its own. Halting at
> the controller adds no correctness benefit and reduces inbound DMA
> bandwidth.
> 
> Add a noc_ensures_ordering flag to struct qcom_pcie_cfg. Clear all
> three halt features on SoCs that set it. Other SoCs keep enabling
> write halt as before.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f..73eb303919c1 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -53,6 +53,7 @@
>  #define PARF_SLV_ADDR_SPACE_SIZE		0x16c
>  #define PARF_MHI_CLOCK_RESET_CTRL		0x174
>  #define PARF_AXI_MSTR_WR_ADDR_HALT		0x178
> +#define PARF_AXI_MSTR_RD_HALT_NO_WRITES		0x1a4
>  #define PARF_AXI_MSTR_WR_ADDR_HALT_V2		0x1a8
>  #define PARF_Q2A_FLUSH				0x1ac
>  #define PARF_LTSSM				0x1b0
> @@ -67,6 +68,7 @@
>  #define PARF_SLV_ADDR_SPACE_SIZE_V2		0x358
>  #define PARF_SLV_ADDR_SPACE_SIZE_V2_HI		0x35c
>  #define PARF_NO_SNOOP_OVERRIDE			0x3d4
> +#define PARF_AXI_MSTR_WR_NS_BDF_HALT		0x4a0
>  #define PARF_ATU_BASE_ADDR			0x634
>  #define PARF_ATU_BASE_ADDR_HI			0x638
>  #define PARF_DEVICE_TYPE			0x1000
> @@ -134,6 +136,9 @@
>  /* PARF_AXI_MSTR_WR_ADDR_HALT register fields */
>  #define EN					BIT(31)
>  
> +/* PARF_AXI_MSTR_RD_HALT_NO_WRITES */
> +#define RD_HALT_NO_WRITES_EN			BIT(0)
> +
>  /* PARF_LTSSM register fields */
>  #define LTSSM_EN				BIT(8)
>  #define PARF_LTSSM_STATE_MASK			GENMASK(5, 0)
> @@ -149,6 +154,9 @@
>  #define WR_NO_SNOOP_OVERRIDE_EN			BIT(1)
>  #define RD_NO_SNOOP_OVERRIDE_EN			BIT(3)
>  
> +/* PARF_AXI_MSTR_WR_NS_BDF_HALT */
> +#define BDF_CHANGE_HALT_EN			BIT(0)
> +
>  /* PARF_DEVICE_TYPE register fields */
>  #define DEVICE_TYPE_RC				0x4
>  
> @@ -275,12 +283,16 @@ struct qcom_pcie_ops {
>    * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
>    * snooping
>    * @firmware_managed: Set if the Root Complex is firmware managed
> +  * @no_l0s: Set if the Root Complex does not support L0s

Spurious change.

> +  * @noc_ensures_ordering: Set if the NoC preserves the ordering of inbound
> +  * transactions from the controller.

@disable_halt: Set to disable the HALT feature on SoCs where it is enabled by
default and the NoC preserves the ordering of inbound transactions from the
controller.

>    */
>  struct qcom_pcie_cfg {
>  	const struct qcom_pcie_ops *ops;
>  	bool override_no_snoop;
>  	bool firmware_managed;
>  	bool no_l0s;
> +	bool noc_ensures_ordering;

Change to 'disable_halt'

>  };
>  
>  struct qcom_pcie_perst {
> @@ -1081,9 +1093,23 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  
>  	pci->l1ss_support = true;
>  
> -	val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> -	val |= EN;
> -	writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> +	if (pcie->cfg->noc_ensures_ordering) {
> +		val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> +		val &= ~EN;
> +		writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> +
> +		val = readl(pcie->parf + PARF_AXI_MSTR_RD_HALT_NO_WRITES);
> +		val &= ~RD_HALT_NO_WRITES_EN;
> +		writel(val, pcie->parf + PARF_AXI_MSTR_RD_HALT_NO_WRITES);
> +
> +		val = readl(pcie->parf + PARF_AXI_MSTR_WR_NS_BDF_HALT);
> +		val &= ~BDF_CHANGE_HALT_EN;
> +		writel(val, pcie->parf + PARF_AXI_MSTR_WR_NS_BDF_HALT);
> +	} else {
> +		val = readl(pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> +		val |= EN;
> +		writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2);
> +	}
>  
>  	return 0;
>  err_disable_clocks:
> @@ -1597,12 +1623,14 @@ static const struct qcom_pcie_cfg cfg_1_0_0 = {
>  
>  static const struct qcom_pcie_cfg cfg_1_9_0 = {
>  	.ops = &ops_1_9_0,
> +	.noc_ensures_ordering = true,
>  };
>  
>  static const struct qcom_pcie_cfg cfg_1_34_0 = {
>  	.ops = &ops_1_9_0,
>  	.override_no_snoop = true,
>  	.no_l0s = true,
> +	.noc_ensures_ordering = true,
>  };
>  
>  static const struct qcom_pcie_cfg cfg_2_1_0 = {
> @@ -1624,15 +1652,18 @@ static const struct qcom_pcie_cfg cfg_2_4_0 = {
>  
>  static const struct qcom_pcie_cfg cfg_2_7_0 = {
>  	.ops = &ops_2_7_0,
> +	.noc_ensures_ordering = true,
>  };
>  
>  static const struct qcom_pcie_cfg cfg_2_9_0 = {
>  	.ops = &ops_2_9_0,
> +	.noc_ensures_ordering = true,

ops_2_9_0 is using qcom_pcie_init_2_9_0(). So this flag has no effect.

- Mani

-- 
மணிவண்ணன் சதாசிவம்