NACK/Cmnt: [PATCH 1/2] UBUNTU: SAUCE: phy: qcom: edp: split power-on sequencing by PHY version
Stefan Bader
stefan.bader at canonical.com
Fri Sep 18 07:44:32 UTC 2026
On 17/09/2026 16:17, Tobias Heider wrote:
> Sorry I fumbled the mail, I originally forgot to include those two in
> the series I sent yesterday.
> They would be quite helpful since they make some OEM laptops work with
> 26.10 and increase our testing capacity.
>
If that is for 26.10/Stonking. things are a bit more relaxed since that
has a limited support timeline. Still SRU should be coming from upstream
linux or at least linux-next since that increases chances that changes
have settled a bit.
-Stefan
> I'll resend them with a source, cover letter and subject.
>
> On Thu, Sep 17, 2026 at 12:38 PM Stefan Bader
> <stefan.bader at canonical.com> wrote:
>>
>> 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/20260918/729f6d5a/attachment-0001.sig>
More information about the kernel-team
mailing list