[PATCH v3 1/2] phy: qcom: edp: Add eDP/DP mode switch support
Yongxing Mou
yongxing.mou at oss.qualcomm.com
Mon Apr 20 05:47:09 PDT 2026
On 3/20/2026 2:36 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 04:28:29PM +0800, Yongxing Mou wrote:
>> The eDP PHY supports both eDP&DP modes, each requires a different table.
>> The current driver doesn't fully support every combo PHY mode and use
>> either the eDP or DP table when enable the platform. In addition, some
>> platforms mismatch between the mode and the table where DP mode uses
>> the eDP table or eDP mode use the DP table.
>>
>> Clean up and correct the tables for currently supported platforms based on
>> the HPG specification.
>>
>> Here lists the tables can be reused across current platforms.
>> DP mode：
>> -sa8775p/sc7280/sc8280xp/x1e80100
>> -glymur
>> eDP mode(low vdiff):
>
> Separate question: should we extend phy_configure_dp_opts with the
> low/high vdiff? Is there a point in providing the ability to toggle
> between low vdiff and high vdiff?
>
Emm ,i haven't found any platform using high vdiff so far, and I'm not
clear in which cases switching between low and high vdiff would be needed.
>> -glymur/sa8775p/sc8280xp/x1e80100
>> -sc7280
>
> I understand your wish to perform all the changes in a single patch, but
> there is one problem with that. Consider this patch regresses one of the
> platforms (I'm looking at Kodiak and SC8180X as they get the biggest set
> of changes). It would be almost impossible to separate, which particular
> change caused the regression. I'd suggest splitting this patch into a
> set of more atomic changes. E.g. the AUX_CFG8 is definitely a separate
> change. Writing swing / pre_emph tables on Kodiak and SC8180X is a
> separate change (or two). Switching each of the platforms to the
> corrected set of tables ideally also should come as a separate change,
> so that in case of a regression the issue would be easier to identify.
>
Thank for point this, will separate the change.
I mostly overlooked SC8180X here, since I assumed it shares the same PHY
as SC7280. However, they are using different PHY sub‑versions. Will add
proper support for it in the next version.
>>
>> Cc: stable at vger.kernel.org
>> Fixes: f199223cb490 ("phy: qcom: Introduce new eDP PHY driver")
>> Signed-off-by: Yongxing Mou <yongxing.mou at oss.qualcomm.com>
>> ---
>> drivers/phy/qualcomm/phy-qcom-edp.c | 90 ++++++++++++++++++++++---------------
>> 1 file changed, 53 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index 7372de05a0b8..36998326bae6 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -87,7 +87,8 @@ struct qcom_edp_phy_cfg {
>> bool is_edp;
>> const u8 *aux_cfg;
>> const u8 *vco_div_cfg;
>> - const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *dp_swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *edp_swing_pre_emph_cfg;
>> const struct phy_ver_ops *ver_ops;
>> };
>>
>> @@ -116,17 +117,17 @@ struct qcom_edp {
>> };
>>
>> static const u8 dp_swing_hbr_rbr[4][4] = {
>> - { 0x08, 0x0f, 0x16, 0x1f },
>> + { 0x07, 0x0f, 0x16, 0x1f },
>> { 0x11, 0x1e, 0x1f, 0xff },
>> { 0x16, 0x1f, 0xff, 0xff },
>> { 0x1f, 0xff, 0xff, 0xff }
>> };
>>
>> static const u8 dp_pre_emp_hbr_rbr[4][4] = {
>> - { 0x00, 0x0d, 0x14, 0x1a },
>> + { 0x00, 0x0e, 0x15, 0x1a },
>> { 0x00, 0x0e, 0x15, 0xff },
>> { 0x00, 0x0e, 0xff, 0xff },
>> - { 0x03, 0xff, 0xff, 0xff }
>> + { 0x04, 0xff, 0xff, 0xff }
>> };
>
> I've checked, at least this table doesn't match SC8180X configuration.
>
Got it.
>>
>> static const u8 dp_swing_hbr2_hbr3[4][4] = {
>> @@ -150,6 +151,20 @@ static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg = {
>> .pre_emphasis_hbr3_hbr2 = &dp_pre_emp_hbr2_hbr3,
>> };
>>
>> +static const u8 dp_pre_emp_hbr_rbr_v8[4][4] = {
>> + { 0x00, 0x0e, 0x15, 0x1a },
>> + { 0x00, 0x0e, 0x15, 0xff },
>> + { 0x00, 0x0e, 0xff, 0xff },
>> + { 0x00, 0xff, 0xff, 0xff }
>> +};
>> +
>> +static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg_v8 = {
>> + .swing_hbr_rbr = &dp_swing_hbr_rbr,
>> + .swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3,
>> + .pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr_v8,
>> + .pre_emphasis_hbr3_hbr2 = &dp_pre_emp_hbr2_hbr3,
>> +};
>> +
>> static const u8 edp_swing_hbr_rbr[4][4] = {
>> { 0x07, 0x0f, 0x16, 0x1f },
>> { 0x0d, 0x16, 0x1e, 0xff },
>> @@ -158,7 +173,7 @@ static const u8 edp_swing_hbr_rbr[4][4] = {
>> };
>>
>> static const u8 edp_pre_emp_hbr_rbr[4][4] = {
>> - { 0x05, 0x12, 0x17, 0x1d },
>> + { 0x05, 0x11, 0x17, 0x1d },
>
> This was changed only for Kodiak. For SC8180X, I assume, we should be
> using the older table.
>
Emm, for SC8180X, eDP low VDIFF (High HBR) only S1 (250mV) P0-0dB
Emphasis Settings "0x08" different from other "generic" tables which is
"0x0B".
>> { 0x05, 0x11, 0x18, 0xff },
>> { 0x06, 0x11, 0xff, 0xff },
>> { 0x00, 0xff, 0xff, 0xff }
>> @@ -172,10 +187,10 @@ static const u8 edp_swing_hbr2_hbr3[4][4] = {
>> };
>>
>> static const u8 edp_pre_emp_hbr2_hbr3[4][4] = {
>
> I think it becomes worth adding version to the "generic" tables. They
> are not that generic anyway.
>
Got it. SC8180X here need a different table..
>> - { 0x08, 0x11, 0x17, 0x1b },
>> - { 0x00, 0x0c, 0x13, 0xff },
>> - { 0x05, 0x10, 0xff, 0xff },
>> - { 0x00, 0xff, 0xff, 0xff }
>> + { 0x0c, 0x15, 0x19, 0x1e },
>> + { 0x0b, 0x15, 0x19, 0xff },
>> + { 0x0e, 0x14, 0xff, 0xff },
>> + { 0x0d, 0xff, 0xff, 0xff }
>
> Current table indeed doesn't match the swing table. Please take care
> about the SC8180X differences (I think, it will need separate set of
> tables).
>
Got it.
>> };
>>
>> static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>> @@ -193,25 +208,25 @@ static const u8 edp_phy_vco_div_cfg_v4[4] = {
>> 0x01, 0x01, 0x02, 0x00,
>> };
>>
>> -static const u8 edp_pre_emp_hbr_rbr_v5[4][4] = {
>> - { 0x05, 0x11, 0x17, 0x1d },
>> - { 0x05, 0x11, 0x18, 0xff },
>> - { 0x06, 0x11, 0xff, 0xff },
>> - { 0x00, 0xff, 0xff, 0xff }
>> +static const u8 edp_swing_hbr2_hbr3_v3[4][4] = {
>> + { 0x06, 0x11, 0x16, 0x1b },
>> + { 0x0b, 0x19, 0x1f, 0xff },
>> + { 0x18, 0x1f, 0xff, 0xff },
>> + { 0x1f, 0xff, 0xff, 0xff }
>> };
>>
>> -static const u8 edp_pre_emp_hbr2_hbr3_v5[4][4] = {
>> +static const u8 edp_pre_emp_hbr2_hbr3_v3[4][4] = {
>> { 0x0c, 0x15, 0x19, 0x1e },
>> - { 0x0b, 0x15, 0x19, 0xff },
>> - { 0x0e, 0x14, 0xff, 0xff },
>> + { 0x09, 0x14, 0x19, 0xff },
>> + { 0x0f, 0x14, 0xff, 0xff },
>> { 0x0d, 0xff, 0xff, 0xff }
>> };
>>
>> -static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v5 = {
>> +static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v3 = {
>> .swing_hbr_rbr = &edp_swing_hbr_rbr,
>> - .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3,
>> - .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr_v5,
>> - .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v5,
>> + .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3_v3,
>> + .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr,
>> + .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v3,
>> };
>>
>> static const u8 edp_phy_aux_cfg_v5[DP_AUX_CFG_SIZE] = {
>> @@ -262,12 +277,7 @@ static int qcom_edp_phy_init(struct phy *phy)
>> DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
>> edp->edp + DP_PHY_PD_CTL);
>>
>> - /*
>> - * TODO: Re-work the conditions around setting the cfg8 value
>> - * when more information becomes available about why this is
>> - * even needed.
>> - */
>> - if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
>> + if (!edp->is_edp)
>> aux_cfg[8] = 0xb7;
>
> This is a separate fix, as it changes the aux_cfg[8] value for Kodiak
> and SC8180X.
>
Got it.
>>
>> writel(0xfc, edp->edp + DP_PHY_MODE);
>> @@ -291,7 +301,7 @@ static int qcom_edp_phy_init(struct phy *phy)
>>
>> static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configure_opts_dp *dp_opts)
>> {
>> - const struct qcom_edp_swing_pre_emph_cfg *cfg = edp->cfg->swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *cfg;
>> unsigned int v_level = 0;
>> unsigned int p_level = 0;
>> u8 ldo_config;
>> @@ -299,11 +309,10 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>> u8 emph;
>> int i;
>>
>> - if (!cfg)
>> - return 0;
>> -
>> if (edp->is_edp)
>> - cfg = &edp_phy_swing_pre_emph_cfg;
>> + cfg = edp->cfg->edp_swing_pre_emph_cfg;
>> + else
>> + cfg = edp->cfg->dp_swing_pre_emph_cfg;
>>
>> for (i = 0; i < dp_opts->lanes; i++) {
>> v_level = max(v_level, dp_opts->voltage[i]);
>> @@ -564,20 +573,24 @@ static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>> .is_edp = false,
>> .aux_cfg = edp_phy_aux_cfg_v5,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v3,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> @@ -585,7 +598,8 @@ static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>> .is_edp = true,
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>
> Ok, we are going to continue using eDP table because of is_edp = true.
>
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> @@ -766,7 +780,8 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>> static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v6,
>> };
>>
>> @@ -945,7 +960,8 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v8 = {
>> static struct qcom_edp_phy_cfg glymur_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v8,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v8,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg_v8,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v8,
>> };
>>
>>
>> --
>> 2.43.0
>>
>>
>> --
>> linux-phy mailing list
>> linux-phy at lists.infradead.org
>> https://lists.infradead.org/mailman/listinfo/linux-phy
>
More information about the linux-phy
mailing list