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

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

[Claude Code Review Bot]

Patch Review

**hclk lifetime concern.** The `hclk` is acquired with `devm_clk_get_optional_enabled()`:

```c
+	dp->hclk = devm_clk_get_optional_enabled(dev, "hclk");
```

This means the clock is enabled at probe time and stays enabled for the entire lifetime of the device (until driver removal). The commit message says "which must be enabled during probing for RK3576 eDP controller," but keeping it permanently enabled may waste power. Consider whether it should instead be enabled/disabled alongside `pclk` in `rockchip_dp_poweron()`/`rockchip_dp_powerdown()`. If it truly needs to be always-on, a comment explaining why would be helpful.

That said, `devm_clk_get_optional_enabled` is a valid pattern used elsewhere in the kernel, and if the clock must be on whenever the device is bound (e.g., for register access), this is correct.

**Missing `edp_mode` GRF field.** The RK3576 chip_data has no `edp_mode` field set:

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

Compare with RK3588 which has:
```c
	.edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
```

The `edp_mode` field is used in `rockchip_dp_poweron()` and `rockchip_dp_powerdown()` via `rockchip_grf_field_write()`. Since the field's `.valid` will be `false` (zero-initialized), `rockchip_grf_field_write()` will return 0 early — so this is functionally safe. But the commit message should clarify whether RK3576 genuinely has no GRF edp_mode bit, or if this is an oversight.

**No `lcdc_sel` either** — this is used in `rockchip_dp_drm_encoder_enable()`. Again, `rockchip_grf_field_write()` will silently no-op due to `.valid = false`. This is consistent with RK3588 which also has no `lcdc_sel`, so this appears intentional.

No blocking issues, but the hclk lifecycle and missing `edp_mode` documentation could use clarification.

---
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

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

[Claude Code Review Bot]

Patch Review

**Status: Looks good**

Adds `rk3576_edp` chip data and OF match table entry:

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

The chip data intentionally omits `edp_mode` and `lcdc_sel` GRF fields. This is safe because `rockchip_grf_field_write()` checks `field->valid` (which defaults to `false` in zero-initialized structs) and returns 0 without writing. If RK3576 genuinely doesn't need GRF mode switching for eDP, this is correct. If it does need a GRF write that was overlooked, eDP would fail to initialize at runtime -- but this has Reviewed-by from two reviewers who presumably tested on hardware.

The OF match table entry is correctly sorted alphabetically between `rk3399-edp` and `rk3588-edp`.

---
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 */ }
+};
```

**Question/concern**: The RK3576 chip data does not set `edp_mode`, unlike RK3588 which has `.edp_mode = GRF_REG_FIELD(0x0000, 0, 0)`. This is *functionally safe* — `rockchip_grf_field_write()` checks `!field->valid` and returns 0 for unset fields (line 107 in `analogix_dp-rockchip.c`). However, the commit message states RK3576 "fully match[es] the proven RK3588 design." If the hardware really does match RK3588, does RK3576 not have an eDP mode selection bit in its VO0 GRF? If it does, it should be populated here. If it genuinely doesn't need one, a brief comment or commit message note explaining why would be helpful.

**Also**: The single-entry array (vs RK3588's two entries) correctly reflects that RK3576 has only one eDP controller.

The DT match table addition maintains alphabetical ordering:

```c
+	{.compatible = "rockchip,rk3576-edp", .data = &rk3576_edp },
```

Overall this patch is fine, but the missing `edp_mode` relative to RK3588 deserves confirmation from the author.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds chip data and OF match table entry:

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

Worth noting: the RK3576 chip data intentionally omits `.edp_mode` and `.lcdc_sel` GRF fields (unlike RK3588 which sets `.edp_mode`). This is safe -- `rockchip_grf_field_write()` at line 101 of the driver checks `field->valid` (which is `false` when zero-initialized) and returns 0 immediately:

```c
static int rockchip_grf_field_write(struct regmap *grf,
				    const struct rockchip_grf_reg_field *field,
				    u32 val)
{
	if (!field->valid)
		return 0;
```

So the `edp_mode` GRF writes in `rockchip_dp_poweron()`/`rockchip_dp_powerdown()` become no-ops for RK3576. This is presumably correct if RK3576 doesn't need GRF-based eDP mode switching, but a brief comment in the chip data or commit message noting this intentional omission would help future readers understand why.

No blocking issues. Series looks ready to merge.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds the platform-specific chip data and OF match entry:

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

**Key observation:** RK3576's chip data does not set `edp_mode` or `lcdc_sel` GRF fields, unlike RK3588 and the older SoCs. I verified that `rockchip_grf_field_write()` checks `!field->valid` and returns 0 early (at `analogix_dp-rockchip.c:107`), so the zero-initialized `edp_mode` field is safe -- the GRF write in `rockchip_dp_poweron()` and `rockchip_dp_powerdown()` will be a no-op. This implies RK3576 doesn't need an eDP mode GRF switch, which is plausible if the VO subsystem handles muxing differently.

Similarly, no `lcdc_sel` means the CRTC selection path in `rockchip_dp_drm_encoder_enable()` will skip the GRF write. This should be fine if RK3576 has a fixed eDP-to-CRTC mapping (no mux).

The OF table entry is correctly ordered alphabetically. The sentinel terminator is present.

**Overall:** This is a clean, well-organized series at v7 maturity. The RK3588 clock fix (patches 1-5) is a proper standalone fix with Fixes tags. The RK3576 support (patches 6-10) reuses the proven RK3588 code path with minimal additions. No correctness bugs found. The series is ready to merge.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds the chip_data and OF match entry:

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

**Design observation:** Unlike `rk3588_edp` which has `.edp_mode = GRF_REG_FIELD(0x0000, 0, 0)`, the RK3576 data has no `edp_mode` field. This means `rockchip_dp_poweron()` at line 147 (`rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1)`) will be a no-op since `.valid` defaults to `false`. Similarly, no `lcdc_sel` field, so the VOP output selection in `rockchip_dp_drm_encoder_enable()` is also a no-op. This is presumably intentional for RK3576 hardware — perhaps the eDP mode is always-on or configured elsewhere, and there's only one VOP path. Given this is v9 with hardware testing, this appears correct.

No blocking issues.

---
Generated by Claude Code Patch Reviewer

[PATCH v10 00/11] Add eDP support for RK3576

[Damon Ding]

Patch 1-5 are to add missing clock "hclk" for RK3588 eDP nodes.
Patch 6 is to use automatic cleanup to fix OF node reference leak
        reported by Sashiko.
Patch 7-8 are to add the RK3576 eDP node.
Patch 9-11 are to support the RK3576 Analogix DP controller.

Damon Ding (11):
  dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock
    for RK3588
  dt-bindings: display: rockchip: analogix-dp: Add per-clock
    descriptions
  arm64: dts: rockchip: Add missing hclk for RK3588 eDP0
  arm64: dts: rockchip: Add missing hclk for RK3588 eDP1
  drm/rockchip: analogix_dp: Enable hclk for RK3588
  drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup
  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        | 47 ++++++++++++++++++-
 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   | 31 +++++++++---
 include/drm/bridge/analogix_dp.h              | 13 ++++-
 8 files changed, 124 insertions(+), 24 deletions(-)

---

Changes in v2:
- Split out separate patches to add the "hclk" clock reference.
- Split out separate patches to enable the "hclk" clock.
- Add Reviewed-by tag.

Changes in v3:
- Add a patch to expand descriptions for clocks of the eDP node.
- Add Reviewed-by tag.

Changes in v4:
- Modify commit msg.

Changes in v5:
- Enforce the correct third clock name on a per-compatible basis.
- Modify the commit msg simultaneously.
- Add Acked-by tag.

Changes in v6:
- Expand more detail commit msg about using hclk instead of grf clock.

Changes in v7:
- List all valid clock names at the top level, and constrain the clock
  count for each platform with minItems/maxItems in allOf.

Changes in v8:
- Fix indentation to 10 for enum in clock-names property.

Changes in v9:
- Restore the explicit clock-names for RK3399 and RK3588 eDP dt-bindings.

Changes in v10:
- Use automatic cleanup to fix OF node reference leak reported by
  Sashiko.

-- 
2.34.1

[PATCH v10 01/11] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

[Damon Ding]

RK3588 eDP controller requires HCLK_VO1 to access the VO1 GRF
registers and enable the video datapath.

Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
phandle reference, which allowed the eDP to work without explicitly
managing the hclk_vo1 clock. However, this is not safe or explicit.

To make the clock dependency explicit, enforce per-SoC clock-names
requirements:
 - RK3288: 2 clocks (dp, pclk)
 - RK3399: 3 clocks (dp, pclk, grf)
 - RK3588: 3 clocks (dp, pclk, hclk)

Do not reuse the 'grf' clock name for RK3588 because it represents
a different clock with distinct control logic:
- The 'grf' clock is only for GRF register access and is toggled
  dynamically during register access.
- The 'hclk' clock controls both GRF access and video datapath
  gating, and must remain enabled during probe.

Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Modify the commit msg.

Changes in v5:
- Enforce the correct third clock name on a per-compatible basis.
- Modify the commit msg simultaneously.

Changes in v6:
- Expand more detail commit msg about using hclk instead of grf clock.

Changes in v7:
- List all valid clock names at the top level, and constrain the clock
  count for each platform with minItems/maxItems in allOf.

Changes in v8:
- Fix indentation to 10 for enum in clock-names property.

Changes in v9:
- Restore the explicit clock-names for RK3399 and RK3588.
---
 .../rockchip/rockchip,analogix-dp.yaml        | 40 ++++++++++++++++++-
 1 file changed, 39 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..6643889f2a9c 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
@@ -60,6 +62,35 @@ required:
 allOf:
   - $ref: /schemas/display/bridge/analogix,dp.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3288-dp
+    then:
+      properties:
+        clocks:
+          maxItems: 2
+        clock-names:
+          maxItems: 2
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3399-edp
+    then:
+      properties:
+        clocks:
+          minItems: 3
+        clock-names:
+          items:
+            - const: dp
+            - const: pclk
+            - const: grf
+
   - if:
       properties:
         compatible:
@@ -68,6 +99,13 @@ allOf:
               - rockchip,rk3588-edp
     then:
       properties:
+        clocks:
+          minItems: 3
+        clock-names:
+          items:
+            - const: dp
+            - const: pclk
+            - const: hclk
         resets:
           minItems: 2
         reset-names:
-- 
2.34.1

[PATCH v10 02/11] dt-bindings: display: rockchip: analogix-dp: Add per-clock descriptions

[Damon Ding]

Supplement dedicated description for each clock in the clocks
property, clarifying the function of each clock input for the
Analogix DP controller binding.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Modify the commit msg.

Changes in v5:
- Add Acked-by tag.
---
 .../bindings/display/rockchip/rockchip,analogix-dp.yaml      | 5 ++++-
 1 file changed, 4 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 6643889f2a9c..1ef1d56ef6e2 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -19,7 +19,10 @@ properties:
 
   clocks:
     minItems: 2
-    maxItems: 3
+    items:
+      - description: Reference clock
+      - description: APB bus clock
+      - description: GRF or AHB bus clock
 
   clock-names:
     minItems: 2
-- 
2.34.1

[PATCH v10 03/11] arm64: dts: rockchip: Add missing hclk for RK3588 eDP0

[Damon Ding]

Add the required HCLK_VO1 bus clock to RK3588 eDP0 node with
corresponding clock-name "hclk". This clock is necessary for the
eDP controller to access video output GRF and work properly.

Previously the clock was enabled implicitly via GRF phandle
reference. Add it explicitly now to align with updated binding.

Fixes: dc79d3d5e7c7 ("arm64: dts: rockchip: Add eDP0 node for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Modify the commit msg.
---
 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 4fb8888c281c..24a5ccbac08c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1712,8 +1712,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 v10 04/11] arm64: dts: rockchip: Add missing hclk for RK3588 eDP1

[Damon Ding]

Add the required HCLK_VO1 bus clock to RK3588 eDP1 node with
corresponding clock-name "hclk". This clock is necessary for
the eDP controller to access video output GRF and work properly.

Previously the clock was enabled implicitly via GRF phandle
reference. Add it explicitly now to align with updated binding.

Fixes: a481bb0b1ad9 ("arm64: dts: rockchip: Add eDP1 dt node for rk3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Modify the commit msg.
---
 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 a2640014ee04..b251bb129cdb 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 v10 05/11] drm/rockchip: analogix_dp: Enable hclk for RK3588

[Damon Ding]

Acquire and enable the HCLK_VO1 bus clock explicitly for RK3588
eDP controller to guarantee register and datapath access.

The clock was previously enabled implicitly via rockchip,vo-grf
phandle reference, which relies on side effect and is fragile.

Fetch optional "hclk" clock in driver to align with updated device
tree binding and keep consistent with hardware clock dependency.

Fixes: 729f8eefdcad ("drm/rockchip: analogix_dp: Add support for RK3588")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v4:
- Modify the commit msg.
---
 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 06072efd7fca..d2af5eb29dbb 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 v10 06/11] drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup

[Damon Ding]

Sashiko reported a reference leak in rockchip_dp_drm_encoder_enable(),
the of_get_child_by_name() function does not call of_node_put() in a
symmetrical way [1].

Fix the device node reference leak by using __free(device_node) to
automatically manage of_node_put() for all device nodes.

Fixes: 729f8eefdcad ("drm/rockchip: analogix_dp: Add support for RK3588")
Link: https://sashiko.dev/#/patchset/20260527024336.191433-1-damon.ding@rock-chips.com?part=5 #1
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index d2af5eb29dbb..b1ed25cefe5e 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -8,6 +8,7 @@
  *         Jeff Chen <jeff.chen@rock-chips.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/component.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of.h>
@@ -206,7 +207,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *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;
@@ -230,18 +230,22 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
 	if (ret < 0)
 		return;
 
-	remote_port_parent = of_graph_get_remote_port_parent(endpoint.local_node);
+	struct device_node *remote_port_parent __free(device_node) =
+		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);
+		struct device_node *ports __free(device_node) =
+			of_get_child_by_name(remote_port_parent, "ports");
+
+		if (ports) {
+			struct device_node *remote_port __free(device_node) =
+				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, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
 	}
-- 
2.34.1

[PATCH v10 07/11] dt-bindings: display: rockchip: analogix-dp: Add support for RK3576

[Damon Ding]

RK3576 integrates an eDP TX controller compatible with the existing
RK3588 hardware design, reuse the same binding configuration directly.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Split out a separate patch to add the "hclk" clock reference.

Chanegs in v4:
- Modify the commit msg.

Changes in v5:
- Add Acked-by tag.
---
 .../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 1ef1d56ef6e2..6548f157fd96 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:
@@ -99,6 +100,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - rockchip,rk3576-edp
               - rockchip,rk3588-edp
     then:
       properties:
-- 
2.34.1

[PATCH v10 08/11] arm64: dts: rockchip: Add eDP node for RK3576

[Damon Ding]

Add full device tree definition for the integrated eDP controller
on RK3576, following the existing RK3588 hardware layout.

Configure required register range, clocks, interrupt, phy, power
domain, reset and grf properties to fully describe the controller.

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.

Changes in v4:
- Modify the commit msg.
---
 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 28175d8200d5..733449cb88b1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1496,6 +1496,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>;
+				};
+			};
+		};
+
 		dp: dp@27e40000 {
 			compatible = "rockchip,rk3576-dp";
 			reg = <0x0 0x27e40000 0x0 0x30000>;
-- 
2.34.1

[PATCH v10 09/11] drm/bridge: analogix_dp: Rename and simplify is_rockchip()

[Damon Ding]

Rename inline helper is_rockchip() to analogix_dp_is_rockchip()
to follow driver namespace convention consistently across code.

Replace chained equality comparisons with switch-case layout
to improve readability and simplify adding new SoC entries later.

Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Suggested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v3:
- Add Reviewed-by tag.

Changes in v4:
- Modify the commit msg.
---
 .../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 8cf6b73bceac..116de3bd83a3 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -870,7 +870,7 @@ static int analogix_dp_bridge_atomic_check(struct drm_bridge *bridge,
 	struct drm_display_info *di = &conn_state->connector->display_info;
 	u32 mask = BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) | BIT(DRM_OUTPUT_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 v10 10/11] drm/bridge: analogix_dp: Add support for RK3576

[Damon Ding]

Add RK3576_EDP device type entry and extend Rockchip check
to match existing hardware capabilities shared with RK3588.

Set identical maximum link rate and lane count parameters
for RK3576 eDP controller to reuse existing RK3588 config.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v3:
- Add Reviewed-by tag.

Changes in v4:
- Modify the commit msg.
---
 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 116de3bd83a3..c8eb3511f92a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1249,6 +1249,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 v10 11/11] drm/rockchip: analogix_dp: Add support for RK3576

[Damon Ding]

RK3576 integrates Analogix eDP 1.3 TX and Samsung combo PHY
hardware blocks that fully match the proven RK3588 design.

Add dedicated chip data table and device tree matching entry
to bring up basic eDP functionality for the RK3576 platform.

Support is limited to RGB output up to 4K@60Hz for now; audio,
PSR and other advanced eDP 1.3 features remain unvalidated.

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

---

Changes in v2:
- Split out a separate patch to enable the "hclk" clock.
- Add Reviewed-by tag.

Changes in v3:
- Add Reviewed-by tag.

Changes in v4:
- Modify the commit msg.
---
 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 b1ed25cefe5e..776954c7d052 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -526,6 +526,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),
@@ -543,6 +551,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 v10 01/11] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

[Conor Dooley]

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

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

Re: (subset) [PATCH v10 00/11] Add eDP support for RK3576

[Heiko Stuebner]

On Mon, 01 Jun 2026 14:50:49 +0800, Damon Ding wrote:
> Patch 1-5 are to add missing clock "hclk" for RK3588 eDP nodes.
> Patch 6 is to use automatic cleanup to fix OF node reference leak
>         reported by Sashiko.
> Patch 7-8 are to add the RK3576 eDP node.
> Patch 9-11 are to support the RK3576 Analogix DP controller.
> 
> Damon Ding (11):
>   dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock
>     for RK3588
>   dt-bindings: display: rockchip: analogix-dp: Add per-clock
>     descriptions
>   arm64: dts: rockchip: Add missing hclk for RK3588 eDP0
>   arm64: dts: rockchip: Add missing hclk for RK3588 eDP1
>   drm/rockchip: analogix_dp: Enable hclk for RK3588
>   drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup
>   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
> 
> [...]

Applied, thanks!

[01/11] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588
        commit: 826ab13cf73e7bd5b268085c9594850f6a7fc7fd
[02/11] dt-bindings: display: rockchip: analogix-dp: Add per-clock descriptions
        commit: 38f41d4e704c2a9bfc025bb17594d53be010a629
[05/11] drm/rockchip: analogix_dp: Enable hclk for RK3588
        commit: 104f20616d72825fdcf56cfdc5f89f4e96fd8dbe
[06/11] drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup
        commit: 87e060521371257ddbb77964b66e60d80afcc7b2

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: (subset) [PATCH v10 00/11] Add eDP support for RK3576

[Heiko Stuebner]

On Mon, 01 Jun 2026 14:50:49 +0800, Damon Ding wrote:
> Patch 1-5 are to add missing clock "hclk" for RK3588 eDP nodes.
> Patch 6 is to use automatic cleanup to fix OF node reference leak
>         reported by Sashiko.
> Patch 7-8 are to add the RK3576 eDP node.
> Patch 9-11 are to support the RK3576 Analogix DP controller.
> 
> Damon Ding (11):
>   dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock
>     for RK3588
>   dt-bindings: display: rockchip: analogix-dp: Add per-clock
>     descriptions
>   arm64: dts: rockchip: Add missing hclk for RK3588 eDP0
>   arm64: dts: rockchip: Add missing hclk for RK3588 eDP1
>   drm/rockchip: analogix_dp: Enable hclk for RK3588
>   drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup
>   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
> 
> [...]

Applied, thanks!

[07/11] dt-bindings: display: rockchip: analogix-dp: Add support for RK3576
        commit: 473e50b27b0020ade5fd97ca04d941e448c5659a
[09/11] drm/bridge: analogix_dp: Rename and simplify is_rockchip()
        commit: 709445fb6fc57aa96d48c8492e84ed4b2a9a4b5e
[10/11] drm/bridge: analogix_dp: Add support for RK3576
        commit: 5442cdc38470ecd6b51562d378b6ed852a4d9d45
[11/11] drm/rockchip: analogix_dp: Add support for RK3576
        commit: 7e60c4e19fd3d3f1203fbb1fdd9b2bb1a1201e65

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

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: 15
Reviewed: 2026-06-04T14:37:01.263725

---

This is a well-structured v10 series adding eDP support for RK3576 to the Rockchip Analogix DP driver, along with prerequisite fixes for RK3588 hclk clock management and an OF node reference leak. The series is logically decomposed: bindings first, then DTS, then driver changes, with bug fixes ordered before new features. The code is clean, review tags are present on several patches, and the incremental approach (reusing RK3588 infrastructure) minimizes risk.

No blocking issues found. A few observations and one question worth answering are noted below.

---
Generated by Claude Code Patch Reviewer

Claude review: dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

[Claude Code Review Bot]

Patch Review

Good approach using per-compatible `allOf`/`if`/`then` blocks to enforce the correct third clock name per SoC. The top-level `clock-names` properly allows both `grf` and `hclk` as the third entry:

```yaml
      - enum:
          - grf
          - hclk
```

Then individual compatible constraints lock each SoC to the correct name. The RK3288 constraint correctly sets `maxItems: 2` to prevent a third clock. RK3399 mandates `grf`, RK3588 mandates `hclk`. Well done.

The Fixes tag is appropriate since the original RK3588 binding didn't enforce the correct clock name.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: dt-bindings: display: rockchip: analogix-dp: Add per-clock descriptions

[Claude Code Review Bot]

Patch Review

Replaces `maxItems: 3` with explicit `items:` descriptions:

```yaml
    items:
      - description: Reference clock
      - description: APB bus clock
      - description: GRF or AHB bus clock
```

This improves documentation while preserving the implicit maxItems of 3 (from three `items:` entries). Combined with the existing `minItems: 2`, 2 or 3 clocks remain valid.

Has Acked-by from Conor Dooley. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: arm64: dts: rockchip: Add missing hclk for RK3588 eDP0

[Claude Code Review Bot]

Patch Review

Straightforward DTS change adding `HCLK_VO1` as third clock to the eDP0 node:

```dts
-		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";
```

Consistent with the binding update in Patch 1. No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: arm64: dts: rockchip: Add missing hclk for RK3588 eDP1

[Claude Code Review Bot]

Patch Review

Identical pattern to Patch 3 but for eDP1. Uses the same `HCLK_VO1` clock (both eDP controllers share the VO1 bus clock). No issues.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Uses `devm_clk_get_optional_enabled()` to acquire and enable the hclk:

```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");
```

Good design choices:
- `optional` ensures backward compatibility with older DTS that lack the third clock
- `enabled` keeps the clock on for the lifetime of the device (correct per the commit message: hclk "must remain enabled during probe")
- The return value is intentionally not stored in the struct since it's devres-managed and never toggled

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: analogix_dp: Fix OF node reference leak via auto cleanup

[Claude Code Review Bot]

Patch Review

Fixes a real leak: in the existing code, `of_get_child_by_name(remote_port_parent, "ports")` returns a reference-counted node that is never put. The fix uses `__free(device_node)` for automatic cleanup:

```c
+	struct device_node *remote_port_parent __free(device_node) =
+		of_graph_get_remote_port_parent(endpoint.local_node);
```

```c
+		struct device_node *ports __free(device_node) =
+			of_get_child_by_name(remote_port_parent, "ports");
+		if (ports) {
+			struct device_node *remote_port __free(device_node) =
+				of_graph_get_remote_port(endpoint.local_node);
```

The scoping is correct: `ports` and `remote_port` are declared inside the `if (remote_port_parent)` block, so `__free` will call `of_node_put()` when those variables leave scope. The `remote_port_parent` variable is declared at function scope and will be freed when the function returns.

One minor note: `of_property_read_u32(remote_port, "reg", &port_id)` is called without checking `remote_port` for NULL. If `of_graph_get_remote_port()` returns NULL, this would be a NULL dereference. However, this is a pre-existing issue not introduced by this patch, and in practice the graph endpoint is already validated by `drm_of_encoder_active_endpoint()` above.

No issues with the fix itself.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds `rockchip,rk3576-edp` to the compatible enum and groups it with RK3588 in the `allOf` constraint:

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

This means RK3576 gets the same clock, reset, and other constraints as RK3588. Has Acked-by from Conor Dooley. No issues.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Complete DTS node for RK3576 eDP controller:

```dts
+		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";
```

Note the hclk here is `HCLK_VO0_ROOT` (not `HCLK_VO1` as in RK3588) since RK3576's eDP is on VO0, not VO1. The node structure follows the same pattern as RK3588. Has two resets (`dp` and `apb`), the phy, power domain, and grf reference.

Has Reviewed-by from Nicolas Frattaroli. No issues.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Clean refactor renaming `is_rockchip()` to `analogix_dp_is_rockchip()` and converting from chained `||` to `switch`:

```c
-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;
+	}
 }
```

The switch also enables compiler warnings if a new enum value is added without updating the switch (with `-Wswitch`). Correct ordering: this patch goes before Patch 10 which adds RK3576_EDP.

Has Reviewed-by from Luca Ceresoli. No issues.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds `RK3576_EDP` to the enum and both the `analogix_dp_is_rockchip()` switch and the `analogix_dp_dt_parse_pdata()` function:

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

RK3576 shares the same max link rate (HBR2, 0x14 = 5.4 Gbps) and 4-lane config as RK3588. Straightforward. Has Reviewed-by from Luca Ceresoli. No issues.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Patch Review

Adds the platform chip_data and OF match entry:

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

**Observation**: Unlike RK3588's chip_data which includes `.edp_mode = GRF_REG_FIELD(0x0000, 0, 0)`, the RK3576 entry has no `edp_mode` field. This means in `rockchip_dp_poweron()`, the call to `rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1)` will be a no-op because `field->valid` is false (zero-initialized). This is safe due to the `valid` check, but it would be worth confirming whether RK3576 truly doesn't need an edp_mode GRF write for the video datapath, or if this is an oversight. The cover letter says "RK3576 integrates an eDP TX controller compatible with the existing RK3588 hardware design" which might suggest it should have one.

Similarly, the `lcdc_sel` field is not set, matching the RK3588 behavior (RK3588 also lacks `lcdc_sel`).

Has Reviewed-by from Nicolas Frattaroli and Luca Ceresoli.

No blocking issues.

---
Generated by Claude Code Patch Reviewer