[PATCH v6 0/5] Add eDP lane mapping support

[PATCH v6 0/5] 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: Add DRM DP helper API to validate DP lane counts
Patch 3: Restore mandatory for samsung,link-rate and samsung,lane-count
         properties
Patch 4: Add validation for samsung,lane-count property as preparation
Patch 5: Implement lane mapping in analogix_dp driver

Damon Ding (5):
  dt-bindings: display: bridge: analogix-dp: Add data-lanes support for
    endpoint
  drm/dp: Add helper to validate DP lane counts
  drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
  drm/bridge: analogix_dp: Add validation for samsung,lane-count
    property
  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    | 70 +++++++++++++++++--
 .../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 ++
 include/drm/display/drm_dp_helper.h           |  6 ++
 7 files changed, 101 insertions(+), 18 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.

Changes in v4:
- Add validation for samsung,lane-count property as preparation.

Changes in v5:
- Add DRM DP helper API to validate DP lane counts.
- Apply DRM DP helper API to check the validity of samsung,lane-count
  property.
- Add Acked-by and Reviewed-by tags.

Changes in v6:
- Add a seperate commit to restore mandatory samsung DP DT properties.
- Add Reviewed-by tag.

-- 
2.34.1

[PATCH v6 1/5] 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.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
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.

Changes in v5
- Add Acked-by and Reviewed-by tags.
---
 .../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 6548f157fd96..39dafe75a680 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
@@ -160,6 +160,7 @@ examples:
           reg = <1>;
 
           edp_out_panel: endpoint {
+            data-lanes = <0 1>;
             remote-endpoint = <&panel_in_edp>;
           };
         };
-- 
2.34.1

[PATCH v6 2/5] drm/dp: Add helper to validate DP lane counts

[Damon Ding]

Add a generic helper function drm_dp_lane_count_is_valid() to check
if a DisplayPort lane count is valid. According to the DP specification,
only 1, 2, or 4 lanes are supported.

This helper avoids duplicating DP lane count validation logic across
individual DisplayPort drivers.

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

---

Changes in v6:
- Add Reviewed-by tag.
---
 include/drm/display/drm_dp_helper.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 8c2d77a032f0..c904cb480d84 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -138,6 +138,12 @@ bool drm_dp_as_sdp_supported(struct drm_dp_aux *aux, const u8 dpcd[DP_RECEIVER_C
 
 int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
 
+static inline bool
+drm_dp_lane_count_is_valid(int lane_count)
+{
+	return lane_count == 1 || lane_count == 2 || lane_count == 4;
+}
+
 static inline int
 drm_dp_max_link_rate(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
-- 
2.34.1

[PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[Damon Ding]

Revert the change that made samsung,link-rate and samsung,lane-count
optional for Exynos DP. Add error checking to fail probe early if the
required DT properties are missing.

If these properties are zero, &link_train.link_rate and
&link_train.lane_count used in the subsequent link training flow will
also be zero, resulting in link training failure. Although all existing
Exynos DP DT nodes already contain these two properties, this potential
logic issue still needs to be fixed.

Fixes: 0d0abd894ead ("drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288")
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 8d3d4a6e6ca2..5240efcd3861 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1244,6 +1244,7 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 {
 	struct device_node *dp_node = dp->dev->of_node;
 	struct video_info *video_info = &dp->video_info;
+	u32 val;
 
 	switch (dp->plat_data->dev_type) {
 	case RK3288_DP:
@@ -1265,10 +1266,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 		 * NOTE: those property parseing code is used for
 		 * providing backward compatibility for samsung platform.
 		 */
-		of_property_read_u32(dp_node, "samsung,link-rate",
-				     &video_info->max_link_rate);
-		of_property_read_u32(dp_node, "samsung,lane-count",
-				     &video_info->max_lane_count);
+		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
+			return dev_err_probe(dp->dev, -EINVAL,
+					     "Failed to get samsung,link-rate\n");
+		video_info->max_link_rate = val;
+		if (of_property_read_u32(dp_node, "samsung,lane-count", &val))
+			return dev_err_probe(dp->dev, -EINVAL,
+					     "Failed to get samsung,lane-count\n");
+		video_info->max_lane_count = val;
 		break;
 	}
 
-- 
2.34.1

[PATCH v6 4/5] drm/bridge: analogix_dp: Add validation for samsung,lane-count property

[Damon Ding]

Add validity check for samsung,lane-count to ensure DT-provided
lane count values are specification-compliant.

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

---

Changes in v5:
- Apply DRM DP helper API to check the validity of DP lane count.

Changes in v6:
- Rebase after the new commit for restoring mandatory properties
  samsung,link-rate and samsung,lane-count.
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 5240efcd3861..039e076d2b8a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1270,7 +1270,8 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
 			return dev_err_probe(dp->dev, -EINVAL,
 					     "Failed to get samsung,link-rate\n");
 		video_info->max_link_rate = val;
-		if (of_property_read_u32(dp_node, "samsung,lane-count", &val))
+		if (of_property_read_u32(dp_node, "samsung,lane-count", &val) ||
+		    !drm_dp_lane_count_is_valid(val))
 			return dev_err_probe(dp->dev, -EINVAL,
 					     "Failed to get samsung,lane-count\n");
 		video_info->max_lane_count = val;
-- 
2.34.1

[PATCH v6 5/5] 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.

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Damon Ding <damon.ding@rock-chips.com>

---

Changes in v2:
- Add lane mapping application scenarios in commit message.

Changes in v5:
- Add Reviewed-by tag.
---
 .../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 039e076d2b8a..cb06347196ce 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1240,6 +1240,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;
@@ -1278,6 +1331,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 c7997677a286..4560597604ad 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 {
@@ -177,7 +179,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

Re: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation.
--

--- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties ---
commit b573dba50acc6087122ffa4a65680322f4d004f8
Author: Damon Ding <damon.ding@rock-chips.com>

drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

This commit reverts a change that made the samsung,link-rate and
samsung,lane-count properties optional for Exynos DP. It adds error checking
to fail the probe early if these properties are missing, aiming to prevent
link training failures caused by zero values.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 8cf6b73bceac0..293d8adfe9856 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
[ ... ]
> @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>  		 * NOTE: those property parseing code is used for
>  		 * providing backward compatibility for samsung platform.
>  		 */
> -		of_property_read_u32(dp_node, "samsung,link-rate",
> -				     &video_info->max_link_rate);
> -		of_property_read_u32(dp_node, "samsung,lane-count",
> -				     &video_info->max_lane_count);
> +		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
> +			return dev_err_probe(dp->dev, -EINVAL,
> +					     "Failed to get samsung,link-rate\n");

[Severity: Critical]
Does failing the probe when this property is missing break the DT ABI?

Since samsung,link-rate is a deprecated property, a modern schema-compliant
Device Tree might intentionally omit it. If a system boots with such a DT,
this check will fail the probe with -EINVAL and disable the display.

Could the driver instead fall back to hardware maximums or sensible defaults
when the deprecated property is absent, rather than returning an error?

> +		video_info->max_link_rate = val;
> +		if (of_property_read_u32(dp_node, "samsung,lane-count", &val))
> +			return dev_err_probe(dp->dev, -EINVAL,
> +					     "Failed to get samsung,lane-count\n");

[Severity: Critical]
Does making samsung,lane-count mandatory similarly break backward compatibility
for valid Device Trees that have intentionally omitted this deprecated property?

> +		video_info->max_lane_count = val;
>  		break;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707040613.1992841-1-damon.ding@rock-chips.com?part=3

Re: [PATCH v6 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `data-lanes` array is interpreted backwards when configuring the `ANALOGIX_DP_LANE_MAP` register, applying the inverse of the intended hardware lane mapping.
- [High] The external PHY's analog configuration (voltage and pre-emphasis) ignores the logical-to-physical lane mapping.
- [Medium] Mismatched endpoint specifiers are used when counting data lanes versus reading the `data-lanes` property, which could lead to parsing the wrong endpoint.
--

--- Patch [5]: [PATCH v6 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping ---
commit 99a86aa8f074546e71df9c8b7f51f7d473ef6dac
Author: Damon Ding <damon.ding@rock-chips.com>

drm/bridge: analogix_dp: Add support for optional data-lanes mapping
    
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 automatically.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b69743837092b..5c5cb6148cf6c 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 = {
>  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);

[Severity: Medium]
Will this return the correct endpoint if there are multiple endpoints on
port 1?

When counting the data lanes, drm_of_get_data_lanes_count_ep() is called with
reg = 0. However, of_graph_get_endpoint_by_regs() uses reg = -1 (wildcard),
which returns the first available endpoint.

If the device tree has multiple endpoints, could this result in counting lanes
on one endpoint but reading the data-lanes property from a different one?

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

[Severity: High]
Does analogix_dp_set_lane_link_training() need to be updated to use this lane
map?

This parsing logic creates a logical-to-physical mapping. However, looking at
analogix_dp_set_lane_link_training(), it appears to still use the logical lane
index to populate the physical PHY configuration structure:

	for (lane = 0; lane < lane_count; lane++) {
		...
		phy_cfg.dp.voltage[lane] = vs;
		phy_cfg.dp.pre[lane] = pe;
	...

Will this cause the external PHY to apply the analog settings to the wrong
physical lanes when an asymmetric mapping is used?

>  	}
>  
>  	for (i = 0; i < LANE_COUNT4 && num_lanes < LANE_COUNT4; i++) {
>  		if (!used[i])
>  			map[num_lanes++] = i;
>  	}
[ ... ]
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index 38fd8d5014d29..45c7652645a13 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)
>  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;

[Severity: High]
Does this logic configure the lane mapping backwards?

The lane map parsed from the device tree establishes lane_map[logical] =
physical. However, this register assignment shifts the physical lane index
(lane_map[i]) by the shift for the physical lane's register field
(LANEi_MAP_SHIFT).

Because the hardware register expects the logical lane number to be written
to the physical lane's bitfield, won't this write the physical lane number
into the physical lane's field instead? This effectively applies the inverse
permutation and could break output for any asymmetric lane mappings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707040613.1992841-1-damon.ding@rock-chips.com?part=5

Re: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[Damon Ding]

On 7/7/2026 12:17 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation.
> --
> 
> --- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties ---
> commit b573dba50acc6087122ffa4a65680322f4d004f8
> Author: Damon Ding <damon.ding@rock-chips.com>
> 
> drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
> 
> This commit reverts a change that made the samsung,link-rate and
> samsung,lane-count properties optional for Exynos DP. It adds error checking
> to fail the probe early if these properties are missing, aiming to prevent
> link training failures caused by zero values.
> 
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 8cf6b73bceac0..293d8adfe9856 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> [ ... ]
>> @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>>   		 * NOTE: those property parseing code is used for
>>   		 * providing backward compatibility for samsung platform.
>>   		 */
>> -		of_property_read_u32(dp_node, "samsung,link-rate",
>> -				     &video_info->max_link_rate);
>> -		of_property_read_u32(dp_node, "samsung,lane-count",
>> -				     &video_info->max_lane_count);
>> +		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
>> +			return dev_err_probe(dp->dev, -EINVAL,
>> +					     "Failed to get samsung,link-rate\n");
> 
> [Severity: Critical]
> Does failing the probe when this property is missing break the DT ABI?
> 
> Since samsung,link-rate is a deprecated property, a modern schema-compliant
> Device Tree might intentionally omit it. If a system boots with such a DT,
> this check will fail the probe with -EINVAL and disable the display.
> 
> Could the driver instead fall back to hardware maximums or sensible defaults
> when the deprecated property is absent, rather than returning an error?
> 

I'm afraid it's difficult to provide a universal sensible default value 
here.

The samsung,lane-count and samsung,link-rate values vary significantly 
across Exynos DP device trees, so a generic fallback to hardware 
maximums cannot guarantee stable link training.

Even though these two properties are marked deprecated, all existing DTS 
files in our projects have already populated them completely. Moreover, 
the current link training logic relies on these two parameters; omitting 
either will directly cause link training failures at runtime.

Therefore, retaining the mandatory property check in probe is a 
reasonable approach for now.

>> +		video_info->max_link_rate = val;
>> +		if (of_property_read_u32(dp_node, "samsung,lane-count", &val))
>> +			return dev_err_probe(dp->dev, -EINVAL,
>> +					     "Failed to get samsung,lane-count\n");
> 
> [Severity: Critical]
> Does making samsung,lane-count mandatory similarly break backward compatibility
> for valid Device Trees that have intentionally omitted this deprecated property?
> 

I don't think so. Missing either property will inevitably cause link 
training failure, and all functional DTs have these deprecated 
properties filled in already.

>> +		video_info->max_lane_count = val;
>>   		break;
>>   	}
>>
> 

Best regards,
Damon

Re: [PATCH v6 0/5] Add eDP lane mapping support

[Damon Ding]

Gentle ping.

On 7/7/2026 12:06 PM, Damon Ding wrote:
> 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: Add DRM DP helper API to validate DP lane counts
> Patch 3: Restore mandatory for samsung,link-rate and samsung,lane-count
>           properties
> Patch 4: Add validation for samsung,lane-count property as preparation
> Patch 5: Implement lane mapping in analogix_dp driver
> 
> Damon Ding (5):
>    dt-bindings: display: bridge: analogix-dp: Add data-lanes support for
>      endpoint
>    drm/dp: Add helper to validate DP lane counts
>    drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
>    drm/bridge: analogix_dp: Add validation for samsung,lane-count
>      property
>    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    | 70 +++++++++++++++++--
>   .../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 ++
>   include/drm/display/drm_dp_helper.h           |  6 ++
>   7 files changed, 101 insertions(+), 18 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.
> 
> Changes in v4:
> - Add validation for samsung,lane-count property as preparation.
> 
> Changes in v5:
> - Add DRM DP helper API to validate DP lane counts.
> - Apply DRM DP helper API to check the validity of samsung,lane-count
>    property.
> - Add Acked-by and Reviewed-by tags.
> 
> Changes in v6:
> - Add a seperate commit to restore mandatory samsung DP DT properties.
> - Add Reviewed-by tag.
> 

Re: [PATCH v6 4/5] drm/bridge: analogix_dp: Add validation for samsung,lane-count property

[Luca Ceresoli]

On Tue, 07 Jul 2026 12:06:12 +0800, Damon Ding <damon.ding@rock-chips.com> wrote:
> Add validity check for samsung,lane-count to ensure DT-provided
> lane count values are specification-compliant.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[Luca Ceresoli]

Hello Damon,

On Wed Jul 15, 2026 at 10:32 AM CEST, Damon Ding wrote:
> On 7/7/2026 12:17 PM, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation.
>> --
>>
>> --- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties ---
>> commit b573dba50acc6087122ffa4a65680322f4d004f8
>> Author: Damon Ding <damon.ding@rock-chips.com>
>>
>> drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
>>
>> This commit reverts a change that made the samsung,link-rate and
>> samsung,lane-count properties optional for Exynos DP. It adds error checking
>> to fail the probe early if these properties are missing, aiming to prevent
>> link training failures caused by zero values.
>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 8cf6b73bceac0..293d8adfe9856 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> [ ... ]
>>> @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>>>   		 * NOTE: those property parseing code is used for
>>>   		 * providing backward compatibility for samsung platform.
>>>   		 */
>>> -		of_property_read_u32(dp_node, "samsung,link-rate",
>>> -				     &video_info->max_link_rate);
>>> -		of_property_read_u32(dp_node, "samsung,lane-count",
>>> -				     &video_info->max_lane_count);
>>> +		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
>>> +			return dev_err_probe(dp->dev, -EINVAL,
>>> +					     "Failed to get samsung,link-rate\n");
>>
>> [Severity: Critical]
>> Does failing the probe when this property is missing break the DT ABI?
>>
>> Since samsung,link-rate is a deprecated property, a modern schema-compliant
>> Device Tree might intentionally omit it. If a system boots with such a DT,
>> this check will fail the probe with -EINVAL and disable the display.
>>
>> Could the driver instead fall back to hardware maximums or sensible defaults
>> when the deprecated property is absent, rather than returning an error?
>>
>
> I'm afraid it's difficult to provide a universal sensible default value
> here.
>
> The samsung,lane-count and samsung,link-rate values vary significantly
> across Exynos DP device trees, so a generic fallback to hardware
> maximums cannot guarantee stable link training.
>
> Even though these two properties are marked deprecated, all existing DTS
> files in our projects have already populated them completely. Moreover,
> the current link training logic relies on these two parameters; omitting
> either will directly cause link training failures at runtime.
>
> Therefore, retaining the mandatory property check in probe is a
> reasonable approach for now.

I'm still not convinced we should turn an optional property into mandatory,
after having been optional since 2016 (based on the commit in the Fixes:
tag).

The sure thing is we cannot break existing devices which upgrade to a new
kernel.

Se the question is: if a device is missing one of these properties, or
both, what happens? The video_info->max_link_rate and/or
video_info->max_lane_count will be zero, correct? And if they are zero, is
there even a remote possibility that the device will work somehow, maybe
only with some rare low resolution or whatever?

If the answer is "yes, there is a remote possibility that one sich device,
with some maybe rare configuration, will work", then no, we cannot make
this property mandatory now. There can be devices out there working without
these proberties, and they would be broken.

If the answer is "there is no way at all a device can work without one or
both properties", with a good explanation based on the code flow and
hardware docs, then we can consider this change.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[Damon Ding]

Hi Luca,

On 8/19/2026 3:32 PM, Luca Ceresoli wrote:
> Hello Damon,
> 
> On Wed Jul 15, 2026 at 10:32 AM CEST, Damon Ding wrote:
>> On 7/7/2026 12:17 PM, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>> - [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation.
>>> --
>>>
>>> --- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties ---
>>> commit b573dba50acc6087122ffa4a65680322f4d004f8
>>> Author: Damon Ding <damon.ding@rock-chips.com>
>>>
>>> drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
>>>
>>> This commit reverts a change that made the samsung,link-rate and
>>> samsung,lane-count properties optional for Exynos DP. It adds error checking
>>> to fail the probe early if these properties are missing, aiming to prevent
>>> link training failures caused by zero values.
>>>
>>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>> index 8cf6b73bceac0..293d8adfe9856 100644
>>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> [ ... ]
>>>> @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>>>>    		 * NOTE: those property parseing code is used for
>>>>    		 * providing backward compatibility for samsung platform.
>>>>    		 */
>>>> -		of_property_read_u32(dp_node, "samsung,link-rate",
>>>> -				     &video_info->max_link_rate);
>>>> -		of_property_read_u32(dp_node, "samsung,lane-count",
>>>> -				     &video_info->max_lane_count);
>>>> +		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
>>>> +			return dev_err_probe(dp->dev, -EINVAL,
>>>> +					     "Failed to get samsung,link-rate\n");
>>>
>>> [Severity: Critical]
>>> Does failing the probe when this property is missing break the DT ABI?
>>>
>>> Since samsung,link-rate is a deprecated property, a modern schema-compliant
>>> Device Tree might intentionally omit it. If a system boots with such a DT,
>>> this check will fail the probe with -EINVAL and disable the display.
>>>
>>> Could the driver instead fall back to hardware maximums or sensible defaults
>>> when the deprecated property is absent, rather than returning an error?
>>>
>>
>> I'm afraid it's difficult to provide a universal sensible default value
>> here.
>>
>> The samsung,lane-count and samsung,link-rate values vary significantly
>> across Exynos DP device trees, so a generic fallback to hardware
>> maximums cannot guarantee stable link training.
>>
>> Even though these two properties are marked deprecated, all existing DTS
>> files in our projects have already populated them completely. Moreover,
>> the current link training logic relies on these two parameters; omitting
>> either will directly cause link training failures at runtime.
>>
>> Therefore, retaining the mandatory property check in probe is a
>> reasonable approach for now.
> 
> I'm still not convinced we should turn an optional property into mandatory,
> after having been optional since 2016 (based on the commit in the Fixes:
> tag).
> 
> The sure thing is we cannot break existing devices which upgrade to a new
> kernel.
> 
> Se the question is: if a device is missing one of these properties, or
> both, what happens? The video_info->max_link_rate and/or
> video_info->max_lane_count will be zero, correct? And if they are zero, is
> there even a remote possibility that the device will work somehow, maybe
> only with some rare low resolution or whatever?
> 
> If the answer is "yes, there is a remote possibility that one sich device,
> with some maybe rare configuration, will work", then no, we cannot make
> this property mandatory now. There can be devices out there working without
> these proberties, and they would be broken.
> 
> If the answer is "there is no way at all a device can work without one or
> both properties", with a good explanation based on the code flow and
> hardware docs, then we can consider this change.
> 

Sorry for the confusion, I just submitted the v7 series which crossed 
with your reply.

To answer your question: there is no way at all a device can work
without these properties. Here is the code flow when either
max_link_rate or max_lane_count is 0 (helped by AI):

   analogix_dp_commit()
     -> analogix_dp_full_link_train(dp, max_lanes = 0, max_rate = 0)

   analogix_dp_full_link_train(max_lanes, max_rate):
       // Read sink capabilities via DPCD and sanitize them
       link_rate   = read_dpcd(DP_MAX_LINK_RATE); // >= 0x06 after fixup
       lane_count  = read_dpcd(DP_MAX_LANE_COUNT);// >= 1 after fixup

       // Clamp by the limits from DT
       if (link_rate > max_rate)                // 0x06 > 0, always true
           link_rate = max_rate;                // link_rate = 0
       if (lane_count > max_lanes)              // 1 > 0, always true
           lane_count = max_lanes;              // lane_count = 0

       // Configure TX with the zeroed values
       set_link_bandwidth(link_rate = 0)
           // writel() is only executed for bwtype == 0x06/0x0a,
           // so LINK_BW_SET is never written and stays at reset value;
           // phy_configure() is called with link_rate = 0.

       set_lane_count(lane_count = 0)
           // writel(0, ANALOGIX_DP_LANE_COUNT_SET) enables 0 lanes;
           // phy_configure() is called with lanes = 0.

       // Program sink for link training
       drm_dp_dpcd_write(DP_LINK_BW_SET, {link_rate = 0, lane_count = 0})
           // DP spec requires link rate in {0x06, 0x0a, 0x14} and
           // lane count in {1, 2, 4}. Writing zeros is illegal, so the
           // sink cannot enter the training state.

       // Training loop
       for (lane = 0; lane < lane_count /* 0 */; lane++)
           // loop body never executes; training_lane[] stays
           // uninitialized and no training register is ever programmed

Since the sanitized sink values are always non-zero (link_rate >= 0x06,
lane_count >= 1), the clamping with a zero maximum unconditionally
forces the training parameters to zero. Clock recovery can never be 
achieved, so link training fails deterministically.

Best regards,
Damon

Re: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties

[Luca Ceresoli]

Hello,

On Wed Aug 19, 2026 at 10:19 AM CEST, Damon Ding wrote:
> Hi Luca,
>
> On 8/19/2026 3:32 PM, Luca Ceresoli wrote:
>> Hello Damon,
>>
>> On Wed Jul 15, 2026 at 10:32 AM CEST, Damon Ding wrote:
>>> On 7/7/2026 12:17 PM, sashiko-bot@kernel.org wrote:
>>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>> - [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation.
>>>> --
>>>>
>>>> --- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties ---
>>>> commit b573dba50acc6087122ffa4a65680322f4d004f8
>>>> Author: Damon Ding <damon.ding@rock-chips.com>
>>>>
>>>> drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties
>>>>
>>>> This commit reverts a change that made the samsung,link-rate and
>>>> samsung,lane-count properties optional for Exynos DP. It adds error checking
>>>> to fail the probe early if these properties are missing, aiming to prevent
>>>> link training failures caused by zero values.
>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>>> index 8cf6b73bceac0..293d8adfe9856 100644
>>>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>> [ ... ]
>>>>> @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>>>>>    		 * NOTE: those property parseing code is used for
>>>>>    		 * providing backward compatibility for samsung platform.
>>>>>    		 */
>>>>> -		of_property_read_u32(dp_node, "samsung,link-rate",
>>>>> -				     &video_info->max_link_rate);
>>>>> -		of_property_read_u32(dp_node, "samsung,lane-count",
>>>>> -				     &video_info->max_lane_count);
>>>>> +		if (of_property_read_u32(dp_node, "samsung,link-rate", &val))
>>>>> +			return dev_err_probe(dp->dev, -EINVAL,
>>>>> +					     "Failed to get samsung,link-rate\n");
>>>>
>>>> [Severity: Critical]
>>>> Does failing the probe when this property is missing break the DT ABI?
>>>>
>>>> Since samsung,link-rate is a deprecated property, a modern schema-compliant
>>>> Device Tree might intentionally omit it. If a system boots with such a DT,
>>>> this check will fail the probe with -EINVAL and disable the display.
>>>>
>>>> Could the driver instead fall back to hardware maximums or sensible defaults
>>>> when the deprecated property is absent, rather than returning an error?
>>>>
>>>
>>> I'm afraid it's difficult to provide a universal sensible default value
>>> here.
>>>
>>> The samsung,lane-count and samsung,link-rate values vary significantly
>>> across Exynos DP device trees, so a generic fallback to hardware
>>> maximums cannot guarantee stable link training.
>>>
>>> Even though these two properties are marked deprecated, all existing DTS
>>> files in our projects have already populated them completely. Moreover,
>>> the current link training logic relies on these two parameters; omitting
>>> either will directly cause link training failures at runtime.
>>>
>>> Therefore, retaining the mandatory property check in probe is a
>>> reasonable approach for now.
>>
>> I'm still not convinced we should turn an optional property into mandatory,
>> after having been optional since 2016 (based on the commit in the Fixes:
>> tag).
>>
>> The sure thing is we cannot break existing devices which upgrade to a new
>> kernel.
>>
>> Se the question is: if a device is missing one of these properties, or
>> both, what happens? The video_info->max_link_rate and/or
>> video_info->max_lane_count will be zero, correct? And if they are zero, is
>> there even a remote possibility that the device will work somehow, maybe
>> only with some rare low resolution or whatever?
>>
>> If the answer is "yes, there is a remote possibility that one sich device,
>> with some maybe rare configuration, will work", then no, we cannot make
>> this property mandatory now. There can be devices out there working without
>> these proberties, and they would be broken.
>>
>> If the answer is "there is no way at all a device can work without one or
>> both properties", with a good explanation based on the code flow and
>> hardware docs, then we can consider this change.
>>
>
> Sorry for the confusion, I just submitted the v7 series which crossed
> with your reply.
>
> To answer your question: there is no way at all a device can work
> without these properties. Here is the code flow when either
> max_link_rate or max_lane_count is 0 (helped by AI):
>
>    analogix_dp_commit()
>      -> analogix_dp_full_link_train(dp, max_lanes = 0, max_rate = 0)
>
>    analogix_dp_full_link_train(max_lanes, max_rate):
>        // Read sink capabilities via DPCD and sanitize them
>        link_rate   = read_dpcd(DP_MAX_LINK_RATE); // >= 0x06 after fixup
>        lane_count  = read_dpcd(DP_MAX_LANE_COUNT);// >= 1 after fixup
>
>        // Clamp by the limits from DT
>        if (link_rate > max_rate)                // 0x06 > 0, always true
>            link_rate = max_rate;                // link_rate = 0
>        if (lane_count > max_lanes)              // 1 > 0, always true
>            lane_count = max_lanes;              // lane_count = 0
>
>        // Configure TX with the zeroed values
>        set_link_bandwidth(link_rate = 0)
>            // writel() is only executed for bwtype == 0x06/0x0a,
>            // so LINK_BW_SET is never written and stays at reset value;
>            // phy_configure() is called with link_rate = 0.
>
>        set_lane_count(lane_count = 0)
>            // writel(0, ANALOGIX_DP_LANE_COUNT_SET) enables 0 lanes;
>            // phy_configure() is called with lanes = 0.
>
>        // Program sink for link training
>        drm_dp_dpcd_write(DP_LINK_BW_SET, {link_rate = 0, lane_count = 0})
>            // DP spec requires link rate in {0x06, 0x0a, 0x14} and
>            // lane count in {1, 2, 4}. Writing zeros is illegal, so the
>            // sink cannot enter the training state.
>
>        // Training loop
>        for (lane = 0; lane < lane_count /* 0 */; lane++)
>            // loop body never executes; training_lane[] stays
>            // uninitialized and no training register is ever programmed
>
> Since the sanitized sink values are always non-zero (link_rate >= 0x06,
> lane_count >= 1), the clamping with a zero maximum unconditionally
> forces the training parameters to zero. Clock recovery can never be
> achieved, so link training fails deterministically.

Thank you very much for the detailed analysis! To it is enough to
proceed. I'll review your v7.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com