[PATCH 0/5] drm/rockchip: vop2: Fix layer cfg done timeout on multi-output setups

[PATCH 0/5] drm/rockchip: vop2: Fix layer cfg done timeout on multi-output setups

[Cristian Ciocaltea]

On RK3588/RK3568 boards with multiple active display outputs, start/stop
transitions may trigger a timeout during overlay layer configuration:

  rockchip-drm display-subsystem: [drm] *ERROR* wait layer cfg done timeout

The shared OVL_LAYER_SEL and OVL_PORT_SEL shadow registers are committed
to the active configuration at the vsync of whichever Video Port is
selected by LAYERSEL_REGDONE_SEL.  When two Video Ports race through
atomic commits, rk3568_vop2_setup_layer_mixer() has two issues that
cause the wait to poll for a value the hardware might not be able to
produce.

Patch 1 fixes passing the wrong target to the wait function, since the
expected value was already overwritten with the current VP's new
layer_sel before reaching the wait.

Patch 2 moves the wait before the LAYERSEL_REGDONE_SEL switch, so the
previous VP's vsync can still latch the pending configuration.

Patches 3 through 5 contain only minor follow-up cleanup.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Cristian Ciocaltea (5):
      drm/rockchip: vop2: Fix wrong wait target in layer cfg done check
      drm/rockchip: vop2: Wait for layer cfg done before switching LAYERSEL_REGDONE_SEL
      drm/rockchip: vop2: Delay old_{layer|port}_sel updates in setup_layer_mixer()
      drm/rockchip: vop2: Drop redundant zero-init in setup_layer_mixer()
      drm/rockchip: vop2: Use vop2->old_layer_sel directly in wait_for_layer_cfg_done()

 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 46 +++++++++++++---------------
 1 file changed, 22 insertions(+), 24 deletions(-)
---
base-commit: d4c14903bf5e28e740516c4fbb7db01e0dedf3af
change-id: 20260504-vop2-layer-cfg-tmout-73617a0a103c

[PATCH 1/5] drm/rockchip: vop2: Fix wrong wait target in layer cfg done check

[Cristian Ciocaltea]

rk3568_vop2_setup_layer_mixer() waits for the previous Video Port (VP)
layer configuration to take effect before writing a new one to the
shared RK3568_OVL_LAYER_SEL shadow register.  However, it passes
vop2->old_layer_sel to rk3568_vop2_wait_for_layer_cfg_done() as the
expected value, which at that point already contains the new VP layer.

This causes the wait to poll for a value that has not been written to
the shadow register yet, resulting in spurious timeouts when two
non-blocking atomic commits race:

  rockchip-drm display-subsystem: [drm] *ERROR* wait layer cfg done timeout [...]

Pass the local old_layer_sel instead, which still holds the value
captured from vop2->old_layer_sel before it was overwritten, i.e. the
previous VP target that the hardware is expected to latch.

Fixes: 3e89a8c68354 ("drm/rockchip: vop2: Fix the update of LAYER/PORT select registers when there are multi display output on rk3588/rk3568")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 02a788a4dfdd..edca0fb16e08 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2307,7 +2307,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 		 * Changes of other VPs' overlays have not taken effect
 		 */
 		if (cfg_done)
-			rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel);
+			rk3568_vop2_wait_for_layer_cfg_done(vop2, old_layer_sel);
 	}
 
 	vop2_writel(vop2, RK3568_OVL_LAYER_SEL, layer_sel);

-- 
2.53.0

[PATCH 2/5] drm/rockchip: vop2: Wait for layer cfg done before switching LAYERSEL_REGDONE_SEL

[Cristian Ciocaltea]

LAYERSEL_REGDONE_SEL mask of RK3568_OVL_CTRL register controls which
Video Port (VP) vsync latches the shared RK3568_OVL_{LAYER|PORT}_SEL
shadow registers into the active configuration.

rk3568_vop2_setup_layer_mixer() overwrites LAYERSEL_REGDONE_SEL to the
current VP ID before waiting for the previous VP layer configuration to
take effect.  As a consequence, the previous VP vsync can no longer
trigger the latch, so the wait polls a value that might never appear.

Move the layer cfg done wait before the RK3568_OVL_CTRL write so the
previous VP vsync can still commit the pending configuration.

Fixes: 3e89a8c68354 ("drm/rockchip: vop2: Fix the update of LAYER/PORT select registers when there are multi display output on rk3588/rk3568")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index edca0fb16e08..5206f01ec787 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2289,15 +2289,6 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 	 *    lead to the configuration of the previous VP being take effect along with the VSYNC
 	 *    of the new VP.
 	 */
-	if (layer_sel != old_layer_sel || port_sel != old_port_sel)
-		ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, vp->id);
-	vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl);
-
-	if (port_sel != old_port_sel) {
-		vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel);
-		vop2_cfg_done(vp);
-		rk3568_vop2_wait_for_port_mux_done(vop2);
-	}
 
 	if (layer_sel != old_layer_sel && atv_layer_sel != old_layer_sel) {
 		cfg_done = vop2_readl(vop2, RK3568_REG_CFG_DONE);
@@ -2310,6 +2301,16 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 			rk3568_vop2_wait_for_layer_cfg_done(vop2, old_layer_sel);
 	}
 
+	if (layer_sel != old_layer_sel || port_sel != old_port_sel)
+		ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, vp->id);
+	vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl);
+
+	if (port_sel != old_port_sel) {
+		vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel);
+		vop2_cfg_done(vp);
+		rk3568_vop2_wait_for_port_mux_done(vop2);
+	}
+
 	vop2_writel(vop2, RK3568_OVL_LAYER_SEL, layer_sel);
 	mutex_unlock(&vop2->ovl_lock);
 }

-- 
2.53.0

[PATCH 3/5] drm/rockchip: vop2: Delay old_{layer|port}_sel updates in setup_layer_mixer()

[Cristian Ciocaltea]

The old_layer_sel and old_port_sel local variables were introduced to
hold the previous VP configuration for comparisons and wait targets,
working around the premature update of vop2->old_layer_sel and
vop2->old_port_sel earlier in the function.

Remove these superfluous locals and instead defer the assignments of
vop2->old_layer_sel and vop2->old_port_sel to just before the
corresponding shadow register writes, where the transition from old to
new logically belongs.

No functional change intended.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 5206f01ec787..1f5e8c2acecd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2136,9 +2136,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 	struct drm_plane *plane;
 	u32 layer_sel = 0;
 	u32 port_sel;
-	u32 old_layer_sel = 0;
 	u32 atv_layer_sel = 0;
-	u32 old_port_sel = 0;
 	u8 layer_id;
 	u8 old_layer_id;
 	u8 layer_sel_id;
@@ -2161,8 +2159,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 	else
 		ovl_ctrl &= ~RK3568_OVL_CTRL__YUV_MODE(vp->id);
 
-	old_port_sel = vop2->old_port_sel;
-	port_sel = old_port_sel;
+	port_sel = vop2->old_port_sel;
 	port_sel &= RK3568_OVL_PORT_SEL__SEL_PORT;
 
 	if (vp0->nlayers)
@@ -2188,8 +2185,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 		port_sel |= FIELD_PREP(RK3588_OVL_PORT_SET__PORT3_MUX, 7);
 
 	atv_layer_sel = vop2_readl(vop2, RK3568_OVL_LAYER_SEL);
-	old_layer_sel = vop2->old_layer_sel;
-	layer_sel = old_layer_sel;
+	layer_sel = vop2->old_layer_sel;
 
 	ofs = 0;
 	for (i = 0; i < vp->id; i++)
@@ -2273,8 +2269,6 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 			     old_win->data->layer_sel_id[vp->id]);
 	}
 
-	vop2->old_layer_sel = layer_sel;
-	vop2->old_port_sel = port_sel;
 	/*
 	 * As the RK3568_OVL_LAYER_SEL and RK3568_OVL_PORT_SEL are shared by all Video Ports,
 	 * and the configuration take effect by one Video Port's vsync.
@@ -2290,7 +2284,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 	 *    of the new VP.
 	 */
 
-	if (layer_sel != old_layer_sel && atv_layer_sel != old_layer_sel) {
+	if (layer_sel != vop2->old_layer_sel && atv_layer_sel != vop2->old_layer_sel) {
 		cfg_done = vop2_readl(vop2, RK3568_REG_CFG_DONE);
 		cfg_done &= (BIT(vop2->data->nr_vps) - 1);
 		cfg_done &= ~BIT(vp->id);
@@ -2298,20 +2292,23 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 		 * Changes of other VPs' overlays have not taken effect
 		 */
 		if (cfg_done)
-			rk3568_vop2_wait_for_layer_cfg_done(vop2, old_layer_sel);
+			rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel);
 	}
 
-	if (layer_sel != old_layer_sel || port_sel != old_port_sel)
+	if (layer_sel != vop2->old_layer_sel || port_sel != vop2->old_port_sel)
 		ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, vp->id);
 	vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl);
 
-	if (port_sel != old_port_sel) {
+	if (port_sel != vop2->old_port_sel) {
+		vop2->old_port_sel = port_sel;
 		vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel);
 		vop2_cfg_done(vp);
 		rk3568_vop2_wait_for_port_mux_done(vop2);
 	}
 
+	vop2->old_layer_sel = layer_sel;
 	vop2_writel(vop2, RK3568_OVL_LAYER_SEL, layer_sel);
+
 	mutex_unlock(&vop2->ovl_lock);
 }
 

-- 
2.53.0

[PATCH 4/5] drm/rockchip: vop2: Drop redundant zero-init in setup_layer_mixer()

[Cristian Ciocaltea]

The layer_sel and atv_layer_sel local variables in
rk3568_vop2_setup_layer_mixer() are unconditionally assigned from
vop2->old_layer_sel and the RK3568_OVL_LAYER_SEL register read,
respectively, before any use.

Remove the superfluous zero-initializers.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 1f5e8c2acecd..0849bd922ffb 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2134,9 +2134,9 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 {
 	struct vop2 *vop2 = vp->vop2;
 	struct drm_plane *plane;
-	u32 layer_sel = 0;
+	u32 layer_sel;
 	u32 port_sel;
-	u32 atv_layer_sel = 0;
+	u32 atv_layer_sel;
 	u8 layer_id;
 	u8 old_layer_id;
 	u8 layer_sel_id;

-- 
2.53.0

[PATCH 5/5] drm/rockchip: vop2: Use vop2->old_layer_sel directly in wait_for_layer_cfg_done()

[Cristian Ciocaltea]

After the old_layer_sel local was removed, the only caller of
rk3568_vop2_wait_for_layer_cfg_done() already passes vop2->old_layer_sel
as the expected value.

Drop the redundant parameter and read the member directly inside the
function.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 0849bd922ffb..1d8473a6dfd1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -2115,7 +2115,7 @@ static u32 rk3568_vop2_read_layer_cfg(struct vop2 *vop2)
 	return vop2_readl(vop2, RK3568_OVL_LAYER_SEL);
 }
 
-static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2, u32 cfg)
+static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2)
 {
 	u32 atv_layer_cfg;
 	int ret;
@@ -2124,10 +2124,10 @@ static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2, u32 cfg)
 	 * Spin until the previous layer configuration is done.
 	 */
 	ret = readx_poll_timeout_atomic(rk3568_vop2_read_layer_cfg, vop2, atv_layer_cfg,
-					atv_layer_cfg == cfg, 10, 50 * 1000);
+					atv_layer_cfg == vop2->old_layer_sel, 10, 50 * 1000);
 	if (ret)
 		drm_err_ratelimited(vop2->drm, "wait layer cfg done timeout: 0x%x--0x%x\n",
-				    atv_layer_cfg, cfg);
+				    atv_layer_cfg, vop2->old_layer_sel);
 }
 
 static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
@@ -2292,7 +2292,7 @@ static void rk3568_vop2_setup_layer_mixer(struct vop2_video_port *vp)
 		 * Changes of other VPs' overlays have not taken effect
 		 */
 		if (cfg_done)
-			rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel);
+			rk3568_vop2_wait_for_layer_cfg_done(vop2);
 	}
 
 	if (layer_sel != vop2->old_layer_sel || port_sel != vop2->old_port_sel)

-- 
2.53.0

Claude review: drm/rockchip: vop2: Fix layer cfg done timeout on multi-output setups

[Claude Code Review Bot]

Overall Series Review

Subject: drm/rockchip: vop2: Fix layer cfg done timeout on multi-output setups
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Patches: 6
Reviewed: 2026-05-05T08:08:45.269819

---

This is a well-structured 5-patch series fixing a real race condition in the VOP2 overlay layer configuration path on RK3568/RK3588 multi-output setups. The cover letter clearly explains the two root causes (patches 1 and 2) and separates the cleanups (patches 3-5).

The two bug fixes are correct: patch 1 fixes passing an already-clobbered value as the wait target, and patch 2 reorders operations so the previous VP's vsync can still latch the pending configuration before LAYERSEL_REGDONE_SEL is switched. The cleanup patches are straightforward and low-risk.

One concern worth raising is the interaction between patches 3 and 5 and the `readx_poll_timeout_atomic` spin in `rk3568_vop2_wait_for_layer_cfg_done()` — see patch 5 comments below.

**Recommendation: Patches 1 and 2 look good for merge (with Fixes tags for stable). Patches 3-5 are clean and correct but patch 5 has a minor concern worth discussing.**

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Fix wrong wait target in layer cfg done check

[Claude Code Review Bot]

Patch Review

**Verdict: Good fix, correct.**

The bug is clear: before this patch, the original code did:
1. `old_layer_sel = vop2->old_layer_sel` (capture old value into local)
2. `vop2->old_layer_sel = layer_sel` (overwrite struct member with new value)
3. `rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel)` — passes the **already-overwritten** value

The fix changes the call to use the local `old_layer_sel` which still holds the captured previous value:

```diff
-		rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel);
+		rk3568_vop2_wait_for_layer_cfg_done(vop2, old_layer_sel);
```

This is a clear correctness fix. The wait was polling for the hardware register to match a value that hadn't even been written to the shadow register yet, guaranteeing a timeout whenever the old and new values differed.

The Fixes tag and commit message are accurate.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Wait for layer cfg done before switching LAYERSEL_REGDONE_SEL

[Claude Code Review Bot]

Patch Review

**Verdict: Good fix, correct.**

This patch reorders the operations so that:
1. **First**: wait for the previous VP's layer config to latch (the wait needs the old VP's vsync to fire)
2. **Then**: write RK3568_OVL_CTRL with LAYERSEL_REGDONE_SEL set to the current VP
3. **Then**: handle port_sel changes

Before this patch, LAYERSEL_REGDONE_SEL was switched to the current VP *before* the wait, meaning the previous VP's vsync could no longer trigger the shadow register latch — a classic ordering bug.

The moved block:
```c
if (layer_sel != old_layer_sel || port_sel != old_port_sel)
    ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, vp->id);
vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl);

if (port_sel != old_port_sel) {
    vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel);
    vop2_cfg_done(vp);
    rk3568_vop2_wait_for_port_mux_done(vop2);
}
```

now appears *after* the layer cfg done wait, which is correct.

Note: the `vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl)` is unconditional (always writes ovl_ctrl even when LAYERSEL_REGDONE_SEL is not set). This was the same before the patch so it's not a regression — but it means we always write OVL_CTRL even in the no-change case. Not a bug, just an observation about pre-existing behavior.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Delay old_{layer|port}_sel updates in setup_layer_mixer()

[Claude Code Review Bot]

Patch Review

**Verdict: Clean refactor, correct.**

This removes the `old_layer_sel` and `old_port_sel` locals that were working around the premature writes to `vop2->old_layer_sel` / `vop2->old_port_sel`. After patches 1 and 2 fixed the ordering, the struct members now hold the correct (previous) values throughout the comparison/wait section, so the locals are no longer needed.

Key change: `vop2->old_layer_sel = layer_sel` and `vop2->old_port_sel = port_sel` are now deferred to just before their corresponding register writes:

```c
if (port_sel != vop2->old_port_sel) {
    vop2->old_port_sel = port_sel;
    vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel);
    ...
}

vop2->old_layer_sel = layer_sel;
vop2_writel(vop2, RK3568_OVL_LAYER_SEL, layer_sel);
```

This is correct and cleaner. Note that `old_port_sel` is updated inside the `if` (only when changed), while `old_layer_sel` is updated unconditionally — this matches the behavior since `layer_sel` is always written to the register. This is fine since the shadow register write is unconditional too.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Drop redundant zero-init in setup_layer_mixer()

[Claude Code Review Bot]

Patch Review

**Verdict: Trivially correct.**

```diff
-	u32 layer_sel = 0;
+	u32 layer_sel;
...
-	u32 atv_layer_sel = 0;
+	u32 atv_layer_sel;
```

Both variables are unconditionally assigned before use (`layer_sel = vop2->old_layer_sel` and `atv_layer_sel = vop2_readl(...)`) after patch 3. The zero-init is indeed dead.

No issues.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/rockchip: vop2: Use vop2->old_layer_sel directly in wait_for_layer_cfg_done()

[Claude Code Review Bot]

Patch Review

**Verdict: Correct but has a subtle consideration.**

This removes the `cfg` parameter from `rk3568_vop2_wait_for_layer_cfg_done()` since the only caller always passes `vop2->old_layer_sel`.

```diff
-static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2, u32 cfg)
+static void rk3568_vop2_wait_for_layer_cfg_done(struct vop2 *vop2)
```

And inside:
```diff
-				atv_layer_cfg == cfg, 10, 50 * 1000);
+				atv_layer_cfg == vop2->old_layer_sel, 10, 50 * 1000);
```

**Minor concern**: `readx_poll_timeout_atomic` re-evaluates the condition expression on each poll iteration. With the parameter version, the target value `cfg` was a stack copy captured before the loop started. With the new version, `vop2->old_layer_sel` is read from the struct on every poll iteration. If another thread (another VP's atomic commit) were to concurrently modify `vop2->old_layer_sel` while this spin is running, the target would shift mid-poll.

In practice, this is likely safe because:
- The function runs under `vop2->ovl_lock` mutex, and all paths that write `vop2->old_layer_sel` (after patch 3) also hold that mutex.
- So no concurrent writer should be able to change the value during the spin.

But this is worth noting — the parameter version was inherently immune to this class of bug, while the new version relies on the caller holding the mutex. If a future code path ever calls this function or modifies `old_layer_sel` without the lock, it would introduce a subtle bug. This is a reasonable tradeoff for code simplicity, but it might be worth a brief comment, or the maintainer may prefer keeping the parameter for robustness.

This is a style/maintainability judgment call rather than a correctness bug.

---

**Summary**: The series is well-crafted. Patches 1 and 2 are clear, correct bug fixes suitable for stable backport. Patches 3-5 are clean follow-up refactors. The only point worth discussing with the author is whether patch 5's removal of the explicit parameter is worth the implicit coupling to the mutex contract — but it's defensible either way.

---
Generated by Claude Code Patch Reviewer