On 15.02.2024 11:35, Abel Vesa wrote: > On 24-01-29 11:40:24, Dmitry Baryshkov wrote: >> On Mon, 29 Jan 2024 at 10:06, Abel Vesa <abel.vesa@xxxxxxxxxx> wrote: >>> >>> On 24-01-29 06:05:09, Dmitry Baryshkov wrote: >>>> On Mon, 29 Jan 2024 at 02:26, Abel Vesa <abel.vesa@xxxxxxxxxx> wrote: >>>>> >>>>> Future platforms should not use different compatibles to differentiate >>>>> between eDP and DP mode. Instead, they should use a single compatible as the >>>>> IP block is the same. It will be the job of the controller to set the submode >>>>> of the PHY accordingly. >>>>> >>>>> The existing platforms will remain with separate compatibles for each mode. >>>>> >>>>> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx> >>>>> --- >>>>> drivers/phy/qualcomm/phy-qcom-edp.c | 71 ++++++++++++++++++++++++++----------- >>>>> 1 file changed, 51 insertions(+), 20 deletions(-) >>>>> >>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c >>>>> index 8e5078304646..af941d6c5588 100644 >>>>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c >>>>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c >>>>> @@ -14,6 +14,7 @@ >>>>> #include <linux/module.h> >>>>> #include <linux/of.h> >>>>> #include <linux/phy/phy.h> >>>>> +#include <linux/phy/phy-dp.h> >>>>> #include <linux/platform_device.h> >>>>> #include <linux/regulator/consumer.h> >>>>> #include <linux/reset.h> >>>>> @@ -68,19 +69,21 @@ >>>>> >>>>> #define TXn_TRAN_DRVR_EMP_EN 0x0078 >>>>> >>>>> -struct qcom_edp_cfg { >>>>> - bool is_dp; >>>>> - >>>>> - /* DP PHY swing and pre_emphasis tables */ >>>>> +struct qcom_edp_swing_pre_emph_cfg { >>>>> const u8 (*swing_hbr_rbr)[4][4]; >>>>> const u8 (*swing_hbr3_hbr2)[4][4]; >>>>> const u8 (*pre_emphasis_hbr_rbr)[4][4]; >>>>> const u8 (*pre_emphasis_hbr3_hbr2)[4][4]; >>>>> }; >>>>> >>>>> +struct qcom_edp_phy_cfg { >>>>> + bool is_edp; >>>>> + const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg; >>>>> +}; >>>>> + >>>>> struct qcom_edp { >>>>> struct device *dev; >>>>> - const struct qcom_edp_cfg *cfg; >>>>> + const struct qcom_edp_phy_cfg *cfg; >>>>> >>>>> struct phy *phy; >>>>> >>>>> @@ -96,6 +99,8 @@ struct qcom_edp { >>>>> >>>>> struct clk_bulk_data clks[2]; >>>>> struct regulator_bulk_data supplies[2]; >>>>> + >>>>> + bool is_edp; >>>>> }; >>>>> >>>>> static const u8 dp_swing_hbr_rbr[4][4] = { >>>>> @@ -126,8 +131,7 @@ static const u8 dp_pre_emp_hbr2_hbr3[4][4] = { >>>>> { 0x04, 0xff, 0xff, 0xff } >>>>> }; >>>>> >>>>> -static const struct qcom_edp_cfg dp_phy_cfg = { >>>>> - .is_dp = true, >>>>> +static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg = { >>>>> .swing_hbr_rbr = &dp_swing_hbr_rbr, >>>>> .swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3, >>>>> .pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr, >>>>> @@ -162,18 +166,28 @@ static const u8 edp_pre_emp_hbr2_hbr3[4][4] = { >>>>> { 0x00, 0xff, 0xff, 0xff } >>>>> }; >>>>> >>>>> -static const struct qcom_edp_cfg edp_phy_cfg = { >>>>> - .is_dp = false, >>>>> +static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = { >>>>> .swing_hbr_rbr = &edp_swing_hbr_rbr, >>>>> .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3, >>>>> .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr, >>>>> .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3, >>>>> }; >>>>> >>>>> +static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = { >>>>> +}; >>>>> + >>>>> +static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = { >>>>> + .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg, >>>>> +}; >>>>> + >>>>> +static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = { >>>>> + .is_edp = true, >>>>> + .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg, >>>>> +}; >>>>> + >>>>> static int qcom_edp_phy_init(struct phy *phy) >>>>> { >>>>> struct qcom_edp *edp = phy_get_drvdata(phy); >>>>> - const struct qcom_edp_cfg *cfg = edp->cfg; >>>>> int ret; >>>>> u8 cfg8; >>>>> >>>>> @@ -200,7 +214,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); >>>>> >>>>> - if (cfg && cfg->is_dp) >>>>> + if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp) >>>> >>>> I think (!edp->is_edp) should be enough here. >>> >>> Actually, in case of DP, the cfg8 needs to be 0xb7 for sc8280xp, while for sc7280 it should be 0x37. >>> >>> So to differentiate between first and second we check if the config >>> provides a swing_pre_emph_cfg >> >> Using swing_pre_emph_cfg to distinguish between those two cases is a pure hack. >> Is there any sensible meaning behind those bits? If not, just put >> those values into the configuration data. > > So the only thing that I was able to find is that this reg has to do > with some TX and RX zero bits preamble. It seems it needs to be > different between DP configurations (with or without swing and pre-emph). > > But leaving that aside, unless we add another knob that actually depends > on swing and pre-emph configuration being available, there is no other > way around it, for now, at least. Until we know better, I'd say this hack is fine, but please leave a comment so that we don't fully forget about it. Konrad