Claude review: Add eDP lane mapping support

Claude review: Add eDP lane mapping support

[Claude Code Review Bot]

Overall Series Review

Subject: Add eDP lane mapping support
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 9
Reviewed: 2026-05-16T11:13:13.157156

---

This is a clean, well-structured 3-patch series that adds configurable eDP/DP physical lane mapping via the `data-lanes` device tree property to the Analogix DP bridge driver. The use case is legitimate — correcting PCB lane swaps and adapting mismatched lane pin definitions between SoC and panel without hardware changes.

The series is logically ordered: DT binding fixups first, then binding additions, then the driver implementation. The driver patch is conservative, falling back to the default identity mapping when no `data-lanes` is specified.

I have a few concerns, mostly around the parsing logic in patch 3 that silently absorbs errors that should probably be propagated, and a question about the deprecated top-level `data-lanes` property added in patch 2.

**Overall: Looks reasonable with minor issues to address.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v3 0/2] Add eDP lane mapping support

[Damon Ding]

This series adds configurable eDP physical lane mapping support via
device tree data-lanes property.

Lane mapping is mainly used for below scenarios:
1. Correct PCB lane swap and differential line routing crossover
   without hardware changes;
2. Adapt mismatched lane pin definitions between SoC and eDP panel;
3. Support multiple panel hardware variants on the same board
   by configuring data-lanes in device tree only.

The series includes driver implementation and device tree binding
updates to support custom lane mapping configuration from endpoint
node, and keeps default linear lane order if no configuration is given.

Patch 1: Add endpoint data-lanes property to analogix-dp binding
Patch 2: Implement lane mapping in analogix_dp driver

Damon Ding (2):
  dt-bindings: display: bridge: analogix-dp: Add data-lanes support for
    endpoint
  drm/bridge: analogix_dp: Add support for optional data-lanes mapping

 .../bindings/display/bridge/analogix,dp.yaml  | 19 +++++--
 .../rockchip/rockchip,analogix-dp.yaml        |  1 +
 .../drm/bridge/analogix/analogix_dp_core.c    | 56 +++++++++++++++++++
 .../drm/bridge/analogix/analogix_dp_core.h    |  4 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 +++--
 .../gpu/drm/bridge/analogix/analogix_dp_reg.h |  4 ++
 6 files changed, 85 insertions(+), 14 deletions(-)

---

Changes in v2:
- Add lane mapping application scenarios in commit message.
- Remove redundant deprecated property 'data-lanes' for eDP node.
- Update port@1 $ref to /schemas/graph.yaml#/$defs/port-base.

Changes in v3:
- Squash [PATCH v2 2/3] into [PATCH v2 1/3].
- Add unevaluatedProperties: false to both the port@1 and endpoint
  nodes.

-- 
2.34.1

[PATCH v3 1/2] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint

[Damon Ding]

Add data-lanes property support to the port@1 endpoint for physical
lane mapping configuration.

Lane mapping is mainly used for below scenarios:
1. Correct PCB lane swap and differential line routing crossover
   without hardware changes;
2. Adapt mismatched lane pin definitions between SoC and eDP panel;
3. Support multiple panel hardware variants on the same board
   by configuring data-lanes in device tree only.

Additionally, add data-lanes setting in Rockchip eDP DT node example
to show actual lane mapping usage.

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

---

Changes in v2:
- Add lane mapping application scenarios in commit message.
- Remove redundant deprecated property 'data-lanes' for eDP node.
- Update port@1 $ref to /schemas/graph.yaml#/$defs/port-base.

Changes in v3:
- Squash data-lanes adjustment of Rockchip eDP DT example into this
  commit.
- Add unevaluatedProperties: false to both the port@1 and endpoint
  nodes.
---
 .../bindings/display/bridge/analogix,dp.yaml  | 19 ++++++++++++++-----
 .../rockchip/rockchip,analogix-dp.yaml        |  1 +
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
index 62f0521b0924..ecf206871cdd 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
@@ -42,13 +42,22 @@ properties:
     properties:
       port@0:
         $ref: /schemas/graph.yaml#/properties/port
-        description:
-          Input node to receive pixel data.
+        description: Input node to receive pixel data.
 
       port@1:
-        $ref: /schemas/graph.yaml#/properties/port
-        description:
-          Port node with one endpoint connected to a dp-connector node.
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: Port node with one endpoint connected to sink device node.
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+                items:
+                  enum: [ 0, 1, 2, 3 ]
 
     required:
       - port@0
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index b4ec101b77a3..cf3ad33a0e86 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -154,6 +154,7 @@ examples:
           reg = <1>;
 
           edp_out_panel: endpoint {
+            data-lanes = <0 1>;
             remote-endpoint = <&panel_in_edp>;
           };
         };
-- 
2.34.1

[PATCH v3 2/2] drm/bridge: analogix_dp: Add support for optional data-lanes mapping

[Damon Ding]

Parse the optional 'data-lanes' device tree property to support
custom physical lane mapping configuration.

If no valid configuration is found, fall back to the default
lane map (0, 1, 2, 3) automatically and keep the driver running.

Lane mapping is mainly used for below scenarios:
1. Correct PCB lane swap and differential line routing crossover
   without hardware changes;
2. Adapt mismatched lane pin definitions between SoC and eDP panel;
3. Support multiple panel hardware variants on the same board
   by configuring data-lanes in device tree only.

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

---

Changes in v2:
- Add lane mapping application scenarios in commit message.
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 56 +++++++++++++++++++
 .../drm/bridge/analogix/analogix_dp_core.h    |  4 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 +++--
 .../gpu/drm/bridge/analogix/analogix_dp_reg.h |  4 ++
 4 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5dc07ff84cd3..d1c6365118de 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1234,6 +1234,59 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
 	.detect = analogix_dp_bridge_detect,
 };
 
+static int analogix_dp_dt_parse_lanes_map(struct analogix_dp_device *dp)
+{
+	struct video_info *video_info = &dp->video_info;
+	struct device_node *endpoint;
+	u32 tmp[LANE_COUNT4];
+	u32 map[LANE_COUNT4] = {0, 1, 2, 3};
+	bool used[LANE_COUNT4] = {false};
+	int num_lanes;
+	int ret, i;
+
+	memcpy(video_info->lane_map, map, sizeof(map));
+
+	num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
+						   video_info->max_lane_count);
+	if (num_lanes < 0)
+		return -EINVAL;
+
+	endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
+	if (!endpoint)
+		return -EINVAL;
+
+	ret = of_property_read_u32_array(endpoint, "data-lanes", tmp, num_lanes);
+	of_node_put(endpoint);
+	if (ret)
+		return -EINVAL;
+
+	for (i = 0; i < num_lanes; i++) {
+		if (tmp[i] >= LANE_COUNT4) {
+			dev_dbg(dp->dev, "data-lanes[%d] = %u is out of range\n", i, tmp[i]);
+			return -EINVAL;
+		}
+
+		if (used[tmp[i]]) {
+			dev_dbg(dp->dev, "data-lanes[%d] = %u is duplicate\n", i, tmp[i]);
+			return -EINVAL;
+		}
+
+		used[tmp[i]] = true;
+		map[i] = tmp[i];
+	}
+
+	for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
+		if (!used[i])
+			map[num_lanes++] = i;
+	}
+
+	dev_dbg(dp->dev, "Using parsed lane map: <%u %u %u %u>\n", map[0], map[1], map[2], map[3]);
+
+	memcpy(video_info->lane_map, map, sizeof(map));
+
+	return 0;
+}
+
 static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 {
 	struct device_node *dp_node = dp->dev->of_node;
@@ -1266,6 +1319,9 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 		break;
 	}
 
+	if (analogix_dp_dt_parse_lanes_map(dp))
+		dev_dbg(dp->dev, "No valid data-lanes found, using default lane map\n");
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 94348c4e3623..a75d0fb8f980 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -137,6 +137,8 @@ struct video_info {
 
 	int max_link_rate;
 	enum link_lane_count_type max_lane_count;
+
+	u32 lane_map[LANE_COUNT4];
 };
 
 struct link_train {
@@ -175,7 +177,7 @@ struct analogix_dp_device {
 /* analogix_dp_reg.c */
 void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable);
 void analogix_dp_stop_video(struct analogix_dp_device *dp);
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable);
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp);
 void analogix_dp_init_analog_param(struct analogix_dp_device *dp);
 void analogix_dp_init_interrupt(struct analogix_dp_device *dp);
 void analogix_dp_reset(struct analogix_dp_device *dp);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index ea8401293a23..c1344a3f013a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -48,16 +48,15 @@ void analogix_dp_stop_video(struct analogix_dp_device *dp)
 	writel(reg, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_1);
 }
 
-void analogix_dp_lane_swap(struct analogix_dp_device *dp, bool enable)
+void analogix_dp_lane_mapping(struct analogix_dp_device *dp)
 {
+	u32 *lane_map = dp->video_info.lane_map;
 	u32 reg;
 
-	if (enable)
-		reg = LANE3_MAP_LOGIC_LANE_0 | LANE2_MAP_LOGIC_LANE_1 |
-		      LANE1_MAP_LOGIC_LANE_2 | LANE0_MAP_LOGIC_LANE_3;
-	else
-		reg = LANE3_MAP_LOGIC_LANE_3 | LANE2_MAP_LOGIC_LANE_2 |
-		      LANE1_MAP_LOGIC_LANE_1 | LANE0_MAP_LOGIC_LANE_0;
+	reg = lane_map[0] << LANE0_MAP_SHIFT;
+	reg |= lane_map[1] << LANE1_MAP_SHIFT;
+	reg |= lane_map[2] << LANE2_MAP_SHIFT;
+	reg |= lane_map[3] << LANE3_MAP_SHIFT;
 
 	writel(reg, dp->reg_base + ANALOGIX_DP_LANE_MAP);
 }
@@ -140,7 +139,7 @@ void analogix_dp_reset(struct analogix_dp_device *dp)
 
 	usleep_range(20, 30);
 
-	analogix_dp_lane_swap(dp, 0);
+	analogix_dp_lane_mapping(dp);
 
 	writel(0x0, dp->reg_base + ANALOGIX_DP_SYS_CTL_1);
 	writel(0x40, dp->reg_base + ANALOGIX_DP_SYS_CTL_2);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index 12735139046c..ac914e37089b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -209,6 +209,10 @@
 #define LANE0_MAP_LOGIC_LANE_1			(0x1 << 0)
 #define LANE0_MAP_LOGIC_LANE_2			(0x2 << 0)
 #define LANE0_MAP_LOGIC_LANE_3			(0x3 << 0)
+#define LANE3_MAP_SHIFT				(6)
+#define LANE2_MAP_SHIFT				(4)
+#define LANE1_MAP_SHIFT				(2)
+#define LANE0_MAP_SHIFT				(0)
 
 /* ANALOGIX_DP_ANALOG_CTL_1 */
 #define TX_TERMINAL_CTRL_50_OHM			(0x1 << 4)
-- 
2.34.1

Claude review: Add eDP lane mapping support

[Claude Code Review Bot]

Overall Series Review

Subject: Add eDP lane mapping support
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 8
Reviewed: 2026-05-25T20:15:51.799166

---

This 3-patch series adds configurable eDP/DP physical lane mapping via the standard `data-lanes` device tree property to the Analogix DP bridge driver. The binding changes (patches 1 & 2) are straightforward. The driver implementation (patch 3) is generally clean and well-structured, with proper validation and fallback to defaults. However, there is a **potential lane mapping inversion bug** in patch 3 that should be examined carefully — the code may be interpreting the `data-lanes` array indices incorrectly relative to the standard DT semantics. The series is also somewhat wasteful in looking up the endpoint node twice.

Overall, the series is a reasonable feature addition with one significant correctness concern and a few minor issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: Add eDP lane mapping support

[Claude Code Review Bot]

Overall Series Review

Subject: Add eDP lane mapping support
Author: Damon Ding <damon.ding@rock-chips.com>
Patches: 3
Reviewed: 2026-05-26T07:20:30.778496

---

This is a clean, well-scoped 2-patch series adding configurable eDP physical lane mapping via the `data-lanes` device tree property to the analogix DP bridge driver. The use cases (PCB lane swap correction, mismatched pin definitions, multi-panel support) are well-motivated. The approach is sound: parse `data-lanes` from the endpoint, fill unused lanes with remaining physical lanes in order, and write the mapping to the `ANALOGIX_DP_LANE_MAP` register using shift-based construction instead of the old boolean swap API.

The series is generally well-written with a few issues worth addressing, ranging from a minor error-handling concern to a redundant OF node lookup. No critical bugs found.

---
Generated by Claude Code Patch Reviewer

Claude review: dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint

[Claude Code Review Bot]

Patch Review

**Looks good overall.** The binding change correctly switches `port@1` from `$ref: /schemas/graph.yaml#/properties/port` to `$ref: /schemas/graph.yaml#/$defs/port-base` to allow extending the port definition, and adds the `data-lanes` property with appropriate constraints.

Minor observations:

1. **`data-lanes` enum values**: The enum `[ 0, 1, 2, 3 ]` correctly constrains individual lane values to valid physical lane indices. The `minItems: 1` / `maxItems: 4` bounds are correct since the driver code fills in unspecified lanes with the remaining unused ones.

2. **Example only shows 2 lanes**: The example in `rockchip,analogix-dp.yaml` adds `data-lanes = <0 1>;` which is fine to demonstrate partial lane specification, though it uses the default identity mapping. A more illustrative example might have shown a swap (e.g., `<1 0>`), but this is not a functional issue.

3. **`unevaluatedProperties: false` on both port and endpoint**: Good — this was requested in v3 review feedback and correctly prevents unrecognized properties.

No issues requiring changes.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/bridge: analogix_dp: Add support for optional data-lanes mapping

[Claude Code Review Bot]

Patch Review

**Generally good, but has a few issues:**

1. **Redundant endpoint lookup**: `analogix_dp_dt_parse_lanes_map()` calls `drm_of_get_data_lanes_count_ep()` which internally does `of_graph_get_endpoint_by_regs()` + `of_node_put()`, and then the function immediately does the same `of_graph_get_endpoint_by_regs()` call again to read the actual `data-lanes` values:

    ```c
    num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
                                               video_info->max_lane_count);
    if (num_lanes < 0)
        return -EINVAL;

    endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
    ```

    This looks up the endpoint twice. Consider using `drm_of_get_data_lanes_count()` (the non-`_ep` variant) on the already-obtained endpoint node, or simply use `of_property_count_u32_elems()` + manual validation to avoid the double lookup. It's not a bug, but it's unnecessary work.

2. **Inconsistent endpoint reg parameter**: The first lookup uses reg `0` (via `drm_of_get_data_lanes_count_ep(..., 1, 0, ...)`), while the second uses reg `-1` (via `of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1)`). Reg `-1` means "any endpoint in port 1", while reg `0` means "endpoint with reg=0 in port 1". If the endpoint doesn't have `reg = <0>` explicitly, these could potentially find different endpoints, or `_count_ep` could fail while `get_endpoint_by_regs` succeeds. This should be made consistent — either use `0` for both or `-1` for both (the `_count_ep` variant doesn't accept `-1`, so the proper fix would be to get the endpoint once with `-1` and then call `drm_of_get_data_lanes_count()` on it directly).

3. **Error semantics when `data-lanes` is absent**: When no `data-lanes` property exists in DT, `drm_of_get_data_lanes_count_ep()` returns `-ENODEV` (property missing). The function returns `-EINVAL` for this case, and the caller prints a debug message:

    ```c
    if (analogix_dp_dt_parse_lanes_map(dp))
        dev_dbg(dp->dev, "No valid data-lanes found, using default lane map\n");
    ```

    This works correctly since the default map `{0, 1, 2, 3}` is written via `memcpy` at the top of the function before any error paths. However, the function returns `-EINVAL` for all error cases (missing property, invalid values, missing endpoint), which is slightly imprecise — returning the actual error code from the helpers would be more idiomatic, even though the caller doesn't distinguish them.

4. **Lane fill logic for partial specification**: When fewer than 4 lanes are specified, the remaining slots are filled with unused physical lane indices in ascending order:

    ```c
    for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
        if (!used[i])
            map[num_lanes++] = i;
    }
    ```

    This is reasonable behavior, but worth documenting somewhere (commit message or binding doc) what the semantics are for partial lane specifications. For example, `data-lanes = <1 0>` means lanes 0→1, 1→0, 2→2, 3→3. Is this the intended hardware behavior? The commit message says "fall back to the default lane map" for absent configuration but doesn't describe partial specification behavior.

5. **New shift defines have unnecessary parentheses**: Minor style nit — the kernel style generally doesn't parenthesize simple integer constants:

    ```c
    #define LANE3_MAP_SHIFT				(6)
    #define LANE2_MAP_SHIFT				(4)
    ```

    These should be just `6`, `4`, `2`, `0` without parentheses, matching other simple constant defines in the same file. The existing `LANEx_MAP_LOGIC_LANE_y` defines use parentheses around the shift *expression* `(0x1 << 6)` which is different and appropriate.

6. **The `analogix_dp_lane_swap` function removal is clean**: The old boolean swap API is replaced with the general mapping API. The only caller was `analogix_dp_reset()` which called it with `enable=0` (identity mapping), so this is a correct generalization. The function rename from `analogix_dp_lane_swap` to `analogix_dp_lane_mapping` is appropriate.

---
Generated by Claude Code Patch Reviewer