This patchset adds support for the X1E80100 eDP/DP PHY and documents its
compatible.
Now, the X1E80100 uses QMP COM v6 registers, so it is added here. This
required configuration-based register offsets support, so that's done
here as well. Then, the legacy "PHY type" specific compatible should not
be used by newer platforms, so the platform-specific configurations have
been added. Rest of it is pretty much variables renaming to make their
use more obvious.
Only tested this on the X1E80100 CRD. Need to test it on at least one
legacy with "PHY type" compatible platforms.
This patchset depends on the QSERDES_V6_COM_SSC_ADJ_PER1 register offset
added by the following patchset:
https://lore.kernel.org/all/20231122-phy-qualcomm-v6-v6-20-v7-new-offsets-v1-0-d9340d362664@linaro.org/
Signed-off-by: Abel Vesa <[email protected]>
---
Changes in v3:
- The whole support for COM v6 register offsets has been reworked from scratch.
- The bindings now document the phy-type property. (dropped Krzysztof's R-b tag)
- New patch for adding PHY_TYPE_EDP into bindings header file.
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Added Krzysztof's R-b tag
- Re-worded commit message for bindings to suggest same PHY can work in
both eDP and DP mode rather than being different PHY types.
- Implemented different qcom_edp_configure_ssc and
qcom_edp_configure_pll for each version of the PHY.
- Dropped the cfg8 override in qcom_edp_phy_init
- Used enum instead of defines for PHY versions
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Abel Vesa (3):
dt-bindings: phy: Add PHY_TYPE_EDP definition
dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles
phy: qcom: edp: Add support for X1E80100 PHY
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 15 +
drivers/phy/qualcomm/phy-qcom-edp.c | 583 +++++++++++++++++----
include/dt-bindings/phy/phy.h | 1 +
3 files changed, 506 insertions(+), 93 deletions(-)
---
base-commit: 629a3b49f3f957e975253c54846090b8d5ed2e9b
change-id: 20231122-phy-qualcomm-edp-x1e80100-a57c15fff32b
Best regards,
--
Abel Vesa <[email protected]>
Add definition for Embedded DisplayPort (eDP) phy type.
Signed-off-by: Abel Vesa <[email protected]>
---
include/dt-bindings/phy/phy.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index 6b901b342348..b1a64508d937 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -23,5 +23,6 @@
#define PHY_TYPE_DPHY 10
#define PHY_TYPE_CPHY 11
#define PHY_TYPE_USXGMII 12
+#define PHY_TYPE_EDP 13
#endif /* _DT_BINDINGS_PHY */
--
2.34.1
The Qualcomm X1E80100 platform has multiple PHYs that can work in both
eDP or DP mode, add compatibles for these. New platforms can use the
phy-type property to specify which mode the PHY should be configured in.
Signed-off-by: Abel Vesa <[email protected]>
---
Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
index 6566353f1a02..3341283577ce 100644
--- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
@@ -21,6 +21,7 @@ properties:
- qcom,sc8180x-edp-phy
- qcom,sc8280xp-dp-phy
- qcom,sc8280xp-edp-phy
+ - qcom,x1e80100-dp-phy
reg:
items:
@@ -59,6 +60,20 @@ required:
additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,x1e80100-dp-phy
+ then:
+ properties:
+ phy-type:
+ description: DP (default) or eDP type
+ enum: [ 6, 13 ]
+ default: 6
+
examples:
- |
phy@aec2a00 {
--
2.34.1
The Qualcomm X1E80100 platform has a number of eDP/DP PHY instances. These
are based on QMP v6. So add support for v6 QMP COM registers by supporting
configuration-based register offsets. For legacy platforms, keep the eDP and DP
compatibles different, but for new platforms, use the phy-type DT property
instead. So add platform specific configs, specify the version and override
the PHY type where compatible dictates it.
Co-developed-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-edp.c | 583 ++++++++++++++++++++++++++++++------
1 file changed, 490 insertions(+), 93 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index 8e5078304646..6fc18bf80db1 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -68,10 +68,333 @@
#define TXn_TRAN_DRVR_EMP_EN 0x0078
-struct qcom_edp_cfg {
- bool is_dp;
+enum dp_qmp_com_version {
+ DP_QMP_VERSION_V4,
+ DP_QMP_VERSION_V6,
+};
+
+enum dp_link_rate {
+ DP_LINK_RATE_1_6_GHZ,
+ DP_LINK_RATE_2_7_GHZ,
+ DP_LINK_RATE_5_4_GHZ,
+ DP_LINK_RATE_8_1_GHZ,
+};
+
+struct qmp_com_regs_layout {
+ unsigned int cmn_status;
+ unsigned int ssc_en_center;
+ unsigned int resetsm_cntrl;
+ unsigned int c_ready_status;
+ unsigned int ssc_per1;
+ unsigned int ssc_per2;
+ unsigned int ssc_step_size1_mode0;
+ unsigned int ssc_step_size2_mode0;
+ unsigned int ssc_adj_per1;
+ unsigned int svs_mode_clk_sel;
+ unsigned int sysclk_en_sel;
+ unsigned int sys_clk_ctrl;
+ unsigned int clk_enable1;
+ unsigned int clk_select;
+ unsigned int sysclk_buf_enable;
+ unsigned int hsclk_sel;
+ unsigned int pll_ivco;
+ unsigned int lock_cmp_en;
+ unsigned int pll_cctrl_mode0;
+ unsigned int pll_rctrl_mode0;
+ unsigned int cp_ctrl_mode0;
+ unsigned int dec_start_mode0;
+ unsigned int div_frac_start1_mode0;
+ unsigned int div_frac_start2_mode0;
+ unsigned int div_frac_start3_mode0;
+ unsigned int cmn_config;
+ unsigned int integloop_gain0_mode0;
+ unsigned int integloop_gain1_mode0;
+ unsigned int vco_tune_map;
+ unsigned int lock_cmp1_mode0;
+ unsigned int lock_cmp2_mode0;
+ unsigned int bg_timer;
+ unsigned int pll_core_clk_div_mode0;
+ unsigned int vco_tune_ctrl;
+ unsigned int pll_bias_en_clk_buflr_en;
+ unsigned int core_clk_en;
+ unsigned int vco_tune1_mode0;
+ unsigned int vco_tune2_mode0;
+ unsigned int bin_vcocal_cmp_code1_mode0;
+ unsigned int bin_vcocal_cmp_code2_mode0;
+};
+
+static const struct qmp_com_regs_layout qmp_v4_com_regs = {
+ .cmn_status = QSERDES_V4_COM_CMN_STATUS,
+ .c_ready_status = QSERDES_V4_COM_C_READY_STATUS,
+ .resetsm_cntrl = QSERDES_V4_COM_RESETSM_CNTRL,
+ .ssc_en_center = QSERDES_V4_COM_SSC_EN_CENTER,
+ .ssc_per1 = QSERDES_V4_COM_SSC_PER1,
+ .ssc_per2 = QSERDES_V4_COM_SSC_PER2,
+ .ssc_step_size1_mode0 = QSERDES_V4_COM_SSC_STEP_SIZE1_MODE0,
+ .ssc_step_size2_mode0 = QSERDES_V4_COM_SSC_STEP_SIZE2_MODE0,
+ .ssc_adj_per1 = QSERDES_V4_COM_SSC_ADJ_PER1,
+ .svs_mode_clk_sel = QSERDES_V4_COM_SVS_MODE_CLK_SEL,
+ .sysclk_en_sel = QSERDES_V4_COM_SYSCLK_EN_SEL,
+ .sys_clk_ctrl = QSERDES_V4_COM_SYS_CLK_CTRL,
+ .sysclk_buf_enable = QSERDES_V4_COM_SYSCLK_BUF_ENABLE,
+ .clk_enable1 = QSERDES_V4_COM_CLK_ENABLE1,
+ .clk_select = QSERDES_V4_COM_CLK_SELECT,
+ .hsclk_sel = QSERDES_V4_COM_HSCLK_SEL,
+ .pll_ivco = QSERDES_V4_COM_PLL_IVCO,
+ .lock_cmp_en = QSERDES_V4_COM_LOCK_CMP_EN,
+ .pll_cctrl_mode0 = QSERDES_V4_COM_PLL_CCTRL_MODE0,
+ .pll_rctrl_mode0 = QSERDES_V4_COM_PLL_RCTRL_MODE0,
+ .cp_ctrl_mode0 = QSERDES_V4_COM_CP_CTRL_MODE0,
+ .dec_start_mode0 = QSERDES_V4_COM_DEC_START_MODE0,
+ .div_frac_start1_mode0 = QSERDES_V4_COM_DIV_FRAC_START1_MODE0,
+ .div_frac_start2_mode0 = QSERDES_V4_COM_DIV_FRAC_START2_MODE0,
+ .div_frac_start3_mode0 = QSERDES_V4_COM_DIV_FRAC_START3_MODE0,
+ .cmn_config = QSERDES_V4_COM_CMN_CONFIG,
+ .integloop_gain0_mode0 = QSERDES_V4_COM_INTEGLOOP_GAIN0_MODE0,
+ .integloop_gain1_mode0 = QSERDES_V4_COM_INTEGLOOP_GAIN1_MODE0,
+ .vco_tune_map = QSERDES_V4_COM_VCO_TUNE_MAP,
+ .lock_cmp1_mode0 = QSERDES_V4_COM_LOCK_CMP1_MODE0,
+ .lock_cmp2_mode0 = QSERDES_V4_COM_LOCK_CMP2_MODE0,
+ .bg_timer = QSERDES_V4_COM_BG_TIMER,
+ .pll_core_clk_div_mode0 = QSERDES_V4_COM_CORECLK_DIV_MODE0,
+ .vco_tune_ctrl = QSERDES_V4_COM_VCO_TUNE_CTRL,
+ .pll_bias_en_clk_buflr_en = QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN,
+ .core_clk_en = QSERDES_V4_COM_CORE_CLK_EN,
+ .vco_tune1_mode0 = QSERDES_V4_COM_VCO_TUNE1_MODE0,
+ .vco_tune2_mode0 = QSERDES_V4_COM_VCO_TUNE2_MODE0,
+};
+
+static const struct qmp_com_regs_layout qmp_v6_com_regs = {
+ .cmn_status = QSERDES_V6_COM_CMN_STATUS,
+ .c_ready_status = QSERDES_V6_COM_C_READY_STATUS,
+ .resetsm_cntrl = QSERDES_V6_COM_RESETSM_CNTRL,
+ .ssc_en_center = QSERDES_V6_COM_SSC_EN_CENTER,
+ .ssc_per1 = QSERDES_V6_COM_SSC_PER1,
+ .ssc_per2 = QSERDES_V6_COM_SSC_PER2,
+ .ssc_step_size1_mode0 = QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0,
+ .ssc_step_size2_mode0 = QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0,
+ .ssc_adj_per1 = QSERDES_V6_COM_SSC_ADJ_PER1,
+ .svs_mode_clk_sel = QSERDES_V6_COM_SVS_MODE_CLK_SEL,
+ .sysclk_en_sel = QSERDES_V6_COM_SYSCLK_EN_SEL,
+ .sys_clk_ctrl = QSERDES_V6_COM_SYS_CLK_CTRL,
+ .sysclk_buf_enable = QSERDES_V6_COM_SYSCLK_BUF_ENABLE,
+ .clk_enable1 = QSERDES_V6_COM_CLK_ENABLE1,
+ .clk_select = QSERDES_V6_COM_CLK_SELECT,
+ .hsclk_sel = QSERDES_V6_COM_HSCLK_SEL_1,
+ .pll_ivco = QSERDES_V6_COM_PLL_IVCO,
+ .lock_cmp_en = QSERDES_V6_COM_LOCK_CMP_EN,
+ .pll_cctrl_mode0 = QSERDES_V6_COM_PLL_CCTRL_MODE0,
+ .pll_rctrl_mode0 = QSERDES_V6_COM_PLL_RCTRL_MODE0,
+ .cp_ctrl_mode0 = QSERDES_V6_COM_CP_CTRL_MODE0,
+ .dec_start_mode0 = QSERDES_V6_COM_DEC_START_MODE0,
+ .div_frac_start1_mode0 = QSERDES_V6_COM_DIV_FRAC_START1_MODE0,
+ .div_frac_start2_mode0 = QSERDES_V6_COM_DIV_FRAC_START2_MODE0,
+ .div_frac_start3_mode0 = QSERDES_V6_COM_DIV_FRAC_START3_MODE0,
+ .cmn_config = QSERDES_V6_COM_CMN_CONFIG_1,
+ .integloop_gain0_mode0 = QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE0,
+ .integloop_gain1_mode0 = QSERDES_V6_COM_INTEGLOOP_GAIN1_MODE0,
+ .vco_tune_map = QSERDES_V6_COM_VCO_TUNE_MAP,
+ .lock_cmp1_mode0 = QSERDES_V6_COM_LOCK_CMP1_MODE0,
+ .lock_cmp2_mode0 = QSERDES_V6_COM_LOCK_CMP2_MODE0,
+ .bg_timer = QSERDES_V6_COM_BG_TIMER,
+ .pll_core_clk_div_mode0 = QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0,
+ .vco_tune_ctrl = QSERDES_V6_COM_VCO_TUNE_CTRL,
+ .pll_bias_en_clk_buflr_en = QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN,
+ .core_clk_en = QSERDES_V6_COM_CORE_CLK_EN,
+ .bin_vcocal_cmp_code1_mode0 = QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE1_MODE0,
+ .bin_vcocal_cmp_code2_mode0 = QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE2_MODE0,
+};
+
+struct qmp_com_init {
+ const u8 ssc_per1;
+ const u8 ssc_per2;
+ const u8 pll_ivco;
+ const u8 cmn_config;
+ const u8 vco_tune1_mode0;
+ const u8 vco_tune2_mode0;
+ const u8 pll_bias_en_clk_buflr_en;
+
+ const u8 *ssc_step1;
+ const u8 *ssc_step2;
+ const u8 *hsclk_sel;
+ const u8 *dec_start_mode0;
+ const u8 *div_frac_start2_mode0;
+ const u8 *div_frac_start3_mode0;
+ const u8 *lock_cmp1_mode0;
+ const u8 *lock_cmp2_mode0;
+ const u8 *code1_mode0;
+ const u8 *code2_mode0;
+};
+
+static const u8 qmp_v4_com_ssc_step1[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x45,
+ [DP_LINK_RATE_2_7_GHZ] = 0x45,
+ [DP_LINK_RATE_5_4_GHZ] = 0x5c,
+ [DP_LINK_RATE_8_1_GHZ] = 0x45,
+};
+
+static const u8 qmp_v4_com_ssc_step2[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x06,
+ [DP_LINK_RATE_2_7_GHZ] = 0x06,
+ [DP_LINK_RATE_5_4_GHZ] = 0x08,
+ [DP_LINK_RATE_8_1_GHZ] = 0x06,
+};
+
+static const u8 qmp_v4_com_hsclk_sel[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x05,
+ [DP_LINK_RATE_2_7_GHZ] = 0x03,
+ [DP_LINK_RATE_5_4_GHZ] = 0x01,
+ [DP_LINK_RATE_8_1_GHZ] = 0x00,
+};
+
+static const u8 qmp_v4_com_dec_start_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x69,
+ [DP_LINK_RATE_2_7_GHZ] = 0x69,
+ [DP_LINK_RATE_5_4_GHZ] = 0x8c,
+ [DP_LINK_RATE_8_1_GHZ] = 0x69,
+};
+
+static const u8 qmp_v4_com_div_frac_start2_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x80,
+ [DP_LINK_RATE_2_7_GHZ] = 0x80,
+ [DP_LINK_RATE_5_4_GHZ] = 0x00,
+ [DP_LINK_RATE_8_1_GHZ] = 0x80,
+};
+
+static const u8 qmp_v4_com_div_frac_start3_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x07,
+ [DP_LINK_RATE_2_7_GHZ] = 0x07,
+ [DP_LINK_RATE_5_4_GHZ] = 0x0a,
+ [DP_LINK_RATE_8_1_GHZ] = 0x07,
+};
+
+static const u8 qmp_v4_com_lock_cmp1_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x6f,
+ [DP_LINK_RATE_2_7_GHZ] = 0x0f,
+ [DP_LINK_RATE_5_4_GHZ] = 0x1f,
+ [DP_LINK_RATE_8_1_GHZ] = 0x2f,
+};
+
+static const u8 qmp_v4_com_lock_cmp2_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x08,
+ [DP_LINK_RATE_2_7_GHZ] = 0x0e,
+ [DP_LINK_RATE_5_4_GHZ] = 0x1c,
+ [DP_LINK_RATE_8_1_GHZ] = 0x2a,
+};
+
+static const struct qmp_com_init qmp_v4_com_init = {
+ .ssc_per1 = 0x36,
+ .ssc_per2 = 0x01,
+ .pll_ivco = 0x0f,
+ .cmn_config = 0x02,
+ .pll_bias_en_clk_buflr_en = 0x17,
+ .ssc_step1 = qmp_v4_com_ssc_step1,
+ .ssc_step2 = qmp_v4_com_ssc_step2,
+ .hsclk_sel = qmp_v4_com_hsclk_sel,
+ .dec_start_mode0 = qmp_v4_com_dec_start_mode0,
+ .div_frac_start2_mode0 = qmp_v4_com_div_frac_start2_mode0,
+ .div_frac_start3_mode0 = qmp_v4_com_div_frac_start3_mode0,
+ .lock_cmp1_mode0 = qmp_v4_com_lock_cmp1_mode0,
+ .lock_cmp2_mode0 = qmp_v4_com_lock_cmp2_mode0,
+ .vco_tune1_mode0 = 0xa0,
+ .vco_tune2_mode0 = 0x03,
+};
+
+static const u8 qmp_v6_com_ssc_step1[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x92,
+ [DP_LINK_RATE_2_7_GHZ] = 0x92,
+ [DP_LINK_RATE_5_4_GHZ] = 0x18,
+ [DP_LINK_RATE_8_1_GHZ] = 0x92,
+};
+
+static const u8 qmp_v6_com_ssc_step2[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x01,
+ [DP_LINK_RATE_2_7_GHZ] = 0x01,
+ [DP_LINK_RATE_5_4_GHZ] = 0x02,
+ [DP_LINK_RATE_8_1_GHZ] = 0x01,
+};
+
+static const u8 qmp_v6_com_hsclk_sel[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x05,
+ [DP_LINK_RATE_2_7_GHZ] = 0x03,
+ [DP_LINK_RATE_5_4_GHZ] = 0x01,
+ [DP_LINK_RATE_8_1_GHZ] = 0x00,
+};
+
+static const u8 qmp_v6_com_dec_start_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x34,
+ [DP_LINK_RATE_2_7_GHZ] = 0x34,
+ [DP_LINK_RATE_5_4_GHZ] = 0x46,
+ [DP_LINK_RATE_8_1_GHZ] = 0x34,
+};
+
+static const u8 qmp_v6_com_div_frac_start2_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0xc0,
+ [DP_LINK_RATE_2_7_GHZ] = 0xc0,
+ [DP_LINK_RATE_5_4_GHZ] = 0x00,
+ [DP_LINK_RATE_8_1_GHZ] = 0xc0,
+};
+
+static const u8 qmp_v6_com_div_frac_start3_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x0b,
+ [DP_LINK_RATE_2_7_GHZ] = 0x0b,
+ [DP_LINK_RATE_5_4_GHZ] = 0x05,
+ [DP_LINK_RATE_8_1_GHZ] = 0x0b,
+};
+
+static const u8 qmp_v6_com_lock_cmp1_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x37,
+ [DP_LINK_RATE_2_7_GHZ] = 0x07,
+ [DP_LINK_RATE_5_4_GHZ] = 0x0f,
+ [DP_LINK_RATE_8_1_GHZ] = 0x17,
+};
- /* DP PHY swing and pre_emphasis tables */
+static const u8 qmp_v6_com_lock_cmp2_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x04,
+ [DP_LINK_RATE_2_7_GHZ] = 0x07,
+ [DP_LINK_RATE_5_4_GHZ] = 0x0e,
+ [DP_LINK_RATE_8_1_GHZ] = 0x15,
+};
+
+static const u8 qmp_v6_com_code1_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x71,
+ [DP_LINK_RATE_2_7_GHZ] = 0x71,
+ [DP_LINK_RATE_5_4_GHZ] = 0x97,
+ [DP_LINK_RATE_8_1_GHZ] = 0x71,
+};
+
+static const u8 qmp_v6_com_code2_mode0[] = {
+ [DP_LINK_RATE_1_6_GHZ] = 0x0c,
+ [DP_LINK_RATE_2_7_GHZ] = 0x0c,
+ [DP_LINK_RATE_5_4_GHZ] = 0x10,
+ [DP_LINK_RATE_8_1_GHZ] = 0x0c,
+};
+
+static const struct qmp_com_init qmp_v6_com_init = {
+ .ssc_per1 = 0x6b,
+ .ssc_per2 = 0x02,
+ .pll_ivco = 0x07,
+ .cmn_config = 0x12,
+ .pll_bias_en_clk_buflr_en = 0x1f,
+ .ssc_step1 = qmp_v6_com_ssc_step1,
+ .ssc_step2 = qmp_v6_com_ssc_step2,
+ .hsclk_sel = qmp_v6_com_hsclk_sel,
+ .dec_start_mode0 = qmp_v6_com_dec_start_mode0,
+ .div_frac_start2_mode0 = qmp_v6_com_div_frac_start2_mode0,
+ .div_frac_start3_mode0 = qmp_v6_com_div_frac_start3_mode0,
+ .lock_cmp1_mode0 = qmp_v6_com_lock_cmp1_mode0,
+ .lock_cmp2_mode0 = qmp_v6_com_lock_cmp2_mode0,
+ .code1_mode0 = qmp_v6_com_code1_mode0,
+ .code2_mode0 = qmp_v6_com_code2_mode0,
+};
+
+struct qmp_phy_cfg {
+ int type;
+ enum dp_qmp_com_version version;
+ bool needs_swing_pre_emph_cfg;
+};
+
+struct qcom_dp_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];
@@ -80,7 +403,9 @@ struct qcom_edp_cfg {
struct qcom_edp {
struct device *dev;
- const struct qcom_edp_cfg *cfg;
+ const struct qcom_dp_swing_pre_emph_cfg *swing_pre_emph_cfg;
+ const struct qmp_com_regs_layout *com_regs;
+ const struct qmp_com_init *init_values;
struct phy *phy;
@@ -96,6 +421,8 @@ struct qcom_edp {
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+
+ bool is_dp;
};
static const u8 dp_swing_hbr_rbr[4][4] = {
@@ -126,8 +453,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_dp_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 +488,40 @@ 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_dp_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 struct qmp_phy_cfg sc7280_dp_phy_cfg = {
+ .type = PHY_TYPE_DP,
+ .version = DP_QMP_VERSION_V4,
+};
+
+static struct qmp_phy_cfg sc8280xp_dp_phy_cfg = {
+ .type = PHY_TYPE_DP,
+ .version = DP_QMP_VERSION_V4,
+ .needs_swing_pre_emph_cfg = true,
+};
+
+static struct qmp_phy_cfg sc8280xp_edp_phy_cfg = {
+ .type = PHY_TYPE_EDP,
+ .version = DP_QMP_VERSION_V4,
+ .needs_swing_pre_emph_cfg = true,
+};
+
+static struct qmp_phy_cfg x1e80100_phy_cfg = {
+ .version = DP_QMP_VERSION_V6,
+ .needs_swing_pre_emph_cfg = true,
+};
+
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;
+ const struct qmp_com_init *init = edp->init_values;
+ const struct qmp_com_regs_layout *regs = edp->com_regs;
int ret;
u8 cfg8;
@@ -190,7 +538,8 @@ static int qcom_edp_phy_init(struct phy *phy)
edp->edp + DP_PHY_PD_CTL);
/* Turn on BIAS current for PHY/PLL */
- writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
+ writel(init->pll_bias_en_clk_buflr_en,
+ edp->pll + regs->pll_bias_en_clk_buflr_en);
writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
msleep(20);
@@ -200,7 +549,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->is_dp)
cfg8 = 0xb7;
else
cfg8 = 0x37;
@@ -234,7 +583,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_cfg *cfg = edp->cfg;
+ const struct qcom_dp_swing_pre_emph_cfg *cfg = edp->swing_pre_emph_cfg;
unsigned int v_level = 0;
unsigned int p_level = 0;
u8 ldo_config;
@@ -261,7 +610,7 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
if (swing == 0xff || emph == 0xff)
return -EINVAL;
- ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+ ldo_config = edp->is_dp ? 0x1 : 0x0;
writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
@@ -291,20 +640,21 @@ static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opt
static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
{
const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
- u32 step1;
- u32 step2;
+ const struct qmp_com_regs_layout *regs = edp->com_regs;
+ const struct qmp_com_init *init = edp->init_values;
+ int link_rate;
+ u8 step1, step2;
+ u8 per1, per2;
switch (dp_opts->link_rate) {
case 1620:
case 2700:
case 8100:
- step1 = 0x45;
- step2 = 0x06;
+ link_rate = DP_LINK_RATE_1_6_GHZ;
break;
case 5400:
- step1 = 0x5c;
- step2 = 0x08;
+ link_rate = DP_LINK_RATE_5_4_GHZ;
break;
default:
@@ -312,12 +662,18 @@ static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
return -EINVAL;
}
- writel(0x01, edp->pll + QSERDES_V4_COM_SSC_EN_CENTER);
- writel(0x00, edp->pll + QSERDES_V4_COM_SSC_ADJ_PER1);
- writel(0x36, edp->pll + QSERDES_V4_COM_SSC_PER1);
- writel(0x01, edp->pll + QSERDES_V4_COM_SSC_PER2);
- writel(step1, edp->pll + QSERDES_V4_COM_SSC_STEP_SIZE1_MODE0);
- writel(step2, edp->pll + QSERDES_V4_COM_SSC_STEP_SIZE2_MODE0);
+ step1 = init->ssc_step1[link_rate];
+ step2 = init->ssc_step2[link_rate];
+
+ per1 = init->ssc_per1;
+ per2 = init->ssc_per2;
+
+ writel(0x01, edp->pll + regs->ssc_en_center);
+ writel(0x00, edp->pll + regs->ssc_adj_per1);
+ writel(per1, edp->pll + regs->ssc_per1);
+ writel(per2, edp->pll + regs->ssc_per2);
+ writel(step1, edp->pll + regs->ssc_step_size1_mode0);
+ writel(step2, edp->pll + regs->ssc_step_size2_mode0);
return 0;
}
@@ -325,48 +681,30 @@ static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
static int qcom_edp_configure_pll(const struct qcom_edp *edp)
{
const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+ const struct qmp_com_regs_layout *regs = edp->com_regs;
+ const struct qmp_com_init *init = edp->init_values;
u32 div_frac_start2_mode0;
u32 div_frac_start3_mode0;
u32 dec_start_mode0;
u32 lock_cmp1_mode0;
u32 lock_cmp2_mode0;
+ u32 code1_mode0;
+ u32 code2_mode0;
u32 hsclk_sel;
+ int link_rate;
switch (dp_opts->link_rate) {
case 1620:
- hsclk_sel = 0x5;
- dec_start_mode0 = 0x69;
- div_frac_start2_mode0 = 0x80;
- div_frac_start3_mode0 = 0x07;
- lock_cmp1_mode0 = 0x6f;
- lock_cmp2_mode0 = 0x08;
+ link_rate = DP_LINK_RATE_1_6_GHZ;
break;
-
case 2700:
- hsclk_sel = 0x3;
- dec_start_mode0 = 0x69;
- div_frac_start2_mode0 = 0x80;
- div_frac_start3_mode0 = 0x07;
- lock_cmp1_mode0 = 0x0f;
- lock_cmp2_mode0 = 0x0e;
+ link_rate = DP_LINK_RATE_2_7_GHZ;
break;
-
case 5400:
- hsclk_sel = 0x1;
- dec_start_mode0 = 0x8c;
- div_frac_start2_mode0 = 0x00;
- div_frac_start3_mode0 = 0x0a;
- lock_cmp1_mode0 = 0x1f;
- lock_cmp2_mode0 = 0x1c;
+ link_rate = DP_LINK_RATE_5_4_GHZ;
break;
-
case 8100:
- hsclk_sel = 0x0;
- dec_start_mode0 = 0x69;
- div_frac_start2_mode0 = 0x80;
- div_frac_start3_mode0 = 0x07;
- lock_cmp1_mode0 = 0x2f;
- lock_cmp2_mode0 = 0x2a;
+ link_rate = DP_LINK_RATE_8_1_GHZ;
break;
default:
@@ -374,36 +712,59 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
return -EINVAL;
}
- writel(0x01, edp->pll + QSERDES_V4_COM_SVS_MODE_CLK_SEL);
- writel(0x0b, edp->pll + QSERDES_V4_COM_SYSCLK_EN_SEL);
- writel(0x02, edp->pll + QSERDES_V4_COM_SYS_CLK_CTRL);
- writel(0x0c, edp->pll + QSERDES_V4_COM_CLK_ENABLE1);
- writel(0x06, edp->pll + QSERDES_V4_COM_SYSCLK_BUF_ENABLE);
- writel(0x30, edp->pll + QSERDES_V4_COM_CLK_SELECT);
- writel(hsclk_sel, edp->pll + QSERDES_V4_COM_HSCLK_SEL);
- writel(0x0f, edp->pll + QSERDES_V4_COM_PLL_IVCO);
- writel(0x08, edp->pll + QSERDES_V4_COM_LOCK_CMP_EN);
- writel(0x36, edp->pll + QSERDES_V4_COM_PLL_CCTRL_MODE0);
- writel(0x16, edp->pll + QSERDES_V4_COM_PLL_RCTRL_MODE0);
- writel(0x06, edp->pll + QSERDES_V4_COM_CP_CTRL_MODE0);
- writel(dec_start_mode0, edp->pll + QSERDES_V4_COM_DEC_START_MODE0);
- writel(0x00, edp->pll + QSERDES_V4_COM_DIV_FRAC_START1_MODE0);
- writel(div_frac_start2_mode0, edp->pll + QSERDES_V4_COM_DIV_FRAC_START2_MODE0);
- writel(div_frac_start3_mode0, edp->pll + QSERDES_V4_COM_DIV_FRAC_START3_MODE0);
- writel(0x02, edp->pll + QSERDES_V4_COM_CMN_CONFIG);
- writel(0x3f, edp->pll + QSERDES_V4_COM_INTEGLOOP_GAIN0_MODE0);
- writel(0x00, edp->pll + QSERDES_V4_COM_INTEGLOOP_GAIN1_MODE0);
- writel(0x00, edp->pll + QSERDES_V4_COM_VCO_TUNE_MAP);
- writel(lock_cmp1_mode0, edp->pll + QSERDES_V4_COM_LOCK_CMP1_MODE0);
- writel(lock_cmp2_mode0, edp->pll + QSERDES_V4_COM_LOCK_CMP2_MODE0);
-
- writel(0x0a, edp->pll + QSERDES_V4_COM_BG_TIMER);
- writel(0x14, edp->pll + QSERDES_V4_COM_CORECLK_DIV_MODE0);
- writel(0x00, edp->pll + QSERDES_V4_COM_VCO_TUNE_CTRL);
- writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
- writel(0x0f, edp->pll + QSERDES_V4_COM_CORE_CLK_EN);
- writel(0xa0, edp->pll + QSERDES_V4_COM_VCO_TUNE1_MODE0);
- writel(0x03, edp->pll + QSERDES_V4_COM_VCO_TUNE2_MODE0);
+ hsclk_sel = init->hsclk_sel[link_rate];
+ dec_start_mode0 = init->dec_start_mode0[link_rate];
+ div_frac_start2_mode0 = init->div_frac_start2_mode0[link_rate];
+ div_frac_start3_mode0 = init->div_frac_start3_mode0[link_rate];
+ lock_cmp1_mode0 = init->lock_cmp1_mode0[link_rate];
+ lock_cmp2_mode0 = init->lock_cmp2_mode0[link_rate];
+
+ if (init->code1_mode0)
+ code1_mode0 = init->code1_mode0[link_rate];
+
+ if (init->code2_mode0)
+ code2_mode0 = init->code2_mode0[link_rate];
+
+ writel(0x01, edp->pll + regs->svs_mode_clk_sel);
+ writel(0x0b, edp->pll + regs->sysclk_en_sel);
+ writel(0x02, edp->pll + regs->sys_clk_ctrl);
+ writel(0x0c, edp->pll + regs->clk_enable1);
+ writel(0x06, edp->pll + regs->sysclk_buf_enable);
+ writel(0x30, edp->pll + regs->clk_select);
+ writel(hsclk_sel, edp->pll + regs->hsclk_sel);
+ writel(init->pll_ivco, edp->pll + regs->pll_ivco);
+ writel(0x08, edp->pll + regs->lock_cmp_en);
+ writel(0x36, edp->pll + regs->pll_cctrl_mode0);
+ writel(0x16, edp->pll + regs->pll_rctrl_mode0);
+ writel(0x06, edp->pll + regs->cp_ctrl_mode0);
+ writel(dec_start_mode0, edp->pll + regs->dec_start_mode0);
+ writel(0x00, edp->pll + regs->div_frac_start1_mode0);
+ writel(div_frac_start2_mode0, edp->pll + regs->div_frac_start2_mode0);
+ writel(div_frac_start3_mode0, edp->pll + regs->div_frac_start3_mode0);
+ writel(init->cmn_config, edp->pll + regs->cmn_config);
+ writel(0x3f, edp->pll + regs->integloop_gain0_mode0);
+ writel(0x00, edp->pll + regs->integloop_gain1_mode0);
+ writel(0x00, edp->pll + regs->vco_tune_map);
+ writel(lock_cmp1_mode0, edp->pll + regs->lock_cmp1_mode0);
+ writel(lock_cmp2_mode0, edp->pll + regs->lock_cmp2_mode0);
+
+ writel(0x0a, edp->pll + regs->bg_timer);
+ writel(0x14, edp->pll + regs->pll_core_clk_div_mode0);
+ writel(0x00, edp->pll + regs->vco_tune_ctrl);
+ writel(0x17, edp->pll + regs->pll_bias_en_clk_buflr_en);
+ writel(0x0f, edp->pll + regs->core_clk_en);
+
+ if (regs->vco_tune1_mode0)
+ writel(init->vco_tune1_mode0, edp->pll + regs->vco_tune1_mode0);
+
+ if (regs->vco_tune2_mode0)
+ writel(init->vco_tune2_mode0, edp->pll + regs->vco_tune2_mode0);
+
+ if (regs->bin_vcocal_cmp_code1_mode0)
+ writel(code1_mode0, edp->pll + regs->bin_vcocal_cmp_code1_mode0);
+
+ if (regs->bin_vcocal_cmp_code2_mode0)
+ writel(code2_mode0, edp->pll + regs->bin_vcocal_cmp_code2_mode0);
return 0;
}
@@ -447,10 +808,10 @@ static int qcom_edp_set_vco_div(const struct qcom_edp *edp, unsigned long *pixel
static int qcom_edp_phy_power_on(struct phy *phy)
{
const struct qcom_edp *edp = phy_get_drvdata(phy);
- const struct qcom_edp_cfg *cfg = edp->cfg;
+ const struct qmp_com_regs_layout *regs = edp->com_regs;
u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
unsigned long pixel_freq;
- u8 ldo_config;
+ u8 ldo_config = 0x0;
int timeout;
int ret;
u32 val;
@@ -462,13 +823,14 @@ static int qcom_edp_phy_power_on(struct phy *phy)
edp->edp + DP_PHY_PD_CTL);
writel(0xfc, edp->edp + DP_PHY_MODE);
- timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
+ timeout = readl_poll_timeout(edp->pll + regs->cmn_status,
val, val & BIT(7), 5, 200);
if (timeout)
return timeout;
- ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+ if (edp->swing_pre_emph_cfg && edp->is_dp)
+ ldo_config = 0x1;
writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
@@ -512,9 +874,9 @@ static int qcom_edp_phy_power_on(struct phy *phy)
writel(0x01, edp->edp + DP_PHY_CFG);
writel(0x09, edp->edp + DP_PHY_CFG);
- writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
+ writel(0x20, edp->pll + regs->resetsm_cntrl);
- timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
+ timeout = readl_poll_timeout(edp->pll + regs->c_ready_status,
val, val & BIT(0), 500, 10000);
if (timeout)
return timeout;
@@ -768,6 +1130,37 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
return devm_of_clk_add_hw_provider(edp->dev, of_clk_hw_onecell_get, data);
}
+static int qcom_edp_setup_phy(struct platform_device *pdev, struct qcom_edp *edp)
+{
+ struct device *dev = &pdev->dev;
+ const struct qmp_phy_cfg *cfg = of_device_get_match_data(dev);
+ enum dp_qmp_com_version version = cfg->version;
+ struct device_node *node = dev->of_node;
+ int type = cfg->type;
+
+ of_property_read_u32(node, "phy-type", &type);
+
+ if (type != PHY_TYPE_DP && type != PHY_TYPE_EDP)
+ return -EINVAL;
+
+ edp->is_dp = (type == PHY_TYPE_DP) ? true : false;
+
+ if (cfg->needs_swing_pre_emph_cfg)
+ edp->swing_pre_emph_cfg = edp->is_dp ?
+ &dp_phy_swing_pre_emph_cfg:
+ &edp_phy_swing_pre_emph_cfg;
+
+ if (version == DP_QMP_VERSION_V6) {
+ edp->com_regs = &qmp_v6_com_regs;
+ edp->init_values = &qmp_v6_com_init;
+ } else {
+ edp->com_regs = &qmp_v4_com_regs;
+ edp->init_values = &qmp_v4_com_init;
+ }
+
+ return 0;
+}
+
static int qcom_edp_phy_probe(struct platform_device *pdev)
{
struct phy_provider *phy_provider;
@@ -780,7 +1173,10 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
return -ENOMEM;
edp->dev = dev;
- edp->cfg = of_device_get_match_data(&pdev->dev);
+
+ ret = qcom_edp_setup_phy(pdev, edp);
+ if (ret)
+ return ret;
edp->edp = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(edp->edp))
@@ -839,10 +1235,11 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
}
static const struct of_device_id qcom_edp_phy_match_table[] = {
- { .compatible = "qcom,sc7280-edp-phy" },
- { .compatible = "qcom,sc8180x-edp-phy" },
- { .compatible = "qcom,sc8280xp-dp-phy", .data = &dp_phy_cfg },
- { .compatible = "qcom,sc8280xp-edp-phy", .data = &edp_phy_cfg },
+ { .compatible = "qcom,sc7280-edp-phy" , .data = &sc7280_dp_phy_cfg, },
+ { .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
+ { .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
+ { .compatible = "qcom,sc8280xp-edp-phy", .data = &sc8280xp_edp_phy_cfg, },
+ { .compatible = "qcom,x1e80100-dp-phy" , .data = &x1e80100_phy_cfg, },
{ }
};
MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
--
2.34.1
On Thu, 7 Dec 2023 at 12:53, Abel Vesa <[email protected]> wrote:
>
> The Qualcomm X1E80100 platform has a number of eDP/DP PHY instances. These
> are based on QMP v6. So add support for v6 QMP COM registers by supporting
> configuration-based register offsets. For legacy platforms, keep the eDP and DP
> compatibles different, but for new platforms, use the phy-type DT property
> instead. So add platform specific configs, specify the version and override
> the PHY type where compatible dictates it.
The phy-type should come as a separate patch. Note, that you also did
not provide the bindings update for this property, which is a must.
Last, but not least. I think that type should come from the host via
the phy_set_mode() call.
>
> Co-developed-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> drivers/phy/qualcomm/phy-qcom-edp.c | 583 ++++++++++++++++++++++++++++++------
> 1 file changed, 490 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index 8e5078304646..6fc18bf80db1 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -68,10 +68,333 @@
>
> #define TXn_TRAN_DRVR_EMP_EN 0x0078
>
> -struct qcom_edp_cfg {
> - bool is_dp;
> +enum dp_qmp_com_version {
> + DP_QMP_VERSION_V4,
> + DP_QMP_VERSION_V6,
> +};
> +
> +enum dp_link_rate {
> + DP_LINK_RATE_1_6_GHZ,
> + DP_LINK_RATE_2_7_GHZ,
> + DP_LINK_RATE_5_4_GHZ,
> + DP_LINK_RATE_8_1_GHZ,
> +};
> +
> +struct qmp_com_regs_layout {
> + unsigned int cmn_status;
> + unsigned int ssc_en_center;
> + unsigned int resetsm_cntrl;
> + unsigned int c_ready_status;
> + unsigned int ssc_per1;
> + unsigned int ssc_per2;
> + unsigned int ssc_step_size1_mode0;
> + unsigned int ssc_step_size2_mode0;
> + unsigned int ssc_adj_per1;
> + unsigned int svs_mode_clk_sel;
> + unsigned int sysclk_en_sel;
> + unsigned int sys_clk_ctrl;
> + unsigned int clk_enable1;
> + unsigned int clk_select;
> + unsigned int sysclk_buf_enable;
> + unsigned int hsclk_sel;
> + unsigned int pll_ivco;
> + unsigned int lock_cmp_en;
> + unsigned int pll_cctrl_mode0;
> + unsigned int pll_rctrl_mode0;
> + unsigned int cp_ctrl_mode0;
> + unsigned int dec_start_mode0;
> + unsigned int div_frac_start1_mode0;
> + unsigned int div_frac_start2_mode0;
> + unsigned int div_frac_start3_mode0;
> + unsigned int cmn_config;
> + unsigned int integloop_gain0_mode0;
> + unsigned int integloop_gain1_mode0;
> + unsigned int vco_tune_map;
> + unsigned int lock_cmp1_mode0;
> + unsigned int lock_cmp2_mode0;
> + unsigned int bg_timer;
> + unsigned int pll_core_clk_div_mode0;
> + unsigned int vco_tune_ctrl;
> + unsigned int pll_bias_en_clk_buflr_en;
> + unsigned int core_clk_en;
> + unsigned int vco_tune1_mode0;
> + unsigned int vco_tune2_mode0;
> + unsigned int bin_vcocal_cmp_code1_mode0;
> + unsigned int bin_vcocal_cmp_code2_mode0;
Unfortunately this is too specific. You depend too much on qserdes
programming details. History has shown that minor details change
between revisions. Compare e.g. qmp_v3_dp_serdes_tbl_rbr vs
qmp_v4_dp_serdes_tbl_rbr vs qmp_v6_dp_serdes_tbl_rbr. I suspect that
we should go either towards per-generation register-value tables (like
we have in the existing combo PHY) or towards per-generation functions
(like I had for
https://patchwork.freedesktop.org/patch/559997/?series=118210&rev=3).
My vote will be towards the former solution. We should really
deduplicate information between phy-qcom-edp.c and
phy-qcom-qmp-combo.c. I haven't looked into that direction for now, I
wanted to get the combo PHYs for msm8998 / qcm2290 out first.
> +};
> +
> +static const struct qmp_com_regs_layout qmp_v4_com_regs = {
> + .cmn_status = QSERDES_V4_COM_CMN_STATUS,
> + .c_ready_status = QSERDES_V4_COM_C_READY_STATUS,
> + .resetsm_cntrl = QSERDES_V4_COM_RESETSM_CNTRL,
> + .ssc_en_center = QSERDES_V4_COM_SSC_EN_CENTER,
> + .ssc_per1 = QSERDES_V4_COM_SSC_PER1,
> + .ssc_per2 = QSERDES_V4_COM_SSC_PER2,
> + .ssc_step_size1_mode0 = QSERDES_V4_COM_SSC_STEP_SIZE1_MODE0,
> + .ssc_step_size2_mode0 = QSERDES_V4_COM_SSC_STEP_SIZE2_MODE0,
> + .ssc_adj_per1 = QSERDES_V4_COM_SSC_ADJ_PER1,
> + .svs_mode_clk_sel = QSERDES_V4_COM_SVS_MODE_CLK_SEL,
> + .sysclk_en_sel = QSERDES_V4_COM_SYSCLK_EN_SEL,
> + .sys_clk_ctrl = QSERDES_V4_COM_SYS_CLK_CTRL,
> + .sysclk_buf_enable = QSERDES_V4_COM_SYSCLK_BUF_ENABLE,
> + .clk_enable1 = QSERDES_V4_COM_CLK_ENABLE1,
> + .clk_select = QSERDES_V4_COM_CLK_SELECT,
> + .hsclk_sel = QSERDES_V4_COM_HSCLK_SEL,
> + .pll_ivco = QSERDES_V4_COM_PLL_IVCO,
> + .lock_cmp_en = QSERDES_V4_COM_LOCK_CMP_EN,
> + .pll_cctrl_mode0 = QSERDES_V4_COM_PLL_CCTRL_MODE0,
> + .pll_rctrl_mode0 = QSERDES_V4_COM_PLL_RCTRL_MODE0,
> + .cp_ctrl_mode0 = QSERDES_V4_COM_CP_CTRL_MODE0,
> + .dec_start_mode0 = QSERDES_V4_COM_DEC_START_MODE0,
> + .div_frac_start1_mode0 = QSERDES_V4_COM_DIV_FRAC_START1_MODE0,
> + .div_frac_start2_mode0 = QSERDES_V4_COM_DIV_FRAC_START2_MODE0,
> + .div_frac_start3_mode0 = QSERDES_V4_COM_DIV_FRAC_START3_MODE0,
> + .cmn_config = QSERDES_V4_COM_CMN_CONFIG,
> + .integloop_gain0_mode0 = QSERDES_V4_COM_INTEGLOOP_GAIN0_MODE0,
> + .integloop_gain1_mode0 = QSERDES_V4_COM_INTEGLOOP_GAIN1_MODE0,
> + .vco_tune_map = QSERDES_V4_COM_VCO_TUNE_MAP,
> + .lock_cmp1_mode0 = QSERDES_V4_COM_LOCK_CMP1_MODE0,
> + .lock_cmp2_mode0 = QSERDES_V4_COM_LOCK_CMP2_MODE0,
> + .bg_timer = QSERDES_V4_COM_BG_TIMER,
> + .pll_core_clk_div_mode0 = QSERDES_V4_COM_CORECLK_DIV_MODE0,
> + .vco_tune_ctrl = QSERDES_V4_COM_VCO_TUNE_CTRL,
> + .pll_bias_en_clk_buflr_en = QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN,
> + .core_clk_en = QSERDES_V4_COM_CORE_CLK_EN,
> + .vco_tune1_mode0 = QSERDES_V4_COM_VCO_TUNE1_MODE0,
> + .vco_tune2_mode0 = QSERDES_V4_COM_VCO_TUNE2_MODE0,
> +};
> +
> +static const struct qmp_com_regs_layout qmp_v6_com_regs = {
> + .cmn_status = QSERDES_V6_COM_CMN_STATUS,
> + .c_ready_status = QSERDES_V6_COM_C_READY_STATUS,
> + .resetsm_cntrl = QSERDES_V6_COM_RESETSM_CNTRL,
> + .ssc_en_center = QSERDES_V6_COM_SSC_EN_CENTER,
> + .ssc_per1 = QSERDES_V6_COM_SSC_PER1,
> + .ssc_per2 = QSERDES_V6_COM_SSC_PER2,
> + .ssc_step_size1_mode0 = QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0,
> + .ssc_step_size2_mode0 = QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0,
> + .ssc_adj_per1 = QSERDES_V6_COM_SSC_ADJ_PER1,
> + .svs_mode_clk_sel = QSERDES_V6_COM_SVS_MODE_CLK_SEL,
> + .sysclk_en_sel = QSERDES_V6_COM_SYSCLK_EN_SEL,
> + .sys_clk_ctrl = QSERDES_V6_COM_SYS_CLK_CTRL,
> + .sysclk_buf_enable = QSERDES_V6_COM_SYSCLK_BUF_ENABLE,
> + .clk_enable1 = QSERDES_V6_COM_CLK_ENABLE1,
> + .clk_select = QSERDES_V6_COM_CLK_SELECT,
> + .hsclk_sel = QSERDES_V6_COM_HSCLK_SEL_1,
> + .pll_ivco = QSERDES_V6_COM_PLL_IVCO,
> + .lock_cmp_en = QSERDES_V6_COM_LOCK_CMP_EN,
> + .pll_cctrl_mode0 = QSERDES_V6_COM_PLL_CCTRL_MODE0,
> + .pll_rctrl_mode0 = QSERDES_V6_COM_PLL_RCTRL_MODE0,
> + .cp_ctrl_mode0 = QSERDES_V6_COM_CP_CTRL_MODE0,
> + .dec_start_mode0 = QSERDES_V6_COM_DEC_START_MODE0,
> + .div_frac_start1_mode0 = QSERDES_V6_COM_DIV_FRAC_START1_MODE0,
> + .div_frac_start2_mode0 = QSERDES_V6_COM_DIV_FRAC_START2_MODE0,
> + .div_frac_start3_mode0 = QSERDES_V6_COM_DIV_FRAC_START3_MODE0,
> + .cmn_config = QSERDES_V6_COM_CMN_CONFIG_1,
> + .integloop_gain0_mode0 = QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE0,
> + .integloop_gain1_mode0 = QSERDES_V6_COM_INTEGLOOP_GAIN1_MODE0,
> + .vco_tune_map = QSERDES_V6_COM_VCO_TUNE_MAP,
> + .lock_cmp1_mode0 = QSERDES_V6_COM_LOCK_CMP1_MODE0,
> + .lock_cmp2_mode0 = QSERDES_V6_COM_LOCK_CMP2_MODE0,
> + .bg_timer = QSERDES_V6_COM_BG_TIMER,
> + .pll_core_clk_div_mode0 = QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0,
> + .vco_tune_ctrl = QSERDES_V6_COM_VCO_TUNE_CTRL,
> + .pll_bias_en_clk_buflr_en = QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN,
> + .core_clk_en = QSERDES_V6_COM_CORE_CLK_EN,
> + .bin_vcocal_cmp_code1_mode0 = QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE1_MODE0,
> + .bin_vcocal_cmp_code2_mode0 = QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE2_MODE0,
> +};
> +
> +struct qmp_com_init {
> + const u8 ssc_per1;
> + const u8 ssc_per2;
> + const u8 pll_ivco;
> + const u8 cmn_config;
> + const u8 vco_tune1_mode0;
> + const u8 vco_tune2_mode0;
> + const u8 pll_bias_en_clk_buflr_en;
> +
> + const u8 *ssc_step1;
> + const u8 *ssc_step2;
> + const u8 *hsclk_sel;
> + const u8 *dec_start_mode0;
> + const u8 *div_frac_start2_mode0;
> + const u8 *div_frac_start3_mode0;
> + const u8 *lock_cmp1_mode0;
> + const u8 *lock_cmp2_mode0;
> + const u8 *code1_mode0;
> + const u8 *code2_mode0;
> +};
> +
> +static const u8 qmp_v4_com_ssc_step1[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x45,
> + [DP_LINK_RATE_2_7_GHZ] = 0x45,
> + [DP_LINK_RATE_5_4_GHZ] = 0x5c,
> + [DP_LINK_RATE_8_1_GHZ] = 0x45,
> +};
> +
> +static const u8 qmp_v4_com_ssc_step2[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x06,
> + [DP_LINK_RATE_2_7_GHZ] = 0x06,
> + [DP_LINK_RATE_5_4_GHZ] = 0x08,
> + [DP_LINK_RATE_8_1_GHZ] = 0x06,
> +};
> +
> +static const u8 qmp_v4_com_hsclk_sel[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x05,
> + [DP_LINK_RATE_2_7_GHZ] = 0x03,
> + [DP_LINK_RATE_5_4_GHZ] = 0x01,
> + [DP_LINK_RATE_8_1_GHZ] = 0x00,
> +};
> +
> +static const u8 qmp_v4_com_dec_start_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x69,
> + [DP_LINK_RATE_2_7_GHZ] = 0x69,
> + [DP_LINK_RATE_5_4_GHZ] = 0x8c,
> + [DP_LINK_RATE_8_1_GHZ] = 0x69,
> +};
> +
> +static const u8 qmp_v4_com_div_frac_start2_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x80,
> + [DP_LINK_RATE_2_7_GHZ] = 0x80,
> + [DP_LINK_RATE_5_4_GHZ] = 0x00,
> + [DP_LINK_RATE_8_1_GHZ] = 0x80,
> +};
> +
> +static const u8 qmp_v4_com_div_frac_start3_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x07,
> + [DP_LINK_RATE_2_7_GHZ] = 0x07,
> + [DP_LINK_RATE_5_4_GHZ] = 0x0a,
> + [DP_LINK_RATE_8_1_GHZ] = 0x07,
> +};
> +
> +static const u8 qmp_v4_com_lock_cmp1_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x6f,
> + [DP_LINK_RATE_2_7_GHZ] = 0x0f,
> + [DP_LINK_RATE_5_4_GHZ] = 0x1f,
> + [DP_LINK_RATE_8_1_GHZ] = 0x2f,
> +};
> +
> +static const u8 qmp_v4_com_lock_cmp2_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x08,
> + [DP_LINK_RATE_2_7_GHZ] = 0x0e,
> + [DP_LINK_RATE_5_4_GHZ] = 0x1c,
> + [DP_LINK_RATE_8_1_GHZ] = 0x2a,
> +};
> +
> +static const struct qmp_com_init qmp_v4_com_init = {
> + .ssc_per1 = 0x36,
> + .ssc_per2 = 0x01,
> + .pll_ivco = 0x0f,
> + .cmn_config = 0x02,
> + .pll_bias_en_clk_buflr_en = 0x17,
> + .ssc_step1 = qmp_v4_com_ssc_step1,
> + .ssc_step2 = qmp_v4_com_ssc_step2,
> + .hsclk_sel = qmp_v4_com_hsclk_sel,
> + .dec_start_mode0 = qmp_v4_com_dec_start_mode0,
> + .div_frac_start2_mode0 = qmp_v4_com_div_frac_start2_mode0,
> + .div_frac_start3_mode0 = qmp_v4_com_div_frac_start3_mode0,
> + .lock_cmp1_mode0 = qmp_v4_com_lock_cmp1_mode0,
> + .lock_cmp2_mode0 = qmp_v4_com_lock_cmp2_mode0,
> + .vco_tune1_mode0 = 0xa0,
> + .vco_tune2_mode0 = 0x03,
> +};
> +
> +static const u8 qmp_v6_com_ssc_step1[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x92,
> + [DP_LINK_RATE_2_7_GHZ] = 0x92,
> + [DP_LINK_RATE_5_4_GHZ] = 0x18,
> + [DP_LINK_RATE_8_1_GHZ] = 0x92,
> +};
> +
> +static const u8 qmp_v6_com_ssc_step2[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x01,
> + [DP_LINK_RATE_2_7_GHZ] = 0x01,
> + [DP_LINK_RATE_5_4_GHZ] = 0x02,
> + [DP_LINK_RATE_8_1_GHZ] = 0x01,
> +};
> +
> +static const u8 qmp_v6_com_hsclk_sel[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x05,
> + [DP_LINK_RATE_2_7_GHZ] = 0x03,
> + [DP_LINK_RATE_5_4_GHZ] = 0x01,
> + [DP_LINK_RATE_8_1_GHZ] = 0x00,
> +};
> +
> +static const u8 qmp_v6_com_dec_start_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x34,
> + [DP_LINK_RATE_2_7_GHZ] = 0x34,
> + [DP_LINK_RATE_5_4_GHZ] = 0x46,
> + [DP_LINK_RATE_8_1_GHZ] = 0x34,
> +};
> +
> +static const u8 qmp_v6_com_div_frac_start2_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0xc0,
> + [DP_LINK_RATE_2_7_GHZ] = 0xc0,
> + [DP_LINK_RATE_5_4_GHZ] = 0x00,
> + [DP_LINK_RATE_8_1_GHZ] = 0xc0,
> +};
> +
> +static const u8 qmp_v6_com_div_frac_start3_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x0b,
> + [DP_LINK_RATE_2_7_GHZ] = 0x0b,
> + [DP_LINK_RATE_5_4_GHZ] = 0x05,
> + [DP_LINK_RATE_8_1_GHZ] = 0x0b,
> +};
> +
> +static const u8 qmp_v6_com_lock_cmp1_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x37,
> + [DP_LINK_RATE_2_7_GHZ] = 0x07,
> + [DP_LINK_RATE_5_4_GHZ] = 0x0f,
> + [DP_LINK_RATE_8_1_GHZ] = 0x17,
> +};
>
> - /* DP PHY swing and pre_emphasis tables */
> +static const u8 qmp_v6_com_lock_cmp2_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x04,
> + [DP_LINK_RATE_2_7_GHZ] = 0x07,
> + [DP_LINK_RATE_5_4_GHZ] = 0x0e,
> + [DP_LINK_RATE_8_1_GHZ] = 0x15,
> +};
> +
> +static const u8 qmp_v6_com_code1_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x71,
> + [DP_LINK_RATE_2_7_GHZ] = 0x71,
> + [DP_LINK_RATE_5_4_GHZ] = 0x97,
> + [DP_LINK_RATE_8_1_GHZ] = 0x71,
> +};
> +
> +static const u8 qmp_v6_com_code2_mode0[] = {
> + [DP_LINK_RATE_1_6_GHZ] = 0x0c,
> + [DP_LINK_RATE_2_7_GHZ] = 0x0c,
> + [DP_LINK_RATE_5_4_GHZ] = 0x10,
> + [DP_LINK_RATE_8_1_GHZ] = 0x0c,
> +};
> +
> +static const struct qmp_com_init qmp_v6_com_init = {
> + .ssc_per1 = 0x6b,
> + .ssc_per2 = 0x02,
> + .pll_ivco = 0x07,
> + .cmn_config = 0x12,
> + .pll_bias_en_clk_buflr_en = 0x1f,
> + .ssc_step1 = qmp_v6_com_ssc_step1,
> + .ssc_step2 = qmp_v6_com_ssc_step2,
> + .hsclk_sel = qmp_v6_com_hsclk_sel,
> + .dec_start_mode0 = qmp_v6_com_dec_start_mode0,
> + .div_frac_start2_mode0 = qmp_v6_com_div_frac_start2_mode0,
> + .div_frac_start3_mode0 = qmp_v6_com_div_frac_start3_mode0,
> + .lock_cmp1_mode0 = qmp_v6_com_lock_cmp1_mode0,
> + .lock_cmp2_mode0 = qmp_v6_com_lock_cmp2_mode0,
> + .code1_mode0 = qmp_v6_com_code1_mode0,
> + .code2_mode0 = qmp_v6_com_code2_mode0,
> +};
> +
> +struct qmp_phy_cfg {
> + int type;
> + enum dp_qmp_com_version version;
> + bool needs_swing_pre_emph_cfg;
> +};
> +
> +struct qcom_dp_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];
> @@ -80,7 +403,9 @@ struct qcom_edp_cfg {
>
> struct qcom_edp {
> struct device *dev;
> - const struct qcom_edp_cfg *cfg;
> + const struct qcom_dp_swing_pre_emph_cfg *swing_pre_emph_cfg;
> + const struct qmp_com_regs_layout *com_regs;
> + const struct qmp_com_init *init_values;
>
> struct phy *phy;
>
> @@ -96,6 +421,8 @@ struct qcom_edp {
>
> struct clk_bulk_data clks[2];
> struct regulator_bulk_data supplies[2];
> +
> + bool is_dp;
> };
>
> static const u8 dp_swing_hbr_rbr[4][4] = {
> @@ -126,8 +453,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_dp_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 +488,40 @@ 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_dp_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 struct qmp_phy_cfg sc7280_dp_phy_cfg = {
> + .type = PHY_TYPE_DP,
> + .version = DP_QMP_VERSION_V4,
> +};
> +
> +static struct qmp_phy_cfg sc8280xp_dp_phy_cfg = {
> + .type = PHY_TYPE_DP,
> + .version = DP_QMP_VERSION_V4,
> + .needs_swing_pre_emph_cfg = true,
> +};
> +
> +static struct qmp_phy_cfg sc8280xp_edp_phy_cfg = {
> + .type = PHY_TYPE_EDP,
> + .version = DP_QMP_VERSION_V4,
> + .needs_swing_pre_emph_cfg = true,
> +};
> +
> +static struct qmp_phy_cfg x1e80100_phy_cfg = {
> + .version = DP_QMP_VERSION_V6,
> + .needs_swing_pre_emph_cfg = true,
> +};
> +
> 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;
> + const struct qmp_com_init *init = edp->init_values;
> + const struct qmp_com_regs_layout *regs = edp->com_regs;
> int ret;
> u8 cfg8;
>
> @@ -190,7 +538,8 @@ static int qcom_edp_phy_init(struct phy *phy)
> edp->edp + DP_PHY_PD_CTL);
>
> /* Turn on BIAS current for PHY/PLL */
> - writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
> + writel(init->pll_bias_en_clk_buflr_en,
> + edp->pll + regs->pll_bias_en_clk_buflr_en);
>
> writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
> msleep(20);
> @@ -200,7 +549,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->is_dp)
> cfg8 = 0xb7;
> else
> cfg8 = 0x37;
> @@ -234,7 +583,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_cfg *cfg = edp->cfg;
> + const struct qcom_dp_swing_pre_emph_cfg *cfg = edp->swing_pre_emph_cfg;
> unsigned int v_level = 0;
> unsigned int p_level = 0;
> u8 ldo_config;
> @@ -261,7 +610,7 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
> if (swing == 0xff || emph == 0xff)
> return -EINVAL;
>
> - ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
> + ldo_config = edp->is_dp ? 0x1 : 0x0;
>
> writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
> writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
> @@ -291,20 +640,21 @@ static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opt
> static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
> {
> const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> - u32 step1;
> - u32 step2;
> + const struct qmp_com_regs_layout *regs = edp->com_regs;
> + const struct qmp_com_init *init = edp->init_values;
> + int link_rate;
> + u8 step1, step2;
> + u8 per1, per2;
>
> switch (dp_opts->link_rate) {
> case 1620:
> case 2700:
> case 8100:
> - step1 = 0x45;
> - step2 = 0x06;
> + link_rate = DP_LINK_RATE_1_6_GHZ;
> break;
>
> case 5400:
> - step1 = 0x5c;
> - step2 = 0x08;
> + link_rate = DP_LINK_RATE_5_4_GHZ;
> break;
>
> default:
> @@ -312,12 +662,18 @@ static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
> return -EINVAL;
> }
>
> - writel(0x01, edp->pll + QSERDES_V4_COM_SSC_EN_CENTER);
> - writel(0x00, edp->pll + QSERDES_V4_COM_SSC_ADJ_PER1);
> - writel(0x36, edp->pll + QSERDES_V4_COM_SSC_PER1);
> - writel(0x01, edp->pll + QSERDES_V4_COM_SSC_PER2);
> - writel(step1, edp->pll + QSERDES_V4_COM_SSC_STEP_SIZE1_MODE0);
> - writel(step2, edp->pll + QSERDES_V4_COM_SSC_STEP_SIZE2_MODE0);
> + step1 = init->ssc_step1[link_rate];
> + step2 = init->ssc_step2[link_rate];
> +
> + per1 = init->ssc_per1;
> + per2 = init->ssc_per2;
> +
> + writel(0x01, edp->pll + regs->ssc_en_center);
> + writel(0x00, edp->pll + regs->ssc_adj_per1);
> + writel(per1, edp->pll + regs->ssc_per1);
> + writel(per2, edp->pll + regs->ssc_per2);
> + writel(step1, edp->pll + regs->ssc_step_size1_mode0);
> + writel(step2, edp->pll + regs->ssc_step_size2_mode0);
>
> return 0;
> }
> @@ -325,48 +681,30 @@ static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
> static int qcom_edp_configure_pll(const struct qcom_edp *edp)
> {
> const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> + const struct qmp_com_regs_layout *regs = edp->com_regs;
> + const struct qmp_com_init *init = edp->init_values;
> u32 div_frac_start2_mode0;
> u32 div_frac_start3_mode0;
> u32 dec_start_mode0;
> u32 lock_cmp1_mode0;
> u32 lock_cmp2_mode0;
> + u32 code1_mode0;
> + u32 code2_mode0;
> u32 hsclk_sel;
> + int link_rate;
>
> switch (dp_opts->link_rate) {
> case 1620:
> - hsclk_sel = 0x5;
> - dec_start_mode0 = 0x69;
> - div_frac_start2_mode0 = 0x80;
> - div_frac_start3_mode0 = 0x07;
> - lock_cmp1_mode0 = 0x6f;
> - lock_cmp2_mode0 = 0x08;
> + link_rate = DP_LINK_RATE_1_6_GHZ;
> break;
> -
> case 2700:
> - hsclk_sel = 0x3;
> - dec_start_mode0 = 0x69;
> - div_frac_start2_mode0 = 0x80;
> - div_frac_start3_mode0 = 0x07;
> - lock_cmp1_mode0 = 0x0f;
> - lock_cmp2_mode0 = 0x0e;
> + link_rate = DP_LINK_RATE_2_7_GHZ;
> break;
> -
> case 5400:
> - hsclk_sel = 0x1;
> - dec_start_mode0 = 0x8c;
> - div_frac_start2_mode0 = 0x00;
> - div_frac_start3_mode0 = 0x0a;
> - lock_cmp1_mode0 = 0x1f;
> - lock_cmp2_mode0 = 0x1c;
> + link_rate = DP_LINK_RATE_5_4_GHZ;
> break;
> -
> case 8100:
> - hsclk_sel = 0x0;
> - dec_start_mode0 = 0x69;
> - div_frac_start2_mode0 = 0x80;
> - div_frac_start3_mode0 = 0x07;
> - lock_cmp1_mode0 = 0x2f;
> - lock_cmp2_mode0 = 0x2a;
> + link_rate = DP_LINK_RATE_8_1_GHZ;
> break;
>
> default:
> @@ -374,36 +712,59 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
> return -EINVAL;
> }
>
> - writel(0x01, edp->pll + QSERDES_V4_COM_SVS_MODE_CLK_SEL);
> - writel(0x0b, edp->pll + QSERDES_V4_COM_SYSCLK_EN_SEL);
> - writel(0x02, edp->pll + QSERDES_V4_COM_SYS_CLK_CTRL);
> - writel(0x0c, edp->pll + QSERDES_V4_COM_CLK_ENABLE1);
> - writel(0x06, edp->pll + QSERDES_V4_COM_SYSCLK_BUF_ENABLE);
> - writel(0x30, edp->pll + QSERDES_V4_COM_CLK_SELECT);
> - writel(hsclk_sel, edp->pll + QSERDES_V4_COM_HSCLK_SEL);
> - writel(0x0f, edp->pll + QSERDES_V4_COM_PLL_IVCO);
> - writel(0x08, edp->pll + QSERDES_V4_COM_LOCK_CMP_EN);
> - writel(0x36, edp->pll + QSERDES_V4_COM_PLL_CCTRL_MODE0);
> - writel(0x16, edp->pll + QSERDES_V4_COM_PLL_RCTRL_MODE0);
> - writel(0x06, edp->pll + QSERDES_V4_COM_CP_CTRL_MODE0);
> - writel(dec_start_mode0, edp->pll + QSERDES_V4_COM_DEC_START_MODE0);
> - writel(0x00, edp->pll + QSERDES_V4_COM_DIV_FRAC_START1_MODE0);
> - writel(div_frac_start2_mode0, edp->pll + QSERDES_V4_COM_DIV_FRAC_START2_MODE0);
> - writel(div_frac_start3_mode0, edp->pll + QSERDES_V4_COM_DIV_FRAC_START3_MODE0);
> - writel(0x02, edp->pll + QSERDES_V4_COM_CMN_CONFIG);
> - writel(0x3f, edp->pll + QSERDES_V4_COM_INTEGLOOP_GAIN0_MODE0);
> - writel(0x00, edp->pll + QSERDES_V4_COM_INTEGLOOP_GAIN1_MODE0);
> - writel(0x00, edp->pll + QSERDES_V4_COM_VCO_TUNE_MAP);
> - writel(lock_cmp1_mode0, edp->pll + QSERDES_V4_COM_LOCK_CMP1_MODE0);
> - writel(lock_cmp2_mode0, edp->pll + QSERDES_V4_COM_LOCK_CMP2_MODE0);
> -
> - writel(0x0a, edp->pll + QSERDES_V4_COM_BG_TIMER);
> - writel(0x14, edp->pll + QSERDES_V4_COM_CORECLK_DIV_MODE0);
> - writel(0x00, edp->pll + QSERDES_V4_COM_VCO_TUNE_CTRL);
> - writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
> - writel(0x0f, edp->pll + QSERDES_V4_COM_CORE_CLK_EN);
> - writel(0xa0, edp->pll + QSERDES_V4_COM_VCO_TUNE1_MODE0);
> - writel(0x03, edp->pll + QSERDES_V4_COM_VCO_TUNE2_MODE0);
> + hsclk_sel = init->hsclk_sel[link_rate];
> + dec_start_mode0 = init->dec_start_mode0[link_rate];
> + div_frac_start2_mode0 = init->div_frac_start2_mode0[link_rate];
> + div_frac_start3_mode0 = init->div_frac_start3_mode0[link_rate];
> + lock_cmp1_mode0 = init->lock_cmp1_mode0[link_rate];
> + lock_cmp2_mode0 = init->lock_cmp2_mode0[link_rate];
> +
> + if (init->code1_mode0)
> + code1_mode0 = init->code1_mode0[link_rate];
> +
> + if (init->code2_mode0)
> + code2_mode0 = init->code2_mode0[link_rate];
> +
> + writel(0x01, edp->pll + regs->svs_mode_clk_sel);
> + writel(0x0b, edp->pll + regs->sysclk_en_sel);
> + writel(0x02, edp->pll + regs->sys_clk_ctrl);
> + writel(0x0c, edp->pll + regs->clk_enable1);
> + writel(0x06, edp->pll + regs->sysclk_buf_enable);
> + writel(0x30, edp->pll + regs->clk_select);
> + writel(hsclk_sel, edp->pll + regs->hsclk_sel);
> + writel(init->pll_ivco, edp->pll + regs->pll_ivco);
> + writel(0x08, edp->pll + regs->lock_cmp_en);
> + writel(0x36, edp->pll + regs->pll_cctrl_mode0);
> + writel(0x16, edp->pll + regs->pll_rctrl_mode0);
> + writel(0x06, edp->pll + regs->cp_ctrl_mode0);
> + writel(dec_start_mode0, edp->pll + regs->dec_start_mode0);
> + writel(0x00, edp->pll + regs->div_frac_start1_mode0);
> + writel(div_frac_start2_mode0, edp->pll + regs->div_frac_start2_mode0);
> + writel(div_frac_start3_mode0, edp->pll + regs->div_frac_start3_mode0);
> + writel(init->cmn_config, edp->pll + regs->cmn_config);
> + writel(0x3f, edp->pll + regs->integloop_gain0_mode0);
> + writel(0x00, edp->pll + regs->integloop_gain1_mode0);
> + writel(0x00, edp->pll + regs->vco_tune_map);
> + writel(lock_cmp1_mode0, edp->pll + regs->lock_cmp1_mode0);
> + writel(lock_cmp2_mode0, edp->pll + regs->lock_cmp2_mode0);
> +
> + writel(0x0a, edp->pll + regs->bg_timer);
> + writel(0x14, edp->pll + regs->pll_core_clk_div_mode0);
> + writel(0x00, edp->pll + regs->vco_tune_ctrl);
> + writel(0x17, edp->pll + regs->pll_bias_en_clk_buflr_en);
> + writel(0x0f, edp->pll + regs->core_clk_en);
> +
> + if (regs->vco_tune1_mode0)
> + writel(init->vco_tune1_mode0, edp->pll + regs->vco_tune1_mode0);
> +
> + if (regs->vco_tune2_mode0)
> + writel(init->vco_tune2_mode0, edp->pll + regs->vco_tune2_mode0);
> +
> + if (regs->bin_vcocal_cmp_code1_mode0)
> + writel(code1_mode0, edp->pll + regs->bin_vcocal_cmp_code1_mode0);
> +
> + if (regs->bin_vcocal_cmp_code2_mode0)
> + writel(code2_mode0, edp->pll + regs->bin_vcocal_cmp_code2_mode0);
>
> return 0;
> }
> @@ -447,10 +808,10 @@ static int qcom_edp_set_vco_div(const struct qcom_edp *edp, unsigned long *pixel
> static int qcom_edp_phy_power_on(struct phy *phy)
> {
> const struct qcom_edp *edp = phy_get_drvdata(phy);
> - const struct qcom_edp_cfg *cfg = edp->cfg;
> + const struct qmp_com_regs_layout *regs = edp->com_regs;
> u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> unsigned long pixel_freq;
> - u8 ldo_config;
> + u8 ldo_config = 0x0;
> int timeout;
> int ret;
> u32 val;
> @@ -462,13 +823,14 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> edp->edp + DP_PHY_PD_CTL);
> writel(0xfc, edp->edp + DP_PHY_MODE);
>
> - timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
> + timeout = readl_poll_timeout(edp->pll + regs->cmn_status,
> val, val & BIT(7), 5, 200);
> if (timeout)
> return timeout;
>
>
> - ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
> + if (edp->swing_pre_emph_cfg && edp->is_dp)
> + ldo_config = 0x1;
>
> writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
> writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
> @@ -512,9 +874,9 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> writel(0x01, edp->edp + DP_PHY_CFG);
> writel(0x09, edp->edp + DP_PHY_CFG);
>
> - writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
> + writel(0x20, edp->pll + regs->resetsm_cntrl);
>
> - timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
> + timeout = readl_poll_timeout(edp->pll + regs->c_ready_status,
> val, val & BIT(0), 500, 10000);
> if (timeout)
> return timeout;
> @@ -768,6 +1130,37 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
> return devm_of_clk_add_hw_provider(edp->dev, of_clk_hw_onecell_get, data);
> }
>
> +static int qcom_edp_setup_phy(struct platform_device *pdev, struct qcom_edp *edp)
> +{
> + struct device *dev = &pdev->dev;
> + const struct qmp_phy_cfg *cfg = of_device_get_match_data(dev);
> + enum dp_qmp_com_version version = cfg->version;
> + struct device_node *node = dev->of_node;
> + int type = cfg->type;
> +
> + of_property_read_u32(node, "phy-type", &type);
> +
> + if (type != PHY_TYPE_DP && type != PHY_TYPE_EDP)
> + return -EINVAL;
> +
> + edp->is_dp = (type == PHY_TYPE_DP) ? true : false;
> +
> + if (cfg->needs_swing_pre_emph_cfg)
> + edp->swing_pre_emph_cfg = edp->is_dp ?
> + &dp_phy_swing_pre_emph_cfg:
> + &edp_phy_swing_pre_emph_cfg;
> +
> + if (version == DP_QMP_VERSION_V6) {
> + edp->com_regs = &qmp_v6_com_regs;
> + edp->init_values = &qmp_v6_com_init;
> + } else {
> + edp->com_regs = &qmp_v4_com_regs;
> + edp->init_values = &qmp_v4_com_init;
> + }
> +
> + return 0;
> +}
> +
> static int qcom_edp_phy_probe(struct platform_device *pdev)
> {
> struct phy_provider *phy_provider;
> @@ -780,7 +1173,10 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> edp->dev = dev;
> - edp->cfg = of_device_get_match_data(&pdev->dev);
> +
> + ret = qcom_edp_setup_phy(pdev, edp);
> + if (ret)
> + return ret;
>
> edp->edp = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(edp->edp))
> @@ -839,10 +1235,11 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id qcom_edp_phy_match_table[] = {
> - { .compatible = "qcom,sc7280-edp-phy" },
> - { .compatible = "qcom,sc8180x-edp-phy" },
> - { .compatible = "qcom,sc8280xp-dp-phy", .data = &dp_phy_cfg },
> - { .compatible = "qcom,sc8280xp-edp-phy", .data = &edp_phy_cfg },
> + { .compatible = "qcom,sc7280-edp-phy" , .data = &sc7280_dp_phy_cfg, },
> + { .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
> + { .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
> + { .compatible = "qcom,sc8280xp-edp-phy", .data = &sc8280xp_edp_phy_cfg, },
> + { .compatible = "qcom,x1e80100-dp-phy" , .data = &x1e80100_phy_cfg, },
> { }
> };
> MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 07/12/2023 11:52, Abel Vesa wrote:
> Add definition for Embedded DisplayPort (eDP) phy type.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 07/12/2023 11:52, Abel Vesa wrote:
> The Qualcomm X1E80100 platform has multiple PHYs that can work in both
> eDP or DP mode, add compatibles for these. New platforms can use the
> phy-type property to specify which mode the PHY should be configured in.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> index 6566353f1a02..3341283577ce 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> @@ -21,6 +21,7 @@ properties:
> - qcom,sc8180x-edp-phy
> - qcom,sc8280xp-dp-phy
> - qcom,sc8280xp-edp-phy
> + - qcom,x1e80100-dp-phy
>
> reg:
> items:
> @@ -59,6 +60,20 @@ required:
>
> additionalProperties: false
Please put it after allOf: block (IOW, allOf: before additonalProperties:)
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,x1e80100-dp-phy
> + then:
> + properties:
> + phy-type:
> + description: DP (default) or eDP type
Properties must be defined in top-level "properties:" block. In
allOf:if:then you only disallow them for other variants.
> + enum: [ 6, 13 ]
> + default: 6
Anyway, I was thinking this should be rather argument to phy-cells.
Best regards,
Krzysztof
On 12/7/23 17:51, Krzysztof Kozlowski wrote:
[...]
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,x1e80100-dp-phy
>> + then:
>> + properties:
>> + phy-type:
>> + description: DP (default) or eDP type
>
> Properties must be defined in top-level "properties:" block. In
> allOf:if:then you only disallow them for other variants.
>
>> + enum: [ 6, 13 ]
>> + default: 6
>
> Anyway, I was thinking this should be rather argument to phy-cells.
I'm not sure I'm for this, because the results would be:
--- device.dts ---
&dp_controller0 {
phys = <&dp_phy0 PHY_EDP>;
};
&dp_controller1 {
phys = <&dp_phy1 PHY_DP>;
};
------------------
as opposed to:
--- device.dts ---
&dp_phy0 {
phy-type <PHY_EDP>;
};
&dp_phy1 {
phy-type = <PHY_DP>;
};
------------------
i.e., we would be saying "this board is connected to this phy
instead" vs "this phy is of this type on this board".
While none of them really fit the "same hw, different config"
situation, I'd vote for the latter one being closer to the
truth
Konrad
On 07/12/2023 20:16, Konrad Dybcio wrote:
>
>
> On 12/7/23 17:51, Krzysztof Kozlowski wrote:
>
> [...]
>
>>> +allOf:
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - qcom,x1e80100-dp-phy
>>> + then:
>>> + properties:
>>> + phy-type:
>>> + description: DP (default) or eDP type
>>
>> Properties must be defined in top-level "properties:" block. In
>> allOf:if:then you only disallow them for other variants.
>>
>>> + enum: [ 6, 13 ]
>>> + default: 6
>>
>> Anyway, I was thinking this should be rather argument to phy-cells.
> I'm not sure I'm for this, because the results would be:
>
> --- device.dts ---
> &dp_controller0 {
> phys = <&dp_phy0 PHY_EDP>;
> };
>
> &dp_controller1 {
> phys = <&dp_phy1 PHY_DP>;
> };
> ------------------
>
> as opposed to:
>
> --- device.dts ---
> &dp_phy0 {
> phy-type <PHY_EDP>;
> };
>
> &dp_phy1 {
> phy-type = <PHY_DP>;
> };
> ------------------
Which is exactly what I proposed/wanted to see.
>
> i.e., we would be saying "this board is connected to this phy
> instead" vs "this phy is of this type on this board".
>
> While none of them really fit the "same hw, different config"
> situation, I'd vote for the latter one being closer to the
> truth
Then maybe I miss the bigger picture, but commit msg clearly says:
"multiple PHYs that can work in both eDP or DP mode"
If this is not the case, describe the hardware correctly in the commit
msg, so people will not ask stupid questions...
Best regards,
Krzysztof
On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 07/12/2023 20:16, Konrad Dybcio wrote:
> >
> >
> > On 12/7/23 17:51, Krzysztof Kozlowski wrote:
> >
> > [...]
> >
> >>> +allOf:
> >>> + - if:
> >>> + properties:
> >>> + compatible:
> >>> + contains:
> >>> + enum:
> >>> + - qcom,x1e80100-dp-phy
> >>> + then:
> >>> + properties:
> >>> + phy-type:
> >>> + description: DP (default) or eDP type
> >>
> >> Properties must be defined in top-level "properties:" block. In
> >> allOf:if:then you only disallow them for other variants.
> >>
> >>> + enum: [ 6, 13 ]
> >>> + default: 6
> >>
> >> Anyway, I was thinking this should be rather argument to phy-cells.
> > I'm not sure I'm for this, because the results would be:
> >
> > --- device.dts ---
> > &dp_controller0 {
> > phys = <&dp_phy0 PHY_EDP>;
> > };
> >
> > &dp_controller1 {
> > phys = <&dp_phy1 PHY_DP>;
> > };
> > ------------------
> >
> > as opposed to:
> >
> > --- device.dts ---
> > &dp_phy0 {
> > phy-type <PHY_EDP>;
> > };
> >
> > &dp_phy1 {
> > phy-type = <PHY_DP>;
> > };
> > ------------------
>
> Which is exactly what I proposed/wanted to see.
>
> >
> > i.e., we would be saying "this board is connected to this phy
> > instead" vs "this phy is of this type on this board".
> >
> > While none of them really fit the "same hw, different config"
> > situation, I'd vote for the latter one being closer to the
> > truth
>
> Then maybe I miss the bigger picture, but commit msg clearly says:
> "multiple PHYs that can work in both eDP or DP mode"
>
> If this is not the case, describe the hardware correctly in the commit
> msg, so people will not ask stupid questions...
There are multiple PHYs (each of them at its own address space). Each
of the PHYs in question can be used either for the DisplayPort output
(directly or through the USB-C) or to drive the eDP panel.
Same applies to the displayport-controller. It can either drive the DP
or eDP output, hardware-wise it is the same.
--
With best wishes
Dmitry
On 08/12/2023 12:04, Dmitry Baryshkov wrote:
> On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 07/12/2023 20:16, Konrad Dybcio wrote:
>>>
>>>
>>> On 12/7/23 17:51, Krzysztof Kozlowski wrote:
>>>
>>> [...]
>>>
>>>>> +allOf:
>>>>> + - if:
>>>>> + properties:
>>>>> + compatible:
>>>>> + contains:
>>>>> + enum:
>>>>> + - qcom,x1e80100-dp-phy
>>>>> + then:
>>>>> + properties:
>>>>> + phy-type:
>>>>> + description: DP (default) or eDP type
>>>>
>>>> Properties must be defined in top-level "properties:" block. In
>>>> allOf:if:then you only disallow them for other variants.
>>>>
>>>>> + enum: [ 6, 13 ]
>>>>> + default: 6
>>>>
>>>> Anyway, I was thinking this should be rather argument to phy-cells.
>>> I'm not sure I'm for this, because the results would be:
>>>
>>> --- device.dts ---
>>> &dp_controller0 {
>>> phys = <&dp_phy0 PHY_EDP>;
>>> };
>>>
>>> &dp_controller1 {
>>> phys = <&dp_phy1 PHY_DP>;
>>> };
>>> ------------------
>>>
>>> as opposed to:
>>>
>>> --- device.dts ---
>>> &dp_phy0 {
>>> phy-type <PHY_EDP>;
>>> };
>>>
>>> &dp_phy1 {
>>> phy-type = <PHY_DP>;
>>> };
>>> ------------------
>>
>> Which is exactly what I proposed/wanted to see.
>>
>>>
>>> i.e., we would be saying "this board is connected to this phy
>>> instead" vs "this phy is of this type on this board".
>>>
>>> While none of them really fit the "same hw, different config"
>>> situation, I'd vote for the latter one being closer to the
>>> truth
>>
>> Then maybe I miss the bigger picture, but commit msg clearly says:
>> "multiple PHYs that can work in both eDP or DP mode"
>>
>> If this is not the case, describe the hardware correctly in the commit
>> msg, so people will not ask stupid questions...
>
> There are multiple PHYs (each of them at its own address space). Each
> of the PHYs in question can be used either for the DisplayPort output
> (directly or through the USB-C) or to drive the eDP panel.
>
> Same applies to the displayport-controller. It can either drive the DP
> or eDP output, hardware-wise it is the same.
Therefore what I proposed was correct - the block which uses the phy
configures its mode. Because this part:
"this phy is of this type on this board".
is not true. The phy is both types.
Best regards,
Krzysztof
On Fri, 8 Dec 2023 at 13:45, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/12/2023 12:04, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 07/12/2023 20:16, Konrad Dybcio wrote:
> >>>
> >>>
> >>> On 12/7/23 17:51, Krzysztof Kozlowski wrote:
> >>>
> >>> [...]
> >>>
> >>>>> +allOf:
> >>>>> + - if:
> >>>>> + properties:
> >>>>> + compatible:
> >>>>> + contains:
> >>>>> + enum:
> >>>>> + - qcom,x1e80100-dp-phy
> >>>>> + then:
> >>>>> + properties:
> >>>>> + phy-type:
> >>>>> + description: DP (default) or eDP type
> >>>>
> >>>> Properties must be defined in top-level "properties:" block. In
> >>>> allOf:if:then you only disallow them for other variants.
> >>>>
> >>>>> + enum: [ 6, 13 ]
> >>>>> + default: 6
> >>>>
> >>>> Anyway, I was thinking this should be rather argument to phy-cells.
> >>> I'm not sure I'm for this, because the results would be:
> >>>
> >>> --- device.dts ---
> >>> &dp_controller0 {
> >>> phys = <&dp_phy0 PHY_EDP>;
> >>> };
> >>>
> >>> &dp_controller1 {
> >>> phys = <&dp_phy1 PHY_DP>;
> >>> };
> >>> ------------------
> >>>
> >>> as opposed to:
> >>>
> >>> --- device.dts ---
> >>> &dp_phy0 {
> >>> phy-type <PHY_EDP>;
> >>> };
> >>>
> >>> &dp_phy1 {
> >>> phy-type = <PHY_DP>;
> >>> };
> >>> ------------------
> >>
> >> Which is exactly what I proposed/wanted to see.
> >>
> >>>
> >>> i.e., we would be saying "this board is connected to this phy
> >>> instead" vs "this phy is of this type on this board".
> >>>
> >>> While none of them really fit the "same hw, different config"
> >>> situation, I'd vote for the latter one being closer to the
> >>> truth
> >>
> >> Then maybe I miss the bigger picture, but commit msg clearly says:
> >> "multiple PHYs that can work in both eDP or DP mode"
> >>
> >> If this is not the case, describe the hardware correctly in the commit
> >> msg, so people will not ask stupid questions...
> >
> > There are multiple PHYs (each of them at its own address space). Each
> > of the PHYs in question can be used either for the DisplayPort output
> > (directly or through the USB-C) or to drive the eDP panel.
> >
> > Same applies to the displayport-controller. It can either drive the DP
> > or eDP output, hardware-wise it is the same.
>
> Therefore what I proposed was correct - the block which uses the phy
> configures its mode. Because this part:
> "this phy is of this type on this board".
> is not true. The phy is both types.
But hopefully you don't mean using #phy-cells here. There are no
sub-PHYs or anything like that.
--
With best wishes
Dmitry
On 08/12/2023 13:17, Dmitry Baryshkov wrote:
>>>>>> Anyway, I was thinking this should be rather argument to phy-cells.
>>>>> I'm not sure I'm for this, because the results would be:
>>>>>
>>>>> --- device.dts ---
>>>>> &dp_controller0 {
>>>>> phys = <&dp_phy0 PHY_EDP>;
>>>>> };
>>>>>
>>>>> &dp_controller1 {
>>>>> phys = <&dp_phy1 PHY_DP>;
>>>>> };
>>>>> ------------------
>>>>>
>>>>> as opposed to:
>>>>>
>>>>> --- device.dts ---
>>>>> &dp_phy0 {
>>>>> phy-type <PHY_EDP>;
>>>>> };
>>>>>
>>>>> &dp_phy1 {
>>>>> phy-type = <PHY_DP>;
>>>>> };
>>>>> ------------------
>>>>
>>>> Which is exactly what I proposed/wanted to see.
>>>>
>>>>>
>>>>> i.e., we would be saying "this board is connected to this phy
>>>>> instead" vs "this phy is of this type on this board".
>>>>>
>>>>> While none of them really fit the "same hw, different config"
>>>>> situation, I'd vote for the latter one being closer to the
>>>>> truth
>>>>
>>>> Then maybe I miss the bigger picture, but commit msg clearly says:
>>>> "multiple PHYs that can work in both eDP or DP mode"
>>>>
>>>> If this is not the case, describe the hardware correctly in the commit
>>>> msg, so people will not ask stupid questions...
>>>
>>> There are multiple PHYs (each of them at its own address space). Each
>>> of the PHYs in question can be used either for the DisplayPort output
>>> (directly or through the USB-C) or to drive the eDP panel.
>>>
>>> Same applies to the displayport-controller. It can either drive the DP
>>> or eDP output, hardware-wise it is the same.
>>
>> Therefore what I proposed was correct - the block which uses the phy
>> configures its mode. Because this part:
>> "this phy is of this type on this board".
>> is not true. The phy is both types.
>
> But hopefully you don't mean using #phy-cells here. There are no
> sub-PHYs or anything like that.
I am exactly talking about phy-cells. Look at first example from Abel's
code.
Best regards,
Krzysztof
On Fri, 8 Dec 2023 at 14:21, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/12/2023 13:17, Dmitry Baryshkov wrote:
> >>>>>> Anyway, I was thinking this should be rather argument to phy-cells.
> >>>>> I'm not sure I'm for this, because the results would be:
> >>>>>
> >>>>> --- device.dts ---
> >>>>> &dp_controller0 {
> >>>>> phys = <&dp_phy0 PHY_EDP>;
> >>>>> };
> >>>>>
> >>>>> &dp_controller1 {
> >>>>> phys = <&dp_phy1 PHY_DP>;
> >>>>> };
> >>>>> ------------------
> >>>>>
> >>>>> as opposed to:
> >>>>>
> >>>>> --- device.dts ---
> >>>>> &dp_phy0 {
> >>>>> phy-type <PHY_EDP>;
> >>>>> };
> >>>>>
> >>>>> &dp_phy1 {
> >>>>> phy-type = <PHY_DP>;
> >>>>> };
> >>>>> ------------------
> >>>>
> >>>> Which is exactly what I proposed/wanted to see.
> >>>>
> >>>>>
> >>>>> i.e., we would be saying "this board is connected to this phy
> >>>>> instead" vs "this phy is of this type on this board".
> >>>>>
> >>>>> While none of them really fit the "same hw, different config"
> >>>>> situation, I'd vote for the latter one being closer to the
> >>>>> truth
> >>>>
> >>>> Then maybe I miss the bigger picture, but commit msg clearly says:
> >>>> "multiple PHYs that can work in both eDP or DP mode"
> >>>>
> >>>> If this is not the case, describe the hardware correctly in the commit
> >>>> msg, so people will not ask stupid questions...
> >>>
> >>> There are multiple PHYs (each of them at its own address space). Each
> >>> of the PHYs in question can be used either for the DisplayPort output
> >>> (directly or through the USB-C) or to drive the eDP panel.
> >>>
> >>> Same applies to the displayport-controller. It can either drive the DP
> >>> or eDP output, hardware-wise it is the same.
> >>
> >> Therefore what I proposed was correct - the block which uses the phy
> >> configures its mode. Because this part:
> >> "this phy is of this type on this board".
> >> is not true. The phy is both types.
> >
> > But hopefully you don't mean using #phy-cells here. There are no
> > sub-PHYs or anything like that.
>
> I am exactly talking about phy-cells. Look at first example from Abel's
> code.
I always had an impression that #foo-cells means that there are
different units within the major handler. I.e. #clock-cells mean that
there are several different clocks, #reset-cells mean that there are
several resets, etc.
Ok, maybe this is not a perfect description. We need cells to identify
a particular instance within the major block. Maybe that sounds more
correct.
For the USB+DP PHY we use #phy-cells to select between USB3 and DP
PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc.
There is a single PHY which works in either of the modes.
Last, but not least, using #phy-cells in this way would create
asymmetry with all the other PHYs (and especially other QMP PHYs)
present on these platforms.
If you feel that phy-type is not an appropriate solution, I'd vote for
not having the type in DT at all, letting the DP controller determine
the proper mode on its own.
--
With best wishes
Dmitry
On 08/12/2023 13:35, Dmitry Baryshkov wrote:
>>>>> Same applies to the displayport-controller. It can either drive the DP
>>>>> or eDP output, hardware-wise it is the same.
>>>>
>>>> Therefore what I proposed was correct - the block which uses the phy
>>>> configures its mode. Because this part:
>>>> "this phy is of this type on this board".
>>>> is not true. The phy is both types.
>>>
>>> But hopefully you don't mean using #phy-cells here. There are no
>>> sub-PHYs or anything like that.
>>
>> I am exactly talking about phy-cells. Look at first example from Abel's
>> code.
>
> I always had an impression that #foo-cells means that there are
> different units within the major handler. I.e. #clock-cells mean that
> there are several different clocks, #reset-cells mean that there are
> several resets, etc.
> Ok, maybe this is not a perfect description. We need cells to identify
> a particular instance within the major block. Maybe that sounds more
> correct.
No, the cells have also meaning of additional arguments. See usage of
phy-type (not the one here, but the correct one) and PWMs.
>
> For the USB+DP PHY we use #phy-cells to select between USB3 and DP
> PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc.
> There is a single PHY which works in either of the modes.
Is it different than case here?
>
> Last, but not least, using #phy-cells in this way would create
> asymmetry with all the other PHYs (and especially other QMP PHYs)
> present on these platforms.
OK. Is phy-type not something different?
>
> If you feel that phy-type is not an appropriate solution, I'd vote for
> not having the type in DT at all, letting the DP controller determine
> the proper mode on its own.
Can we do it? That's BTW the best option.
Best regards,
Krzysztof
On Fri, 8 Dec 2023 at 14:47, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/12/2023 13:35, Dmitry Baryshkov wrote:
> >>>>> Same applies to the displayport-controller. It can either drive the DP
> >>>>> or eDP output, hardware-wise it is the same.
> >>>>
> >>>> Therefore what I proposed was correct - the block which uses the phy
> >>>> configures its mode. Because this part:
> >>>> "this phy is of this type on this board".
> >>>> is not true. The phy is both types.
> >>>
> >>> But hopefully you don't mean using #phy-cells here. There are no
> >>> sub-PHYs or anything like that.
> >>
> >> I am exactly talking about phy-cells. Look at first example from Abel's
> >> code.
> >
> > I always had an impression that #foo-cells means that there are
> > different units within the major handler. I.e. #clock-cells mean that
> > there are several different clocks, #reset-cells mean that there are
> > several resets, etc.
> > Ok, maybe this is not a perfect description. We need cells to identify
> > a particular instance within the major block. Maybe that sounds more
> > correct.
>
> No, the cells have also meaning of additional arguments. See usage of
> phy-type (not the one here, but the correct one) and PWMs.
phy-type being used for the 7nm DSI PHY, where it specify exactly the
same thing: whether the connected device uses D-PHY or C-PHY modes of
the PHY.
cdns,phy-type - selecs between PCIe, DP, USB3 or other modes of the PHY
ti/emif.txt: phy-type specifies which PHY is attached / used in the controller
xlnx,phy-type: deprecated in favour of phy-mode, selects MII mode for the PHY
marvell,xenon-phy-type: I _think_ this specifies the actual PHY
attached to the controller in hardware.
> > For the USB+DP PHY we use #phy-cells to select between USB3 and DP
> > PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc.
> > There is a single PHY which works in either of the modes.
>
> Is it different than case here?
Hmm, I was not clear enough.
USB+DP = two different PHYs in the same hardware block.
DP-eDP = single PHY, working in one of the modes.
>
> >
> > Last, but not least, using #phy-cells in this way would create
> > asymmetry with all the other PHYs (and especially other QMP PHYs)
> > present on these platforms.
>
> OK. Is phy-type not something different?
No. It doesn't redefine what we already have for other QMP PHYs, it
defines new property.
>
> >
> > If you feel that phy-type is not an appropriate solution, I'd vote for
> > not having the type in DT at all, letting the DP controller determine
> > the proper mode on its own.
>
> Can we do it? That's BTW the best option.
That's a good question. We have separate -dp and -edp compatibles for
the DP controller, but I think those also should go, at least for
newer platforms. And the reason is the same, there is a single
hardware block, just two modes of operation. See mdss_dp3 in the
X13s's DT file.
I had a thought of using aux-bus presence to determine whether the
controller is working in the DP or eDP modes. But this might need
additional care for older DT files.
--
With best wishes
Dmitry