Claude review: drm/msm/dp: add dp_mst_drm to manage DP MST bridge operations

Claude Code Review Bot
2026-04-12T00:16:19Z

Patch Review

This is the largest and most complex patch — the MST bridge operations.

**State access bug in atomic_check**:
```c
+ mst_state = to_drm_dp_mst_topology_state(mst->mst_mgr.base.state);
+ if (IS_ERR(mst_state))
+ return PTR_ERR(mst_state);
+
+ if (!dfixed_trunc(mst_state->pbn_div)) {
+ mst_state->pbn_div =
+ drm_dp_get_vc_payload_bw(mst_conn->dp_panel->link_info.rate,
+ mst_conn->dp_panel->link_info.num_lanes);
+ }
```

`to_drm_dp_mst_topology_state(mst->mst_mgr.base.state)` reads the *current committed* state, not the state being built in this atomic transaction. This should use `drm_atomic_get_mst_topology_state(state, &mst->mst_mgr)` to get (or create) the MST state within the current atomic transaction. Modifying the current state during atomic_check violates the atomic model and can cause races with concurrent modesets. Note that `to_drm_dp_mst_topology_state` also cannot return an error pointer — it's a `container_of` macro — so the `IS_ERR` check is also dead code.

Additionally, the same function has a similar issue in `_msm_dp_mst_bridge_pre_enable_part2()`:
```c
+ mst_state = to_drm_dp_mst_topology_state(mst->mst_mgr.base.state);
+ payload = drm_atomic_get_mst_payload_state(mst_state, port);
```
In pre_enable this is less dangerous since the state is committed by then, but `drm_atomic_get_new_mst_topology_state()` from the atomic_state parameter would be more correct and consistent with `pre_enable_part1` which correctly uses it.

**Missing error check on slots**:
```c
+ slots = drm_dp_atomic_find_time_slots(state, &mst->mst_mgr, mst_conn->mst_port, pbn);
+
+ drm_dbg_dp(drm_bridge->dev, "add slots, conn:%d pbn:%d slots:%d rc:%d\n",
+ connector->base.id, pbn, slots, rc);
```
`slots` is logged but never checked for errors (negative return). The function returns 0 unconditionally.

---
Generated by Claude Code Patch Reviewer