On 8/19/2026 4:04 PM, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
to consider:
- [High] Logic error in hardware lane mapping configuration causes inverted
assignments for non-symmetric maps.
- [Low] Device Tree endpoint lookup inconsistency when querying lane counts
versus reading properties.
--
--- Patch [5]: [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional
data-lanes mapping ---
commit 1c15eb6132eedc3d606168644e5f97918c8e3651
Author: Damon Ding <[email protected]>
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 039e076d2b8ab..cb06347196cef 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)
+{
[ ... ]
+ 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);
Hi Luca,
Thanks for taking a look at previous v6 patch series. This Sashiko
warning was also present in v6. Sorry for CC'ing you out‑of‑the‑blue
without prior heads‑up.
[Severity: Low]
Does passing -1 here potentially retrieve a different endpoint from the one
validated with reg=0 above?
If multiple endpoints exist on the DP port and the target endpoint is not
positionally first, could this mismatch result in reading the data-lanes
property from the wrong endpoint?
For Analogix DP, port@1 will only ever have a single endpoint.
Multi‑output configurations are not supported by the hardware, so
passing -1 cannot select a mismatched endpoint. There is no risk of
retrieving the 'data‑lanes' property from an unintended endpoint.
+ if (!endpoint)
+ return -EINVAL;
+
+ ret = of_property_read_u32_array(endpoint, "data-lanes", tmp,
num_lanes);
+ of_node_put(endpoint);
[ ... ]
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index ea8401293a23a..c1344a3f013ab 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;
[Severity: High]
Does this logic incorrectly assign the physical lane value to the logical
lane's field?
By shifting the physical lane index (lane_map[i]) into the register field
assigned to the logical lane index (LANE<i>_MAP_SHIFT), it appears this
inherently applies the inverse of the requested mapping.
While this silently succeeds for symmetric maps (e.g., swapping 0<->3, 1<->2),
will it fail for any non-symmetric map (e.g., <1 2 3 0>) by causing physical
lanes to carry the wrong logical lanes?
Should the bitwise construction instead be reg |= i << (lane_map[i] * 2)?
writel(reg, dp->reg_base + ANALOGIX_DP_LANE_MAP);
}
lane_map[] uses index as physical lane, value as logical lane.
This matches training_lane[] / cr_loop[] in struct link_train, whose
indexes are physical lanes for DPCD access during link training.
LANE<i>_MAP_SHIFT configures which logical lane is routed onto physical
lane i, so current register logic is correct.
Best regards,
Damon
