Re: [PATCH v3 1/2] phy: qcom: edp: Add eDP/DP mode switch support
From: Yongxing Mou
Date: Mon Apr 20 2026 - 08:47:29 EST
On 3/20/2026 2:36 PM, Dmitry Baryshkov wrote:
On Mon, Mar 02, 2026 at 04:28:29PM +0800, Yongxing Mou wrote: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.
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?
Thank for point this, will separate the change.-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.
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.
Got it.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: f199223cb490 ("phy: qcom: Introduce new eDP PHY driver")
Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
---
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.
Emm, for SC8180X, eDP low VDIFF (High HBR) only S1 (250mV) P0-0dB Emphasis Settings "0x08" different from other "generic" tables which is "0x0B".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.
Got it. SC8180X here need a different table..{ 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.- { 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.
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@xxxxxxxxxxxxxxxxxxx
https://lists.infradead.org/mailman/listinfo/linux-phy