[PATCH v1 00/10] Add eDP support for RK3588

[PATCH v1 00/10] Add eDP support for RK3588

[Damon Ding]

These patchs have been tested with a 1536x2048p60 eDP panel on
RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
on RK3588 EVB1 board.

Patch 1~3 are the RK3588 eDP support of Rockchip analogix_dp driver.
Patch 4   is the eDP mode support of samsung hdptx phy driver.
Patch 5~6 are the Rk3588 eDP support of Aanalogix DP driver. Add phy
          interfaces is to configure the HDMI/eDP TX Combo PHY.
Patch 7~8 are the renaming of hdptxphy node. It is not only used by
          HDMI display but also for the eDP display.
Patch 9   is the addition of RK3588 eDP0 node.
Patch 10  is to enable the eDP0 display on RK3588S EVB1 board.

Damon Ding (10):
  drm/rockchip: analogix_dp: Use formalized struct definition for grf
    field
  dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
  drm/rockchip: analogix_dp: Add support for RK3588
  phy: phy-rockchip-samsung-hdptx: Add support for eDP mode
  drm/bridge: analogix_dp: add support for RK3588
  drm/bridge: analogix_dp: Add support for phy configuration.
  dt-bindings: display: rockchip: Fix label name of hdptxphy for RK3588
    HDMI TX Controller
  arm64: dts: rockchip: Fix label name of hdptxphy for RK3588
  arm64: dts: rockchip: Add eDP0 node for RK3588
  arch64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

 .../rockchip/rockchip,analogix-dp.yaml        |   1 +
 .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml  |   2 +-
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  33 +-
 .../dts/rockchip/rk3588-coolpi-cm5-evb.dts    |   2 +-
 .../rockchip/rk3588-coolpi-cm5-genbook.dts    |   2 +-
 .../boot/dts/rockchip/rk3588-evb1-v10.dts     |   2 +-
 .../rk3588-friendlyelec-cm3588-nas.dts        |   2 +-
 .../arm64/boot/dts/rockchip/rk3588-jaguar.dts |   2 +-
 .../boot/dts/rockchip/rk3588-nanopc-t6.dtsi   |   2 +-
 .../dts/rockchip/rk3588-orangepi-5-plus.dts   |   2 +-
 .../boot/dts/rockchip/rk3588-rock-5b.dts      |   2 +-
 .../boot/dts/rockchip/rk3588-tiger-haikou.dts |   2 +-
 .../boot/dts/rockchip/rk3588s-coolpi-4b.dts   |   2 +-
 .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  84 ++
 .../dts/rockchip/rk3588s-indiedroid-nova.dts  |   2 +-
 .../boot/dts/rockchip/rk3588s-nanopi-r6.dtsi  |   2 +-
 .../boot/dts/rockchip/rk3588s-odroid-m2.dts   |   2 +-
 .../boot/dts/rockchip/rk3588s-orangepi-5.dtsi |   2 +-
 .../boot/dts/rockchip/rk3588s-rock-5a.dts     |   2 +-
 .../boot/dts/rockchip/rk3588s-rock-5c.dts     |   2 +-
 .../drm/bridge/analogix/analogix_dp_core.c    |   8 +-
 .../drm/bridge/analogix/analogix_dp_core.h    |   2 +
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  90 ++
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 112 ++-
 .../phy/rockchip/phy-rockchip-samsung-hdptx.c | 936 +++++++++++++++++-
 include/drm/bridge/analogix_dp.h              |   3 +-
 26 files changed, 1206 insertions(+), 97 deletions(-)

-- 
2.34.1

[PATCH v1 01/10] drm/rockchip: analogix_dp: Use formalized struct definition for grf field

[Damon Ding]

The formalized struct definition will makes grf field operations more
concise and easier to extend.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 77 +++++++++++--------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d3341edfe4f4..edb08a816c3f 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -32,26 +32,29 @@
 
 #include "rockchip_drm_drv.h"
 
-#define RK3288_GRF_SOC_CON6		0x25c
-#define RK3288_EDP_LCDC_SEL		BIT(5)
-#define RK3399_GRF_SOC_CON20		0x6250
-#define RK3399_EDP_LCDC_SEL		BIT(5)
-
-#define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
-
 #define PSR_WAIT_LINE_FLAG_TIMEOUT_MS	100
 
+#define GRF_REG_FIELD(_reg, _lsb, _msb) {	\
+				.reg = _reg,	\
+				.lsb = _lsb,	\
+				.msb = _msb,	\
+				.valid = true,	\
+				}
+
+struct rockchip_grf_reg_field {
+	u32 reg;
+	u32 lsb;
+	u32 msb;
+	bool valid;
+};
+
 /**
  * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
- * @lcdsel_grf_reg: grf register offset of lcdc select
- * @lcdsel_big: reg value of selecting vop big for eDP
- * @lcdsel_lit: reg value of selecting vop little for eDP
+ * @lcdc_sel: grf register field of lcdc_sel
  * @chip_type: specific chip type
  */
 struct rockchip_dp_chip_data {
-	u32	lcdsel_grf_reg;
-	u32	lcdsel_big;
-	u32	lcdsel_lit;
+	const struct rockchip_grf_reg_field lcdc_sel;
 	u32	chip_type;
 };
 
@@ -84,6 +87,26 @@ static struct rockchip_dp_device *pdata_encoder_to_dp(struct analogix_dp_plat_da
 	return container_of(plat_data, struct rockchip_dp_device, plat_data);
 }
 
+static int rockchip_grf_write(struct regmap *grf, u32 reg, u32 mask, u32 val)
+{
+	return regmap_write(grf, reg, (mask << 16) | (val & mask));
+}
+
+static int rockchip_grf_field_write(struct regmap *grf,
+				    const struct rockchip_grf_reg_field *field,
+				    u32 val)
+{
+	u32 mask;
+
+	if (!field->valid)
+		return 0;
+
+	mask = GENMASK(field->msb, field->lsb);
+	val <<= field->lsb;
+
+	return rockchip_grf_write(grf, field->reg, mask, val);
+}
+
 static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
 {
 	reset_control_assert(dp->rst);
@@ -181,7 +204,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
 	int ret;
-	u32 val;
 
 	crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
 	if (!crtc)
@@ -192,24 +214,19 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	if (old_crtc_state && old_crtc_state->self_refresh_active)
 		return;
 
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
-	if (ret < 0)
-		return;
-
-	if (ret)
-		val = dp->data->lcdsel_lit;
-	else
-		val = dp->data->lcdsel_big;
-
-	DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
-
 	ret = clk_prepare_enable(dp->grfclk);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dp->dev, "failed to enable grfclk %d\n", ret);
 		return;
 	}
 
-	ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
+	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+	if (ret < 0)
+		return;
+
+	DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
+
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, ret);
 	if (ret != 0)
 		DRM_DEV_ERROR(dp->dev, "Could not write to GRF: %d\n", ret);
 
@@ -448,17 +465,13 @@ static DEFINE_RUNTIME_DEV_PM_OPS(rockchip_dp_pm_ops, rockchip_dp_suspend,
 		rockchip_dp_resume, NULL);
 
 static const struct rockchip_dp_chip_data rk3399_edp = {
-	.lcdsel_grf_reg = RK3399_GRF_SOC_CON20,
-	.lcdsel_big = HIWORD_UPDATE(0, RK3399_EDP_LCDC_SEL),
-	.lcdsel_lit = HIWORD_UPDATE(RK3399_EDP_LCDC_SEL, RK3399_EDP_LCDC_SEL),
 	.chip_type = RK3399_EDP,
+	.lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
 };
 
 static const struct rockchip_dp_chip_data rk3288_dp = {
-	.lcdsel_grf_reg = RK3288_GRF_SOC_CON6,
-	.lcdsel_big = HIWORD_UPDATE(0, RK3288_EDP_LCDC_SEL),
-	.lcdsel_lit = HIWORD_UPDATE(RK3288_EDP_LCDC_SEL, RK3288_EDP_LCDC_SEL),
 	.chip_type = RK3288_DP,
+	.lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
 };
 
 static const struct of_device_id rockchip_dp_dt_ids[] = {
-- 
2.34.1

[PATCH v1 02/10] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Damon Ding]

Add the compatible "rockchip,rk3588-edp".

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../bindings/display/rockchip/rockchip,analogix-dp.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index 60dedf9b2be7..b82b693532e1 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -15,6 +15,7 @@ properties:
     enum:
       - rockchip,rk3288-dp
       - rockchip,rk3399-edp
+      - rockchip,rk3588-edp
 
   clocks:
     minItems: 2
-- 
2.34.1

Re: [PATCH v1 02/10] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Krzysztof Kozlowski]

On 27/11/2024 08:51, Damon Ding wrote:
> Add the compatible "rockchip,rk3588-edp".

This we see from the diff. Say something about hardware and why it is
not compatible with existing variants.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>



Best regards,
Krzysztof

Re: [PATCH v1 02/10] dt-bindings: display: rockchip: analogix-dp: Add support for RK3588

[Damon Ding]

Hi Krzysztof,

On 2024/11/27 18:23, Krzysztof Kozlowski wrote:
> On 27/11/2024 08:51, Damon Ding wrote:
>> Add the compatible "rockchip,rk3588-edp".
> 
> This we see from the diff. Say something about hardware and why it is
> not compatible with existing variants.

In response to your kind suggestion, I will add more details in next 
version, specifically:

Compared with RK3288/RK3399, the HBR2 link rate support is the main
improvement of RK3588 eDP TX controller, and there are also two
independent eDP display interfaces on RK3588 Soc.

> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> 
> 
> Best regards,
> Krzysztof
> 
> 

Best regards,
Damon

[PATCH v1 03/10] drm/rockchip: analogix_dp: Add support for RK3588

[Damon Ding]

RK3588 integrates the analogix eDP 1.3 TX controller IP and the HDMI/eDP
TX Combo PHY based on a Samsung IP block.

Add just the basic support for now, i.e. RGB output up to 4K@60Hz, without
the tests of audio, PSR and other eDP 1.3 specific features.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 41 +++++++++++++++++--
 include/drm/bridge/analogix_dp.h              |  3 +-
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index edb08a816c3f..e5792deb7885 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -51,10 +51,12 @@ struct rockchip_grf_reg_field {
 /**
  * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
  * @lcdc_sel: grf register field of lcdc_sel
+ * @edp_mode: grf register field of edp_mode
  * @chip_type: specific chip type
  */
 struct rockchip_dp_chip_data {
 	const struct rockchip_grf_reg_field lcdc_sel;
+	const struct rockchip_grf_reg_field edp_mode;
 	u32	chip_type;
 };
 
@@ -134,12 +136,21 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data)
 		return ret;
 	}
 
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1);
+	if (ret != 0)
+		DRM_DEV_ERROR(dp->dev, "failed to set edp mode %d\n", ret);
+
 	return ret;
 }
 
 static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
 {
 	struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
+	int ret;
+
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 0);
+	if (ret != 0)
+		DRM_DEV_ERROR(dp->dev, "failed to set edp mode %d\n", ret);
 
 	clk_disable_unprepare(dp->pclk);
 
@@ -203,6 +214,10 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	struct rockchip_dp_device *dp = encoder_to_dp(encoder);
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
+	struct of_endpoint endpoint;
+	struct device_node *remote_port, *remote_port_parent;
+	char name[32];
+	u32 port_id;
 	int ret;
 
 	crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
@@ -220,13 +235,27 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 		return;
 	}
 
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+	ret = drm_of_encoder_active_endpoint(dp->dev->of_node, encoder, &endpoint);
 	if (ret < 0)
 		return;
 
-	DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
+	remote_port_parent = of_graph_get_remote_port_parent(endpoint.local_node);
+	if (remote_port_parent) {
+		if (of_get_child_by_name(remote_port_parent, "ports")) {
+			remote_port = of_graph_get_remote_port(endpoint.local_node);
+			of_property_read_u32(remote_port, "reg", &port_id);
+			of_node_put(remote_port);
+			sprintf(name, "%s vp%d", remote_port_parent->full_name, port_id);
+		} else {
+			sprintf(name, "%s %s",
+				remote_port_parent->full_name, endpoint.id ? "vopl" : "vopb");
+		}
+		of_node_put(remote_port_parent);
+
+		DRM_DEV_DEBUG(dp->dev, "%s output to edp\n", name);
+	}
 
-	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, ret);
+	ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, endpoint.id);
 	if (ret != 0)
 		DRM_DEV_ERROR(dp->dev, "Could not write to GRF: %d\n", ret);
 
@@ -474,9 +503,15 @@ static const struct rockchip_dp_chip_data rk3288_dp = {
 	.lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
 };
 
+static const struct rockchip_dp_chip_data rk3588_edp = {
+	.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
+	.chip_type = RK3588_EDP,
+};
+
 static const struct of_device_id rockchip_dp_dt_ids[] = {
 	{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
 	{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
+	{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 6002c5666031..54086cb2d97d 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -15,11 +15,12 @@ enum analogix_dp_devtype {
 	EXYNOS_DP,
 	RK3288_DP,
 	RK3399_EDP,
+	RK3588_EDP,
 };
 
 static inline bool is_rockchip(enum analogix_dp_devtype type)
 {
-	return type == RK3288_DP || type == RK3399_EDP;
+	return type == RK3288_DP || type == RK3399_EDP || type == RK3588_EDP;
 }
 
 struct analogix_dp_plat_data {
-- 
2.34.1

[PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
pre-emphasis configurations of each link rate have been verified according
to the eDP 1.3 requirements.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../phy/rockchip/phy-rockchip-samsung-hdptx.c | 936 +++++++++++++++++-
 1 file changed, 893 insertions(+), 43 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
index 9f084697dd05..ba06e360e550 100644
--- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
+++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
@@ -25,6 +25,7 @@
 #define HDPTX_I_PLL_EN			BIT(7)
 #define HDPTX_I_BIAS_EN			BIT(6)
 #define HDPTX_I_BGR_EN			BIT(5)
+#define HDPTX_MODE_SEL			BIT(0)
 #define GRF_HDPTX_STATUS		0x80
 #define HDPTX_O_PLL_LOCK_DONE		BIT(3)
 #define HDPTX_O_PHY_CLK_RDY		BIT(2)
@@ -44,6 +45,7 @@
 #define LANE_REG(n)			HDTPX_REG(n, 0300, 062d)
 
 /* CMN_REG(0008) */
+#define OVRD_LCPLL_EN_MASK		BIT(7)
 #define LCPLL_EN_MASK			BIT(6)
 #define LCPLL_LCVCO_MODE_EN_MASK	BIT(4)
 /* CMN_REG(001e) */
@@ -61,49 +63,110 @@
 /* CMN_REG(002f) */
 #define LCPLL_SDC_DENOMINATOR_MASK	GENMASK(7, 2)
 #define LCPLL_SDC_NDIV_RSTN		BIT(0)
+/* CMN_REG(003c) */
+#define ANA_LCPLL_RESERVED7_MASK	BIT(7)
 /* CMN_REG(003d) */
+#define OVRD_ROPLL_EN_MASK		BIT(7)
+#define ROPLL_EN_MASK			BIT(6)
 #define ROPLL_LCVCO_EN			BIT(4)
+/* CMN_REG(0046) */
+#define ROPLL_ANA_CPP_CTRL_COARSE_MASK	GENMASK(7, 4)
+#define ROPLL_ANA_CPP_CTRL_FINE_MASK	GENMASK(3, 0)
+/* CMN_REG(0047) */
+#define ROPLL_ANA_LPF_C_SEL_COARSE_MASK	GENMASK(5, 3)
+#define ROPLL_ANA_LPF_C_SEL_FINE_MASK	GENMASK(2, 0)
 /* CMN_REG(004e) */
 #define ROPLL_PI_EN			BIT(5)
+/* CMN_REG(0051) */
+#define ROPLL_PMS_MDIV_MASK		GENMASK(7, 0)
+/* CMN_REG(0055) */
+#define ROPLL_PMS_MDIV_AFC_MASK		GENMASK(7, 0)
+/* CMN_REG(0059) */
+#define ANA_ROPLL_PMS_PDIV_MASK		GENMASK(7, 4)
+#define ANA_ROPLL_PMS_REFDIV_MASK	GENMASK(3, 0)
+/* CMN_REG(005a) */
+#define ROPLL_PMS_SDIV_RBR_MASK		GENMASK(7, 4)
+#define ROPLL_PMS_SDIV_HBR_MASK		GENMASK(3, 0)
+/* CMN_REG(005b) */
+#define ROPLL_PMS_SDIV_HBR2_MASK	GENMASK(7, 4)
 /* CMN_REG(005c) */
 #define ROPLL_PMS_IQDIV_RSTN		BIT(5)
 /* CMN_REG(005e) */
 #define ROPLL_SDM_EN_MASK		BIT(6)
-#define ROPLL_SDM_FRAC_EN_RBR		BIT(3)
-#define ROPLL_SDM_FRAC_EN_HBR		BIT(2)
-#define ROPLL_SDM_FRAC_EN_HBR2		BIT(1)
-#define ROPLL_SDM_FRAC_EN_HBR3		BIT(0)
+#define OVRD_ROPLL_SDM_RSTN_MASK	BIT(5)
+#define ROPLL_SDM_RSTN_MASK		BIT(4)
+#define ROPLL_SDC_FRAC_EN_RBR_MASK	BIT(3)
+#define ROPLL_SDC_FRAC_EN_HBR_MASK	BIT(2)
+#define ROPLL_SDC_FRAC_EN_HBR2_MASK	BIT(1)
+/* CMN_REG(005f) */
+#define OVRD_ROPLL_SDC_RSTN_MASK	BIT(5)
+#define ROPLL_SDC_RSTN_MASK		BIT(4)
+/* CMN_REG(0060)  */
+#define ROPLL_SDM_DENOMINATOR_MASK	GENMASK(7, 0)
 /* CMN_REG(0064) */
 #define ROPLL_SDM_NUM_SIGN_RBR_MASK	BIT(3)
+#define ROPLL_SDM_NUM_SIGN_HBR_MASK	BIT(2)
+#define ROPLL_SDM_NUM_SIGN_HBR2_MASK	BIT(1)
+/* CMN_REG(0065) */
+#define ROPLL_SDM_NUM_MASK		GENMASK(7, 0)
 /* CMN_REG(0069) */
 #define ROPLL_SDC_N_RBR_MASK		GENMASK(2, 0)
+/* CMN_REG(006a) */
+#define ROPLL_SDC_N_HBR_MASK		GENMASK(5, 3)
+#define ROPLL_SDC_N_HBR2_MASK		GENMASK(2, 0)
+/* CMN_REG(006b) */
+#define ROPLL_SDC_N_HBR3_MASK		GENMASK(3, 1)
+/* CMN_REG(006c) */
+#define ROPLL_SDC_NUM_MASK		GENMASK(5, 0)
+/* cmn_reg0070 */
+#define ROPLL_SDC_DENO_MASK		GENMASK(5, 0)
 /* CMN_REG(0074) */
-#define ROPLL_SDC_NDIV_RSTN		BIT(2)
-#define ROPLL_SSC_EN			BIT(0)
+#define OVRD_ROPLL_SDC_NDIV_RSTN_MASK	BIT(3)
+#define ROPLL_SDC_NDIV_RSTN_MASK	BIT(2)
+#define OVRD_ROPLL_SSC_EN_MASK		BIT(1)
+#define ROPLL_SSC_EN_MASK		BIT(0)
+/* CMN_REG(0075) */
+#define ANA_ROPLL_SSC_FM_DEVIATION_MASK	GENMASK(5, 0)
+/* CMN_REG(0076) */
+#define ANA_ROPLL_SSC_FM_FREQ_MASK	GENMASK(6, 2)
+/* CMN_REG(0077) */
+#define ANA_ROPLL_SSC_CLK_DIV_SEL_MASK	GENMASK(6, 3)
 /* CMN_REG(0081) */
-#define OVRD_PLL_CD_CLK_EN		BIT(8)
-#define PLL_CD_HSCLK_EAST_EN		BIT(0)
+#define ANA_PLL_CD_TX_SER_RATE_SEL_MASK	BIT(3)
+#define ANA_PLL_CD_HSCLK_WEST_EN_MASK	BIT(1)
+#define ANA_PLL_CD_HSCLK_EAST_EN_MASK	BIT(0)
+/* CMN_REG(0082) */
+#define ANA_PLL_CD_VREG_GAIN_CTRL_MASK	GENMASK(3, 0)
+/* CMN_REG(0083) */
+#define ANA_PLL_CD_VREG_ICTRL_MASK	GENMASK(6, 5)
+/* CMN_REG(0084) */
+#define PLL_LCRO_CLK_SEL_MASK		BIT(5)
+/* CMN_REG(0085) */
+#define ANA_PLL_SYNC_LOSS_DET_MODE_MASK	GENMASK(1, 0)
 /* CMN_REG(0086) */
 #define PLL_PCG_POSTDIV_SEL_MASK	GENMASK(7, 4)
 #define PLL_PCG_CLK_SEL_MASK		GENMASK(3, 1)
 #define PLL_PCG_CLK_EN			BIT(0)
 /* CMN_REG(0087) */
-#define PLL_FRL_MODE_EN			BIT(3)
-#define PLL_TX_HS_CLK_EN		BIT(2)
+#define ANA_PLL_FRL_MODE_EN_MASK	BIT(3)
+#define ANA_PLL_TX_HS_CLK_EN_MASK	BIT(2)
 /* CMN_REG(0089) */
 #define LCPLL_ALONE_MODE		BIT(1)
+/* CMN_REG(0095) */
+#define DP_TX_LINK_BW_MASK		GENMASK(1, 0)
 /* CMN_REG(0097) */
-#define DIG_CLK_SEL			BIT(1)
-#define ROPLL_REF			BIT(1)
-#define LCPLL_REF			0
+#define DIG_CLK_SEL_MASK		BIT(1)
+#define LCPLL_REF			BIT(1)
+#define ROPLL_REF			0
 /* CMN_REG(0099) */
+#define SSC_EN_MASK			GENMASK(7, 6)
 #define CMN_ROPLL_ALONE_MODE		BIT(2)
 #define ROPLL_ALONE_MODE		BIT(2)
 /* CMN_REG(009a) */
-#define HS_SPEED_SEL			BIT(0)
+#define HS_SPEED_SEL_MASK		BIT(0)
 #define DIV_10_CLOCK			BIT(0)
 /* CMN_REG(009b) */
-#define IS_SPEED_SEL			BIT(4)
+#define LS_SPEED_SEL_MASK		BIT(4)
 #define LINK_SYMBOL_CLOCK		BIT(4)
 #define LINK_SYMBOL_CLOCK1_2		0
 
@@ -118,6 +181,8 @@
 /* SB_REG(0104) */
 #define OVRD_SB_EN_MASK			BIT(5)
 #define SB_EN_MASK			BIT(4)
+#define OVRD_SB_AUX_EN_MASK		BIT(1)
+#define SB_AUX_EN_MASK			BIT(0)
 /* SB_REG(0105) */
 #define OVRD_SB_EARC_CMDC_EN_MASK	BIT(6)
 #define SB_EARC_CMDC_EN_MASK		BIT(5)
@@ -126,6 +191,8 @@
 #define ANA_SB_TX_LLVL_PROG_MASK	GENMASK(6, 4)
 /* SB_REG(0109) */
 #define ANA_SB_DMRX_AFC_DIV_RATIO_MASK	GENMASK(2, 0)
+/* SB_REG(010d) */
+#define ANA_SB_DMRX_LPBK_DATA_MASK	BIT(4)
 /* SB_REG(010f) */
 #define OVRD_SB_VREG_EN_MASK		BIT(7)
 #define SB_VREG_EN_MASK			BIT(6)
@@ -133,6 +200,7 @@
 #define SB_VREG_LPF_BYPASS_MASK		BIT(4)
 #define ANA_SB_VREG_GAIN_CTRL_MASK	GENMASK(3, 0)
 /* SB_REG(0110) */
+#define ANA_SB_VREG_OUT_SEL_MASK	BIT(1)
 #define ANA_SB_VREG_REF_SEL_MASK	BIT(0)
 /* SB_REG(0113) */
 #define SB_RX_RCAL_OPT_CODE_MASK	GENMASK(5, 4)
@@ -147,13 +215,24 @@
 #define AFC_RSTN_DELAY_TIME_MASK	GENMASK(6, 4)
 /* SB_REG(0117) */
 #define FAST_PULSE_TIME_MASK		GENMASK(3, 0)
+/* SB_REG(0118) */
+#define SB_TG_EARC_DMRX_RECVRD_CLK_CNT_MASK	GENMASK(7, 0)
+/* SB_REG(011a) */
+#define SB_TG_CNT_RUN_NO_7_0_MASK	GENMASK(7, 0)
 /* SB_REG(011b) */
 #define SB_EARC_SIG_DET_BYPASS_MASK	BIT(4)
 #define SB_AFC_TOL_MASK			GENMASK(3, 0)
+/* SB_REG(011c) */
+#define SB_AFC_STB_NUM_MASK		GENMASK(3, 0)
+/* SB_REG(011d) */
+#define SB_TG_OSC_CNT_MIN_MASK		GENMASK(7, 0)
+/* SB_REG(011e) */
+#define SB_TG_OSC_CNT_MAX_MASK		GENMASK(7, 0)
 /* SB_REG(011f) */
 #define SB_PWM_AFC_CTRL_MASK		GENMASK(7, 2)
 #define SB_RCAL_RSTN_MASK		BIT(1)
 /* SB_REG(0120) */
+#define SB_AUX_EN_IN_MASK		BIT(7)
 #define SB_EARC_EN_MASK			BIT(1)
 #define SB_EARC_AFC_EN_MASK		BIT(2)
 /* SB_REG(0123) */
@@ -165,35 +244,65 @@
 #define HDMI_MODE			BIT(2)
 #define HDMI_TMDS_FRL_SEL		BIT(1)
 /* LNTOP_REG(0206) */
-#define DATA_BUS_SEL			BIT(0)
+#define DATA_BUS_WIDTH_MASK		GENMASK(2, 1)
+#define DATA_BUS_WIDTH_SEL_MASK		BIT(0)
 #define DATA_BUS_36_40			BIT(0)
 /* LNTOP_REG(0207) */
 #define LANE_EN				0xf
 #define ALL_LANE_EN			0xf
 
-/* LANE_REG(0312) */
-#define LN0_TX_SER_RATE_SEL_RBR		BIT(5)
-#define LN0_TX_SER_RATE_SEL_HBR		BIT(4)
-#define LN0_TX_SER_RATE_SEL_HBR2	BIT(3)
-#define LN0_TX_SER_RATE_SEL_HBR3	BIT(2)
-/* LANE_REG(0412) */
-#define LN1_TX_SER_RATE_SEL_RBR		BIT(5)
-#define LN1_TX_SER_RATE_SEL_HBR		BIT(4)
-#define LN1_TX_SER_RATE_SEL_HBR2	BIT(3)
-#define LN1_TX_SER_RATE_SEL_HBR3	BIT(2)
-/* LANE_REG(0512) */
-#define LN2_TX_SER_RATE_SEL_RBR		BIT(5)
-#define LN2_TX_SER_RATE_SEL_HBR		BIT(4)
-#define LN2_TX_SER_RATE_SEL_HBR2	BIT(3)
-#define LN2_TX_SER_RATE_SEL_HBR3	BIT(2)
-/* LANE_REG(0612) */
-#define LN3_TX_SER_RATE_SEL_RBR		BIT(5)
-#define LN3_TX_SER_RATE_SEL_HBR		BIT(4)
-#define LN3_TX_SER_RATE_SEL_HBR2	BIT(3)
-#define LN3_TX_SER_RATE_SEL_HBR3	BIT(2)
+/* LANE_REG(0301) */
+#define OVRD_LN_TX_DRV_EI_EN_MASK	BIT(7)
+#define LN_TX_DRV_EI_EN_MASK		BIT(6)
+/* LANE_REG(0303) */
+#define OVRD_LN_TX_DRV_LVL_CTRL_MASK	BIT(5)
+#define LN_TX_DRV_LVL_CTRL_MASK		GENMASK(4, 0)
+/* LANE_REG(0304)  */
+#define OVRD_LN_TX_DRV_POST_LVL_CTRL_MASK	BIT(4)
+#define LN_TX_DRV_POST_LVL_CTRL_MASK	GENMASK(3, 0)
+/* LANE_REG(0305) */
+#define OVRD_LN_TX_DRV_PRE_LVL_CTRL_MASK	BIT(6)
+#define LN_TX_DRV_PRE_LVL_CTRL_MASK	GENMASK(5, 2)
+/* LANE_REG(0306) */
+#define LN_ANA_TX_DRV_IDRV_IDN_CTRL_MASK	GENMASK(7, 5)
+#define LN_ANA_TX_DRV_IDRV_IUP_CTRL_MASK	GENMASK(4, 2)
+#define LN_ANA_TX_DRV_ACCDRV_EN_MASK	BIT(0)
+/* LANE_REG(0307) */
+#define LN_ANA_TX_DRV_ACCDRV_POL_SEL_MASK	BIT(6)
+#define LN_ANA_TX_DRV_ACCDRV_CTRL_MASK	GENMASK(5, 3)
+/* LANE_REG(030a) */
+#define LN_ANA_TX_JEQ_EN_MASK		BIT(4)
+#define LN_TX_JEQ_EVEN_CTRL_RBR_MASK	GENMASK(3, 0)
+/* LANE_REG(030b) */
+#define LN_TX_JEQ_EVEN_CTRL_HBR_MASK	GENMASK(7, 4)
+#define LN_TX_JEQ_EVEN_CTRL_HBR2_MASK	GENMASK(3, 0)
+/* LANE_REG(030c) */
+#define LN_TX_JEQ_ODD_CTRL_RBR_MASK	GENMASK(3, 0)
+/* LANE_REG(030d) */
+#define LN_TX_JEQ_ODD_CTRL_HBR_MASK	GENMASK(7, 4)
+#define LN_TX_JEQ_ODD_CTRL_HBR2_MASK	GENMASK(3, 0)
+/* LANE_REG(0310) */
+#define LN_ANA_TX_SYNC_LOSS_DET_MODE_MASK	GENMASK(1, 0)
+/* LANE_REG(0311) */
+#define LN_TX_SER_40BIT_EN_RBR_MASK	BIT(3)
+#define LN_TX_SER_40BIT_EN_HBR_MASK	BIT(2)
+#define LN_TX_SER_40BIT_EN_HBR2_MASK	BIT(1)
+/* LANE_REG(0316) */
+#define LN_ANA_TX_SER_VREG_GAIN_CTRL_MASK	GENMASK(3, 0)
+/* LANE_REG(031B) */
+#define LN_ANA_TX_RESERVED_MASK		GENMASK(7, 0)
+/* LANE_REG(031e) */
+#define LN_POLARITY_INV_MASK		BIT(2)
+#define LN_LANE_MODE_MASK		BIT(1)
 
 #define HDMI20_MAX_RATE			600000000
 
+enum dp_link_rate {
+	DP_BW_RBR,
+	DP_BW_HBR,
+	DP_BW_HBR2,
+};
+
 struct lcpll_config {
 	u32 bit_rate;
 	u8 lcvco_mode_en;
@@ -255,6 +364,19 @@ struct ropll_config {
 	u8 cd_tx_ser_rate_sel;
 };
 
+struct tx_drv_ctrl {
+	u8 tx_drv_lvl_ctrl;
+	u8 tx_drv_post_lvl_ctrl;
+	u8 ana_tx_drv_idrv_idn_ctrl;
+	u8 ana_tx_drv_idrv_iup_ctrl;
+	u8 ana_tx_drv_accdrv_en;
+	u8 ana_tx_drv_accdrv_ctrl;
+	u8 tx_drv_pre_lvl_ctrl;
+	u8 ana_tx_jeq_en;
+	u8 tx_jeq_even_ctrl;
+	u8 tx_jeq_odd_ctrl;
+};
+
 enum rk_hdptx_reset {
 	RST_PHY = 0,
 	RST_APB,
@@ -560,6 +682,90 @@ static const struct reg_sequence rk_hdtpx_tmds_lane_init_seq[] = {
 	REG_SEQ0(LANE_REG(0606), 0x1c),
 };
 
+static struct tx_drv_ctrl tx_drv_ctrl_rbr[4][4] = {
+	/* voltage swing 0, pre-emphasis 0->3 */
+	{
+		{ 0x2, 0x0, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0x4, 0x3, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0x7, 0x6, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xd, 0xc, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 1, pre-emphasis 0->2 */
+	{
+		{ 0x4, 0x0, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0x9, 0x5, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xc, 0x8, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 2, pre-emphasis 0->1 */
+	{
+		{ 0x8, 0x0, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0xc, 0x5, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 3, pre-emphasis 0 */
+	{
+		{ 0xb, 0x0, 0x7, 0x7, 0x1, 0x4, 0x1, 0x1, 0x7, 0x7 },
+	}
+};
+
+static struct tx_drv_ctrl tx_drv_ctrl_hbr[4][4] = {
+	/* voltage swing 0, pre-emphasis 0->3 */
+	{
+		{ 0x2, 0x0, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0x5, 0x4, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0x9, 0x8, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xd, 0xc, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 1, pre-emphasis 0->2 */
+	{
+		{ 0x6, 0x1, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0xa, 0x6, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xc, 0x8, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 2, pre-emphasis 0->1 */
+	{
+		{ 0x9, 0x1, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0xd, 0x6, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 3, pre-emphasis 0 */
+	{
+		{ 0xc, 0x1, 0x7, 0x7, 0x1, 0x4, 0x1, 0x1, 0x7, 0x7 },
+	}
+};
+
+static struct tx_drv_ctrl tx_drv_ctrl_hbr2[4][4] = {
+	/* voltage swing 0, pre-emphasis 0->3 */
+	{
+		{ 0x2, 0x1, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0x5, 0x4, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0x9, 0x8, 0x4, 0x6, 0x1, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xd, 0xc, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 1, pre-emphasis 0->2 */
+	{
+		{ 0x6, 0x1, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0xb, 0x7, 0x4, 0x6, 0x0, 0x4, 0x0, 0x1, 0x7, 0x7 },
+		{ 0xd, 0x9, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 2, pre-emphasis 0->1 */
+	{
+		{ 0x8, 0x1, 0x4, 0x6, 0x0, 0x4, 0x1, 0x1, 0x7, 0x7 },
+		{ 0xc, 0x6, 0x7, 0x7, 0x1, 0x7, 0x0, 0x1, 0x7, 0x7 },
+	},
+
+	/* voltage swing 3, pre-emphasis 0 */
+	{
+		{ 0xb, 0x0, 0x7, 0x7, 0x1, 0x4, 0x1, 0x1, 0x7, 0x7 },
+	}
+};
+
 static bool rk_hdptx_phy_is_rw_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -911,11 +1117,297 @@ static int rk_hdptx_phy_consumer_put(struct rk_hdptx_phy *hdptx, bool force)
 	return ret;
 }
 
+static void rk_hdptx_dp_reset(struct rk_hdptx_phy *hdptx)
+{
+	reset_control_assert(hdptx->rsts[RST_LANE].rstc);
+	reset_control_assert(hdptx->rsts[RST_CMN].rstc);
+	reset_control_assert(hdptx->rsts[RST_INIT].rstc);
+
+	reset_control_assert(hdptx->rsts[RST_APB].rstc);
+	udelay(10);
+	reset_control_deassert(hdptx->rsts[RST_APB].rstc);
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(0301),
+			   OVRD_LN_TX_DRV_EI_EN_MASK | LN_TX_DRV_EI_EN_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_EI_EN_MASK, 1) |
+			   FIELD_PREP(LN_TX_DRV_EI_EN_MASK, 0));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0401),
+			   OVRD_LN_TX_DRV_EI_EN_MASK | LN_TX_DRV_EI_EN_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_EI_EN_MASK, 1) |
+			   FIELD_PREP(LN_TX_DRV_EI_EN_MASK, 0));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0501),
+			   OVRD_LN_TX_DRV_EI_EN_MASK | LN_TX_DRV_EI_EN_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_EI_EN_MASK, 1) |
+			   FIELD_PREP(LN_TX_DRV_EI_EN_MASK, 0));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0601),
+			   OVRD_LN_TX_DRV_EI_EN_MASK | LN_TX_DRV_EI_EN_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_EI_EN_MASK, 1) |
+			   FIELD_PREP(LN_TX_DRV_EI_EN_MASK, 0));
+
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x0));
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_BIAS_EN << 16 | FIELD_PREP(HDPTX_I_BIAS_EN, 0x0));
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_BGR_EN << 16 | FIELD_PREP(HDPTX_I_BGR_EN, 0x0));
+}
+
+static void rk_hdptx_dp_pll_init(struct rk_hdptx_phy *hdptx)
+{
+	regmap_update_bits(hdptx->regmap, CMN_REG(003c), ANA_LCPLL_RESERVED7_MASK,
+			   FIELD_PREP(ANA_LCPLL_RESERVED7_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0046),
+			   ROPLL_ANA_CPP_CTRL_COARSE_MASK | ROPLL_ANA_CPP_CTRL_FINE_MASK,
+			   FIELD_PREP(ROPLL_ANA_CPP_CTRL_COARSE_MASK, 0xe) |
+			   FIELD_PREP(ROPLL_ANA_CPP_CTRL_FINE_MASK, 0xe));
+	regmap_update_bits(hdptx->regmap, CMN_REG(0047),
+			   ROPLL_ANA_LPF_C_SEL_COARSE_MASK |
+			   ROPLL_ANA_LPF_C_SEL_FINE_MASK,
+			   FIELD_PREP(ROPLL_ANA_LPF_C_SEL_COARSE_MASK, 0x4) |
+			   FIELD_PREP(ROPLL_ANA_LPF_C_SEL_FINE_MASK, 0x4));
+
+	regmap_write(hdptx->regmap, CMN_REG(0051), FIELD_PREP(ROPLL_PMS_MDIV_MASK, 0x87));
+	regmap_write(hdptx->regmap, CMN_REG(0052), FIELD_PREP(ROPLL_PMS_MDIV_MASK, 0x71));
+	regmap_write(hdptx->regmap, CMN_REG(0053), FIELD_PREP(ROPLL_PMS_MDIV_MASK, 0x71));
+
+	regmap_write(hdptx->regmap, CMN_REG(0055),
+		     FIELD_PREP(ROPLL_PMS_MDIV_AFC_MASK, 0x87));
+	regmap_write(hdptx->regmap, CMN_REG(0056),
+		     FIELD_PREP(ROPLL_PMS_MDIV_AFC_MASK, 0x71));
+	regmap_write(hdptx->regmap, CMN_REG(0057),
+		     FIELD_PREP(ROPLL_PMS_MDIV_AFC_MASK, 0x71));
+
+	regmap_write(hdptx->regmap, CMN_REG(0059),
+		     FIELD_PREP(ANA_ROPLL_PMS_PDIV_MASK, 0x1) |
+		     FIELD_PREP(ANA_ROPLL_PMS_REFDIV_MASK, 0x1));
+	regmap_write(hdptx->regmap, CMN_REG(005a),
+		     FIELD_PREP(ROPLL_PMS_SDIV_RBR_MASK, 0x3) |
+		     FIELD_PREP(ROPLL_PMS_SDIV_HBR_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(005b), ROPLL_PMS_SDIV_HBR2_MASK,
+			   FIELD_PREP(ROPLL_PMS_SDIV_HBR2_MASK, 0x0));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(005e), ROPLL_SDM_EN_MASK,
+			   FIELD_PREP(ROPLL_SDM_EN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(005e),
+			   OVRD_ROPLL_SDM_RSTN_MASK | ROPLL_SDM_RSTN_MASK,
+			   FIELD_PREP(OVRD_ROPLL_SDM_RSTN_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_SDM_RSTN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(005e), ROPLL_SDC_FRAC_EN_RBR_MASK,
+			   FIELD_PREP(ROPLL_SDC_FRAC_EN_RBR_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(005e), ROPLL_SDC_FRAC_EN_HBR_MASK,
+			   FIELD_PREP(ROPLL_SDC_FRAC_EN_HBR_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(005e), ROPLL_SDC_FRAC_EN_HBR2_MASK,
+			   FIELD_PREP(ROPLL_SDC_FRAC_EN_HBR2_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(005f),
+			   OVRD_ROPLL_SDC_RSTN_MASK | ROPLL_SDC_RSTN_MASK,
+			   FIELD_PREP(OVRD_ROPLL_SDC_RSTN_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_SDC_RSTN_MASK, 0x1));
+	regmap_write(hdptx->regmap, CMN_REG(0060),
+		     FIELD_PREP(ROPLL_SDM_DENOMINATOR_MASK, 0x21));
+	regmap_write(hdptx->regmap, CMN_REG(0061),
+		     FIELD_PREP(ROPLL_SDM_DENOMINATOR_MASK, 0x27));
+	regmap_write(hdptx->regmap, CMN_REG(0062),
+		     FIELD_PREP(ROPLL_SDM_DENOMINATOR_MASK, 0x27));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0064),
+			   ROPLL_SDM_NUM_SIGN_RBR_MASK |
+			   ROPLL_SDM_NUM_SIGN_HBR_MASK |
+			   ROPLL_SDM_NUM_SIGN_HBR2_MASK,
+			   FIELD_PREP(ROPLL_SDM_NUM_SIGN_RBR_MASK, 0x0) |
+			   FIELD_PREP(ROPLL_SDM_NUM_SIGN_HBR_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_SDM_NUM_SIGN_HBR2_MASK, 0x1));
+	regmap_write(hdptx->regmap, CMN_REG(0065),
+		     FIELD_PREP(ROPLL_SDM_NUM_MASK, 0x0));
+	regmap_write(hdptx->regmap, CMN_REG(0066),
+		     FIELD_PREP(ROPLL_SDM_NUM_MASK, 0xd));
+	regmap_write(hdptx->regmap, CMN_REG(0067),
+		     FIELD_PREP(ROPLL_SDM_NUM_MASK, 0xd));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0069), ROPLL_SDC_N_RBR_MASK,
+			   FIELD_PREP(ROPLL_SDC_N_RBR_MASK, 0x2));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(006a),
+			   ROPLL_SDC_N_HBR_MASK | ROPLL_SDC_N_HBR2_MASK,
+			   FIELD_PREP(ROPLL_SDC_N_HBR_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_SDC_N_HBR2_MASK, 0x1));
+
+	regmap_write(hdptx->regmap, CMN_REG(006c),
+		     FIELD_PREP(ROPLL_SDC_NUM_MASK, 0x3));
+	regmap_write(hdptx->regmap, CMN_REG(006d),
+		     FIELD_PREP(ROPLL_SDC_NUM_MASK, 0x7));
+	regmap_write(hdptx->regmap, CMN_REG(006e),
+		     FIELD_PREP(ROPLL_SDC_NUM_MASK, 0x7));
+
+	regmap_write(hdptx->regmap, CMN_REG(0070),
+		     FIELD_PREP(ROPLL_SDC_DENO_MASK, 0x8));
+	regmap_write(hdptx->regmap, CMN_REG(0071),
+		     FIELD_PREP(ROPLL_SDC_DENO_MASK, 0x18));
+	regmap_write(hdptx->regmap, CMN_REG(0072),
+		     FIELD_PREP(ROPLL_SDC_DENO_MASK, 0x18));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0074),
+			   OVRD_ROPLL_SDC_NDIV_RSTN_MASK | ROPLL_SDC_NDIV_RSTN_MASK,
+			   FIELD_PREP(OVRD_ROPLL_SDC_NDIV_RSTN_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_SDC_NDIV_RSTN_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0077), ANA_ROPLL_SSC_CLK_DIV_SEL_MASK,
+			   FIELD_PREP(ANA_ROPLL_SSC_CLK_DIV_SEL_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0081), ANA_PLL_CD_TX_SER_RATE_SEL_MASK,
+			   FIELD_PREP(ANA_PLL_CD_TX_SER_RATE_SEL_MASK, 0x0));
+	regmap_update_bits(hdptx->regmap, CMN_REG(0081),
+			   ANA_PLL_CD_HSCLK_EAST_EN_MASK | ANA_PLL_CD_HSCLK_WEST_EN_MASK,
+			   FIELD_PREP(ANA_PLL_CD_HSCLK_EAST_EN_MASK, 0x1) |
+			   FIELD_PREP(ANA_PLL_CD_HSCLK_WEST_EN_MASK, 0x0));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0082), ANA_PLL_CD_VREG_GAIN_CTRL_MASK,
+			   FIELD_PREP(ANA_PLL_CD_VREG_GAIN_CTRL_MASK, 0x4));
+	regmap_update_bits(hdptx->regmap, CMN_REG(0083), ANA_PLL_CD_VREG_ICTRL_MASK,
+			   FIELD_PREP(ANA_PLL_CD_VREG_ICTRL_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(0084), PLL_LCRO_CLK_SEL_MASK,
+			   FIELD_PREP(PLL_LCRO_CLK_SEL_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(0085), ANA_PLL_SYNC_LOSS_DET_MODE_MASK,
+			   FIELD_PREP(ANA_PLL_SYNC_LOSS_DET_MODE_MASK, 0x3));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0087), ANA_PLL_TX_HS_CLK_EN_MASK,
+			   FIELD_PREP(ANA_PLL_TX_HS_CLK_EN_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0097), DIG_CLK_SEL_MASK,
+			   FIELD_PREP(DIG_CLK_SEL_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0099), CMN_ROPLL_ALONE_MODE,
+			   FIELD_PREP(CMN_ROPLL_ALONE_MODE, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(009a), HS_SPEED_SEL_MASK,
+			   FIELD_PREP(HS_SPEED_SEL_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, CMN_REG(009b), LS_SPEED_SEL_MASK,
+			   FIELD_PREP(LS_SPEED_SEL_MASK, 0x1));
+}
+
+static int rk_hdptx_dp_aux_init(struct rk_hdptx_phy *hdptx)
+{
+	u32 status;
+	int ret;
+
+	regmap_update_bits(hdptx->regmap, SB_REG(0102), ANA_SB_RXTERM_OFFSP_MASK,
+			   FIELD_PREP(ANA_SB_RXTERM_OFFSP_MASK, 0x3));
+	regmap_update_bits(hdptx->regmap, SB_REG(0103), ANA_SB_RXTERM_OFFSN_MASK,
+			   FIELD_PREP(ANA_SB_RXTERM_OFFSN_MASK, 0x3));
+	regmap_update_bits(hdptx->regmap, SB_REG(0104), SB_AUX_EN_MASK,
+			   FIELD_PREP(SB_AUX_EN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, SB_REG(0105), ANA_SB_TX_HLVL_PROG_MASK,
+			   FIELD_PREP(ANA_SB_TX_HLVL_PROG_MASK, 0x7));
+	regmap_update_bits(hdptx->regmap, SB_REG(0106), ANA_SB_TX_LLVL_PROG_MASK,
+			   FIELD_PREP(ANA_SB_TX_LLVL_PROG_MASK, 0x7));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(010d), ANA_SB_DMRX_LPBK_DATA_MASK,
+			   FIELD_PREP(ANA_SB_DMRX_LPBK_DATA_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(010f), ANA_SB_VREG_GAIN_CTRL_MASK,
+			   FIELD_PREP(ANA_SB_VREG_GAIN_CTRL_MASK, 0x0));
+	regmap_update_bits(hdptx->regmap, SB_REG(0110),
+			   ANA_SB_VREG_OUT_SEL_MASK | ANA_SB_VREG_REF_SEL_MASK,
+			   FIELD_PREP(ANA_SB_VREG_OUT_SEL_MASK, 0x1) |
+			   FIELD_PREP(ANA_SB_VREG_REF_SEL_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(0113),
+			   SB_RX_RCAL_OPT_CODE_MASK | SB_RX_RTERM_CTRL_MASK,
+			   FIELD_PREP(SB_RX_RCAL_OPT_CODE_MASK, 0x1) |
+			   FIELD_PREP(SB_RX_RTERM_CTRL_MASK, 0x3));
+	regmap_update_bits(hdptx->regmap, SB_REG(0114),
+			   SB_TG_SB_EN_DELAY_TIME_MASK | SB_TG_RXTERM_EN_DELAY_TIME_MASK,
+			   FIELD_PREP(SB_TG_SB_EN_DELAY_TIME_MASK, 0x2) |
+			   FIELD_PREP(SB_TG_RXTERM_EN_DELAY_TIME_MASK, 0x2));
+	regmap_update_bits(hdptx->regmap, SB_REG(0115),
+			   SB_READY_DELAY_TIME_MASK | SB_TG_OSC_EN_DELAY_TIME_MASK,
+			   FIELD_PREP(SB_READY_DELAY_TIME_MASK, 0x2) |
+			   FIELD_PREP(SB_TG_OSC_EN_DELAY_TIME_MASK, 0x2));
+	regmap_update_bits(hdptx->regmap, SB_REG(0116),
+			   AFC_RSTN_DELAY_TIME_MASK,
+			   FIELD_PREP(AFC_RSTN_DELAY_TIME_MASK, 0x2));
+	regmap_update_bits(hdptx->regmap, SB_REG(0117),
+			   FAST_PULSE_TIME_MASK,
+			   FIELD_PREP(FAST_PULSE_TIME_MASK, 0x4));
+	regmap_update_bits(hdptx->regmap, SB_REG(0118),
+			   SB_TG_EARC_DMRX_RECVRD_CLK_CNT_MASK,
+			   FIELD_PREP(SB_TG_EARC_DMRX_RECVRD_CLK_CNT_MASK, 0xa));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(011a), SB_TG_CNT_RUN_NO_7_0_MASK,
+			   FIELD_PREP(SB_TG_CNT_RUN_NO_7_0_MASK, 0x3));
+	regmap_update_bits(hdptx->regmap, SB_REG(011b),
+			   SB_EARC_SIG_DET_BYPASS_MASK | SB_AFC_TOL_MASK,
+			   FIELD_PREP(SB_EARC_SIG_DET_BYPASS_MASK, 0x1) |
+			   FIELD_PREP(SB_AFC_TOL_MASK, 0x3));
+	regmap_update_bits(hdptx->regmap, SB_REG(011c), SB_AFC_STB_NUM_MASK,
+			   FIELD_PREP(SB_AFC_STB_NUM_MASK, 0x4));
+	regmap_update_bits(hdptx->regmap, SB_REG(011d), SB_TG_OSC_CNT_MIN_MASK,
+			   FIELD_PREP(SB_TG_OSC_CNT_MIN_MASK, 0x67));
+	regmap_update_bits(hdptx->regmap, SB_REG(011e), SB_TG_OSC_CNT_MAX_MASK,
+			   FIELD_PREP(SB_TG_OSC_CNT_MAX_MASK, 0x6a));
+	regmap_update_bits(hdptx->regmap, SB_REG(011f), SB_PWM_AFC_CTRL_MASK,
+			   FIELD_PREP(SB_PWM_AFC_CTRL_MASK, 0x5));
+	regmap_update_bits(hdptx->regmap, SB_REG(011f), SB_RCAL_RSTN_MASK,
+			   FIELD_PREP(SB_RCAL_RSTN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, SB_REG(0120), SB_AUX_EN_IN_MASK,
+			   FIELD_PREP(SB_AUX_EN_IN_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(0102), OVRD_SB_RXTERM_EN_MASK,
+			   FIELD_PREP(OVRD_SB_RXTERM_EN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, SB_REG(0103), OVRD_SB_RX_RESCAL_DONE_MASK,
+			   FIELD_PREP(OVRD_SB_RX_RESCAL_DONE_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, SB_REG(0104), OVRD_SB_EN_MASK,
+			   FIELD_PREP(OVRD_SB_EN_MASK, 0x1));
+	regmap_update_bits(hdptx->regmap, SB_REG(0104), OVRD_SB_AUX_EN_MASK,
+			   FIELD_PREP(OVRD_SB_AUX_EN_MASK, 0x1));
+
+	regmap_update_bits(hdptx->regmap, SB_REG(010f), OVRD_SB_VREG_EN_MASK,
+			   FIELD_PREP(OVRD_SB_VREG_EN_MASK, 0x1));
+
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_BGR_EN << 16 | FIELD_PREP(HDPTX_I_BGR_EN, 0x1));
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_BIAS_EN << 16 | FIELD_PREP(HDPTX_I_BIAS_EN, 0x1));
+	usleep_range(20, 25);
+
+	reset_control_deassert(hdptx->rsts[RST_INIT].rstc);
+	usleep_range(20, 25);
+	reset_control_deassert(hdptx->rsts[RST_CMN].rstc);
+	usleep_range(20, 25);
+
+	regmap_update_bits(hdptx->regmap, SB_REG(0103), OVRD_SB_RX_RESCAL_DONE_MASK,
+			   FIELD_PREP(OVRD_SB_RX_RESCAL_DONE_MASK, 0x1));
+	usleep_range(100, 110);
+	regmap_update_bits(hdptx->regmap, SB_REG(0104), SB_EN_MASK,
+			   FIELD_PREP(SB_EN_MASK, 0x1));
+	usleep_range(100, 110);
+	regmap_update_bits(hdptx->regmap, SB_REG(0102), SB_RXTERM_EN_MASK,
+			   FIELD_PREP(SB_RXTERM_EN_MASK, 0x1));
+	usleep_range(20, 25);
+	regmap_update_bits(hdptx->regmap, SB_REG(010f), SB_VREG_EN_MASK,
+			   FIELD_PREP(SB_VREG_EN_MASK, 0x1));
+	usleep_range(20, 25);
+	regmap_update_bits(hdptx->regmap, SB_REG(0104), SB_AUX_EN_MASK,
+			   FIELD_PREP(SB_AUX_EN_MASK, 0x1));
+	usleep_range(100, 110);
+
+	ret = regmap_read_poll_timeout(hdptx->grf, GRF_HDPTX_STATUS,
+				       status, FIELD_GET(HDPTX_O_SB_RDY, status),
+				       50, 1000);
+	if (ret) {
+		dev_err(hdptx->dev, "Failed to get phy sb ready: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int rk_hdptx_phy_power_on(struct phy *phy)
 {
 	struct rk_hdptx_phy *hdptx = phy_get_drvdata(phy);
 	int bus_width = phy_get_bus_width(hdptx->phy);
-	int ret;
+	enum phy_mode mode = phy_get_mode(phy);
+	int ret, lane;
 
 	/*
 	 * FIXME: Temporary workaround to pass pixel_clk_rate
@@ -927,13 +1419,43 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
 	dev_dbg(hdptx->dev, "%s bus_width=%x rate=%u\n",
 		__func__, bus_width, rate);
 
-	ret = rk_hdptx_phy_consumer_get(hdptx, rate);
-	if (ret)
-		return ret;
+	if (mode == PHY_MODE_DP) {
+		rk_hdptx_dp_reset(hdptx);
 
-	ret = rk_hdptx_ropll_tmds_mode_config(hdptx, rate);
-	if (ret)
-		rk_hdptx_phy_consumer_put(hdptx, true);
+		for (lane = 0; lane < 4; lane++) {
+			regmap_update_bits(hdptx->regmap, LANE_REG(031e) + 0x400 * lane,
+					   LN_POLARITY_INV_MASK | LN_LANE_MODE_MASK,
+					   FIELD_PREP(LN_POLARITY_INV_MASK, 0) |
+					   FIELD_PREP(LN_LANE_MODE_MASK, 1));
+		}
+
+		regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+			     HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x1));
+
+		regmap_update_bits(hdptx->regmap, LNTOP_REG(0200), PROTOCOL_SEL,
+				   FIELD_PREP(PROTOCOL_SEL, 0x0));
+		regmap_update_bits(hdptx->regmap, LNTOP_REG(0206), DATA_BUS_WIDTH_MASK,
+				   FIELD_PREP(DATA_BUS_WIDTH_MASK, 0x1));
+		regmap_update_bits(hdptx->regmap, LNTOP_REG(0206), DATA_BUS_WIDTH_SEL_MASK,
+				   FIELD_PREP(DATA_BUS_WIDTH_SEL_MASK, 0x0));
+
+		rk_hdptx_dp_pll_init(hdptx);
+
+		ret = rk_hdptx_dp_aux_init(hdptx);
+		if (ret)
+			pm_runtime_put(hdptx->dev);
+	} else {
+		regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+			     HDPTX_MODE_SEL << 16 | FIELD_PREP(HDPTX_MODE_SEL, 0x0));
+
+		ret = rk_hdptx_phy_consumer_get(hdptx, rate);
+		if (ret)
+			return ret;
+
+		ret = rk_hdptx_ropll_tmds_mode_config(hdptx, rate);
+		if (ret)
+			rk_hdptx_phy_consumer_put(hdptx, true);
+	}
 
 	return ret;
 }
@@ -945,9 +1467,337 @@ static int rk_hdptx_phy_power_off(struct phy *phy)
 	return rk_hdptx_phy_consumer_put(hdptx, false);
 }
 
+static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
+				 int submode)
+{
+	return 0;
+}
+
+static int rk_hdptx_phy_verify_config(struct rk_hdptx_phy *hdptx,
+				      struct phy_configure_opts_dp *dp)
+{
+	int i;
+
+	if (dp->set_rate) {
+		switch (dp->link_rate) {
+		case 1620:
+		case 2700:
+		case 5400:
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	if (dp->set_lanes) {
+		switch (dp->lanes) {
+		case 0:
+		case 1:
+		case 2:
+		case 4:
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	if (dp->set_voltages) {
+		for (i = 0; i < dp->lanes; i++) {
+			if (dp->voltage[i] > 3 || dp->pre[i] > 3)
+				return -EINVAL;
+
+			if (dp->voltage[i] + dp->pre[i] > 3)
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int rk_hdptx_phy_set_rate(struct rk_hdptx_phy *hdptx,
+				 struct phy_configure_opts_dp *dp)
+{
+	u32 bw, status;
+	int ret;
+
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x0));
+
+	switch (dp->link_rate) {
+	case 1620:
+		bw = DP_BW_RBR;
+		break;
+	case 2700:
+		bw = DP_BW_HBR;
+		break;
+	case 5400:
+		bw = DP_BW_HBR2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0008), OVRD_LCPLL_EN_MASK | LCPLL_EN_MASK,
+			   FIELD_PREP(OVRD_LCPLL_EN_MASK, 0x1) |
+			   FIELD_PREP(LCPLL_EN_MASK, 0x0));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(003d), OVRD_ROPLL_EN_MASK | ROPLL_EN_MASK,
+			   FIELD_PREP(OVRD_ROPLL_EN_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_EN_MASK, 0x1));
+
+	if (dp->ssc) {
+		regmap_update_bits(hdptx->regmap, CMN_REG(0074),
+				   OVRD_ROPLL_SSC_EN_MASK | ROPLL_SSC_EN_MASK,
+				   FIELD_PREP(OVRD_ROPLL_SSC_EN_MASK, 0x1) |
+				   FIELD_PREP(ROPLL_SSC_EN_MASK, 0x1));
+		regmap_write(hdptx->regmap, CMN_REG(0075),
+			     FIELD_PREP(ANA_ROPLL_SSC_FM_DEVIATION_MASK, 0xc));
+		regmap_update_bits(hdptx->regmap, CMN_REG(0076),
+				   ANA_ROPLL_SSC_FM_FREQ_MASK,
+				   FIELD_PREP(ANA_ROPLL_SSC_FM_FREQ_MASK, 0x1f));
+
+		regmap_update_bits(hdptx->regmap, CMN_REG(0099), SSC_EN_MASK,
+				   FIELD_PREP(SSC_EN_MASK, 0x2));
+	} else {
+		regmap_update_bits(hdptx->regmap, CMN_REG(0074),
+				   OVRD_ROPLL_SSC_EN_MASK | ROPLL_SSC_EN_MASK,
+				   FIELD_PREP(OVRD_ROPLL_SSC_EN_MASK, 0x1) |
+				   FIELD_PREP(ROPLL_SSC_EN_MASK, 0x0));
+		regmap_write(hdptx->regmap, CMN_REG(0075),
+			     FIELD_PREP(ANA_ROPLL_SSC_FM_DEVIATION_MASK, 0x20));
+		regmap_update_bits(hdptx->regmap, CMN_REG(0076),
+				   ANA_ROPLL_SSC_FM_FREQ_MASK,
+				   FIELD_PREP(ANA_ROPLL_SSC_FM_FREQ_MASK, 0xc));
+
+		regmap_update_bits(hdptx->regmap, CMN_REG(0099), SSC_EN_MASK,
+				   FIELD_PREP(SSC_EN_MASK, 0x0));
+	}
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0095), DP_TX_LINK_BW_MASK,
+			   FIELD_PREP(DP_TX_LINK_BW_MASK, bw));
+
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x1));
+
+	ret = regmap_read_poll_timeout(hdptx->grf, GRF_HDPTX_STATUS,
+				       status, FIELD_GET(HDPTX_O_PLL_LOCK_DONE, status),
+				       50, 1000);
+	if (ret) {
+		dev_err(hdptx->dev, "Failed to get phy pll lock: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void rk_hdptx_phy_lane_disable(struct rk_hdptx_phy *hdptx)
+{
+	reset_control_assert(hdptx->rsts[RST_LANE].rstc);
+
+	regmap_update_bits(hdptx->regmap, LNTOP_REG(0207), LANE_EN,
+			   FIELD_PREP(LANE_EN, 0x0));
+
+	regmap_write(hdptx->grf, GRF_HDPTX_CON0,
+		     HDPTX_I_PLL_EN << 16 | FIELD_PREP(HDPTX_I_PLL_EN, 0x0));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(0008), OVRD_LCPLL_EN_MASK | LCPLL_EN_MASK,
+			   FIELD_PREP(OVRD_LCPLL_EN_MASK, 0x1) |
+			   FIELD_PREP(LCPLL_EN_MASK, 0x0));
+
+	regmap_update_bits(hdptx->regmap, CMN_REG(003d), OVRD_ROPLL_EN_MASK | ROPLL_EN_MASK,
+			   FIELD_PREP(OVRD_ROPLL_EN_MASK, 0x1) |
+			   FIELD_PREP(ROPLL_EN_MASK, 0x0));
+}
+
+static int rk_hdptx_phy_set_lanes(struct rk_hdptx_phy *hdptx,
+				  struct phy_configure_opts_dp *dp)
+{
+	if (!dp->lanes) {
+		rk_hdptx_phy_lane_disable(hdptx);
+		return 0;
+	}
+
+	regmap_update_bits(hdptx->regmap, LNTOP_REG(0207), LANE_EN,
+			   FIELD_PREP(LANE_EN, GENMASK(dp->lanes - 1, 0)));
+
+	return 0;
+}
+
+static void rk_hdptx_phy_set_voltage(struct rk_hdptx_phy *hdptx,
+				     struct phy_configure_opts_dp *dp,
+				     u8 lane)
+{
+	const struct tx_drv_ctrl *ctrl;
+	u32 offset = lane * 0x400;
+
+	switch (dp->link_rate) {
+	case 1620:
+		ctrl = &tx_drv_ctrl_rbr[dp->voltage[lane]][dp->pre[lane]];
+		regmap_update_bits(hdptx->regmap, LANE_REG(030a) + offset,
+				   LN_TX_JEQ_EVEN_CTRL_RBR_MASK,
+				   FIELD_PREP(LN_TX_JEQ_EVEN_CTRL_RBR_MASK,
+				   ctrl->tx_jeq_even_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(030c) + offset,
+				   LN_TX_JEQ_ODD_CTRL_RBR_MASK,
+				   FIELD_PREP(LN_TX_JEQ_ODD_CTRL_RBR_MASK,
+				   ctrl->tx_jeq_odd_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(0311) + offset,
+				   LN_TX_SER_40BIT_EN_RBR_MASK,
+				   FIELD_PREP(LN_TX_SER_40BIT_EN_RBR_MASK, 0x1));
+		break;
+	case 2700:
+		ctrl = &tx_drv_ctrl_hbr[dp->voltage[lane]][dp->pre[lane]];
+		regmap_update_bits(hdptx->regmap, LANE_REG(030b) + offset,
+				   LN_TX_JEQ_EVEN_CTRL_HBR_MASK,
+				   FIELD_PREP(LN_TX_JEQ_EVEN_CTRL_HBR_MASK,
+				   ctrl->tx_jeq_even_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(030d) + offset,
+				   LN_TX_JEQ_ODD_CTRL_HBR_MASK,
+				   FIELD_PREP(LN_TX_JEQ_ODD_CTRL_HBR_MASK,
+				   ctrl->tx_jeq_odd_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(0311) + offset,
+				   LN_TX_SER_40BIT_EN_HBR_MASK,
+				   FIELD_PREP(LN_TX_SER_40BIT_EN_HBR_MASK, 0x1));
+		break;
+	case 5400:
+	default:
+		ctrl = &tx_drv_ctrl_hbr2[dp->voltage[lane]][dp->pre[lane]];
+		regmap_update_bits(hdptx->regmap, LANE_REG(030b) + offset,
+				   LN_TX_JEQ_EVEN_CTRL_HBR2_MASK,
+				   FIELD_PREP(LN_TX_JEQ_EVEN_CTRL_HBR2_MASK,
+				   ctrl->tx_jeq_even_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(030d) + offset,
+				   LN_TX_JEQ_ODD_CTRL_HBR2_MASK,
+				   FIELD_PREP(LN_TX_JEQ_ODD_CTRL_HBR2_MASK,
+				   ctrl->tx_jeq_odd_ctrl));
+		regmap_update_bits(hdptx->regmap, LANE_REG(0311) + offset,
+				   LN_TX_SER_40BIT_EN_HBR2_MASK,
+				   FIELD_PREP(LN_TX_SER_40BIT_EN_HBR2_MASK, 0x1));
+		break;
+	}
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(0303) + offset,
+			   OVRD_LN_TX_DRV_LVL_CTRL_MASK | LN_TX_DRV_LVL_CTRL_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_LVL_CTRL_MASK, 0x1) |
+			   FIELD_PREP(LN_TX_DRV_LVL_CTRL_MASK,
+				      ctrl->tx_drv_lvl_ctrl));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0304) + offset,
+			   OVRD_LN_TX_DRV_POST_LVL_CTRL_MASK |
+			   LN_TX_DRV_POST_LVL_CTRL_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_POST_LVL_CTRL_MASK, 0x1) |
+			   FIELD_PREP(LN_TX_DRV_POST_LVL_CTRL_MASK,
+				      ctrl->tx_drv_post_lvl_ctrl));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0305) + offset,
+			   OVRD_LN_TX_DRV_PRE_LVL_CTRL_MASK |
+			   LN_TX_DRV_PRE_LVL_CTRL_MASK,
+			   FIELD_PREP(OVRD_LN_TX_DRV_PRE_LVL_CTRL_MASK, 0x1) |
+			   FIELD_PREP(LN_TX_DRV_PRE_LVL_CTRL_MASK,
+				      ctrl->tx_drv_pre_lvl_ctrl));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0306) + offset,
+			   LN_ANA_TX_DRV_IDRV_IDN_CTRL_MASK |
+			   LN_ANA_TX_DRV_IDRV_IUP_CTRL_MASK |
+			   LN_ANA_TX_DRV_ACCDRV_EN_MASK,
+			   FIELD_PREP(LN_ANA_TX_DRV_IDRV_IDN_CTRL_MASK,
+				      ctrl->ana_tx_drv_idrv_idn_ctrl) |
+			   FIELD_PREP(LN_ANA_TX_DRV_IDRV_IUP_CTRL_MASK,
+				      ctrl->ana_tx_drv_idrv_iup_ctrl) |
+			   FIELD_PREP(LN_ANA_TX_DRV_ACCDRV_EN_MASK,
+				      ctrl->ana_tx_drv_accdrv_en));
+	regmap_update_bits(hdptx->regmap, LANE_REG(0307) + offset,
+			   LN_ANA_TX_DRV_ACCDRV_POL_SEL_MASK |
+			   LN_ANA_TX_DRV_ACCDRV_CTRL_MASK,
+			   FIELD_PREP(LN_ANA_TX_DRV_ACCDRV_POL_SEL_MASK, 0x1) |
+			   FIELD_PREP(LN_ANA_TX_DRV_ACCDRV_CTRL_MASK,
+				      ctrl->ana_tx_drv_accdrv_ctrl));
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(030a) + offset,
+			   LN_ANA_TX_JEQ_EN_MASK,
+			   FIELD_PREP(LN_ANA_TX_JEQ_EN_MASK, ctrl->ana_tx_jeq_en));
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(0310) + offset,
+			   LN_ANA_TX_SYNC_LOSS_DET_MODE_MASK,
+			   FIELD_PREP(LN_ANA_TX_SYNC_LOSS_DET_MODE_MASK, 0x3));
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(0316) + offset,
+			   LN_ANA_TX_SER_VREG_GAIN_CTRL_MASK,
+			   FIELD_PREP(LN_ANA_TX_SER_VREG_GAIN_CTRL_MASK, 0x2));
+
+	regmap_update_bits(hdptx->regmap, LANE_REG(031b) + offset,
+			   LN_ANA_TX_RESERVED_MASK,
+			   FIELD_PREP(LN_ANA_TX_RESERVED_MASK, 0x1));
+}
+
+static int rk_hdptx_phy_set_voltages(struct rk_hdptx_phy *hdptx,
+				     struct phy_configure_opts_dp *dp)
+{
+	u8 lane;
+	u32 status;
+	int ret;
+
+	for (lane = 0; lane < dp->lanes; lane++)
+		rk_hdptx_phy_set_voltage(hdptx, dp, lane);
+
+	reset_control_deassert(hdptx->rsts[RST_LANE].rstc);
+
+	ret = regmap_read_poll_timeout(hdptx->grf, GRF_HDPTX_STATUS,
+				       status, FIELD_GET(HDPTX_O_PHY_RDY, status),
+				       50, 5000);
+	if (ret) {
+		dev_err(hdptx->dev, "Failed to get phy ready: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rk_hdptx_phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct rk_hdptx_phy *hdptx = phy_get_drvdata(phy);
+	enum phy_mode mode = phy_get_mode(phy);
+	int ret;
+
+	if (mode != PHY_MODE_DP)
+		return -EINVAL;
+
+	ret = rk_hdptx_phy_verify_config(hdptx, &opts->dp);
+	if (ret) {
+		dev_err(hdptx->dev, "invalid params for phy configure\n");
+		return ret;
+	}
+
+	if (opts->dp.set_rate) {
+		ret = rk_hdptx_phy_set_rate(hdptx, &opts->dp);
+		if (ret) {
+			dev_err(hdptx->dev, "failed to set rate: %d\n", ret);
+			return ret;
+		}
+	}
+
+	if (opts->dp.set_lanes) {
+		ret = rk_hdptx_phy_set_lanes(hdptx, &opts->dp);
+		if (ret) {
+			dev_err(hdptx->dev, "failed to set lanes: %d\n", ret);
+			return ret;
+		}
+	}
+
+	if (opts->dp.set_voltages) {
+		ret = rk_hdptx_phy_set_voltages(hdptx, &opts->dp);
+		if (ret) {
+			dev_err(hdptx->dev, "failed to set voltages: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static const struct phy_ops rk_hdptx_phy_ops = {
 	.power_on  = rk_hdptx_phy_power_on,
 	.power_off = rk_hdptx_phy_power_off,
+	.set_mode  = rk_hdptx_phy_set_mode,
+	.configure = rk_hdptx_phy_configure,
 	.owner	   = THIS_MODULE,
 };
 
-- 
2.34.1

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Heiko Stübner]

Hi Damon,

Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
> pre-emphasis configurations of each link rate have been verified according
> to the eDP 1.3 requirements.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---

[ ... huge block of DP phy support ...]

yes that block was huge, but I also don't see a way to split that up in a
useful way, so it should be fine.


> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> +				 int submode)
> +{
> +	return 0;
> +}

I think it might make sense to go the same way as the DCPHY and also
naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .

See [0] for Sebastians initial suggestion regarding the DC-PHY.
The naneng combophy already uses that scheme of mode-selection too.

There is of course the issue of backwards-compatibility, but that can be
worked around in the binding with something like:

 '#phy-cells':
    enum: [0, 1]
    description: |
      If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
      If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
        - PHY_TYPE_HDMI
        - PHY_TYPE_DP
      See include/dt-bindings/phy/phy.h for constants.

PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
but PHY_TYPE_DP is already there.

That way we would standardize on one form of accessing phy-types
on rk3588 :-) .

Also see the Mediatek CSI rx phy doing this too already [1]


Heiko

[0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Hi Heiko:

On 2024/11/27 17:29, Heiko Stübner wrote:
> Hi Damon,
> 
> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
>> Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
>> pre-emphasis configurations of each link rate have been verified according
>> to the eDP 1.3 requirements.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> ---
> 
> [ ... huge block of DP phy support ...]
> 
> yes that block was huge, but I also don't see a way to split that up in a
> useful way, so it should be fine.
> 

As for the huge block of DP phy support, I will try to use the existing 
rk_hdptx_multi_reg_write() to set regs in next version, maybe the way 
can make the codes more concise.

> 
>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
>> +				 int submode)
>> +{
>> +	return 0;
>> +}
> 
> I think it might make sense to go the same way as the DCPHY and also
> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> 
> See [0] for Sebastians initial suggestion regarding the DC-PHY.
> The naneng combophy already uses that scheme of mode-selection too.
> 
> There is of course the issue of backwards-compatibility, but that can be
> worked around in the binding with something like:
> 
>   '#phy-cells':
>      enum: [0, 1]
>      description: |
>        If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
>        If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
>          - PHY_TYPE_HDMI
>          - PHY_TYPE_DP
>        See include/dt-bindings/phy/phy.h for constants.
> 
> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> but PHY_TYPE_DP is already there.
> 
> That way we would standardize on one form of accessing phy-types
> on rk3588 :-) .
> 
> Also see the Mediatek CSI rx phy doing this too already [1]
> 
> 
> Heiko
> 
> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> 

It is really a nice way to separate HDMI and DP modes.

> 
> 
> 
> 

Best regards,
Damon

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Heiko Stübner]

Hi Damon,

Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
> Hi Heiko:
> 
> On 2024/11/27 17:29, Heiko Stübner wrote:
> > Hi Damon,
> > 
> > Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> >> Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
> >> pre-emphasis configurations of each link rate have been verified according
> >> to the eDP 1.3 requirements.
> >>
> >> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> >> ---
> > 
> > [ ... huge block of DP phy support ...]
> > 
> > yes that block was huge, but I also don't see a way to split that up in a
> > useful way, so it should be fine.
> > 
> 
> As for the huge block of DP phy support, I will try to use the existing 
> rk_hdptx_multi_reg_write() to set regs in next version, maybe the way 
> can make the codes more concise.

I actually did like the the dp-side of the phy code.

That you need to add all the DP stuff can't be helped and I actually find
real functions nicer than having anonymous register writes.

I.e. the hdmi-side with its register lists does write "magic" values to
registers.

So personally I'd just leave the dp-functions as is please, until someone
does complain (I was not trying to complain, just mentioned why I cut
it from the reply :-) )


Thanks
Heiko


> >> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> >> +				 int submode)
> >> +{
> >> +	return 0;
> >> +}
> > 
> > I think it might make sense to go the same way as the DCPHY and also
> > naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> > 
> > See [0] for Sebastians initial suggestion regarding the DC-PHY.
> > The naneng combophy already uses that scheme of mode-selection too.
> > 
> > There is of course the issue of backwards-compatibility, but that can be
> > worked around in the binding with something like:
> > 
> >   '#phy-cells':
> >      enum: [0, 1]
> >      description: |
> >        If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
> >        If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
> >          - PHY_TYPE_HDMI
> >          - PHY_TYPE_DP
> >        See include/dt-bindings/phy/phy.h for constants.
> > 
> > PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> > but PHY_TYPE_DP is already there.
> > 
> > That way we would standardize on one form of accessing phy-types
> > on rk3588 :-) .
> > 
> > Also see the Mediatek CSI rx phy doing this too already [1]
> > 
> > 
> > Heiko
> > 
> > [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> > 
> 
> It is really a nice way to separate HDMI and DP modes.
> 
> > 
> > 
> > 
> > 
> 
> Best regards,
> Damon
> 
> 

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Hi Heiko,

On 2024/11/27 19:04, Heiko Stübner wrote:
> Hi Damon,
> 
> Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
>> Hi Heiko:
>>
>> On 2024/11/27 17:29, Heiko Stübner wrote:
>>> Hi Damon,
>>>
>>> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
>>>> Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
>>>> pre-emphasis configurations of each link rate have been verified according
>>>> to the eDP 1.3 requirements.
>>>>
>>>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>>> ---
>>>
>>> [ ... huge block of DP phy support ...]
>>>
>>> yes that block was huge, but I also don't see a way to split that up in a
>>> useful way, so it should be fine.
>>>
>>
>> As for the huge block of DP phy support, I will try to use the existing
>> rk_hdptx_multi_reg_write() to set regs in next version, maybe the way
>> can make the codes more concise.
> 
> I actually did like the the dp-side of the phy code.
> 
> That you need to add all the DP stuff can't be helped and I actually find
> real functions nicer than having anonymous register writes.
> 
> I.e. the hdmi-side with its register lists does write "magic" values to
> registers.
> 
> So personally I'd just leave the dp-functions as is please, until someone
> does complain (I was not trying to complain, just mentioned why I cut
> it from the reply :-) )
> 
> 
> Thanks
> Heiko
> 
> 
>>>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
>>>> +				 int submode)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>
>>> I think it might make sense to go the same way as the DCPHY and also
>>> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
>>>
>>> See [0] for Sebastians initial suggestion regarding the DC-PHY.
>>> The naneng combophy already uses that scheme of mode-selection too.
>>>
>>> There is of course the issue of backwards-compatibility, but that can be
>>> worked around in the binding with something like:
>>>
>>>    '#phy-cells':
>>>       enum: [0, 1]
>>>       description: |
>>>         If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
>>>         If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
>>>           - PHY_TYPE_HDMI
>>>           - PHY_TYPE_DP
>>>         See include/dt-bindings/phy/phy.h for constants.
>>>
>>> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
>>> but PHY_TYPE_DP is already there.
>>>
>>> That way we would standardize on one form of accessing phy-types
>>> on rk3588 :-) .
>>>
>>> Also see the Mediatek CSI rx phy doing this too already [1]
>>>
>>>
>>> Heiko
>>>
>>> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
>>>
>>
>> It is really a nice way to separate HDMI and DP modes.

I apologize for reopening the discussion about the phy-types setting.

With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic 
switching can be achieved, which just depends on the right setting of
enum phy_mode in include/linux/phy/phy.h. So the previous way of 
configuring phy mode may be also good.

And other phys may want to support dynamic switching too, like the 
Rockchip USBDP combo phy.

>>
>>>
>>>
>>>
>>>
>>
>> Best regards,
>> Damon
>>
>>
> 
> 
> 
> 
> 
> 
Best regards,
Damon

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Heiko Stübner]

Hi Damon,

Am Freitag, 29. November 2024, 03:43:57 CET schrieb Damon Ding:
> On 2024/11/27 19:04, Heiko Stübner wrote:
> > Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
> >> On 2024/11/27 17:29, Heiko Stübner wrote:
> >>> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> >>>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> >>>> +				 int submode)
> >>>> +{
> >>>> +	return 0;
> >>>> +}
> >>>
> >>> I think it might make sense to go the same way as the DCPHY and also
> >>> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> >>>
> >>> See [0] for Sebastians initial suggestion regarding the DC-PHY.
> >>> The naneng combophy already uses that scheme of mode-selection too.
> >>>
> >>> There is of course the issue of backwards-compatibility, but that can be
> >>> worked around in the binding with something like:
> >>>
> >>>    '#phy-cells':
> >>>       enum: [0, 1]
> >>>       description: |
> >>>         If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
> >>>         If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
> >>>           - PHY_TYPE_HDMI
> >>>           - PHY_TYPE_DP
> >>>         See include/dt-bindings/phy/phy.h for constants.
> >>>
> >>> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> >>> but PHY_TYPE_DP is already there.
> >>>
> >>> That way we would standardize on one form of accessing phy-types
> >>> on rk3588 :-) .
> >>>
> >>> Also see the Mediatek CSI rx phy doing this too already [1]
> >>>
> >>>
> >>> Heiko
> >>>
> >>> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> >>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> >>>
> >>
> >> It is really a nice way to separate HDMI and DP modes.
> 
> I apologize for reopening the discussion about the phy-types setting.

there is definitly no need to apologize. We're trying to find the best
solution afterall :-) .


> With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic 
> switching can be achieved, which just depends on the right setting of
> enum phy_mode in include/linux/phy/phy.h. So the previous way of 
> configuring phy mode may be also good.

I think the deciding factor is, is there a use-case for needing to switch
modes at runtime.

I do think the mode for the dc-phy and also the hdptx-phy is pretty much
decided by the board design.

I.e. when you end up in a DP-connector (or eDP-panel) on your board you
need DP mode, and when you end up in a hdmi-connector you need the
HDMI phy mode.

So I think the phy-mode for the hdptx-phy is largely dictated by the static
board definition (like devicetree), hence going with the dt-argument for
the mode.

Like similar to the Naneng combophy, selecting its mode via argument
because deciding if it ends up in a sata port is a board-design thing.


Is there a use-case where you need to switch at runtime between
hdmi and eDP? Like starting the phy in eDP mode but then needing
to switch to HDMI mode, while the device is running?


> And other phys may want to support dynamic switching too, like the 
> Rockchip USBDP combo phy.

I guess USBDP is special in that in also does both modes dynamical
depending on its use (like type-c with option DP altmode)


Have a great weekend
Heiko

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5874 bytes --]

Hi,

On Sat, Nov 30, 2024 at 09:25:12PM +0100, Heiko Stübner wrote:
> Am Freitag, 29. November 2024, 03:43:57 CET schrieb Damon Ding:
> > On 2024/11/27 19:04, Heiko Stübner wrote:
> > > Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
> > >> On 2024/11/27 17:29, Heiko Stübner wrote:
> > >>> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> > >>>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> > >>>> +				 int submode)
> > >>>> +{
> > >>>> +	return 0;
> > >>>> +}
> > >>>analogix_dp_phy_power_on
> > >>> I think it might make sense to go the same way as the DCPHY and also
> > >>> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> > >>>
> > >>> See [0] for Sebastians initial suggestion regarding the DC-PHY.
> > >>> The naneng combophy already uses that scheme of mode-selection too.
> > >>>
> > >>> There is of course the issue of backwards-compatibility, but that can be
> > >>> worked around in the binding with something like:
> > >>>
> > >>>    '#phy-cells':
> > >>>       enum: [0, 1]
> > >>>       description: |
> > >>>         If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
> > >>>         If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
> > >>>           - PHY_TYPE_HDMI
> > >>>           - PHY_TYPE_DP
> > >>>         See include/dt-bindings/phy/phy.h for constants.
> > >>>
> > >>> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> > >>> but PHY_TYPE_DP is already there.
> > >>>
> > >>> That way we would standardize on one form of accessing phy-types
> > >>> on rk3588 :-) .
> > >>>
> > >>> Also see the Mediatek CSI rx phy doing this too already [1]
> > >>>
> > >>>
> > >>> Heiko
> > >>>
> > >>> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> > >>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> > >>>
> > >>
> > >> It is really a nice way to separate HDMI and DP modes.
> > 
> > I apologize for reopening the discussion about the phy-types setting.
> 
> there is definitly no need to apologize. We're trying to find the best
> solution afterall :-) .
> 
> > With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic 
> > switching can be achieved, which just depends on the right setting of
> > enum phy_mode in include/linux/phy/phy.h. So the previous way of 
> > configuring phy mode may be also good.
> 
> I think the deciding factor is, is there a use-case for needing to switch
> modes at runtime.
> 
> I do think the mode for the dc-phy and also the hdptx-phy is pretty much
> decided by the board design.
> 
> I.e. when you end up in a DP-connector (or eDP-panel) on your board you
> need DP mode, and when you end up in a hdmi-connector you need the
> HDMI phy mode.
> 
> So I think the phy-mode for the hdptx-phy is largely dictated by the static
> board definition (like devicetree), hence going with the dt-argument for
> the mode.
> 
> Like similar to the Naneng combophy, selecting its mode via argument
> because deciding if it ends up in a sata port is a board-design thing.
>
> Is there a use-case where you need to switch at runtime between
> HDMI and eDP? Like starting the phy in eDP mode but then needing
> to switch to HDMI mode, while the device is running?

I believe the eDP controller can only use the PHY in eDP mode and
the HDMI controller can only use it in HDMI mode. So in order to
support runtime switching, the following options are possible:

1. Enable both controllers, the PHY decides which one is really
   used, the other one is basically a non-functional dummy device
   until the PHY is reconfigured. This requires the set_mode()
   callback, since the HDMI and eDP drivers both expect their
   PHY to be enabled.

2. Properly enable / disable the used controller, so that only one
   controller is active at the same time. In this case the switching
   is handled one layer above and the PHY has nothing to do with it.
   The phy_enable call from each controller would just set it up in
   the right mode.

I guess option 1 is the hacked solution, which is easier to
implement as DRM's hotplug abilities are quite limited at the moment
as far as I know. I think the second solution looks much cleaner and
should be prefered for upstream. That solution does not require
calling set_mode() for runtime switching making this whole argument
void :)

FWIW I think the DT argument based mode setting and the runtime set_mode
are not necessarily mutual exclusive. In theory one could support both
and adding set_mode support later does not change any ABI as far as
I can see. Basically handle it like pin mux/configuration settings,
which are usually automatically handled by the device core based on
DT information, except for some drivers which have special needs.

> > And other phys may want to support dynamic switching too, like the 
> > Rockchip USBDP combo phy.
> 
> I guess USBDP is special in that in also does both modes dynamical
> depending on its use (like type-c with option DP altmode)

The USBDP PHY is indeed quite different from the PHYs in your list
above, but for a different reason: All the combined PHYs you listed
above only support one mode at the same time. E.g. you need to
decide between PCIe, SATA and USB3 mode for the Naneng combophy.

For the USBDP PHY the modes are not mutually exclusive. The USB
controller can request the USBDP PHY in USB mode at the same time
as the DP controller requests it in DP mode. Some additional
registers configure how the lanes are being muxed. A typcial setup
would be 2 lanes for USB3 and 2 lanes for DP.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Hi,

On 2024/12/2 6:59, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Nov 30, 2024 at 09:25:12PM +0100, Heiko Stübner wrote:
>> Am Freitag, 29. November 2024, 03:43:57 CET schrieb Damon Ding:
>>> On 2024/11/27 19:04, Heiko Stübner wrote:
>>>> Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
>>>>> On 2024/11/27 17:29, Heiko Stübner wrote:
>>>>>> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
>>>>>>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
>>>>>>> +				 int submode)
>>>>>>> +{
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>> analogix_dp_phy_power_on
>>>>>> I think it might make sense to go the same way as the DCPHY and also
>>>>>> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
>>>>>>
>>>>>> See [0] for Sebastians initial suggestion regarding the DC-PHY.
>>>>>> The naneng combophy already uses that scheme of mode-selection too.
>>>>>>
>>>>>> There is of course the issue of backwards-compatibility, but that can be
>>>>>> worked around in the binding with something like:
>>>>>>
>>>>>>     '#phy-cells':
>>>>>>        enum: [0, 1]
>>>>>>        description: |
>>>>>>          If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
>>>>>>          If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
>>>>>>            - PHY_TYPE_HDMI
>>>>>>            - PHY_TYPE_DP
>>>>>>          See include/dt-bindings/phy/phy.h for constants.
>>>>>>
>>>>>> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
>>>>>> but PHY_TYPE_DP is already there.
>>>>>>
>>>>>> That way we would standardize on one form of accessing phy-types
>>>>>> on rk3588 :-) .
>>>>>>
>>>>>> Also see the Mediatek CSI rx phy doing this too already [1]
>>>>>>
>>>>>>
>>>>>> Heiko
>>>>>>
>>>>>> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
>>>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
>>>>>>
>>>>>
>>>>> It is really a nice way to separate HDMI and DP modes.
>>>
>>> I apologize for reopening the discussion about the phy-types setting.
>>
>> there is definitly no need to apologize. We're trying to find the best
>> solution afterall :-) .
>>
>>> With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic
>>> switching can be achieved, which just depends on the right setting of
>>> enum phy_mode in include/linux/phy/phy.h. So the previous way of
>>> configuring phy mode may be also good.
>>
>> I think the deciding factor is, is there a use-case for needing to switch
>> modes at runtime.
>>
>> I do think the mode for the dc-phy and also the hdptx-phy is pretty much
>> decided by the board design.
>>
>> I.e. when you end up in a DP-connector (or eDP-panel) on your board you
>> need DP mode, and when you end up in a hdmi-connector you need the
>> HDMI phy mode.
>>
>> So I think the phy-mode for the hdptx-phy is largely dictated by the static
>> board definition (like devicetree), hence going with the dt-argument for
>> the mode.
>>
>> Like similar to the Naneng combophy, selecting its mode via argument
>> because deciding if it ends up in a sata port is a board-design thing.
>>
>> Is there a use-case where you need to switch at runtime between
>> HDMI and eDP? Like starting the phy in eDP mode but then needing
>> to switch to HDMI mode, while the device is running?

Indeed, we have the board as you described, on which the DP-connector 
and HDMI-connector both have been configured.

And the dynamic switching is more useful for RK3576, which has the same 
eDP/HDMI design as RK3588 but only one eDP controller/HDMI 
controller/HDPTX phy. We can only enable both of eDP/HDMI by this way.

> 
> I believe the eDP controller can only use the PHY in eDP mode and
> the HDMI controller can only use it in HDMI mode. So in order to
> support runtime switching, the following options are possible:
> 
> 1. Enable both controllers, the PHY decides which one is really
>     used, the other one is basically a non-functional dummy device
>     until the PHY is reconfigured. This requires the set_mode()
>     callback, since the HDMI and eDP drivers both expect their
>     PHY to be enabled.
> 
> 2. Properly enable / disable the used controller, so that only one
>     controller is active at the same time. In this case the switching
>     is handled one layer above and the PHY has nothing to do with it.
>     The phy_enable call from each controller would just set it up in
>     the right mode.
> 
> I guess option 1 is the hacked solution, which is easier to
> implement as DRM's hotplug abilities are quite limited at the moment
> as far as I know. I think the second solution looks much cleaner and
> should be prefered for upstream. That solution does not require
> calling set_mode() for runtime switching making this whole argument
> void :)
> 

Your friendly and detailed analysis has brought me some valuable 
insights. :)

The option 2 is really a better way to support the dynamic switching, 
and we still need the .set_mode() to select the configurations for 
either eDP or HDMI in HDPTX phy at the controller level. Would you mind
elaborating on the useful way to choose the phy mode for the second 
solution?

> FWIW I think the DT argument based mode setting and the runtime set_mode
> are not necessarily mutual exclusive. In theory one could support both
> and adding set_mode support later does not change any ABI as far as
> I can see. Basically handle it like pin mux/configuration settings,
> which are usually automatically handled by the device core based on
> DT information, except for some drivers which have special needs.
> 

>>> And other phys may want to support dynamic switching too, like the
>>> Rockchip USBDP combo phy.
>>
>> I guess USBDP is special in that in also does both modes dynamical
>> depending on its use (like type-c with option DP altmode)
> 
> The USBDP PHY is indeed quite different from the PHYs in your list
> above, but for a different reason: All the combined PHYs you listed
> above only support one mode at the same time. E.g. you need to
> decide between PCIe, SATA and USB3 mode for the Naneng combophy.
> 
> For the USBDP PHY the modes are not mutually exclusive. The USB
> controller can request the USBDP PHY in USB mode at the same time
> as the DP controller requests it in DP mode. Some additional
> registers configure how the lanes are being muxed. A typcial setup
> would be 2 lanes for USB3 and 2 lanes for DP.
> 
> Greetings,
> 
> -- Sebastian

Best regards,
Damon

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 7840 bytes --]

Hi,

On Mon, Dec 02, 2024 at 11:28:13AM +0800, Damon Ding wrote:
> Hi,
> 
> On 2024/12/2 6:59, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Sat, Nov 30, 2024 at 09:25:12PM +0100, Heiko Stübner wrote:
> > > Am Freitag, 29. November 2024, 03:43:57 CET schrieb Damon Ding:
> > > > On 2024/11/27 19:04, Heiko Stübner wrote:
> > > > > Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
> > > > > > On 2024/11/27 17:29, Heiko Stübner wrote:
> > > > > > > Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> > > > > > > > +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> > > > > > > > +				 int submode)
> > > > > > > > +{
> > > > > > > > +	return 0;
> > > > > > > > +}
> > > > > > > analogix_dp_phy_power_on
> > > > > > > I think it might make sense to go the same way as the DCPHY and also
> > > > > > > naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> > > > > > > 
> > > > > > > See [0] for Sebastians initial suggestion regarding the DC-PHY.
> > > > > > > The naneng combophy already uses that scheme of mode-selection too.
> > > > > > > 
> > > > > > > There is of course the issue of backwards-compatibility, but that can be
> > > > > > > worked around in the binding with something like:
> > > > > > > 
> > > > > > >     '#phy-cells':
> > > > > > >        enum: [0, 1]
> > > > > > >        description: |
> > > > > > >          If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
> > > > > > >          If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
> > > > > > >            - PHY_TYPE_HDMI
> > > > > > >            - PHY_TYPE_DP
> > > > > > >          See include/dt-bindings/phy/phy.h for constants.
> > > > > > > 
> > > > > > > PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> > > > > > > but PHY_TYPE_DP is already there.
> > > > > > > 
> > > > > > > That way we would standardize on one form of accessing phy-types
> > > > > > > on rk3588 :-) .
> > > > > > > 
> > > > > > > Also see the Mediatek CSI rx phy doing this too already [1]
> > > > > > > 
> > > > > > > 
> > > > > > > Heiko
> > > > > > > 
> > > > > > > [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> > > > > > > 
> > > > > > 
> > > > > > It is really a nice way to separate HDMI and DP modes.
> > > > 
> > > > I apologize for reopening the discussion about the phy-types setting.
> > > 
> > > there is definitly no need to apologize. We're trying to find the best
> > > solution afterall :-) .
> > > 
> > > > With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic
> > > > switching can be achieved, which just depends on the right setting of
> > > > enum phy_mode in include/linux/phy/phy.h. So the previous way of
> > > > configuring phy mode may be also good.
> > > 
> > > I think the deciding factor is, is there a use-case for needing to switch
> > > modes at runtime.
> > > 
> > > I do think the mode for the dc-phy and also the hdptx-phy is pretty much
> > > decided by the board design.
> > > 
> > > I.e. when you end up in a DP-connector (or eDP-panel) on your board you
> > > need DP mode, and when you end up in a hdmi-connector you need the
> > > HDMI phy mode.
> > > 
> > > So I think the phy-mode for the hdptx-phy is largely dictated by the static
> > > board definition (like devicetree), hence going with the dt-argument for
> > > the mode.
> > > 
> > > Like similar to the Naneng combophy, selecting its mode via argument
> > > because deciding if it ends up in a sata port is a board-design thing.
> > > 
> > > Is there a use-case where you need to switch at runtime between
> > > HDMI and eDP? Like starting the phy in eDP mode but then needing
> > > to switch to HDMI mode, while the device is running?
> 
> Indeed, we have the board as you described, on which the DP-connector and
> HDMI-connector both have been configured.
> 
> And the dynamic switching is more useful for RK3576, which has the same
> eDP/HDMI design as RK3588 but only one eDP controller/HDMI controller/HDPTX
> phy. We can only enable both of eDP/HDMI by this way.
> 
> > 
> > I believe the eDP controller can only use the PHY in eDP mode and
> > the HDMI controller can only use it in HDMI mode. So in order to
> > support runtime switching, the following options are possible:
> > 
> > 1. Enable both controllers, the PHY decides which one is really
> >     used, the other one is basically a non-functional dummy device
> >     until the PHY is reconfigured. This requires the set_mode()
> >     callback, since the HDMI and eDP drivers both expect their
> >     PHY to be enabled.
> > 
> > 2. Properly enable / disable the used controller, so that only one
> >     controller is active at the same time. In this case the switching
> >     is handled one layer above and the PHY has nothing to do with it.
> >     The phy_enable call from each controller would just set it up in
> >     the right mode.
> > 
> > I guess option 1 is the hacked solution, which is easier to
> > implement as DRM's hotplug abilities are quite limited at the moment
> > as far as I know. I think the second solution looks much cleaner and
> > should be prefered for upstream. That solution does not require
> > calling set_mode() for runtime switching making this whole argument
> > void :)
> > 
> 
> Your friendly and detailed analysis has brought me some valuable insights.
> :)
> 
> The option 2 is really a better way to support the dynamic switching, and we
> still need the .set_mode() to select the configurations for either eDP or
> HDMI in HDPTX phy at the controller level. Would you mind
> elaborating on the useful way to choose the phy mode for the second
> solution?

The xlate function to handle the arguments is called when the PHY
device is looked up. So the devm_phy_get(dp->dev, "dp") call in
analogix_dp_probe() would setup the PHY in DP mode.

Similarily the call to devm_of_phy_get_by_index() in
dw_hdmi_qp_rockchip_bind() would set the PHY in HDMI mode.

So the PHY mode is correct as long as only one controller driver
is bound at the same time.

Greetings,

-- Sebastian

> > FWIW I think the DT argument based mode setting and the runtime set_mode
> > are not necessarily mutual exclusive. In theory one could support both
> > and adding set_mode support later does not change any ABI as far as
> > I can see. Basically handle it like pin mux/configuration settings,
> > which are usually automatically handled by the device core based on
> > DT information, except for some drivers which have special needs.
> > 
> 
> > > > And other phys may want to support dynamic switching too, like the
> > > > Rockchip USBDP combo phy.
> > > 
> > > I guess USBDP is special in that in also does both modes dynamical
> > > depending on its use (like type-c with option DP altmode)
> > 
> > The USBDP PHY is indeed quite different from the PHYs in your list
> > above, but for a different reason: All the combined PHYs you listed
> > above only support one mode at the same time. E.g. you need to
> > decide between PCIe, SATA and USB3 mode for the Naneng combophy.
> > 
> > For the USBDP PHY the modes are not mutually exclusive. The USB
> > controller can request the USBDP PHY in USB mode at the same time
> > as the DP controller requests it in DP mode. Some additional
> > registers configure how the lanes are being muxed. A typcial setup
> > would be 2 lanes for USB3 and 2 lanes for DP.
> > 
> > Greetings,
> > 
> > -- Sebastian
> 
> Best regards,
> Damon
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Hi Sebastian,

On 2024/12/2 22:41, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Dec 02, 2024 at 11:28:13AM +0800, Damon Ding wrote:
>> Hi,
>>
>> On 2024/12/2 6:59, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> On Sat, Nov 30, 2024 at 09:25:12PM +0100, Heiko Stübner wrote:
>>>> Am Freitag, 29. November 2024, 03:43:57 CET schrieb Damon Ding:
>>>>> On 2024/11/27 19:04, Heiko Stübner wrote:
>>>>>> Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
>>>>>>> On 2024/11/27 17:29, Heiko Stübner wrote:
>>>>>>>> Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
>>>>>>>>> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
>>>>>>>>> +				 int submode)
>>>>>>>>> +{
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>> analogix_dp_phy_power_on
>>>>>>>> I think it might make sense to go the same way as the DCPHY and also
>>>>>>>> naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
>>>>>>>>
>>>>>>>> See [0] for Sebastians initial suggestion regarding the DC-PHY.
>>>>>>>> The naneng combophy already uses that scheme of mode-selection too.
>>>>>>>>
>>>>>>>> There is of course the issue of backwards-compatibility, but that can be
>>>>>>>> worked around in the binding with something like:
>>>>>>>>
>>>>>>>>      '#phy-cells':
>>>>>>>>         enum: [0, 1]
>>>>>>>>         description: |
>>>>>>>>           If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
>>>>>>>>           If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
>>>>>>>>             - PHY_TYPE_HDMI
>>>>>>>>             - PHY_TYPE_DP
>>>>>>>>           See include/dt-bindings/phy/phy.h for constants.
>>>>>>>>
>>>>>>>> PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
>>>>>>>> but PHY_TYPE_DP is already there.
>>>>>>>>
>>>>>>>> That way we would standardize on one form of accessing phy-types
>>>>>>>> on rk3588 :-) .
>>>>>>>>
>>>>>>>> Also see the Mediatek CSI rx phy doing this too already [1]
>>>>>>>>
>>>>>>>>
>>>>>>>> Heiko
>>>>>>>>
>>>>>>>> [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
>>>>>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
>>>>>>>>
>>>>>>>
>>>>>>> It is really a nice way to separate HDMI and DP modes.
>>>>>
>>>>> I apologize for reopening the discussion about the phy-types setting.
>>>>
>>>> there is definitly no need to apologize. We're trying to find the best
>>>> solution afterall :-) .
>>>>
>>>>> With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic
>>>>> switching can be achieved, which just depends on the right setting of
>>>>> enum phy_mode in include/linux/phy/phy.h. So the previous way of
>>>>> configuring phy mode may be also good.
>>>>
>>>> I think the deciding factor is, is there a use-case for needing to switch
>>>> modes at runtime.
>>>>
>>>> I do think the mode for the dc-phy and also the hdptx-phy is pretty much
>>>> decided by the board design.
>>>>
>>>> I.e. when you end up in a DP-connector (or eDP-panel) on your board you
>>>> need DP mode, and when you end up in a hdmi-connector you need the
>>>> HDMI phy mode.
>>>>
>>>> So I think the phy-mode for the hdptx-phy is largely dictated by the static
>>>> board definition (like devicetree), hence going with the dt-argument for
>>>> the mode.
>>>>
>>>> Like similar to the Naneng combophy, selecting its mode via argument
>>>> because deciding if it ends up in a sata port is a board-design thing.
>>>>
>>>> Is there a use-case where you need to switch at runtime between
>>>> HDMI and eDP? Like starting the phy in eDP mode but then needing
>>>> to switch to HDMI mode, while the device is running?
>>
>> Indeed, we have the board as you described, on which the DP-connector and
>> HDMI-connector both have been configured.
>>
>> And the dynamic switching is more useful for RK3576, which has the same
>> eDP/HDMI design as RK3588 but only one eDP controller/HDMI controller/HDPTX
>> phy. We can only enable both of eDP/HDMI by this way.
>>
>>>
>>> I believe the eDP controller can only use the PHY in eDP mode and
>>> the HDMI controller can only use it in HDMI mode. So in order to
>>> support runtime switching, the following options are possible:
>>>
>>> 1. Enable both controllers, the PHY decides which one is really
>>>      used, the other one is basically a non-functional dummy device
>>>      until the PHY is reconfigured. This requires the set_mode()
>>>      callback, since the HDMI and eDP drivers both expect their
>>>      PHY to be enabled.
>>>
>>> 2. Properly enable / disable the used controller, so that only one
>>>      controller is active at the same time. In this case the switching
>>>      is handled one layer above and the PHY has nothing to do with it.
>>>      The phy_enable call from each controller would just set it up in
>>>      the right mode.
>>>
>>> I guess option 1 is the hacked solution, which is easier to
>>> implement as DRM's hotplug abilities are quite limited at the moment
>>> as far as I know. I think the second solution looks much cleaner and
>>> should be prefered for upstream. That solution does not require
>>> calling set_mode() for runtime switching making this whole argument
>>> void :)
>>>
>>
>> Your friendly and detailed analysis has brought me some valuable insights.
>> :)
>>
>> The option 2 is really a better way to support the dynamic switching, and we
>> still need the .set_mode() to select the configurations for either eDP or
>> HDMI in HDPTX phy at the controller level. Would you mind
>> elaborating on the useful way to choose the phy mode for the second
>> solution?
> 
> The xlate function to handle the arguments is called when the PHY
> device is looked up. So the devm_phy_get(dp->dev, "dp") call in
> analogix_dp_probe() would setup the PHY in DP mode.
> 
> Similarily the call to devm_of_phy_get_by_index() in
> dw_hdmi_qp_rockchip_bind() would set the PHY in HDMI mode.
> 
> So the PHY mode is correct as long as only one controller driver
> is bound at the same time.
> 

Firstly, the term "the HDMI and eDP dynamic switching" can be somewhat 
misleading, because the eDP usually does not support hot plug. The 
RK3588 eDP is often used as DP, and it actually supports DP 1.2. 
Therefore, it is better to use the "the HDMI and DP dynamic switching" 
description.

Indeed, the devm_phy_get(dp->dev, "dp") and devm_of_phy_get_by_index() 
will help to get the phy reference in .probe() or .bind().

However, the phy_set_mode() may be still needed in the HDMI and DP 
dynamic switching application scenarios. We need the enum phy_mode 
PHY_MODE_DP/PHY_MODE_HDMI to differentiate the configuration processes 
in .power_on(), .power_off() and .configure() of struct phy_ops, which 
will be called in conjunction with plugging in and unplugging an HDMI or 
DP cable.

Therefore, I believe we should retain the .set_mode() function.

> Greetings,
> 
> -- Sebastian
> 
>>> FWIW I think the DT argument based mode setting and the runtime set_mode
>>> are not necessarily mutual exclusive. In theory one could support both
>>> and adding set_mode support later does not change any ABI as far as
>>> I can see. Basically handle it like pin mux/configuration settings,
>>> which are usually automatically handled by the device core based on
>>> DT information, except for some drivers which have special needs.
>>>
>>
>>>>> And other phys may want to support dynamic switching too, like the
>>>>> Rockchip USBDP combo phy.
>>>>
>>>> I guess USBDP is special in that in also does both modes dynamical
>>>> depending on its use (like type-c with option DP altmode)
>>>
>>> The USBDP PHY is indeed quite different from the PHYs in your list
>>> above, but for a different reason: All the combined PHYs you listed
>>> above only support one mode at the same time. E.g. you need to
>>> decide between PCIe, SATA and USB3 mode for the Naneng combophy.
>>>
>>> For the USBDP PHY the modes are not mutually exclusive. The USB
>>> controller can request the USBDP PHY in USB mode at the same time
>>> as the DP controller requests it in DP mode. Some additional
>>> registers configure how the lanes are being muxed. A typcial setup
>>> would be 2 lanes for USB3 and 2 lanes for DP.
>>>
>>> Greetings,
>>>
>>> -- Sebastian
>>
>> Best regards,
>> Damon
>>

Best regards,
Damon

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

Hello Damon,

On Thu, Dec 05, 2024 at 09:13:33AM +0800, Damon Ding wrote:
> Firstly, the term "the HDMI and eDP dynamic switching" can be somewhat
> misleading, because the eDP usually does not support hot plug. The RK3588
> eDP is often used as DP, and it actually supports DP 1.2. Therefore, it is
> better to use the "the HDMI and DP dynamic switching" description.

The part unclear to me is how the dynamic switching is supposed to
happen. Looking at the TRM the hotplug detect signals also seem to be
shared between HDMI and eDP. Can the RK3588S EVB distinguish if HDMI
or eDP has been plugged, or does this require some user interaction
to set the right mode?

> Indeed, the devm_phy_get(dp->dev, "dp") and devm_of_phy_get_by_index() will
> help to get the phy reference in .probe() or .bind().
> 
> However, the phy_set_mode() may be still needed in the HDMI and DP dynamic
> switching application scenarios. We need the enum phy_mode
> PHY_MODE_DP/PHY_MODE_HDMI to differentiate the configuration processes in
> .power_on(), .power_off() and .configure() of struct phy_ops, which will be
> called in conjunction with plugging in and unplugging an HDMI or DP cable.

I suppose you could fetch the PHY in power_on() and release it in
power_off(). But using phy_set_mode() might indeed be better here.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Damon Ding]

Hi Sebastian,

On 2024/12/6 2:04, Sebastian Reichel wrote:
> Hello Damon,
> 
> On Thu, Dec 05, 2024 at 09:13:33AM +0800, Damon Ding wrote:
>> Firstly, the term "the HDMI and eDP dynamic switching" can be somewhat
>> misleading, because the eDP usually does not support hot plug. The RK3588
>> eDP is often used as DP, and it actually supports DP 1.2. Therefore, it is
>> better to use the "the HDMI and DP dynamic switching" description.
> 
> The part unclear to me is how the dynamic switching is supposed to
> happen. Looking at the TRM the hotplug detect signals also seem to be
> shared between HDMI and eDP. Can the RK3588S EVB distinguish if HDMI
> or eDP has been plugged, or does this require some user interaction
> to set the right mode?

Indeed, HDMI and eDP share the same pin for hotplug detect. However, 
some users may connect the hotplug detection pin of DP-connector with an 
unexpected pin that can not support the iomux of hotplug detect function 
on RK3588 SoC. This could be due to a flaw in the hardware design, a 
conflict in pin multiplexing, or other factors. Therefore, we support 
the GPIO HDP function for the eDP, as DP also supports this for the same 
reasons.

If the dynamic switching is enabled, HDMI detects the HPD signal through 
  the hotplug detect function pin, while eDP uses one of the available 
GPIO pins to do this.

What's more, if the user connects an HDMI cable first and than connects 
a DP cable as well, despite our clear instruction against using HDMI and 
eDP simultaneously, the status register of GRF will indicate that HDMI 
has been connected. Meanwhile, during the HPD detection process for eDP, 
it will return "connector_status_disconnected". The reverse scenario 
also applies.

> 
>> Indeed, the devm_phy_get(dp->dev, "dp") and devm_of_phy_get_by_index() will
>> help to get the phy reference in .probe() or .bind().
>>
>> However, the phy_set_mode() may be still needed in the HDMI and DP dynamic
>> switching application scenarios. We need the enum phy_mode
>> PHY_MODE_DP/PHY_MODE_HDMI to differentiate the configuration processes in
>> .power_on(), .power_off() and .configure() of struct phy_ops, which will be
>> called in conjunction with plugging in and unplugging an HDMI or DP cable.
> 
> I suppose you could fetch the PHY in power_on() and release it in
> power_off(). But using phy_set_mode() might indeed be better here.
> 

As a future expansion, the .set_mode() can also be helpful in the txffe 
level adjustment for HDMI 2.1. :)

> -- Sebastian

Best regards,
Damon

[PATCH v1 05/10] drm/bridge: analogix_dp: add support for RK3588

[Damon Ding]

Add max_link_rate and max_lane_count configs for RK3588.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index bfa88409a7ff..6f10d88a34c5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1513,6 +1513,10 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 		video_info->max_link_rate = 0x0A;
 		video_info->max_lane_count = 0x04;
 		break;
+	case RK3588_EDP:
+		video_info->max_link_rate = 0x14;
+		video_info->max_lane_count = 0x04;
+		break;
 	case EXYNOS_DP:
 		/*
 		 * NOTE: those property parseing code is used for
-- 
2.34.1

[PATCH v1 06/10] drm/bridge: analogix_dp: Add support for phy configuration.

[Damon Ding]

Add support to configurate link rate, lane count, voltage swing and
pre-emphasis with phy_configure(). It is helpful in application scenarios
where analogix controller is mixed with the phy of other vendors.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../drm/bridge/analogix/analogix_dp_core.c    |  4 +-
 .../drm/bridge/analogix/analogix_dp_core.h    |  2 +
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 90 +++++++++++++++++++
 3 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 6f10d88a34c5..7624ed13cdbf 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1672,7 +1672,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_probe);
 
 int analogix_dp_suspend(struct analogix_dp_device *dp)
 {
-	phy_power_off(dp->phy);
+	analogix_dp_phy_power_off(dp);
 
 	if (dp->plat_data->power_off)
 		dp->plat_data->power_off(dp->plat_data);
@@ -1696,7 +1696,7 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
 	if (dp->plat_data->power_on)
 		dp->plat_data->power_on(dp->plat_data);
 
-	phy_power_on(dp->phy);
+	analogix_dp_phy_power_on(dp);
 
 	analogix_dp_init_dp(dp);
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 774d11574b09..a76079d61768 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -232,5 +232,7 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
 			     struct dp_sdp *vsc, bool blocking);
 ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 			     struct drm_dp_aux_msg *msg);
+void analogix_dp_phy_power_on(struct analogix_dp_device *dp);
+void analogix_dp_phy_power_off(struct analogix_dp_device *dp);
 
 #endif /* _ANALOGIX_DP_CORE_H */
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 3afc73c858c4..809bb0c72d18 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -11,6 +11,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/phy/phy.h>
 
 #include <drm/bridge/analogix_dp.h>
 
@@ -513,10 +514,27 @@ void analogix_dp_enable_sw_function(struct analogix_dp_device *dp)
 void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
 {
 	u32 reg;
+	int ret;
 
 	reg = bwtype;
 	if ((bwtype == DP_LINK_BW_2_7) || (bwtype == DP_LINK_BW_1_62))
 		writel(reg, dp->reg_base + ANALOGIX_DP_LINK_BW_SET);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		phy_cfg.dp.lanes = dp->link_train.lane_count;
+		phy_cfg.dp.link_rate =
+			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
+		phy_cfg.dp.set_lanes = false;
+		phy_cfg.dp.set_rate = true;
+		phy_cfg.dp.set_voltages = false;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
@@ -530,9 +548,24 @@ void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
 void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count)
 {
 	u32 reg;
+	int ret;
 
 	reg = count;
 	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_COUNT_SET);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		phy_cfg.dp.lanes = dp->link_train.lane_count;
+		phy_cfg.dp.set_lanes = true;
+		phy_cfg.dp.set_rate = false;
+		phy_cfg.dp.set_voltages = false;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
@@ -546,10 +579,39 @@ void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
 void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp)
 {
 	u8 lane;
+	int ret;
 
 	for (lane = 0; lane < dp->link_train.lane_count; lane++)
 		writel(dp->link_train.training_lane[lane],
 		       dp->reg_base + ANALOGIX_DP_LN0_LINK_TRAINING_CTL + 4 * lane);
+
+	if (dp->phy) {
+		union phy_configure_opts phy_cfg = {0};
+
+		for (lane = 0; lane < dp->link_train.lane_count; lane++) {
+			u8 training_lane = dp->link_train.training_lane[lane];
+			u8 vs, pe;
+
+			vs = (training_lane & DP_TRAIN_VOLTAGE_SWING_MASK) >>
+			     DP_TRAIN_VOLTAGE_SWING_SHIFT;
+			pe = (training_lane & DP_TRAIN_PRE_EMPHASIS_MASK) >>
+			     DP_TRAIN_PRE_EMPHASIS_SHIFT;
+			phy_cfg.dp.voltage[lane] = vs;
+			phy_cfg.dp.pre[lane] = pe;
+		}
+
+		phy_cfg.dp.lanes = dp->link_train.lane_count;
+		phy_cfg.dp.link_rate =
+			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
+		phy_cfg.dp.set_lanes = false;
+		phy_cfg.dp.set_rate = false;
+		phy_cfg.dp.set_voltages = true;
+		ret = phy_configure(dp->phy, &phy_cfg);
+		if (ret && ret != -EOPNOTSUPP) {
+			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
+			return;
+		}
+	}
 }
 
 u32 analogix_dp_get_lane_link_training(struct analogix_dp_device *dp, u8 lane)
@@ -1053,3 +1115,31 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
 
 	return -EREMOTEIO;
 }
+
+void analogix_dp_phy_power_on(struct analogix_dp_device *dp)
+{
+	int ret;
+
+	ret = phy_set_mode(dp->phy, PHY_MODE_DP);
+	if (ret) {
+		dev_err(dp->dev, "%s: phy_set_mode() failed: %d\n", __func__, ret);
+		return;
+	}
+
+	ret = phy_power_on(dp->phy);
+	if (ret) {
+		dev_err(dp->dev, "%s: phy_power_on() failed: %d\n", __func__, ret);
+		return;
+	}
+}
+
+void analogix_dp_phy_power_off(struct analogix_dp_device *dp)
+{
+	int ret;
+
+	ret = phy_power_off(dp->phy);
+	if (ret) {
+		dev_err(dp->dev, "%s: phy_power_off() failed: %d\n", __func__, ret);
+		return;
+	}
+}
-- 
2.34.1

Re: [PATCH v1 06/10] drm/bridge: analogix_dp: Add support for phy configuration.

[Dmitry Baryshkov]

On Wed, Nov 27, 2024 at 03:51:53PM +0800, Damon Ding wrote:
> Add support to configurate link rate, lane count, voltage swing and
> pre-emphasis with phy_configure(). It is helpful in application scenarios
> where analogix controller is mixed with the phy of other vendors.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
>  .../drm/bridge/analogix/analogix_dp_core.c    |  4 +-
>  .../drm/bridge/analogix/analogix_dp_core.h    |  2 +
>  .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 90 +++++++++++++++++++
>  3 files changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6f10d88a34c5..7624ed13cdbf 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1672,7 +1672,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_probe);
>  
>  int analogix_dp_suspend(struct analogix_dp_device *dp)
>  {
> -	phy_power_off(dp->phy);
> +	analogix_dp_phy_power_off(dp);

Why?

>  
>  	if (dp->plat_data->power_off)
>  		dp->plat_data->power_off(dp->plat_data);
> @@ -1696,7 +1696,7 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
>  	if (dp->plat_data->power_on)
>  		dp->plat_data->power_on(dp->plat_data);
>  
> -	phy_power_on(dp->phy);
> +	analogix_dp_phy_power_on(dp);

Why?

>  
>  	analogix_dp_init_dp(dp);
>  
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 774d11574b09..a76079d61768 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -232,5 +232,7 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>  			     struct dp_sdp *vsc, bool blocking);
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>  			     struct drm_dp_aux_msg *msg);
> +void analogix_dp_phy_power_on(struct analogix_dp_device *dp);
> +void analogix_dp_phy_power_off(struct analogix_dp_device *dp);
>  
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 3afc73c858c4..809bb0c72d18 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -11,6 +11,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
>  
>  #include <drm/bridge/analogix_dp.h>
>  
> @@ -513,10 +514,27 @@ void analogix_dp_enable_sw_function(struct analogix_dp_device *dp)
>  void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
>  {
>  	u32 reg;
> +	int ret;
>  
>  	reg = bwtype;
>  	if ((bwtype == DP_LINK_BW_2_7) || (bwtype == DP_LINK_BW_1_62))
>  		writel(reg, dp->reg_base + ANALOGIX_DP_LINK_BW_SET);
> +
> +	if (dp->phy) {
> +		union phy_configure_opts phy_cfg = {0};
> +
> +		phy_cfg.dp.lanes = dp->link_train.lane_count;

You can drop this, .set_lanes is false.

> +		phy_cfg.dp.link_rate =
> +			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
> +		phy_cfg.dp.set_lanes = false;
> +		phy_cfg.dp.set_rate = true;
> +		phy_cfg.dp.set_voltages = false;

You don't need to set those to false, it's cleared by = {0};

> +		ret = phy_configure(dp->phy, &phy_cfg);
> +		if (ret && ret != -EOPNOTSUPP) {
> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
> +			return;
> +		}
> +	}
>  }
>  
>  void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
> @@ -530,9 +548,24 @@ void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
>  void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count)
>  {
>  	u32 reg;
> +	int ret;
>  
>  	reg = count;
>  	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_COUNT_SET);
> +
> +	if (dp->phy) {
> +		union phy_configure_opts phy_cfg = {0};
> +
> +		phy_cfg.dp.lanes = dp->link_train.lane_count;
> +		phy_cfg.dp.set_lanes = true;
> +		phy_cfg.dp.set_rate = false;
> +		phy_cfg.dp.set_voltages = false;

Likewise

> +		ret = phy_configure(dp->phy, &phy_cfg);
> +		if (ret && ret != -EOPNOTSUPP) {
> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
> +			return;
> +		}
> +	}
>  }
>  
>  void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
> @@ -546,10 +579,39 @@ void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
>  void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp)
>  {
>  	u8 lane;
> +	int ret;
>  
>  	for (lane = 0; lane < dp->link_train.lane_count; lane++)
>  		writel(dp->link_train.training_lane[lane],
>  		       dp->reg_base + ANALOGIX_DP_LN0_LINK_TRAINING_CTL + 4 * lane);
> +
> +	if (dp->phy) {
> +		union phy_configure_opts phy_cfg = {0};
> +
> +		for (lane = 0; lane < dp->link_train.lane_count; lane++) {
> +			u8 training_lane = dp->link_train.training_lane[lane];
> +			u8 vs, pe;
> +
> +			vs = (training_lane & DP_TRAIN_VOLTAGE_SWING_MASK) >>
> +			     DP_TRAIN_VOLTAGE_SWING_SHIFT;
> +			pe = (training_lane & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> +			     DP_TRAIN_PRE_EMPHASIS_SHIFT;
> +			phy_cfg.dp.voltage[lane] = vs;
> +			phy_cfg.dp.pre[lane] = pe;
> +		}
> +
> +		phy_cfg.dp.lanes = dp->link_train.lane_count;
> +		phy_cfg.dp.link_rate =
> +			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;

You can drop these two.

> +		phy_cfg.dp.set_lanes = false;
> +		phy_cfg.dp.set_rate = false;
> +		phy_cfg.dp.set_voltages = true;
> +		ret = phy_configure(dp->phy, &phy_cfg);
> +		if (ret && ret != -EOPNOTSUPP) {
> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
> +			return;
> +		}
> +	}
>  }
>  
>  u32 analogix_dp_get_lane_link_training(struct analogix_dp_device *dp, u8 lane)
> @@ -1053,3 +1115,31 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>  
>  	return -EREMOTEIO;
>  }
> +
> +void analogix_dp_phy_power_on(struct analogix_dp_device *dp)
> +{
> +	int ret;
> +
> +	ret = phy_set_mode(dp->phy, PHY_MODE_DP);
> +	if (ret) {
> +		dev_err(dp->dev, "%s: phy_set_mode() failed: %d\n", __func__, ret);
> +		return;
> +	}
> +
> +	ret = phy_power_on(dp->phy);
> +	if (ret) {
> +		dev_err(dp->dev, "%s: phy_power_on() failed: %d\n", __func__, ret);

There is already a dev_err() call in phy_power_on().

> +		return;
> +	}
> +}
> +
> +void analogix_dp_phy_power_off(struct analogix_dp_device *dp)
> +{
> +	int ret;
> +
> +	ret = phy_power_off(dp->phy);
> +	if (ret) {
> +		dev_err(dp->dev, "%s: phy_power_off() failed: %d\n", __func__, ret);

There is already a dev_err() call in phy_power_off().

> +		return;
> +	}
> +}
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

-- 
With best wishes
Dmitry

Re: [PATCH v1 06/10] drm/bridge: analogix_dp: Add support for phy configuration.

[Damon Ding]

Hi Dmitry,

On 2024/11/30 16:53, Dmitry Baryshkov wrote:
> On Wed, Nov 27, 2024 at 03:51:53PM +0800, Damon Ding wrote:
>> Add support to configurate link rate, lane count, voltage swing and
>> pre-emphasis with phy_configure(). It is helpful in application scenarios
>> where analogix controller is mixed with the phy of other vendors.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> ---
>>   .../drm/bridge/analogix/analogix_dp_core.c    |  4 +-
>>   .../drm/bridge/analogix/analogix_dp_core.h    |  2 +
>>   .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 90 +++++++++++++++++++
>>   3 files changed, 94 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 6f10d88a34c5..7624ed13cdbf 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1672,7 +1672,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_probe);
>>   
>>   int analogix_dp_suspend(struct analogix_dp_device *dp)
>>   {
>> -	phy_power_off(dp->phy);
>> +	analogix_dp_phy_power_off(dp);
> 
> Why?
> 
>>   
>>   	if (dp->plat_data->power_off)
>>   		dp->plat_data->power_off(dp->plat_data);
>> @@ -1696,7 +1696,7 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
>>   	if (dp->plat_data->power_on)
>>   		dp->plat_data->power_on(dp->plat_data);
>>   
>> -	phy_power_on(dp->phy);
>> +	analogix_dp_phy_power_on(dp);
> 
> Why?
> 

These changes for phy_power_on()/phy_power_off() may be unnecessary. I 
will just put the phy_set_mode() before phy_power_on() in next version.

>>   
>>   	analogix_dp_init_dp(dp);
>>   
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index 774d11574b09..a76079d61768 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -232,5 +232,7 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>>   			     struct dp_sdp *vsc, bool blocking);
>>   ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>   			     struct drm_dp_aux_msg *msg);
>> +void analogix_dp_phy_power_on(struct analogix_dp_device *dp);
>> +void analogix_dp_phy_power_off(struct analogix_dp_device *dp);
>>   
>>   #endif /* _ANALOGIX_DP_CORE_H */
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> index 3afc73c858c4..809bb0c72d18 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/io.h>
>>   #include <linux/iopoll.h>
>> +#include <linux/phy/phy.h>
>>   
>>   #include <drm/bridge/analogix_dp.h>
>>   
>> @@ -513,10 +514,27 @@ void analogix_dp_enable_sw_function(struct analogix_dp_device *dp)
>>   void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype)
>>   {
>>   	u32 reg;
>> +	int ret;
>>   
>>   	reg = bwtype;
>>   	if ((bwtype == DP_LINK_BW_2_7) || (bwtype == DP_LINK_BW_1_62))
>>   		writel(reg, dp->reg_base + ANALOGIX_DP_LINK_BW_SET);
>> +
>> +	if (dp->phy) {
>> +		union phy_configure_opts phy_cfg = {0};
>> +
>> +		phy_cfg.dp.lanes = dp->link_train.lane_count;
> 
> You can drop this, .set_lanes is false.
> 
>> +		phy_cfg.dp.link_rate =
>> +			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
>> +		phy_cfg.dp.set_lanes = false;
>> +		phy_cfg.dp.set_rate = true;
>> +		phy_cfg.dp.set_voltages = false;
> 
> You don't need to set those to false, it's cleared by = {0};
> 
>> +		ret = phy_configure(dp->phy, &phy_cfg);
>> +		if (ret && ret != -EOPNOTSUPP) {
>> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
>> +			return;
>> +		}
>> +	}
>>   }
>>   
>>   void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
>> @@ -530,9 +548,24 @@ void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype)
>>   void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count)
>>   {
>>   	u32 reg;
>> +	int ret;
>>   
>>   	reg = count;
>>   	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_COUNT_SET);
>> +
>> +	if (dp->phy) {
>> +		union phy_configure_opts phy_cfg = {0};
>> +
>> +		phy_cfg.dp.lanes = dp->link_train.lane_count;
>> +		phy_cfg.dp.set_lanes = true;
>> +		phy_cfg.dp.set_rate = false;
>> +		phy_cfg.dp.set_voltages = false;
> 
> Likewise
> 
>> +		ret = phy_configure(dp->phy, &phy_cfg);
>> +		if (ret && ret != -EOPNOTSUPP) {
>> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
>> +			return;
>> +		}
>> +	}
>>   }
>>   
>>   void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
>> @@ -546,10 +579,39 @@ void analogix_dp_get_lane_count(struct analogix_dp_device *dp, u32 *count)
>>   void analogix_dp_set_lane_link_training(struct analogix_dp_device *dp)
>>   {
>>   	u8 lane;
>> +	int ret;
>>   
>>   	for (lane = 0; lane < dp->link_train.lane_count; lane++)
>>   		writel(dp->link_train.training_lane[lane],
>>   		       dp->reg_base + ANALOGIX_DP_LN0_LINK_TRAINING_CTL + 4 * lane);
>> +
>> +	if (dp->phy) {
>> +		union phy_configure_opts phy_cfg = {0};
>> +
>> +		for (lane = 0; lane < dp->link_train.lane_count; lane++) {
>> +			u8 training_lane = dp->link_train.training_lane[lane];
>> +			u8 vs, pe;
>> +
>> +			vs = (training_lane & DP_TRAIN_VOLTAGE_SWING_MASK) >>
>> +			     DP_TRAIN_VOLTAGE_SWING_SHIFT;
>> +			pe = (training_lane & DP_TRAIN_PRE_EMPHASIS_MASK) >>
>> +			     DP_TRAIN_PRE_EMPHASIS_SHIFT;
>> +			phy_cfg.dp.voltage[lane] = vs;
>> +			phy_cfg.dp.pre[lane] = pe;
>> +		}
>> +
>> +		phy_cfg.dp.lanes = dp->link_train.lane_count;
>> +		phy_cfg.dp.link_rate =
>> +			drm_dp_bw_code_to_link_rate(dp->link_train.link_rate) / 100;
> 
> You can drop these two.

Indeed, all the needless assignments for phy_configure() will be deleted 
in next version.

> 
>> +		phy_cfg.dp.set_lanes = false;
>> +		phy_cfg.dp.set_rate = false;
>> +		phy_cfg.dp.set_voltages = true;
>> +		ret = phy_configure(dp->phy, &phy_cfg);
>> +		if (ret && ret != -EOPNOTSUPP) {
>> +			dev_err(dp->dev, "%s: phy_configure() failed: %d\n", __func__, ret);
>> +			return;
>> +		}
>> +	}
>>   }
>>   
>>   u32 analogix_dp_get_lane_link_training(struct analogix_dp_device *dp, u8 lane)
>> @@ -1053,3 +1115,31 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>>   
>>   	return -EREMOTEIO;
>>   }
>> +
>> +void analogix_dp_phy_power_on(struct analogix_dp_device *dp)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_set_mode(dp->phy, PHY_MODE_DP);
>> +	if (ret) {
>> +		dev_err(dp->dev, "%s: phy_set_mode() failed: %d\n", __func__, ret);
>> +		return;
>> +	}
>> +
>> +	ret = phy_power_on(dp->phy);
>> +	if (ret) {
>> +		dev_err(dp->dev, "%s: phy_power_on() failed: %d\n", __func__, ret);
> 
> There is already a dev_err() call in phy_power_on().
> 
>> +		return;
>> +	}
>> +}
>> +
>> +void analogix_dp_phy_power_off(struct analogix_dp_device *dp)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_power_off(dp->phy);
>> +	if (ret) {
>> +		dev_err(dp->dev, "%s: phy_power_off() failed: %d\n", __func__, ret);
> 
> There is already a dev_err() call in phy_power_off().
> 
>> +		return;
>> +	}
>> +}
>> -- 
>> 2.34.1
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

Best regards,
Damon

[PATCH v1 07/10] dt-bindings: display: rockchip: Fix label name of hdptxphy for RK3588 HDMI TX Controller

[Damon Ding]

The hdptxphy is a combo transmit-PHY for HDMI2.1 TMDS Link, FRL Link, DP
and eDP Link. Therefore, it is better to name it hdptxphy0 other than
hdptxphy_hdmi0, which will be referenced by both hdmi0 and edp0 nodes.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
index d8e761865f27..7a1ae31cc535 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
@@ -156,7 +156,7 @@ examples:
                      <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH 0>,
                      <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH 0>;
         interrupt-names = "avp", "cec", "earc", "main", "hpd";
-        phys = <&hdptxphy_hdmi0>;
+        phys = <&hdptxphy0>;
         power-domains = <&power RK3588_PD_VO1>;
         resets = <&cru SRST_HDMITX0_REF>, <&cru SRST_HDMIHDP0>;
         reset-names = "ref", "hdp";
-- 
2.34.1

Re: [PATCH v1 07/10] dt-bindings: display: rockchip: Fix label name of hdptxphy for RK3588 HDMI TX Controller

[Rob Herring (Arm)]

On Wed, 27 Nov 2024 15:51:54 +0800, Damon Ding wrote:
> The hdptxphy is a combo transmit-PHY for HDMI2.1 TMDS Link, FRL Link, DP
> and eDP Link. Therefore, it is better to name it hdptxphy0 other than
> hdptxphy_hdmi0, which will be referenced by both hdmi0 and edp0 nodes.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
>  .../bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v1 08/10] arm64: dts: rockchip: Fix label name of hdptxphy for RK3588

[Damon Ding]

The hdptxphy is a combo transmit-PHY for HDMI2.1 TMDS Link, FRL Link, DP
and eDP Link. Therefore, it is better to name it hdptxphy0 other than
hdptxphy_hdmi0, which will be referenced by both hdmi0 and edp0 nodes.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi                 | 4 ++--
 arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts        | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-genbook.dts    | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts              | 2 +-
 .../boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts      | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts                | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi            | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts       | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts               | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou.dts          | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts            | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-indiedroid-nova.dts      | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi           | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-odroid-m2.dts            | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi          | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts              | 2 +-
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts              | 2 +-
 17 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index a337f3fb8377..7e125897b0cd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1386,7 +1386,7 @@ hdmi0: hdmi@fde80000 {
 			     <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH 0>;
 		interrupt-names = "avp", "cec", "earc", "main", "hpd";
-		phys = <&hdptxphy_hdmi0>;
+		phys = <&hdptxphy0>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&hdmim0_tx0_cec &hdmim0_tx0_hpd
 			     &hdmim0_tx0_scl &hdmim0_tx0_sda>;
@@ -2806,7 +2806,7 @@ dmac2: dma-controller@fed10000 {
 		#dma-cells = <1>;
 	};
 
-	hdptxphy_hdmi0: phy@fed60000 {
+	hdptxphy0: phy@fed60000 {
 		compatible = "rockchip,rk3588-hdptx-phy";
 		reg = <0x0 0xfed60000 0x0 0x2000>;
 		clocks = <&cru CLK_USB2PHY_HDPTXRXPHY_REF>, <&cru PCLK_HDPTX0>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts
index 9d525c8ff725..9eda69722665 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts
@@ -129,7 +129,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-genbook.dts b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-genbook.dts
index 92f0ed83c990..d1be72eb63a4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-genbook.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-genbook.dts
@@ -166,7 +166,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
index d6e464cdc536..094235d22cd1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
@@ -328,7 +328,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts b/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts
index b3a04ca370bb..8171fbfd819a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-friendlyelec-cm3588-nas.dts
@@ -335,7 +335,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 90f823b2c219..ef56380530f9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -303,7 +303,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
index cb350727d116..0d9b5020acc0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
@@ -360,7 +360,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
index 9f5a38b290bf..f14e96d96207 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
@@ -296,7 +296,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index c44d001da169..a047c579c93d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -220,7 +220,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou.dts
index 3187b4918a30..795d8175e654 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou.dts
@@ -189,7 +189,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
index 9c394f733bbf..f471baca6d31 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-coolpi-4b.dts
@@ -236,7 +236,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-indiedroid-nova.dts b/arch/arm64/boot/dts/rockchip/rk3588s-indiedroid-nova.dts
index 4a3aa80f2226..74a4f03e05e3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-indiedroid-nova.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-indiedroid-nova.dts
@@ -278,7 +278,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
index 76a6e8e517e9..482f5917043f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-r6.dtsi
@@ -251,7 +251,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-odroid-m2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-odroid-m2.dts
index 8f034c6d494c..1463bd36b1b2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-odroid-m2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-odroid-m2.dts
@@ -264,7 +264,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
index d86aeacca238..9e16960b8705 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
@@ -197,7 +197,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index 70a43432bdc5..676cc4fec269 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -334,7 +334,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
index 9b14d5383cdc..bf74789e3f51 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
@@ -278,7 +278,7 @@ hdmi0_out_con: endpoint {
 	};
 };
 
-&hdptxphy_hdmi0 {
+&hdptxphy0 {
 	status = "okay";
 };
 
-- 
2.34.1

[PATCH v1 09/10] arm64: dts: rockchip: Add eDP0 node for RK3588

[Damon Ding]

Add support for the eDP0 output on RK3588 SoC.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 7e125897b0cd..240b0103c957 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1411,6 +1411,35 @@ hdmi0_out: port@1 {
 		};
 	};
 
+	edp0: edp@fdec0000 {
+		compatible = "rockchip,rk3588-edp";
+		reg = <0x0 0xfdec0000 0x0 0x1000>;
+		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru CLK_EDP0_200M>;
+		clock-names = "dp", "pclk", "spdif";
+		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH 0>;
+		phys = <&hdptxphy0>;
+		phy-names = "dp";
+		power-domains = <&power RK3588_PD_VO1>;
+		resets = <&cru SRST_EDP0_24M>, <&cru SRST_P_EDP0>;
+		reset-names = "dp", "apb";
+		rockchip,grf = <&vo1_grf>;
+		#sound-dai-cells = <1>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			edp0_in: port@0 {
+				reg = <0>;
+			};
+
+			edp0_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu_m0: qos@fdf35000 {
 		compatible = "rockchip,rk3588-qos", "syscon";
 		reg = <0x0 0xfdf35000 0x0 0x20>;
-- 
2.34.1

[PATCH v1 10/10] arch64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

[Damon Ding]

Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../boot/dts/rockchip/rk3588s-evb1-v10.dts    | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
index bc4077575beb..67c2e6d1bece 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
@@ -9,6 +9,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/soc/rockchip,vop2.h>
 #include <dt-bindings/usb/pd.h>
 #include "rk3588s.dtsi"
 
@@ -116,10 +117,56 @@ masterdai: simple-audio-card,codec {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
+		brightness-levels = <
+			  0   1   2   3   4   5   6   7
+			  8   9  10  11  12  13  14  15
+			 16  17  18  19  20  21  22  23
+			 24  25  26  27  28  29  30  31
+			 32  33  34  35  36  37  38  39
+			 40  41  42  43  44  45  46  47
+			 48  49  50  51  52  53  54  55
+			 56  57  58  59  60  61  62  63
+			 64  65  66  67  68  69  70  71
+			 72  73  74  75  76  77  78  79
+			 80  81  82  83  84  85  86  87
+			 88  89  90  91  92  93  94  95
+			 96  97  98  99 100 101 102 103
+			104 105 106 107 108 109 110 111
+			112 113 114 115 116 117 118 119
+			120 121 122 123 124 125 126 127
+			128 129 130 131 132 133 134 135
+			136 137 138 139 140 141 142 143
+			144 145 146 147 148 149 150 151
+			152 153 154 155 156 157 158 159
+			160 161 162 163 164 165 166 167
+			168 169 170 171 172 173 174 175
+			176 177 178 179 180 181 182 183
+			184 185 186 187 188 189 190 191
+			192 193 194 195 196 197 198 199
+			200 201 202 203 204 205 206 207
+			208 209 210 211 212 213 214 215
+			216 217 218 219 220 221 222 223
+			224 225 226 227 228 229 230 231
+			232 233 234 235 236 237 238 239
+			240 241 242 243 244 245 246 247
+			248 249 250 251 252 253 254 255>;
+		default-brightness-level = <200>;
 		power-supply = <&vcc3v3_lcd_edp>;
 		pwms = <&pwm12 0 25000 0>;
 	};
 
+	edp_panel: edp-panel {
+		compatible = "lg,lp079qx1-sp0v";
+		backlight = <&backlight>;
+		power-supply = <&vcc3v3_lcd_edp>;
+
+		port {
+			panel_in_edp: endpoint {
+				remote-endpoint = <&edp_out_panel>;
+			};
+		};
+	};
+
 	combophy_avdd0v85: regulator-combophy-avdd0v85 {
 		compatible = "regulator-fixed";
 		regulator-name = "combophy_avdd0v85";
@@ -238,6 +285,27 @@ &combphy2_psu {
 	status = "okay";
 };
 
+&edp0 {
+	force-hpd;
+	status = "okay";
+};
+
+&edp0_in {
+	edp0_in_vp2: endpoint {
+		remote-endpoint = <&vp2_out_edp0>;
+	};
+};
+
+&edp0_out {
+	edp_out_panel: endpoint {
+		remote-endpoint = <&panel_in_edp>;
+	};
+};
+
+&hdptxphy0 {
+	status = "okay";
+};
+
 &i2c3 {
 	status = "okay";
 
@@ -399,6 +467,7 @@ usbc0_int: usbc0-int {
 };
 
 &pwm12 {
+	pinctrl-0 = <&pwm12m1_pins>;
 	status = "okay";
 };
 
@@ -1168,3 +1237,18 @@ usbdp_phy0_dp_altmode_mux: endpoint@1 {
 		};
 	};
 };
+
+&vop_mmu {
+	status = "okay";
+};
+
+&vop {
+	status = "okay";
+};
+
+&vp2 {
+	vp2_out_edp0: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
+		reg = <ROCKCHIP_VOP2_EP_EDP0>;
+		remote-endpoint = <&edp0_in_vp2>;
+	};
+};
-- 
2.34.1

Re: [PATCH v1 10/10] arch64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board

[Heiko Stübner]

Hi Damon,

Am Mittwoch, 27. November 2024, 08:51:57 CET schrieb Damon Ding:
> Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board.
> 
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
>  .../boot/dts/rockchip/rk3588s-evb1-v10.dts    | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> index bc4077575beb..67c2e6d1bece 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,vop2.h>
>  #include <dt-bindings/usb/pd.h>
>  #include "rk3588s.dtsi"
>  
> @@ -116,10 +117,56 @@ masterdai: simple-audio-card,codec {
>  
>  	backlight: backlight {
>  		compatible = "pwm-backlight";
> +		brightness-levels = <
> +			  0   1   2   3   4   5   6   7
> +			  8   9  10  11  12  13  14  15
> +			 16  17  18  19  20  21  22  23
> +			 24  25  26  27  28  29  30  31
> +			 32  33  34  35  36  37  38  39
> +			 40  41  42  43  44  45  46  47
> +			 48  49  50  51  52  53  54  55
> +			 56  57  58  59  60  61  62  63
> +			 64  65  66  67  68  69  70  71
> +			 72  73  74  75  76  77  78  79
> +			 80  81  82  83  84  85  86  87
> +			 88  89  90  91  92  93  94  95
> +			 96  97  98  99 100 101 102 103
> +			104 105 106 107 108 109 110 111
> +			112 113 114 115 116 117 118 119
> +			120 121 122 123 124 125 126 127
> +			128 129 130 131 132 133 134 135
> +			136 137 138 139 140 141 142 143
> +			144 145 146 147 148 149 150 151
> +			152 153 154 155 156 157 158 159
> +			160 161 162 163 164 165 166 167
> +			168 169 170 171 172 173 174 175
> +			176 177 178 179 180 181 182 183
> +			184 185 186 187 188 189 190 191
> +			192 193 194 195 196 197 198 199
> +			200 201 202 203 204 205 206 207
> +			208 209 210 211 212 213 214 215
> +			216 217 218 219 220 221 222 223
> +			224 225 226 227 228 229 230 231
> +			232 233 234 235 236 237 238 239
> +			240 241 242 243 244 245 246 247
> +			248 249 250 251 252 253 254 255>;

manual brightness-levels shouldn't be needed anymore and the backlight
can create those automatically.

So both the brightness-levels and default-brightness-level should be able
to go away?

> +		default-brightness-level = <200>;
>  		power-supply = <&vcc3v3_lcd_edp>;
>  		pwms = <&pwm12 0 25000 0>;
>  	};
>  


Heiko

Re: [PATCH v1 00/10] Add eDP support for RK3588

[Heiko Stübner]

Hi Daemon,

Am Mittwoch, 27. November 2024, 08:51:47 CET schrieb Damon Ding:
> These patchs have been tested with a 1536x2048p60 eDP panel on
> RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
> on RK3588 EVB1 board.
> 
> Patch 1~3 are the RK3588 eDP support of Rockchip analogix_dp driver.
> Patch 4   is the eDP mode support of samsung hdptx phy driver.
> Patch 5~6 are the Rk3588 eDP support of Aanalogix DP driver. Add phy
>           interfaces is to configure the HDMI/eDP TX Combo PHY.
> Patch 7~8 are the renaming of hdptxphy node. It is not only used by
>           HDMI display but also for the eDP display.
> Patch 9   is the addition of RK3588 eDP0 node.
> Patch 10  is to enable the eDP0 display on RK3588S EVB1 board.

Could you maybe also bring over functionality for real bridge-handling?
That way we'd have support for things like the dp-connector bridge.

In the 6.1 vendor-tree I've found commits
94e598190128 ("drm/rockchip: analogix_dp: Add support for external bridge")
437e0a901b14 ("drm/bridge: analogix_dp: Support split mode for bridge chain")

needing a bit of cleanup of course, but that would get rid of the driver
not handling the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag too.

With a bit of streamlining, we could maybe even get rid of the panel-part
completely, similar to how the dw-dsi controllers do it [0]


Thanks
Heiko

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#n335
devm_drm_of_get_bridge() combines drm_of_find_panel_or_bridge()
with devm_drm_panel_bridge_add(), so indepent of it being either a
panel or other bridge, the driver below only needs to handle bridges.

> Damon Ding (10):
>   drm/rockchip: analogix_dp: Use formalized struct definition for grf
>     field
>   dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
>   drm/rockchip: analogix_dp: Add support for RK3588
>   phy: phy-rockchip-samsung-hdptx: Add support for eDP mode
>   drm/bridge: analogix_dp: add support for RK3588
>   drm/bridge: analogix_dp: Add support for phy configuration.
>   dt-bindings: display: rockchip: Fix label name of hdptxphy for RK3588
>     HDMI TX Controller
>   arm64: dts: rockchip: Fix label name of hdptxphy for RK3588
>   arm64: dts: rockchip: Add eDP0 node for RK3588
>   arch64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board
> 
>  .../rockchip/rockchip,analogix-dp.yaml        |   1 +
>  .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml  |   2 +-
>  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  33 +-
>  .../dts/rockchip/rk3588-coolpi-cm5-evb.dts    |   2 +-
>  .../rockchip/rk3588-coolpi-cm5-genbook.dts    |   2 +-
>  .../boot/dts/rockchip/rk3588-evb1-v10.dts     |   2 +-
>  .../rk3588-friendlyelec-cm3588-nas.dts        |   2 +-
>  .../arm64/boot/dts/rockchip/rk3588-jaguar.dts |   2 +-
>  .../boot/dts/rockchip/rk3588-nanopc-t6.dtsi   |   2 +-
>  .../dts/rockchip/rk3588-orangepi-5-plus.dts   |   2 +-
>  .../boot/dts/rockchip/rk3588-rock-5b.dts      |   2 +-
>  .../boot/dts/rockchip/rk3588-tiger-haikou.dts |   2 +-
>  .../boot/dts/rockchip/rk3588s-coolpi-4b.dts   |   2 +-
>  .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  84 ++
>  .../dts/rockchip/rk3588s-indiedroid-nova.dts  |   2 +-
>  .../boot/dts/rockchip/rk3588s-nanopi-r6.dtsi  |   2 +-
>  .../boot/dts/rockchip/rk3588s-odroid-m2.dts   |   2 +-
>  .../boot/dts/rockchip/rk3588s-orangepi-5.dtsi |   2 +-
>  .../boot/dts/rockchip/rk3588s-rock-5a.dts     |   2 +-
>  .../boot/dts/rockchip/rk3588s-rock-5c.dts     |   2 +-
>  .../drm/bridge/analogix/analogix_dp_core.c    |   8 +-
>  .../drm/bridge/analogix/analogix_dp_core.h    |   2 +
>  .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  90 ++
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 112 ++-
>  .../phy/rockchip/phy-rockchip-samsung-hdptx.c | 936 +++++++++++++++++-
>  include/drm/bridge/analogix_dp.h              |   3 +-
>  26 files changed, 1206 insertions(+), 97 deletions(-)
> 
> 

Re: [PATCH v1 00/10] Add eDP support for RK3588

[Damon Ding]

Hi Heiko,

On 2024/12/6 22:35, Heiko Stübner wrote:
> Hi Daemon,
> 
> Am Mittwoch, 27. November 2024, 08:51:47 CET schrieb Damon Ding:
>> These patchs have been tested with a 1536x2048p60 eDP panel on
>> RK3588S EVB1 board, and HDMI 1080P/4K display also has been verified
>> on RK3588 EVB1 board.
>>
>> Patch 1~3 are the RK3588 eDP support of Rockchip analogix_dp driver.
>> Patch 4   is the eDP mode support of samsung hdptx phy driver.
>> Patch 5~6 are the Rk3588 eDP support of Aanalogix DP driver. Add phy
>>            interfaces is to configure the HDMI/eDP TX Combo PHY.
>> Patch 7~8 are the renaming of hdptxphy node. It is not only used by
>>            HDMI display but also for the eDP display.
>> Patch 9   is the addition of RK3588 eDP0 node.
>> Patch 10  is to enable the eDP0 display on RK3588S EVB1 board.
> 
> Could you maybe also bring over functionality for real bridge-handling?
> That way we'd have support for things like the dp-connector bridge.
> 
> In the 6.1 vendor-tree I've found commits
> 94e598190128 ("drm/rockchip: analogix_dp: Add support for external bridge")
> 437e0a901b14 ("drm/bridge: analogix_dp: Support split mode for bridge chain")
> 
> needing a bit of cleanup of course, but that would get rid of the driver
> not handling the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag too.
> 
> With a bit of streamlining, we could maybe even get rid of the panel-part
> completely, similar to how the dw-dsi controllers do it [0]
> 
> 

Indeed, the patches related to the bridge support have not been included 
in this series. My intention was to first implement the basic display 
functionality, and then gradually add other features, including the 
support for bridge-handling. Otherwise, this series of patches may be 
too large. :-)

What's more, RK3576 SoC also supports the eDP display, so the subsequent 
patch series will come soon.

> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#n335
> devm_drm_of_get_bridge() combines drm_of_find_panel_or_bridge()
> with devm_drm_panel_bridge_add(), so indepent of it being either a
> panel or other bridge, the driver below only needs to handle bridges.
> 
>> Damon Ding (10):
>>    drm/rockchip: analogix_dp: Use formalized struct definition for grf
>>      field
>>    dt-bindings: display: rockchip: analogix-dp: Add support for RK3588
>>    drm/rockchip: analogix_dp: Add support for RK3588
>>    phy: phy-rockchip-samsung-hdptx: Add support for eDP mode
>>    drm/bridge: analogix_dp: add support for RK3588
>>    drm/bridge: analogix_dp: Add support for phy configuration.
>>    dt-bindings: display: rockchip: Fix label name of hdptxphy for RK3588
>>      HDMI TX Controller
>>    arm64: dts: rockchip: Fix label name of hdptxphy for RK3588
>>    arm64: dts: rockchip: Add eDP0 node for RK3588
>>    arch64: dts: rockchip: Enable eDP0 display on RK3588S EVB1 board
>>
>>   .../rockchip/rockchip,analogix-dp.yaml        |   1 +
>>   .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml  |   2 +-
>>   arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  33 +-
>>   .../dts/rockchip/rk3588-coolpi-cm5-evb.dts    |   2 +-
>>   .../rockchip/rk3588-coolpi-cm5-genbook.dts    |   2 +-
>>   .../boot/dts/rockchip/rk3588-evb1-v10.dts     |   2 +-
>>   .../rk3588-friendlyelec-cm3588-nas.dts        |   2 +-
>>   .../arm64/boot/dts/rockchip/rk3588-jaguar.dts |   2 +-
>>   .../boot/dts/rockchip/rk3588-nanopc-t6.dtsi   |   2 +-
>>   .../dts/rockchip/rk3588-orangepi-5-plus.dts   |   2 +-
>>   .../boot/dts/rockchip/rk3588-rock-5b.dts      |   2 +-
>>   .../boot/dts/rockchip/rk3588-tiger-haikou.dts |   2 +-
>>   .../boot/dts/rockchip/rk3588s-coolpi-4b.dts   |   2 +-
>>   .../boot/dts/rockchip/rk3588s-evb1-v10.dts    |  84 ++
>>   .../dts/rockchip/rk3588s-indiedroid-nova.dts  |   2 +-
>>   .../boot/dts/rockchip/rk3588s-nanopi-r6.dtsi  |   2 +-
>>   .../boot/dts/rockchip/rk3588s-odroid-m2.dts   |   2 +-
>>   .../boot/dts/rockchip/rk3588s-orangepi-5.dtsi |   2 +-
>>   .../boot/dts/rockchip/rk3588s-rock-5a.dts     |   2 +-
>>   .../boot/dts/rockchip/rk3588s-rock-5c.dts     |   2 +-
>>   .../drm/bridge/analogix/analogix_dp_core.c    |   8 +-
>>   .../drm/bridge/analogix/analogix_dp_core.h    |   2 +
>>   .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  90 ++
>>   .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 112 ++-
>>   .../phy/rockchip/phy-rockchip-samsung-hdptx.c | 936 +++++++++++++++++-
>>   include/drm/bridge/analogix_dp.h              |   3 +-
>>   26 files changed, 1206 insertions(+), 97 deletions(-)
>>
>>
> 
> 
> 
> 
> 
> 
Best regards,
Damon