* Claude review: Add eDP lane mapping support
2026-05-14 7:01 [PATCH v1 0/3] " Damon Ding
@ 2026-05-16 1:13 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 1:13 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 0/3] Add eDP lane mapping support
@ 2026-05-21 11:44 Damon Ding
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Damon Ding @ 2026-05-21 11:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
This series adds configurable eDP/DP 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: Add data-lanes property example in rockchip analogix-dp binding
Patch 3: Implement lane mapping in analogix_dp driver
Damon Ding (3):
dt-bindings: display: bridge: analogix-dp: Add data-lanes support for
endpoint
dt-bindings: rockchip: analogix-dp: Add data-lanes example
drm/bridge: analogix_dp: Add support for optional data-lanes mapping
.../bindings/display/bridge/analogix,dp.yaml | 17 ++++--
.../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, 83 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.
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-21 11:44 [PATCH v2 0/3] Add eDP lane mapping support Damon Ding
@ 2026-05-21 11:44 ` Damon Ding
2026-05-21 19:50 ` Conor Dooley
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-21 11:44 ` [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example Damon Ding
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Damon Ding @ 2026-05-21 11:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, 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.
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.
---
.../bindings/display/bridge/analogix,dp.yaml | 17 ++++++++++++-----
1 file changed, 12 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..e34fdb21adb4 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
@@ -42,13 +42,20 @@ 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
+ description: Port node with one endpoint connected to sink device node.
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+ items:
+ enum: [ 0, 1, 2, 3 ]
required:
- port@0
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example
2026-05-21 11:44 [PATCH v2 0/3] Add eDP lane mapping support Damon Ding
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
@ 2026-05-21 11:44 ` Damon Ding
2026-05-21 19:47 ` Conor Dooley
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-21 11:44 ` [PATCH v2 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
2026-05-25 10:15 ` Claude review: Add eDP lane mapping support Claude Code Review Bot
3 siblings, 2 replies; 14+ messages in thread
From: Damon Ding @ 2026-05-21 11:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Damon Ding
Add data-lanes setting in endpoint example to show actual lane mapping
usage.
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
.../bindings/display/rockchip/rockchip,analogix-dp.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
index bb75d898a5c5..cf75c926318b 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -151,6 +151,7 @@ examples:
reg = <1>;
edp_out_panel: endpoint {
+ data-lanes = <0 1>;
remote-endpoint = <&panel_in_edp>;
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping
2026-05-21 11:44 [PATCH v2 0/3] Add eDP lane mapping support Damon Ding
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
2026-05-21 11:44 ` [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example Damon Ding
@ 2026-05-21 11:44 ` Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-25 10:15 ` Claude review: Add eDP lane mapping support Claude Code Review Bot
3 siblings, 1 reply; 14+ messages in thread
From: Damon Ding @ 2026-05-21 11:44 UTC (permalink / raw)
To: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, nicolas.frattaroli,
cristian.ciocaltea, sebastian.reichel, dmitry.baryshkov,
luca.ceresoli, dianders, m.szyprowski, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example
2026-05-21 11:44 ` [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example Damon Ding
@ 2026-05-21 19:47 ` Conor Dooley
2026-05-22 1:25 ` Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2026-05-21 19:47 UTC (permalink / raw)
To: Damon Ding
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]
On Thu, May 21, 2026 at 07:44:58PM +0800, Damon Ding wrote:
> Add data-lanes setting in endpoint example to show actual lane mapping
> usage.
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Squash this with patch 1.
pw-bot: changes-requested
Thanks,
Conor.
> ---
> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> index bb75d898a5c5..cf75c926318b 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> @@ -151,6 +151,7 @@ examples:
> reg = <1>;
>
> edp_out_panel: endpoint {
> + data-lanes = <0 1>;
> remote-endpoint = <&panel_in_edp>;
> };
> };
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
@ 2026-05-21 19:50 ` Conor Dooley
2026-05-22 1:32 ` Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2026-05-21 19:50 UTC (permalink / raw)
To: Damon Ding
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]
On Thu, May 21, 2026 at 07:44:57PM +0800, Damon Ding wrote:
> 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.
>
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
Sashiko complaint here looks valid.
pw-bot: changes-requested
Thanks,
Conor.
>
> ---
>
> 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.
> ---
> .../bindings/display/bridge/analogix,dp.yaml | 17 ++++++++++++-----
> 1 file changed, 12 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..e34fdb21adb4 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
> @@ -42,13 +42,20 @@ 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
> + description: Port node with one endpoint connected to sink device node.
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + properties:
> + data-lanes:
> + minItems: 1
> + maxItems: 4
> + items:
> + enum: [ 0, 1, 2, 3 ]
>
> required:
> - port@0
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example
2026-05-21 19:47 ` Conor Dooley
@ 2026-05-22 1:25 ` Damon Ding
0 siblings, 0 replies; 14+ messages in thread
From: Damon Ding @ 2026-05-22 1:25 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
On 5/22/2026 3:47 AM, Conor Dooley wrote:
> On Thu, May 21, 2026 at 07:44:58PM +0800, Damon Ding wrote:
>> Add data-lanes setting in endpoint example to show actual lane mapping
>> usage.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>
> Squash this with patch 1.
> pw-bot: changes-requested
>
Yes, will fix in v3
Best regards,
Damon
>
>> ---
>> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index bb75d898a5c5..cf75c926318b 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -151,6 +151,7 @@ examples:
>> reg = <1>;
>>
>> edp_out_panel: endpoint {
>> + data-lanes = <0 1>;
>> remote-endpoint = <&panel_in_edp>;
>> };
>> };
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-21 19:50 ` Conor Dooley
@ 2026-05-22 1:32 ` Damon Ding
0 siblings, 0 replies; 14+ messages in thread
From: Damon Ding @ 2026-05-22 1:32 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
On 5/22/2026 3:50 AM, Conor Dooley wrote:
> On Thu, May 21, 2026 at 07:44:57PM +0800, Damon Ding wrote:
>> 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.
>>
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>
> Sashiko complaint here looks valid.
> pw-bot: changes-requested
>
Yes, unevaluatedProperties: false should be added to both the port@1
and endpoint nodes.
Will fix in v3.
Best regards,
Damon
>
>>
>> ---
>>
>> 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.
>> ---
>> .../bindings/display/bridge/analogix,dp.yaml | 17 ++++++++++++-----
>> 1 file changed, 12 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..e34fdb21adb4 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,dp.yaml
>> @@ -42,13 +42,20 @@ 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
>> + description: Port node with one endpoint connected to sink device node.
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> + properties:
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> + items:
>> + enum: [ 0, 1, 2, 3 ]
>>
>> required:
>> - port@0
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: Add eDP lane mapping support
2026-05-21 11:44 [PATCH v2 0/3] Add eDP lane mapping support Damon Ding
` (2 preceding siblings ...)
2026-05-21 11:44 ` [PATCH v2 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
@ 2026-05-25 10:15 ` Claude Code Review Bot
3 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 10:15 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
2026-05-21 19:50 ` Conor Dooley
@ 2026-05-25 10:15 ` Claude Code Review Bot
1 sibling, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 10:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks reasonable, minor comments**
The change from `$ref: /schemas/graph.yaml#/properties/port` to `$ref: /schemas/graph.yaml#/$defs/port-base` on port@1 is correct — `port-base` is needed to allow extending the port with additional endpoint properties, since `port` has `additionalProperties: false`.
The endpoint data-lanes constraint looks correct:
```yaml
endpoint:
$ref: /schemas/media/video-interfaces.yaml#
properties:
data-lanes:
minItems: 1
maxItems: 4
items:
enum: [ 0, 1, 2, 3 ]
```
**Minor**: Consider adding `unevaluatedProperties: false` on the endpoint node to prevent unvalidated properties from slipping through. Without it, the schema permits any additional properties on the endpoint beyond what `video-interfaces.yaml` and this local schema define.
The cosmetic description reformatting (multi-line to single-line for port@0 and port@1) is fine but unrelated to the functional change — could be split out for clarity, though not worth blocking over.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: dt-bindings: rockchip: analogix-dp: Add data-lanes example
2026-05-21 11:44 ` [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example Damon Ding
2026-05-21 19:47 ` Conor Dooley
@ 2026-05-25 10:15 ` Claude Code Review Bot
1 sibling, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 10:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Fine**
Simple addition of `data-lanes = <0 1>;` to the example endpoint:
```
edp_out_panel: endpoint {
+ data-lanes = <0 1>;
remote-endpoint = <&panel_in_edp>;
};
```
This is an identity mapping with only 2 lanes, so it doesn't demonstrate lane reordering. Consider using a non-trivial example like `data-lanes = <1 0>;` to show actual lane swapping — that would better illustrate the feature's purpose.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: drm/bridge: analogix_dp: Add support for optional data-lanes mapping
2026-05-21 11:44 ` [PATCH v2 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
@ 2026-05-25 10:15 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 10:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Needs revision — potential mapping inversion bug and minor issues**
#### 1. Potential lane mapping semantic bug (important)
The `data-lanes` DT standard (from `video-interfaces.yaml`) defines: **position = logical lane index, value = physical lane index**. So `data-lanes = <1 2 3 0>` means logical lane 0 is on physical lane 1, etc.
The register `ANALOGIX_DP_LANE_MAP` maps in the opposite direction: `LANE_N_MAP` specifies which **logical** lane **physical** lane N carries.
The code writes the `data-lanes` array directly into the register:
```c
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;
```
This treats `data-lanes[physical] = logical`, but the standard says `data-lanes[logical] = physical`. These interpretations yield the same result only for self-inverse permutations (involutions) like `<1 0 3 2>` or `<3 2 1 0>`. For a permutation like `<1 2 3 0>`, the mapping would be wrong.
For example, `data-lanes = <1 2 3 0>` means:
- Logical 0 → Physical 1, Logical 1 → Physical 2, Logical 2 → Physical 3, Logical 3 → Physical 0
The correct register values should be the **inverse** permutation:
- Physical 0 carries Logical 3 → LANE0_MAP = 3
- Physical 1 carries Logical 0 → LANE1_MAP = 0
But the code writes LANE0_MAP = 1, LANE1_MAP = 2, which is incorrect.
**Recommendation**: Either compute the inverse permutation when programming the register, or document clearly in the binding that this driver interprets `data-lanes` as physical-to-logical (non-standard). The former is preferred for DT convention compliance.
#### 2. Redundant endpoint lookup
The endpoint node is fetched twice — once inside `drm_of_get_data_lanes_count_ep()` and once explicitly:
```c
num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
video_info->max_lane_count);
...
endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
```
Consider using `of_graph_get_endpoint_by_regs()` once, then calling `drm_of_get_data_lanes_count()` (the non-`_ep` variant) on the resulting endpoint node directly, reading the property from the same node.
#### 3. Inconsistent endpoint reg parameter
The first call uses `reg=0`:
```c
drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, ...)
```
The second uses `reg=-1` (wildcard):
```c
of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1)
```
If there were multiple endpoints under port@1, these could find different endpoints. Use the same value in both calls to ensure consistency.
#### 4. Early default initialization is partially redundant
```c
u32 map[LANE_COUNT4] = {0, 1, 2, 3};
...
memcpy(video_info->lane_map, map, sizeof(map));
```
The `memcpy` at line 230 sets the default immediately, which is good for the early-return paths. But the `map` array itself is initialized at declaration with the same values, so this is fine functionally — just slightly redundant since the two copies serve the same purpose.
#### 5. Minor style: unnecessary parentheses on shift defines
```c
#define LANE3_MAP_SHIFT (6)
#define LANE2_MAP_SHIFT (4)
```
Kernel coding style doesn't require parentheses around simple integer constants. Consider `#define LANE3_MAP_SHIFT 6` for consistency with the existing defines in the same file (e.g., `(0x0 << 6)` uses parentheses around expressions, not bare constants).
#### 6. The rename from `analogix_dp_lane_swap` to `analogix_dp_lane_mapping` is clean
The old boolean `enable` parameter is replaced by reading from `dp->video_info.lane_map`, and the single call site in `analogix_dp_reset()` is updated. No external callers exist. This is a clean API evolution.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: Add eDP lane mapping support
2026-05-25 9:47 [PATCH v3 0/2] " Damon Ding
@ 2026-05-25 21:20 ` Claude Code Review Bot
0 siblings, 0 replies; 14+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:20 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-05-25 21:20 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 11:44 [PATCH v2 0/3] Add eDP lane mapping support Damon Ding
2026-05-21 11:44 ` [PATCH v2 1/3] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
2026-05-21 19:50 ` Conor Dooley
2026-05-22 1:32 ` Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-21 11:44 ` [PATCH v2 2/3] dt-bindings: rockchip: analogix-dp: Add data-lanes example Damon Ding
2026-05-21 19:47 ` Conor Dooley
2026-05-22 1:25 ` Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-21 11:44 ` [PATCH v2 3/3] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
2026-05-25 10:15 ` Claude review: " Claude Code Review Bot
2026-05-25 10:15 ` Claude review: Add eDP lane mapping support Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-05-25 9:47 [PATCH v3 0/2] " Damon Ding
2026-05-25 21:20 ` Claude review: " Claude Code Review Bot
2026-05-14 7:01 [PATCH v1 0/3] " Damon Ding
2026-05-16 1:13 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox