Claude review: Add eDP support for RK3576

Claude review: Add eDP support for RK3576

[Claude Code Review Bot]

Overall Series Review

Subject: Add eDP support for RK3576
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 6
Reviewed: 2026-03-11T13:27:10.390206

---

This is a small, straightforward 4-patch series adding eDP support for the RK3576 SoC, leveraging the existing Analogix DP infrastructure already used by RK3288/RK3399/RK3588. The cover letter states it was tested on RK3576 EVB1 hardware. The series is generally well-structured and follows existing patterns, but there are a few issues in the DT binding patch and a concern about the hclk lifecycle management.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 0/9] Add eDP support for RK3576

[Damon Ding]

These patchs have been tested with a 1536x2048p60 eDP panel on
RK3576 EVB1 board (hardware modified specially).

Patch 1~4 are to add missing clock "hclk" for RK3588 eDP nodes.
Patch 5~6 are to add the RK3576 eDP node.
Patch 7~9 are to support the RK3576 Analogix DP controller.

Damon Ding (9):
  dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk"
    for the third clock
  arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP0 nodes.
  arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP1 nodes.
  drm/rockchip: analogix_dp: Enable clock "hclk" for RK3588
  dt-bindings: display: rockchip: analogix-dp: Add support for RK3576
  arm64: dts: rockchip: Add eDP node for RK3576
  drm/bridge: analogix_dp: Rename and simplify is_rockchip()
  drm/bridge: analogix_dp: Add support for RK3576
  drm/rockchip: analogix_dp: Add support for RK3576

 .../rockchip/rockchip,analogix-dp.yaml        |  6 +++-
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      | 28 +++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  4 +--
 .../arm64/boot/dts/rockchip/rk3588-extra.dtsi |  4 +--
 .../drm/bridge/analogix/analogix_dp_core.c    |  3 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 ++++++------
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   | 15 ++++++++++
 include/drm/bridge/analogix_dp.h              | 13 +++++++--
 8 files changed, 74 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH v2 1/9] dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk" for the third clock

[Damon Ding]

The RK3588 eDP controller needs the video datapath clock "hclk" to work
well. Previously, it works without explicitly adding this clock because
the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.

Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 .../bindings/display/rockchip/rockchip,analogix-dp.yaml       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index d99b23b88cc5..d2bc8636b626 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -26,7 +26,9 @@ properties:
     items:
       - const: dp
       - const: pclk
-      - const: grf
+      - enum:
+          - grf
+          - hclk
 
   power-domains:
     maxItems: 1
-- 
2.34.1

[PATCH v2 2/9] arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP0 nodes.

[Damon Ding]

The RK3588 eDP controller needs the video datapath clock "hclk" to work
well. Previously, it works without explicitly adding this clock because
the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.

Fixes: dc79d3d5e7c7 ("arm64: dts: rockchip: Add eDP0 node for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 2a7921793020..4cf87d0c82cd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1648,8 +1648,8 @@ hdmi0_out: port@1 {
 	edp0: edp@fdec0000 {
 		compatible = "rockchip,rk3588-edp";
 		reg = <0x0 0xfdec0000 0x0 0x1000>;
-		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>;
-		clock-names = "dp", "pclk";
+		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru HCLK_VO1>;
+		clock-names = "dp", "pclk", "hclk";
 		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH 0>;
 		phys = <&hdptxphy0>;
 		phy-names = "dp";
-- 
2.34.1

[PATCH v2 3/9] arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP1 nodes.

[Damon Ding]

The RK3588 eDP controller needs the video datapath clock "hclk" to work
well. Previously, it works without explicitly adding this clock because
the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.

Fixes: a481bb0b1ad9 ("arm64: dts: rockchip: Add eDP1 dt node for rk3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
index 6e5a58428bba..71fc4fc9e556 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
@@ -285,8 +285,8 @@ hdmi1_out: port@1 {
 	edp1: edp@fded0000 {
 		compatible = "rockchip,rk3588-edp";
 		reg = <0x0 0xfded0000 0x0 0x1000>;
-		clocks = <&cru CLK_EDP1_24M>, <&cru PCLK_EDP1>;
-		clock-names = "dp", "pclk";
+		clocks = <&cru CLK_EDP1_24M>, <&cru PCLK_EDP1>, <&cru HCLK_VO1>;
+		clock-names = "dp", "pclk", "hclk";
 		interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH 0>;
 		phys = <&hdptxphy1>;
 		phy-names = "dp";
-- 
2.34.1

[PATCH v2 4/9] drm/rockchip: analogix_dp: Enable clock "hclk" for RK3588

[Damon Ding]

The RK3588 eDP controller needs the video datapath clock "hclk" to work
well. Previously, it works without explicitly adding this clock because
the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.

Fixes: 729f8eefdcad ("drm/rockchip: analogix_dp: Add support for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 832e9766bef0..aa01ede4945e 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -311,6 +311,7 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 {
 	struct device *dev = dp->dev;
 	struct device_node *np = dev->of_node;
+	struct clk *clk;
 
 	dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
 	if (IS_ERR(dp->grf))
@@ -327,6 +328,11 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
 		return dev_err_probe(dev, PTR_ERR(dp->pclk),
 				     "failed to get pclk property\n");
 
+	clk = devm_clk_get_optional_enabled(dev, "hclk");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "failed to get hclk property\n");
+
 	dp->rst = devm_reset_control_get(dev, "dp");
 	if (IS_ERR(dp->rst))
 		return dev_err_probe(dev, PTR_ERR(dp->rst),
-- 
2.34.1

[PATCH v2 5/9] dt-bindings: display: rockchip: analogix-dp: Add support for RK3576

[Damon Ding]

The eDP TX controller on RK3576 is the same as that on RK3588.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

-----

Changes in v2:
- Split out a separate patch to add the "hclk" clock reference.
---
 .../bindings/display/rockchip/rockchip,analogix-dp.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index d2bc8636b626..4496a43881f9 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,rk3576-edp
       - rockchip,rk3588-edp
 
   clocks:
@@ -67,6 +68,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - rockchip,rk3576-edp
               - rockchip,rk3588-edp
     then:
       properties:
-- 
2.34.1

[PATCH v2 6/9] arm64: dts: rockchip: Add eDP node for RK3576

[Damon Ding]

Add support for the eDP output on RK3576 SoC.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

-----

Changes in v2:
- Add Reviewed-by tag.
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 28 ++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index a86fc6b4e8c4..14900a66d3e1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1446,6 +1446,34 @@ hdmi_out: port@1 {
 			};
 		};
 
+		edp: edp@27dc0000 {
+			compatible = "rockchip,rk3576-edp";
+			reg = <0x0 0x27dc0000 0x0 0x1000>;
+			clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru HCLK_VO0_ROOT>;
+			clock-names = "dp", "pclk", "hclk";
+			interrupts = <GIC_SPI 365 IRQ_TYPE_LEVEL_HIGH>;
+			phys = <&hdptxphy>;
+			phy-names = "dp";
+			power-domains = <&power RK3576_PD_VO0>;
+			resets = <&cru SRST_EDP0_24M>, <&cru SRST_P_EDP0>;
+			reset-names = "dp", "apb";
+			rockchip,grf = <&vo0_grf>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				edp_in: port@0 {
+					reg = <0>;
+				};
+
+				edp_out: port@1 {
+					reg = <1>;
+				};
+			};
+		};
+
 		sai7: sai@27ed0000 {
 			compatible = "rockchip,rk3576-sai";
 			reg = <0x0 0x27ed0000 0x0 0x1000>;
-- 
2.34.1

[PATCH v2 7/9] drm/bridge: analogix_dp: Rename and simplify is_rockchip()

[Damon Ding]

Rename is_rockchip() to analogix_dp_is_rockchip() for naming consistency
and readability, and simplify the code with switch.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Suggested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../gpu/drm/bridge/analogix/analogix_dp_core.c |  2 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c  | 18 +++++++++---------
 include/drm/bridge/analogix_dp.h               | 11 +++++++++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index fe7158d9edde..045fdb77531f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -869,7 +869,7 @@ static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
 	struct drm_display_info *di = &conn_state->connector->display_info;
 	u32 mask = DRM_COLOR_FORMAT_YCBCR444 | DRM_COLOR_FORMAT_YCBCR422;
 
-	if (is_rockchip(dp->plat_data->dev_type)) {
+	if (analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
 		if ((di->color_formats & mask)) {
 			DRM_DEBUG_KMS("Swapping display color format from YUV to RGB\n");
 			di->color_formats &= ~mask;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 38fd8d5014d2..6207ded7ffd5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -72,7 +72,7 @@ void analogix_dp_init_analog_param(struct analogix_dp_device *dp)
 	reg = SEL_24M | TX_DVDD_BIT_1_0625V;
 	writel(reg, dp->reg_base + ANALOGIX_DP_ANALOG_CTL_2);
 
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) {
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
 		reg = REF_CLK_24M;
 		if (dp->plat_data->dev_type == RK3288_DP)
 			reg ^= REF_CLK_MASK;
@@ -123,7 +123,7 @@ void analogix_dp_reset(struct analogix_dp_device *dp)
 	analogix_dp_stop_video(dp);
 	analogix_dp_enable_video_mute(dp, 0);
 
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 		reg = RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N |
 			SW_FUNC_EN_N;
 	else
@@ -233,7 +233,7 @@ void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable)
 	u32 mask = DP_PLL_PD;
 	u32 pd_addr = ANALOGIX_DP_PLL_CTL;
 
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) {
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
 		pd_addr = ANALOGIX_DP_PD;
 		mask = RK_PLL_PD;
 	}
@@ -254,12 +254,12 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
 	u32 phy_pd_addr = ANALOGIX_DP_PHY_PD;
 	u32 mask;
 
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 		phy_pd_addr = ANALOGIX_DP_PD;
 
 	switch (block) {
 	case AUX_BLOCK:
-		if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+		if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 			mask = RK_AUX_PD;
 		else
 			mask = AUX_PD;
@@ -317,7 +317,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
 		 * to power off everything instead of DP_PHY_PD in
 		 * Rockchip
 		 */
-		if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+		if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 			mask = DP_INC_BG;
 		else
 			mask = DP_PHY_PD;
@@ -329,7 +329,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
 			reg &= ~mask;
 
 		writel(reg, dp->reg_base + phy_pd_addr);
-		if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+		if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 			usleep_range(10, 15);
 		break;
 	case POWER_ALL:
@@ -465,7 +465,7 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp)
 	analogix_dp_reset_aux(dp);
 
 	/* AUX_BIT_PERIOD_EXPECTED_DELAY doesn't apply to Rockchip IP */
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type))
 		reg = 0;
 	else
 		reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3);
@@ -837,7 +837,7 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp)
 	u32 reg;
 
 	reg = readl(dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
-	if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) {
+	if (dp->plat_data && analogix_dp_is_rockchip(dp->plat_data->dev_type)) {
 		reg &= ~(RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N);
 	} else {
 		reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N);
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 854af692229b..7b670dd769e9 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -19,9 +19,16 @@ enum analogix_dp_devtype {
 	RK3588_EDP,
 };
 
-static inline bool is_rockchip(enum analogix_dp_devtype type)
+static inline bool analogix_dp_is_rockchip(enum analogix_dp_devtype type)
 {
-	return type == RK3288_DP || type == RK3399_EDP || type == RK3588_EDP;
+	switch (type) {
+	case RK3288_DP:
+	case RK3399_EDP:
+	case RK3588_EDP:
+		return true;
+	default:
+		return false;
+	}
 }
 
 struct analogix_dp_plat_data {
-- 
2.34.1

[PATCH v2 8/9] drm/bridge: analogix_dp: Add support for RK3576

[Damon Ding]

Expand enum analogix_dp_devtype with RK3576_EDP, and add max_link_rate
and max_lane_count configs for it.

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

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 045fdb77531f..641f37fbea27 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1248,6 +1248,7 @@ 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 RK3576_EDP:
 	case RK3588_EDP:
 		video_info->max_link_rate = 0x14;
 		video_info->max_lane_count = 0x04;
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 7b670dd769e9..0e0b87abee59 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -16,6 +16,7 @@ enum analogix_dp_devtype {
 	EXYNOS_DP,
 	RK3288_DP,
 	RK3399_EDP,
+	RK3576_EDP,
 	RK3588_EDP,
 };
 
@@ -24,6 +25,7 @@ static inline bool analogix_dp_is_rockchip(enum analogix_dp_devtype type)
 	switch (type) {
 	case RK3288_DP:
 	case RK3399_EDP:
+	case RK3576_EDP:
 	case RK3588_EDP:
 		return true;
 	default:
-- 
2.34.1

[PATCH v2 9/9] drm/rockchip: analogix_dp: Add support for RK3576

[Damon Ding]

RK3576 integrates the Analogix eDP 1.3 TX controller IP and the HDMI/eDP
TX Combo PHY based on a Samsung IP block - both of which are the same as
those on RK3588.

The patch currently adds only the basic support, specifically RGB output
up to 4K@60Hz, without the tests for audio, PSR and other eDP 1.3 specific
features.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

-----

Changes in v2:
- Split out a separate patch to enable the "hclk" clock.
- Add Reviewed-by tag.
---
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index aa01ede4945e..942f81edddfa 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -520,6 +520,14 @@ static const struct rockchip_dp_chip_data rk3288_dp[] = {
 	{ /* sentinel */ }
 };
 
+static const struct rockchip_dp_chip_data rk3576_edp[] = {
+	{
+		.chip_type = RK3576_EDP,
+		.reg = 0x27dc0000,
+	},
+	{ /* sentinel */ }
+};
+
 static const struct rockchip_dp_chip_data rk3588_edp[] = {
 	{
 		.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
@@ -537,6 +545,7 @@ static const struct rockchip_dp_chip_data 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,rk3576-edp", .data = &rk3576_edp },
 	{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
 	{}
 };
-- 
2.34.1

Re: [PATCH v2 1/9] dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk" for the third clock

[Krzysztof Kozlowski]

On Thu, Mar 19, 2026 at 06:40:23PM +0800, Damon Ding wrote:
> The RK3588 eDP controller needs the video datapath clock "hclk" to work
> well. Previously, it works without explicitly adding this clock because
> the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.
> 
> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
> ---
>  .../bindings/display/rockchip/rockchip,analogix-dp.yaml       | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> index d99b23b88cc5..d2bc8636b626 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> @@ -26,7 +26,9 @@ properties:
>      items:
>        - const: dp
>        - const: pclk
> -      - const: grf
> +      - enum:
> +          - grf
> +          - hclk

You just told me it is the same clock, no? I asked that and you
confirmed? Or you replied with an answer not relevant to the question?

If this device takes reference to grf, why does it also take grf clock?
Devices MUST NOT take clocks on behalf of other devices, so please
answer with a list of all clock inputs according to datasheet/manual.

Best regards,
Krzysztof

Re: [PATCH v2 1/9] dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk" for the third clock

[Krzysztof Kozlowski]

On 20/03/2026 10:19, Krzysztof Kozlowski wrote:
> On Thu, Mar 19, 2026 at 06:40:23PM +0800, Damon Ding wrote:
>> The RK3588 eDP controller needs the video datapath clock "hclk" to work
>> well. Previously, it works without explicitly adding this clock because
>> the 'rockchip,vo-grf = <&vo1_grf>' property implicitly enables HCLK_VO1.
>>
>> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>> ---

BTW, all your patches (this and previous set... actually, everything you
ever sent IIRC) end up marked as spam. I don't see anything in the
headers pointing to the issue. There is no score assigned and
DMARC+SPF+DKIM pass.

However it seems you are using @rock-chips.com and SMTP server 163.com,
so maybe that difference. Or maybe it is rock-chips lack of DMARC policy
(p=none dis=none)?

Regardless of the reason, if you do not ever receive answers from
people, it is likely that is the reason.

Best regards,
Krzysztof

Claude review: Add eDP support for RK3576

[Claude Code Review Bot]

Overall Series Review

Subject: Add eDP support for RK3576
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 12
Reviewed: 2026-03-22T04:26:14.323016

---

This is a well-structured 9-patch series adding eDP support for the Rockchip RK3576 SoC to the analogix DP bridge driver. The series is logically organized: patches 1-4 fix a missing "hclk" clock for existing RK3588 support (with Fixes tags), patch 5-6 add dt-bindings and DTS for RK3576, and patches 7-9 add the driver-side RK3576 support.

The changes are straightforward and follow existing patterns. The series has a few minor issues worth addressing, mostly around the dt-bindings and a questionable `devm_clk_get_optional_enabled` usage.

---
Generated by Claude Code Patch Reviewer

Claude review: dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk" for the third clock

[Claude Code Review Bot]

Patch Review

The change from a fixed `const: grf` to an `enum: [grf, hclk]` for the third clock is correct for allowing both the existing RK3288/RK3399 "grf" clock and the new RK3588/RK3576 "hclk" clock.

However, the binding should ideally be more constrained using `allOf`/`if`/`then` blocks to enforce that RK3288/RK3399 use "grf" and RK3588/RK3576 use "hclk", rather than allowing either name on any compatible. As it stands, nothing prevents a DT author from specifying "hclk" on rk3288 or "grf" on rk3588. This is a minor binding correctness concern.

The Fixes tag referencing the RK3588 binding addition is appropriate.

---
Generated by Claude Code Patch Reviewer

Claude review: arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP0 nodes.

[Claude Code Review Bot]

Patch Review

Straightforward DTS fix adding the third clock (`HCLK_VO1`) to the eDP0 node. The change from 2 to 3 clocks with the added "hclk" name is correct and consistent with the binding update in patch 1.

Looks correct.

---
Generated by Claude Code Patch Reviewer

Claude review: arm64: dts: rockchip: Add missing clock "hclk" for RK3588 eDP1 nodes.

[Claude Code Review Bot]

Patch Review

Same as patch 2 but for eDP1. Looks correct.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: analogix_dp: Enable clock "hclk" for RK3588

[Claude Code Review Bot]

Patch Review

```c
+	clk = devm_clk_get_optional_enabled(dev, "hclk");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "failed to get hclk property\n");
```

**Concern**: Using `devm_clk_get_optional_enabled()` means the clock is enabled at probe time and stays enabled for the lifetime of the device. This is different from how `pclk` is handled — `pclk` is stored in `dp->pclk` and explicitly enabled/disabled in `rockchip_dp_poweron()`/`rockchip_dp_powerdown()`. If `hclk` is a video datapath clock needed for operation, it would be more power-efficient to enable/disable it alongside `pclk` in the power on/off callbacks. That said, if the clock domain is shared and always-on in practice, this approach is simpler and acceptable.

The error message says "failed to get hclk property" — "property" is misleading for a clock; "failed to get hclk clock" would be clearer, matching the terminology used elsewhere.

The Fixes tag is appropriate.

---
Generated by Claude Code Patch Reviewer

Claude review: dt-bindings: display: rockchip: analogix-dp: Add support for RK3576

[Claude Code Review Bot]

Patch Review

```yaml
+      - rockchip,rk3576-edp
```

Adding `rockchip,rk3576-edp` to the compatible enum and to the `if`/`then` block that requires 2 resets is correct.

Minor: The cover letter says "The eDP TX controller on RK3576 is the same as that on RK3588" — if that's the case, consider whether a fallback compatible like `compatible = "rockchip,rk3576-edp", "rockchip,rk3588-edp"` would be appropriate (documented in the binding). This is a DT best practice for SoCs sharing the same IP block, though it requires driver cooperation.

---
Generated by Claude Code Patch Reviewer

Claude review: arm64: dts: rockchip: Add eDP node for RK3576

[Claude Code Review Bot]

Patch Review

The DTS node looks well-formed. It follows the pattern of the RK3588 eDP nodes with appropriate RK3576-specific clock/reset/interrupt references.

One observation: The RK3588 eDP nodes do not have a `power-domains` property in the upstream DTS (only clocks, resets, phys, etc.), but this RK3576 node adds `power-domains = <&power RK3576_PD_VO0>`. This is likely correct for RK3576 but worth confirming it's needed.

Reviewed-by tag from Nicolas Frattaroli is present. Looks good.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: analogix_dp: Rename and simplify is_rockchip()

[Claude Code Review Bot]

Patch Review

The rename from `is_rockchip()` to `analogix_dp_is_rockchip()` improves namespace consistency. The switch-based implementation is cleaner and makes adding new Rockchip variants easier (no need to extend a chain of `||`).

This is a clean mechanical change. The `Suggested-by` tag crediting Nicolas Frattaroli is noted.

One nit: `Suggested-by` should conventionally appear before `Signed-off-by` in the commit message, not after. Currently:
```
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Suggested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
```

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: analogix_dp: Add support for RK3576

[Claude Code Review Bot]

Patch Review

```c
+	case RK3576_EDP:
 	case RK3588_EDP:
 		video_info->max_link_rate = 0x14;
 		video_info->max_lane_count = 0x04;
```

RK3576_EDP falls through to the RK3588_EDP case, sharing the same max_link_rate (0x14 = HBR2, 5.4 Gbps) and max_lane_count (4 lanes). Since the cover letter states the eDP TX controller IP is the same as RK3588, this is correct.

The enum addition and `analogix_dp_is_rockchip()` update are straightforward.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: analogix_dp: Add support for RK3576

[Claude Code Review Bot]

Patch Review

```c
+static const struct rockchip_dp_chip_data rk3576_edp[] = {
+	{
+		.chip_type = RK3576_EDP,
+		.reg = 0x27dc0000,
+	},
+	{ /* sentinel */ }
+};
```

**Notable difference from RK3588**: The RK3576 chip_data does not set `.edp_mode`, unlike RK3588 which has:
```c
.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
```

This means `rockchip_dp_poweron()` will call `rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1)` which checks `field->valid` (which will be `false` since `.edp_mode` is zero-initialized) and returns 0 — so it's a no-op. This is correct behavior if RK3576 doesn't need the edp_mode GRF write, but it would be worth a brief comment or commit message note explaining why this isn't needed for RK3576.

Similarly, `.lcdc_sel` is not set, so the GRF write in `rockchip_dp_drm_encoder_enable()` will also be a no-op. This seems intentional but should be confirmed — does RK3576 not need VOP output routing via GRF?

The Reviewed-by tag from Nicolas Frattaroli is present. The change is otherwise straightforward and follows existing patterns.

---
Generated by Claude Code Patch Reviewer