Re: [PATCH v5 2/3] PCI: qcom: Add equalization settings for 16 GT/s

[Shashank Babu Chinta Venkata <quic_schintav@quicinc.com>]

On 8/26/24 00:55, Manivannan Sadhasivam wrote:
> On Wed, Aug 21, 2024 at 10:08:43AM -0700, Shashank Babu Chinta Venkata wrote:
>> During high data transmission rates such as 16 GT/s , there is an
>> increased risk of signal loss due to poor channel quality and
>> interference. This can impact receiver's ability to capture signals
>> accurately. Hence, signal compensation is achieved through appropriate
>> lane equalization settings at both transmitter and receiver. This will
>> result in increased PCIe signal strength.
>>
>> Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> ---
>>  drivers/pci/controller/dwc/pcie-designware.h  | 12 ++++++
>>  drivers/pci/controller/dwc/pcie-qcom-common.c | 37 +++++++++++++++++++
>>  drivers/pci/controller/dwc/pcie-qcom-common.h |  1 +
>>  drivers/pci/controller/dwc/pcie-qcom-ep.c     |  3 ++
>>  drivers/pci/controller/dwc/pcie-qcom.c        |  3 ++
>>  5 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 53c4c8f399c8..50265a2fbb9f 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -126,6 +126,18 @@
>>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT	24
>>  #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK	GENMASK(25, 24)
>>  
>> +#define GEN3_EQ_CONTROL_OFF			0x8a8
>> +#define GEN3_EQ_CONTROL_OFF_FB_MODE		GENMASK(3, 0)
>> +#define GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE	BIT(4)
>> +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC	GENMASK(23, 8)
>> +#define GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL	BIT(24)
>> +
>> +#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF          0x8ac
>> +#define GEN3_EQ_FMDC_T_MIN_PHASE23		GENMASK(4, 0)
>> +#define GEN3_EQ_FMDC_N_EVALS			GENMASK(9, 5)
>> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA	GENMASK(13, 10)
>> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA	GENMASK(17, 14)
>> +
>>  #define PCIE_PORT_MULTI_LANE_CTRL	0x8C0
>>  #define PORT_MLTI_UPCFG_SUPPORT		BIT(7)
>>  
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.c b/drivers/pci/controller/dwc/pcie-qcom-common.c
>> index 1d8992147bba..e085075557cd 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-common.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-common.c
>> @@ -15,6 +15,43 @@
>>  #include "pcie-designware.h"
>>  #include "pcie-qcom-common.h"
>>  
>> +void qcom_pcie_common_set_16gt_eq_settings(struct dw_pcie *pci)
>> +{
>> +	u32 reg;
>> +
>> +	/*
>> +	 * GEN3_RELATED_OFF register is repurposed to apply equalization
>> +	 * settings at various data transmission rates through registers
>> +	 * namely GEN3_EQ_*. RATE_SHADOW_SEL bit field of GEN3_RELATED_OFF
>> +	 * determines data rate for which this equalization settings are
>> +	 * applied.
>> +	 */
>> +	reg = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
>> +	reg &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
>> +	reg &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
>> +	reg |= FIELD_PREP(GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK, 0x1);
>> +	dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, reg);
>> +
>> +	reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF);
>> +	reg &= ~(GEN3_EQ_FMDC_T_MIN_PHASE23 |
>> +		GEN3_EQ_FMDC_N_EVALS |
>> +		GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA |
>> +		GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA);
>> +	reg |= FIELD_PREP(GEN3_EQ_FMDC_T_MIN_PHASE23, 0x1) |
>> +		FIELD_PREP(GEN3_EQ_FMDC_N_EVALS, 0xd) |
>> +		FIELD_PREP(GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA, 0x5) |
>> +		FIELD_PREP(GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA, 0x5);
>> +	dw_pcie_writel_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF, reg);
>> +
>> +	reg = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
>> +	reg &= ~(GEN3_EQ_CONTROL_OFF_FB_MODE |
>> +		GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE |
>> +		GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL |
>> +		GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC);
>> +	dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
>> +}
>> +EXPORT_SYMBOL_GPL(qcom_pcie_common_set_16gt_eq_settings);
>> +
>>  struct icc_path *qcom_pcie_common_icc_get_resource(struct dw_pcie *pci, const char *path)
>>  {
>>  	struct icc_path *icc_p;
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.h b/drivers/pci/controller/dwc/pcie-qcom-common.h
>> index 897fa18e618a..c281582de12c 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-common.h
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-common.h
>> @@ -13,3 +13,4 @@
>>  struct icc_path *qcom_pcie_common_icc_get_resource(struct dw_pcie *pci, const char *path);
>>  int qcom_pcie_common_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem, u32 bandwidth);
>>  void qcom_pcie_common_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem);
>> +void qcom_pcie_common_set_16gt_eq_settings(struct dw_pcie *pci);
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index e1860026e134..823e33a4d745 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -455,6 +455,9 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
>>  		goto err_disable_resources;
>>  	}
>>  
>> +	if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
> 
> Abel reported that 'pci->link_gen' is not updated unless the 'max-link-speed'
> property is set in DT on his platform. I fixed that issue locally and this
> series will depend on those patches.
> 
> Provided that you are having issues with your build environment as discussed
> offline, I'd like to take over the series to combine my patches and address the
> review comments. Let me know if you are OK with this or not.
> 
> - Mani
> 

Sure Mani.You can include my patches in your series.

Thanks
Shashank
>> +		qcom_pcie_common_set_16gt_eq_settings(pci);
>> +
>>  	/*
>>  	 * The physical address of the MMIO region which is exposed as the BAR
>>  	 * should be written to MHI BASE registers.
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index ee32590f1506..829b34391af1 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -280,6 +280,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
>>  {
>>  	struct qcom_pcie *pcie = to_qcom_pcie(pci);
>>  
>> +	if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
>> +		qcom_pcie_common_set_16gt_eq_settings(pci);
>> +
>>  	/* Enable Link Training state machine */
>>  	if (pcie->cfg->ops->ltssm_enable)
>>  		pcie->cfg->ops->ltssm_enable(pcie);
>> -- 
>> 2.46.0
>>
>