| Message ID | 20240419001013.28788-3-quic_schintav@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add 16GT/s equalization and margining settings | expand |
On Thu, Apr 18, 2024 at 05:09:35PM -0700, Shashank Babu Chinta Venkata wrote: PCI: qcom: Add PCIe link equalization settings for 16 GT/s > GEN3_RELATED_OFFSET is being used to determine data rate of shadow > registers. Select data rate as 16GT/s and set appropriate equilization > settings to improve link stability for 16GT/s data rate. > How about: 'To improve the PCIe link stability while running at the rate of 16 GT/s, set the appropriate link equalization settings for both RC and EP controllers.' However, I don't understand the statement about 'GEN3_RELATED_OFFSET' and 'shadow registers'. Please reword it to make it understandable. > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > --- > drivers/pci/controller/dwc/pcie-designware.h | 12 ++++++++ > drivers/pci/controller/dwc/pcie-qcom-common.c | 30 +++++++++++++++++++ > 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, 49 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 26dae4837462..ad771bb52d29 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -122,6 +122,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(n) FIELD_PREP(GENMASK(3, 0), n) > +#define GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE(n) FIELD_PREP(BIT(4), n) > +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC(n) FIELD_PREP(GENMASK(23, 8), n) > +#define GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL(n) FIELD_PREP(BIT(24), n) > + > +#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF 0x8ac > +#define GEN3_EQ_FMDC_T_MIN_PHASE23(n) FIELD_PREP(GENMASK(4, 0), n) > +#define GEN3_EQ_FMDC_N_EVALS(n) FIELD_PREP(GENMASK(9, 5), n) > +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA(n) FIELD_PREP(GENMASK(13, 10), n) > +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA(n) FIELD_PREP(GENMASK(17, 14), n) > + > #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 dc2120ec5fef..a6f3eb4c3ee6 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom-common.c > +++ b/drivers/pci/controller/dwc/pcie-qcom-common.c > @@ -16,6 +16,36 @@ > #define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ > Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) > > +void qcom_pcie_common_set_16gt_eq_settings(struct dw_pcie *pci) > +{ > + u32 reg; > + > + /* > + * GEN3_RELATED_OFF is repurposed to be used with GEN4(16GT/s) rate > + * as well based on RATE_SHADOW_SEL_MASK settings on this register. Same comment as above. > + */ > + 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 |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT); Use FIELD_PREP() > + 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(0) | You are reading 'reg' above and then overwriting immediately. > + GEN3_EQ_FMDC_N_EVALS(0xD) | '0xd' > + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA(0x5) | > + 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(0) | Same as above. - Mani
On 4/19/24 02:09, Shashank Babu Chinta Venkata wrote: > GEN3_RELATED_OFFSET is being used to determine data rate of shadow > registers. Select data rate as 16GT/s and set appropriate equilization > settings to improve link stability for 16GT/s data rate. > > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > --- [...] > + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF); > + reg = GEN3_EQ_FMDC_T_MIN_PHASE23(0) | > + GEN3_EQ_FMDC_N_EVALS(0xD) | > + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA(0x5) | > + 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(0) | > + GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE(0) | > + GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL(0) | > + GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC(0); > + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg); Also, any chance we could get some explanations as to what these magic values mean? Preferably in the form of a #define for each one Konrad
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 26dae4837462..ad771bb52d29 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -122,6 +122,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(n) FIELD_PREP(GENMASK(3, 0), n) +#define GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE(n) FIELD_PREP(BIT(4), n) +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC(n) FIELD_PREP(GENMASK(23, 8), n) +#define GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL(n) FIELD_PREP(BIT(24), n) + +#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF 0x8ac +#define GEN3_EQ_FMDC_T_MIN_PHASE23(n) FIELD_PREP(GENMASK(4, 0), n) +#define GEN3_EQ_FMDC_N_EVALS(n) FIELD_PREP(GENMASK(9, 5), n) +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA(n) FIELD_PREP(GENMASK(13, 10), n) +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA(n) FIELD_PREP(GENMASK(17, 14), n) + #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 dc2120ec5fef..a6f3eb4c3ee6 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-common.c +++ b/drivers/pci/controller/dwc/pcie-qcom-common.c @@ -16,6 +16,36 @@ #define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) +void qcom_pcie_common_set_16gt_eq_settings(struct dw_pcie *pci) +{ + u32 reg; + + /* + * GEN3_RELATED_OFF is repurposed to be used with GEN4(16GT/s) rate + * as well based on RATE_SHADOW_SEL_MASK settings on this register. + */ + 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 |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT); + 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(0) | + GEN3_EQ_FMDC_N_EVALS(0xD) | + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA(0x5) | + 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(0) | + GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE(0) | + GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL(0) | + GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC(0); + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg); +} +EXPORT_SYMBOL_GPL(qcom_pcie_common_set_16gt_eq_settings); + int qcom_pcie_common_icc_get_resource(struct dw_pcie *pci, struct icc_path **icc_mem_p) { *icc_mem_p = devm_of_icc_get(pci->dev, "pcie-mem"); diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.h b/drivers/pci/controller/dwc/pcie-qcom-common.h index f0520d7301da..e72c651b0d28 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-common.h +++ b/drivers/pci/controller/dwc/pcie-qcom-common.h @@ -10,3 +10,4 @@ int qcom_pcie_common_icc_get_resource(struct dw_pcie *pci, struct icc_path **icc_mem_p); int qcom_pcie_common_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem); 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 11c99b358147..cb75a874f76c 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c @@ -438,6 +438,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) + 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 71f011daad1d..acf66f974edc 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -263,6 +263,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);
GEN3_RELATED_OFFSET is being used to determine data rate of shadow registers. Select data rate as 16GT/s and set appropriate equilization settings to improve link stability for 16GT/s data rate. Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> --- drivers/pci/controller/dwc/pcie-designware.h | 12 ++++++++ drivers/pci/controller/dwc/pcie-qcom-common.c | 30 +++++++++++++++++++ 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, 49 insertions(+)