* [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog
@ 2026-07-27 15:05 Francesco Magazzu
2026-07-27 15:05 ` [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors " Francesco Magazzu
2026-07-27 15:05 ` [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds Francesco Magazzu
0 siblings, 2 replies; 5+ messages in thread
From: Francesco Magazzu @ 2026-07-27 15:05 UTC (permalink / raw)
To: dri-devel; +Cc: nouveau
Follow-up to "[PATCH v2 0/4] drm/nouveau: fix list cursor use after loop
in the clk pstate paths" (20260712123616.1180830-1-postadelmaga@gmail.com).
Sashiko AI review flagged three pre-existing issues while reviewing patch
2/4 of that series; the third (clk->func->prog()'s error being clobbered
by nvkm_volt_set_id() in nvkm_cstate_prog()) is already fixed by patch 4/4
of that series. These two patches address the other two:
1. nvkm_pstate_prog() drops the RAM reclock error and falls through to
core clock programming anyway.
2. nvkm_pstate_prog() commits clk->pstate to the new state before the
hardware transition has actually succeeded.
Francesco Magazzu (2):
drm/nouveau/clk: don't ignore RAM clock programming errors in
nvkm_pstate_prog
drm/nouveau/clk: only update clk->pstate after hardware programming
succeeds
nvkm/subdev/clk/base.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors in nvkm_pstate_prog
2026-07-27 15:05 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
@ 2026-07-27 15:05 ` Francesco Magazzu
2026-07-27 15:05 ` [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds Francesco Magazzu
1 sibling, 0 replies; 5+ messages in thread
From: Francesco Magazzu @ 2026-07-27 15:05 UTC (permalink / raw)
To: dri-devel; +Cc: nouveau
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 related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds
2026-07-27 15:05 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2026-07-27 15:05 ` [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors " Francesco Magazzu
@ 2026-07-27 15:05 ` Francesco Magazzu
1 sibling, 0 replies; 5+ messages in thread
From: Francesco Magazzu @ 2026-07-27 15:05 UTC (permalink / raw)
To: dri-devel; +Cc: nouveau
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 related [flat|nested] 5+ messages in thread
* [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog
@ 2026-07-27 15:28 Francesco Magazzu
2026-07-27 15:28 ` Francesco Magazzu
0 siblings, 1 reply; 5+ 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
Follow-up to "[PATCH v2 0/4] drm/nouveau: fix list cursor use after loop
in the clk pstate paths" (20260712123616.1180830-1-postadelmaga@gmail.com).
Sashiko AI review flagged three pre-existing issues while reviewing patch
2/4 of that series; the third (clk->func->prog()'s error being clobbered
by nvkm_volt_set_id() in nvkm_cstate_prog()) is already fixed by patch 4/4
of that series. These two patches address the other two:
1. nvkm_pstate_prog() drops the RAM reclock error and falls through to
core clock programming anyway.
2. nvkm_pstate_prog() commits clk->pstate to the new state before the
hardware transition has actually succeeded.
Francesco Magazzu (2):
drm/nouveau/clk: don't ignore RAM clock programming errors in
nvkm_pstate_prog
drm/nouveau/clk: only update clk->pstate after hardware programming
succeeds
nvkm/subdev/clk/base.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [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
0 siblings, 0 replies; 5+ 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
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 related [flat|nested] 5+ messages in thread
* [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors in nvkm_pstate_prog
@ 2026-07-27 15:28 ` Francesco Magazzu
0 siblings, 0 replies; 5+ 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 related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-27 15:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 15:05 [PATCH 0/2] drm/nouveau/clk: fix RAM error handling and pstate desync in nvkm_pstate_prog Francesco Magazzu
2026-07-27 15:05 ` [PATCH 1/2] drm/nouveau/clk: don't ignore RAM clock programming errors " Francesco Magazzu
2026-07-27 15:05 ` [PATCH 2/2] drm/nouveau/clk: only update clk->pstate after hardware programming succeeds Francesco Magazzu
-- strict thread matches above, loose matches on Subject: below --
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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.