[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <af65b744-7538-4929-9ab4-8ee42e17b8d1@quicinc.com>
Date: Tue, 3 Sep 2024 11:32:13 -0700
From: Shashank Babu Chinta Venkata <quic_schintav@...cinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: <agross@...nel.org>, <andersson@...nel.org>, <konrad.dybcio@...aro.org>,
<mani@...nel.org>, <quic_msarkar@...cinc.com>,
<quic_kraravin@...cinc.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof WilczyĆski <kw@...ux.com>,
Rob Herring
<robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Jingoo Han
<jingoohan1@...il.com>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
Serge Semin <fancer.lancer@...il.com>,
Niklas Cassel <cassel@...nel.org>,
Conor Dooley <conor.dooley@...rochip.com>,
<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] PCI: qcom: Add equalization settings for 16 GT/s
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@...cinc.com>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.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
>>
>
Powered by blists - more mailing lists