[PATCH] phy: qualcomm: qmp-combo: update DP PHY PLL programming on Glymur
Nabige Aala
nabige.aala at oss.qualcomm.com
Thu Aug 20 23:04:14 PDT 2026
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 at oss.qualcomm.com>
>>
>> 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 at oss.qualcomm.com>
>> Signed-off-by: Mahadevan P<mahadevan.p at oss.qualcomm.com>
>> Signed-off-by: Nabige Aala<nabige.aala at oss.qualcomm.com>
>> ---
>> 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@oss.qualcomm.com/
>> ---
>> 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,
> There are too many changes squashed into one commit, which makes it
> 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,
>>
>> [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);
> If we are writing 0xa4 here, why does qmp_v8_dp_aux_init() write 0x06?
Ack, According to HPG in both places it should be 0x06.
I will correct in next revision.
>> +
>> + 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;
>> +}
> This is exactly the same as a part of qmp_v456_configure_dp_phy().
> Please use it in that function too (and name correspondingly).
Ack, I will update in the next revision.
>> +
>> 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]);
> No more VCO?
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 */
>> 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),
> Why? Should we poll both BIT(0) and BIT(1)?
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 at oss.qualcomm.com>
>>
More information about the linux-phy
mailing list