Re: [PATCH] phy: qualcomm: qmp-combo: update DP PHY PLL programming on Glymur
From: Nabige Aala
Date: Fri Aug 21 2026 - 02:04:50 EST
On 7/22/2026 4:56 AM, Dmitry Baryshkov wrote:
On Tue, Jul 21, 2026 at 10:48:33AM +0530, Nabige Aala wrote:
From: Ritesh Kumar<ritesh.kumar@xxxxxxxxxxxxxxxx>There are too many changes squashed into one commit, which makes it
The existing DP PHY PLL and AUX configuration for the Glymur platform
does not fully follow the Hardware Programming Guide requirements for
DP over Type-C, which results in DP link bring-up failures.
Update the DP PHY programming sequence and PLL-related register
settings to align with the latest HPG recommendations. With this
change, DP link training completes successfully on Glymur-based
platforms.
Fixes: d10736db98d2 ("phy: qualcomm: qmp-combo: Add DP offsets and settings for Glymur platforms")
Signed-off-by: Ritesh Kumar<ritesh.kumar@xxxxxxxxxxxxxxxx>
Signed-off-by: Mahadevan P<mahadevan.p@xxxxxxxxxxxxxxxx>
Signed-off-by: Nabige Aala<nabige.aala@xxxxxxxxxxxxxxxx>
---
The existing DP PHY PLL and AUX configuration for the Glymur platform
does not fully follow the Hardware Programming Guide requirements for
DP over Type-C, which results in DP link bring-up failures.
Update the DP PHY programming sequence and PLL-related register
settings to align with the latest HPG recommendations. With this
change, DP link training completes successfully on Glymur-based
platforms.
Changes in v2:
- Use the DP-specific v8 COM status register definitions
(DP_QSERDES_V8_COM_C_READY_STATUS and DP_QSERDES_V8_COM_CMN_STATUS)
instead of adding new N3 USB/DP variants.(Konrad)
- Drop the qmp_v8_combo_configure_dp_mode() implementation and
reuse the common qmp_combo_configure_dp_mode() helper, restoring
orientation-specific DP_PHY_MODE programming (0x4c/0x5c) and addressing
review comments regarding DP_PHY_MODE handling.
- Include phy-qcom-qmp-dp-qserdes-com-v8.h for the DP COM register
definitions.
- Update patch author to Ritesh Kumar.(Konrad)
- Link to v1:https://lore.kernel.org/all/20260419-glymur_dp-v1-1-ad1067a8e8ae@xxxxxxxxxxxxxxxx/
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 190 ++++++++++++++++++++----------
1 file changed, 129 insertions(+), 61 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1..c98abae40a32 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -50,6 +50,7 @@
#include "phy-qcom-qmp-dp-phy-v6.h"
#include "phy-qcom-qmp-dp-phy-v8.h"
+#include "phy-qcom-qmp-dp-qserdes-com-v8.h"
#include "phy-qcom-qmp-usb43-pcs-v8.h"
/* QPHY_V3_DP_COM_RESET_OVRD_CTRL register bits */
@@ -283,8 +284,8 @@ static const unsigned int qmp_v8_n3_usb43dpphy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_DP_AON_TOGGLE_ENABLE] = QPHY_V8_PCS_AON_DP_AON_TOGGLE_ENABLE,
[QPHY_COM_RESETSM_CNTRL] = QSERDES_V8_COM_RESETSM_CNTRL,
- [QPHY_COM_C_READY_STATUS] = QSERDES_V8_COM_C_READY_STATUS,
- [QPHY_COM_CMN_STATUS] = QSERDES_V8_COM_CMN_STATUS,
+ [QPHY_COM_C_READY_STATUS] = DP_QSERDES_V8_COM_C_READY_STATUS,
+ [QPHY_COM_CMN_STATUS] = DP_QSERDES_V8_COM_CMN_STATUS,
harder to review. Could you please separate them into the logical
smaller chunks? Register layout fix, tables and other values update,
configuration rework, etc.
Ack, I will split in more logical chunks.
[QPHY_COM_BIAS_EN_CLKBUFLR_EN] = QSERDES_V8_COM_BIAS_EN_CLKBUFLR_EN,If we are writing 0xa4 here, why does qmp_v8_dp_aux_init() write 0x06?
[QPHY_DP_PHY_STATUS] = QSERDES_V8_DP_PHY_STATUS,
+static int qmp_v8_helper_configure_dp_phy(struct qmp_combo *qmp)
+{
+ const struct qmp_phy_cfg *cfg = qmp->cfg;
+ u32 status;
+ int ret;
+
+ writel(0x0f, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG_1);
+
+ qmp_combo_configure_dp_mode(qmp);
+
+ writel(0x13, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG1);
+ writel(0xa4, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG2);
Ack, According to HPG in both places it should be 0x06.
I will correct in next revision.
+This is exactly the same as a part of qmp_v456_configure_dp_phy().
+ writel(0x05, qmp->dp_dp_phy + QSERDES_V4_DP_PHY_TX0_TX1_LANE_CTL);
+ writel(0x05, qmp->dp_dp_phy + QSERDES_V4_DP_PHY_TX2_TX3_LANE_CTL);
+
+ ret = qmp->cfg->configure_dp_clocks(qmp);
+ if (ret)
+ return ret;
+
+ writel(0x01, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
+ writel(0x05, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
+ writel(0x01, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
+ writel(0x09, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
+
+ writel(0x20, qmp->dp_serdes + cfg->regs[QPHY_COM_RESETSM_CNTRL]);
+
+ if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_C_READY_STATUS],
+ status,
+ ((status & BIT(0)) > 0),
+ 500,
+ 10000))
+ return -ETIMEDOUT;
+
+ if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_CMN_STATUS],
+ status,
+ ((status & BIT(0)) > 0),
+ 500,
+ 10000))
+ return -ETIMEDOUT;
+
+ if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_CMN_STATUS],
+ status,
+ ((status & BIT(1)) > 0),
+ 500,
+ 10000))
+ return -ETIMEDOUT;
+
+ return 0;
+}
Please use it in that function too (and name correspondingly).
Ack, I will update in the next revision.
+No more VCO?
static void qmp_v8_dp_aux_init(struct qmp_combo *qmp)
{
const struct qmp_phy_cfg *cfg = qmp->cfg;
- writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_PSR_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
- DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
- qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
+ writel(0x3f, qmp->dp_serdes + QSERDES_V8_USB43_COM_CLK_FWD_CONFIG_1);
+
+ writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
+ DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
+ DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
+ qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
/* Turn on BIAS current for PHY/PLL */
- writel(0x1c, qmp->dp_serdes + cfg->regs[QPHY_COM_BIAS_EN_CLKBUFLR_EN]);
+ writel(0x17, qmp->dp_serdes + cfg->regs[QPHY_COM_BIAS_EN_CLKBUFLR_EN]);
writel(0x00, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG0);
writel(0x13, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG1);
@@ -3361,8 +3420,8 @@ static void qmp_v4_configure_dp_tx(struct qmp_combo *qmp)
writel(0x27, qmp->dp_tx + cfg->regs[QPHY_TX_TX_DRV_LVL]);
writel(0x27, qmp->dp_tx2 + cfg->regs[QPHY_TX_TX_DRV_LVL]);
- writel(0x20, qmp->dp_tx + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
- writel(0x20, qmp->dp_tx2 + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
+ writel(0x2b, qmp->dp_tx + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
+ writel(0x2b, qmp->dp_tx2 + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
qmp_combo_configure_dp_swing(qmp);
}
@@ -3370,48 +3429,41 @@ static void qmp_v4_configure_dp_tx(struct qmp_combo *qmp)
static int qmp_v8_configure_dp_clocks(struct qmp_combo *qmp)
{
const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
- u32 phy_vco_div;
unsigned long pixel_freq;
- const struct qmp_phy_cfg *cfg = qmp->cfg;
switch (dp_opts->link_rate) {
case 1620:
- phy_vco_div = 0x4;
pixel_freq = 1620000000UL / 2;
break;
case 2700:
- phy_vco_div = 0x2;
pixel_freq = 2700000000UL / 2;
break;
case 5400:
- phy_vco_div = 0x4;
pixel_freq = 5400000000UL / 4;
break;
case 8100:
- phy_vco_div = 0x3;
pixel_freq = 8100000000UL / 6;
break;
default:
/* Other link rates aren't supported */
return -EINVAL;
}
- writel(phy_vco_div, qmp->dp_dp_phy + cfg->regs[QPHY_DP_PHY_VCO_DIV]);
According to glymur HPG "QPHY_DP_PHY_VCO_DIV register does not participate in the
divider of vco_divided_clk, it can be left as it is- No impact"
/* disable core reset tsync */Why? Should we poll both BIT(0) and BIT(1)?
writel(0x09, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
- writel(0x04, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_AUXLESS_SETUP_CYC);
- writel(0x08, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_AUXLESS_SILENCE_CYC);
+ writel(0x09, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_AUXLESS_SETUP_CYC);
+ writel(0x11, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_AUXLESS_SILENCE_CYC);
writel(0x08, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LFPS_CYC);
- writel(0x11, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LFPS_PERIOD);
+ writel(0x33, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LFPS_PERIOD);
writel(0x3e, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TSYNC_OVRD);
writel(0x05, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TX2_TX3_LANE_CTL);
writel(0x05, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TX0_TX1_LANE_CTL);
writel(0x01, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_AUXLESS_CFG1);
- writel(0x11, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LFPS_PERIOD);
+ writel(0x33, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LFPS_PERIOD);
writel(0x1f, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LN0_DRV_LVL);
- writel(0x1f, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LN1_DRV_LVL);
+ writel(0x02, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_LN1_DRV_LVL);
clk_set_rate(qmp->dp_link_hw.clk, dp_opts->link_rate * 100000);
clk_set_rate(qmp->dp_pixel_hw.clk, pixel_freq);
@@ -3558,7 +3610,7 @@ static int qmp_v8_configure_dp_phy(struct qmp_combo *qmp)
u32 status;
int ret;
- ret = qmp_v456_configure_dp_phy(qmp);
+ ret = qmp_v8_helper_configure_dp_phy(qmp);
if (ret < 0)
return ret;
@@ -3570,13 +3622,13 @@ static int qmp_v8_configure_dp_phy(struct qmp_combo *qmp)
} else if (dp_opts->lanes == 2) {
bias0_en = reverse ? 0x3f : 0x15;
bias1_en = reverse ? 0x15 : 0x3f;
- drvr0_en = 0x10;
- drvr1_en = 0x10;
+ drvr0_en = 0x30;
+ drvr1_en = 0x30;
} else {
bias0_en = 0x3f;
bias1_en = 0x3f;
- drvr0_en = 0x34;
- drvr1_en = 0x34;
+ drvr0_en = 0x30;
+ drvr1_en = 0x30;
}
writel(drvr0_en, qmp->dp_tx + cfg->regs[QPHY_TX_HIGHZ_DRVR_EN]);
@@ -3584,6 +3636,14 @@ static int qmp_v8_configure_dp_phy(struct qmp_combo *qmp)
writel(drvr1_en, qmp->dp_tx2 + cfg->regs[QPHY_TX_HIGHZ_DRVR_EN]);
writel(bias1_en, qmp->dp_tx2 + cfg->regs[QPHY_TX_TRANSCEIVER_BIAS_EN]);
+ writel(0x03, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TSYNC_OVRD);
+ writel(0x23, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TSYNC_OVRD);
+ writel(0x22, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TSYNC_OVRD);
+
+ writel(0x0a, qmp->dp_tx + QSERDES_V8_LALB_RESET_TSYNC_EN_CTRL);
+ writel(0x0a, qmp->dp_tx2 + QSERDES_V8_LALB_RESET_TSYNC_EN_CTRL);
+
+ writel(0x3e, qmp->dp_dp_phy + QSERDES_V8_DP_PHY_TSYNC_OVRD);
writel(0x08, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
udelay(100);
writel(0x09, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
@@ -3591,16 +3651,24 @@ static int qmp_v8_configure_dp_phy(struct qmp_combo *qmp)
if (readl_poll_timeout(qmp->dp_dp_phy + cfg->regs[QPHY_DP_PHY_STATUS],
status,
- ((status & BIT(1)) > 0),
+ ((status & BIT(0)) > 0),
According to glymur HPG expected readback value is 0x01. So only BIT(0) would be enough.
Thanks,
Nabige
500,
10000))
return -ETIMEDOUT;
- writel(0x00, qmp->dp_tx + cfg->regs[QPHY_TX_TX_DRV_LVL]);
- writel(0x00, qmp->dp_tx2 + cfg->regs[QPHY_TX_TX_DRV_LVL]);
+ if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_CMN_STATUS],
+ status,
+ ((status & BIT(0)) > 0),
+ 500,
+ 10000))
+ return -ETIMEDOUT;
- writel(0x2b, qmp->dp_tx + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
- writel(0x2b, qmp->dp_tx2 + cfg->regs[QPHY_TX_TX_EMP_POST1_LVL]);
+ if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_CMN_STATUS],
+ status,
+ ((status & BIT(1)) > 0),
+ 500,
+ 10000))
+ return -ETIMEDOUT;
return 0;
}
---
base-commit: 6946bc5406f7fe2cb024419eca681bb0280f3461
change-id: 20260720-glymur-phy-conf-fc878ed134b1
Best regards,
--
Nabige Aala<nabige.aala@xxxxxxxxxxxxxxxx>