Re: [PATCH] phy: qualcomm: qmp-combo: update DP PHY PLL programming on Glymur

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<[email protected]>
>>
>> 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<[email protected]>
>> Signed-off-by: Mahadevan P<[email protected]>
>> Signed-off-by: Nabige Aala<[email protected]>
>> ---
>> 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/[email protected]/
>> ---
>>   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<[email protected]>
>>

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy