* [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors in nvkm_pstate_prog
2026-07-27 15:28 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
@ 2026-07-27 15:28 ` Francesco Magazzu
2026-07-27 15:28 ` [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds Francesco Magazzu
2026-08-01 12:11 ` [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2 siblings, 0 replies; 4+ messages in thread
From: Francesco Magazzu @ 2026-07-27 15:28 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich
Cc: dri-devel, nouveau, linux-kernel, Karol Herbst
The RAM reclock loop discards its own return value: once the do-while
exits, ram->func->tidy() runs and the function falls straight into
nvkm_cstate_prog(), whose result becomes the return value regardless of
whether ram->func->calc()/prog() failed. A negative ret from RAM
programming is silently dropped, so a failed memory clock transition is
never reported and the core clock still gets reprogrammed on top of it.
Return the RAM error (after tidy() has still run) instead of falling
through to the core clock path.
Found by Sashiko AI review (https://sashiko.dev) while reviewing
"[PATCH v2 2/4] drm/nouveau/clk: don't use the pstate cursor after the
loop" (20260712123616.1180830-1-postadelmaga@gmail.com) as a
pre-existing issue.
Signed-off-by: Francesco Magazzu <postadelmaga@gmail.com>
---
nvkm/subdev/clk/base.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/nvkm/subdev/clk/base.c b/nvkm/subdev/clk/base.c
index 44dc86b..737bec8 100644
--- a/nvkm/subdev/clk/base.c
+++ b/nvkm/subdev/clk/base.c
@@ -298,6 +298,8 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
ret = ram->func->prog(ram);
} while (ret > 0);
ram->func->tidy(ram);
+ if (ret < 0)
+ return ret;
}
return nvkm_cstate_prog(clk, pstate, NVKM_CLK_CSTATE_HIGHEST);
--
2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds
2026-07-27 15:28 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2026-07-27 15:28 ` [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors " Francesco Magazzu
@ 2026-07-27 15:28 ` Francesco Magazzu
2026-08-01 12:11 ` [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2 siblings, 0 replies; 4+ messages in thread
From: Francesco Magazzu @ 2026-07-27 15:28 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich
Cc: dri-devel, nouveau, linux-kernel, Karol Herbst
nvkm_pstate_prog() wrote clk->pstate = pstatei before attempting the RAM
and core clock reprogramming. If either step failed, the software state
tracker had already committed to the new pstate even though the hardware
transition never completed, and a later re-request for that same state
could be skipped (pstate != clk->pstate would be false), leaving the
hardware stuck out of sync with what the driver believes it's running.
Move the assignment to after nvkm_cstate_prog() returns, and only commit
it on success.
Found by Sashiko AI review (https://sashiko.dev) while reviewing
"[PATCH v2 2/4] drm/nouveau/clk: don't use the pstate cursor after the
loop" (20260712123616.1180830-1-postadelmaga@gmail.com) as a
pre-existing issue.
Signed-off-by: Francesco Magazzu <postadelmaga@gmail.com>
---
nvkm/subdev/clk/base.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/nvkm/subdev/clk/base.c b/nvkm/subdev/clk/base.c
index 737bec8..91c51d4 100644
--- a/nvkm/subdev/clk/base.c
+++ b/nvkm/subdev/clk/base.c
@@ -285,7 +285,6 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
return -EINVAL;
nvkm_debug(subdev, "setting performance state %d\n", pstatei);
- clk->pstate = pstatei;
nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
@@ -302,7 +301,11 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
return ret;
}
- return nvkm_cstate_prog(clk, pstate, NVKM_CLK_CSTATE_HIGHEST);
+ ret = nvkm_cstate_prog(clk, pstate, NVKM_CLK_CSTATE_HIGHEST);
+ if (ret == 0)
+ clk->pstate = pstatei;
+
+ return ret;
}
static void
--
2.55.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog
2026-07-27 15:28 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2026-07-27 15:28 ` [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors " Francesco Magazzu
2026-07-27 15:28 ` [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds Francesco Magazzu
@ 2026-08-01 12:11 ` Francesco Magazzu
2 siblings, 0 replies; 4+ messages in thread
From: Francesco Magazzu @ 2026-08-01 12:11 UTC (permalink / raw)
To: lyude, dakr; +Cc: dri-devel, nouveau, linux-kernel, kherbst
Note: this series applies on top of my earlier "[PATCH v2 0/4]
drm/nouveau: fix list cursor use after loop in the clk pstate paths"
(20260712123616.1180830-1-postadelmaga@gmail.com, sent 2026-07-12), not
on a clean tree -- patch 2/2 here touches the same nvkm_pstate_prog()
that v2's patch 2/4 modifies.
Both patches fix pre-existing issues an automated review (Sashiko)
found in v2's patch 2/4 while reviewing it: nvkm_pstate_prog() silently
discarding RAM clock programming errors, and clk->pstate being updated
before hardware programming actually succeeds. Neither bug was
introduced by v2 itself.
If it's easier to review as one unit, happy to resend as a v3 of the
original series with these two folded in -- let me know what you'd
prefer.
Francesco
^ permalink raw reply [flat|nested] 4+ messages in thread