NACK/Cmnt: [PATCH 1/2] UBUNTU: SAUCE: phy: qcom: edp: split power-on sequencing by PHY version
Stefan Bader
stefan.bader at canonical.com
Thu Sep 17 10:37:54 UTC 2026
On 17/09/2026 11:43, Tobias Heider via kernel-team wrote:
> From: Bjorn Andersson <bjorn.andersson at oss.qualcomm.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2167217
>
> The eDP PHY power-on sequence has grown version-specific differences,
> but qcom_edp_phy_power_on() still contains the sequence as one shared
> flow. This makes it difficult to add newer PHY programming without
> interleaving it with the v4/v6 sequence.
>
> Split the power-on sequence into version callbacks for the parts that
> differ between PHY revisions:
>
> - pre-power-on preparation
> - TX programming before PLL enable
> - rate/PCS programming
> - lane programming after PLL enable
> - final status polling
>
> Keep the existing v4/v6 sequence as the v46 implementation, and wire v8
> to the same callbacks for now. This keeps the programming unchanged
> while making the next patch a v8-only programming update.
>
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Bjorn Andersson <bjorn.andersson at oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio at oss.qualcomm.com>
> Signed-off-by: Tobias Heider <tobias.heider at canonical.com>
> ---
Rejected for the following reasons:
- No cover email and unclear target (see
https://canonical-kernel-docs.readthedocs-hosted.com/en/latest/reference/stable-patch-format/#subject-line)
- non-upstream patch with unclear history
Given the heavily increased burden of maintaining security we have to
stick to upstream as close as possible.
-Stefan> drivers/phy/qualcomm/phy-qcom-edp.c | 175
++++++++++++++++++++--------
> 1 file changed, 125 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index a391c29463ef6..ecd7b535c903f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -48,13 +48,10 @@
> /* LANE_TXn registers */
> #define TXn_CLKBUF_ENABLE 0x0000
> #define TXn_TX_EMP_POST1_LVL 0x0004
> -
> #define TXn_TX_DRV_LVL 0x0014
> #define TXn_TX_DRV_LVL_OFFSET 0x0018
> #define TXn_RESET_TSYNC_EN 0x001c
> -#define TXn_LDO_CONFIG 0x0084
> #define TXn_TX_BAND 0x0028
> -
> #define TXn_RES_CODE_LANE_OFFSET_TX0 0x0044
> #define TXn_RES_CODE_LANE_OFFSET_TX1 0x0048
>
> @@ -64,6 +61,7 @@
> #define TXn_LANE_MODE_1 0x0064
>
> #define TXn_TRAN_DRVR_EMP_EN 0x0078
> +#define TXn_LDO_CONFIG 0x0084
>
> struct qcom_edp_swing_pre_emph_cfg {
> const u8 (*swing_hbr_rbr)[4][4];
> @@ -82,6 +80,12 @@ struct phy_ver_ops {
> int (*com_configure_pll)(const struct qcom_edp *edp);
> int (*com_configure_ssc)(const struct qcom_edp *edp);
> int (*com_ldo_config)(const struct qcom_edp *edp);
> + int (*prepare_power_on)(const struct qcom_edp *edp);
> + int (*configure_tx_pre_pll)(const struct qcom_edp *edp);
> + int (*configure_rate_pcs)(const struct qcom_edp *edp,
> + unsigned long *pixel_freq);
> + void (*configure_lanes_after_pll)(const struct qcom_edp *edp);
> + int (*finish_power_on)(const struct qcom_edp *edp);
> };
>
> struct qcom_edp_phy_cfg {
> @@ -117,6 +121,13 @@ struct qcom_edp {
> bool is_edp;
> };
>
> +static int qcom_edp_prepare_power_on_v46(const struct qcom_edp *edp);
> +static int qcom_edp_configure_tx_pre_pll_v46(const struct qcom_edp *edp);
> +static int qcom_edp_configure_rate_pcs_v46(const struct qcom_edp *edp,
> + unsigned long *pixel_freq);
> +static void qcom_edp_configure_lanes_after_pll_v46(const struct qcom_edp *edp);
> +static int qcom_edp_finish_power_on_v46(const struct qcom_edp *edp);
> +
> static const u8 dp_swing_hbr_rbr[4][4] = {
> { 0x07, 0x0f, 0x16, 0x1f },
> { 0x11, 0x1e, 0x1f, 0xff },
> @@ -653,6 +664,11 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v3 = {
> .com_configure_pll = qcom_edp_com_configure_pll_v4,
> .com_configure_ssc = qcom_edp_com_configure_ssc_v4,
> .com_ldo_config = qcom_edp_ldo_config_v3,
> + .prepare_power_on = qcom_edp_prepare_power_on_v46,
> + .configure_tx_pre_pll = qcom_edp_configure_tx_pre_pll_v46,
> + .configure_rate_pcs = qcom_edp_configure_rate_pcs_v46,
> + .configure_lanes_after_pll = qcom_edp_configure_lanes_after_pll_v46,
> + .finish_power_on = qcom_edp_finish_power_on_v46,
> };
>
> static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
> @@ -663,6 +679,11 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
> .com_configure_pll = qcom_edp_com_configure_pll_v4,
> .com_configure_ssc = qcom_edp_com_configure_ssc_v4,
> .com_ldo_config = qcom_edp_ldo_config_v4,
> + .prepare_power_on = qcom_edp_prepare_power_on_v46,
> + .configure_tx_pre_pll = qcom_edp_configure_tx_pre_pll_v46,
> + .configure_rate_pcs = qcom_edp_configure_rate_pcs_v46,
> + .configure_lanes_after_pll = qcom_edp_configure_lanes_after_pll_v46,
> + .finish_power_on = qcom_edp_finish_power_on_v46,
> };
>
> static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
> @@ -898,6 +919,11 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
> .com_configure_pll = qcom_edp_com_configure_pll_v6,
> .com_configure_ssc = qcom_edp_com_configure_ssc_v6,
> .com_ldo_config = qcom_edp_ldo_config_v6,
> + .prepare_power_on = qcom_edp_prepare_power_on_v46,
> + .configure_tx_pre_pll = qcom_edp_configure_tx_pre_pll_v46,
> + .configure_rate_pcs = qcom_edp_configure_rate_pcs_v46,
> + .configure_lanes_after_pll = qcom_edp_configure_lanes_after_pll_v46,
> + .finish_power_on = qcom_edp_finish_power_on_v46,
> };
>
> static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
> @@ -1079,6 +1105,11 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v8 = {
> .com_configure_pll = qcom_edp_com_configure_pll_v8,
> .com_configure_ssc = qcom_edp_com_configure_ssc_v8,
> .com_ldo_config = qcom_edp_ldo_config_v6,
> + .prepare_power_on = qcom_edp_prepare_power_on_v46,
> + .configure_tx_pre_pll = qcom_edp_configure_tx_pre_pll_v46,
> + .configure_rate_pcs = qcom_edp_configure_rate_pcs_v46,
> + .configure_lanes_after_pll = qcom_edp_configure_lanes_after_pll_v46,
> + .finish_power_on = qcom_edp_finish_power_on_v46,
> };
>
> static struct qcom_edp_phy_cfg glymur_phy_cfg = {
> @@ -1089,81 +1120,49 @@ static struct qcom_edp_phy_cfg glymur_phy_cfg = {
> .ver_ops = &qcom_edp_phy_ops_v8,
> };
>
> -static int qcom_edp_phy_power_on(struct phy *phy)
> +static int qcom_edp_prepare_power_on_v46(const struct qcom_edp *edp)
> {
> - const struct qcom_edp *edp = phy_get_drvdata(phy);
> - u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> - unsigned long pixel_freq;
> - int ret;
> - u32 val;
> - u8 cfg1;
> -
> - ret = edp->cfg->ver_ops->com_power_on(edp);
> - if (ret)
> - return ret;
> -
> - ret = edp->cfg->ver_ops->com_ldo_config(edp);
> - if (ret)
> - return ret;
> -
> - writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
> - writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
> -
> - if (edp->dp_opts.ssc) {
> - ret = qcom_edp_configure_ssc(edp);
> - if (ret)
> - return ret;
> - }
> -
> - ret = qcom_edp_configure_pll(edp);
> - if (ret)
> - return ret;
> + return 0;
> +}
>
> - /* TX Lane configuration */
> +static int qcom_edp_configure_tx_pre_pll_v46(const struct qcom_edp *edp)
> +{
> writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
> writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
>
> - /* TX-0 register configuration */
> writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> writel(0x0f, edp->tx0 + TXn_CLKBUF_ENABLE);
> writel(0x03, edp->tx0 + TXn_RESET_TSYNC_EN);
> writel(0x01, edp->tx0 + TXn_TRAN_DRVR_EMP_EN);
> writel(0x04, edp->tx0 + TXn_TX_BAND);
>
> - /* TX-1 register configuration */
> writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> writel(0x0f, edp->tx1 + TXn_CLKBUF_ENABLE);
> writel(0x03, edp->tx1 + TXn_RESET_TSYNC_EN);
> writel(0x01, edp->tx1 + TXn_TRAN_DRVR_EMP_EN);
> writel(0x04, edp->tx1 + TXn_TX_BAND);
>
> - ret = qcom_edp_set_vco_div(edp, &pixel_freq);
> - if (ret)
> - return ret;
> -
> - writel(0x01, edp->edp + DP_PHY_CFG);
> - writel(0x05, edp->edp + DP_PHY_CFG);
> - writel(0x01, edp->edp + DP_PHY_CFG);
> - writel(0x09, edp->edp + DP_PHY_CFG);
> + return 0;
> +}
>
> - ret = edp->cfg->ver_ops->com_resetsm_cntrl(edp);
> - if (ret)
> - return ret;
> +static void qcom_edp_configure_lanes_after_pll_v46(const struct qcom_edp *edp)
> +{
> + u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> + u8 cfg1;
>
> - writel(0x19, edp->edp + DP_PHY_CFG);
> writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> writel(0x00, edp->tx0 + TXn_TX_POL_INV);
> writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> +
> writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
> writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
> writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
> writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
> writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
> writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
> -
> writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
> writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
> @@ -1194,14 +1193,90 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> writel(drvr1_en, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> writel(bias1_en, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> writel(cfg1, edp->edp + DP_PHY_CFG_1);
> +}
> +
> +static int qcom_edp_configure_rate_pcs_v46(const struct qcom_edp *edp,
> + unsigned long *pixel_freq)
> +{
> + return qcom_edp_set_vco_div(edp, pixel_freq);
> +}
> +
> +static int qcom_edp_start_pll(const struct qcom_edp *edp)
> +{
> + int ret;
> +
> + writel(0x01, edp->edp + DP_PHY_CFG);
> + writel(0x05, edp->edp + DP_PHY_CFG);
> + writel(0x01, edp->edp + DP_PHY_CFG);
> + writel(0x09, edp->edp + DP_PHY_CFG);
> +
> + ret = edp->cfg->ver_ops->com_resetsm_cntrl(edp);
> + if (ret)
> + return ret;
> +
> + writel(0x19, edp->edp + DP_PHY_CFG);
> +
> + return 0;
> +}
> +
> +static int qcom_edp_finish_power_on_v46(const struct qcom_edp *edp)
> +{
> + u32 val;
>
> writel(0x18, edp->edp + DP_PHY_CFG);
> usleep_range(100, 1000);
> -
> writel(0x19, edp->edp + DP_PHY_CFG);
>
> - ret = readl_poll_timeout(edp->edp + DP_PHY_STATUS,
> - val, val & BIT(1), 500, 10000);
> + return readl_poll_timeout(edp->edp + DP_PHY_STATUS, val, val & BIT(1),
> + 500, 10000);
> +}
> +
> +static int qcom_edp_phy_power_on(struct phy *phy)
> +{
> + const struct qcom_edp *edp = phy_get_drvdata(phy);
> + unsigned long pixel_freq;
> + int ret;
> +
> + ret = edp->cfg->ver_ops->com_power_on(edp);
> + if (ret)
> + return ret;
> +
> + ret = edp->cfg->ver_ops->prepare_power_on(edp);
> + if (ret)
> + return ret;
> +
> + ret = edp->cfg->ver_ops->com_ldo_config(edp);
> + if (ret)
> + return ret;
> +
> + writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
> + writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
> +
> + if (edp->dp_opts.ssc) {
> + ret = qcom_edp_configure_ssc(edp);
> + if (ret)
> + return ret;
> + }
> +
> + ret = qcom_edp_configure_pll(edp);
> + if (ret)
> + return ret;
> +
> + ret = edp->cfg->ver_ops->configure_tx_pre_pll(edp);
> + if (ret)
> + return ret;
> +
> + ret = edp->cfg->ver_ops->configure_rate_pcs(edp, &pixel_freq);
> + if (ret)
> + return ret;
> +
> + ret = qcom_edp_start_pll(edp);
> + if (ret)
> + return ret;
> +
> + edp->cfg->ver_ops->configure_lanes_after_pll(edp);
> +
> + ret = edp->cfg->ver_ops->finish_power_on(edp);
> if (ret)
> return ret;
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260917/f6114aed/attachment.sig>
More information about the kernel-team
mailing list