* [PATCH v4 0/3] drm/nouveau: GSP runtime PM fixes
@ 2026-07-27 17:47 ` Lyude Paul
0 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: Dave Airlie, Andy Shevchenko, Maarten Lankhorst, Ben Skeggs,
Kees Cook, Simona Vetter, Maxime Ripard, Danilo Krummrich
Runtime suspend/resume has been notoriously flakey since we started
using the R570 GSP firmware, and possibly was also flakey for some time
when we introduced GSP support initially.
These fixes get runtime PM to be dramatically more stable then it was
before. Unfortunately even with this patch series, there are still some
remaining issues that need to be fixed. These likely stem from the
currently different code path that we need to take with these fixes in
order to get suspend/resume working.
While we're making progress in understanding what exactly we're doing
differently from OpenRM that makes this the case, it will likely take us
a while before we actually have things working well enough to copy what
OpenRM does. Taking precisely the same path that OpenRM takes however,
will require some additional work - and at the very least these fixes do
appear to get suspend/resume and runtime PM working the vast majority of
the time.
Thanks for nvidia for providing context into what all of these various
flags and settings do!
Previous version of this patch series:
https://patchwork.freedesktop.org/series/169457/
Lyude Paul (3):
Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"
drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
drm/nouveau/gsp/r570: Enable S/R Display workaround in GSP
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 8 ++++----
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 3 ++-
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h | 8 ++++++++
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
6 files changed, 17 insertions(+), 8 deletions(-)
base-commit: df0311845915442ab75d03ee89fb443b444c1370
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/3] Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"
2026-07-27 17:47 ` Lyude Paul
@ 2026-07-27 17:47 ` Lyude Paul
-1 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Timur Tabi, Dave Airlie, Andy Shevchenko,
Maarten Lankhorst, Ben Skeggs, Kees Cook, Simona Vetter,
David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
Danilo Krummrich, Lyude Paul
This reverts commit 8302d0afeaec0bc57d951dd085e0cffe997d4d18.
It turns out this looked like the right fix on some systems, but it's not -
as this causes runtime PM to actually fail on many a laptop.
Fixes: 8302d0afeaec ("nouveau/gsp: fix suspend/resume regression on r570 firmware")
Cc: <stable@vger.kernel.org> # v6.19+
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 8 ++++----
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
index f128330f30d7b..40bf83ea33ac0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
@@ -208,7 +208,7 @@ r535_fbsr_resume(struct nvkm_gsp *gsp)
}
static int
-r535_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
+r535_fbsr_suspend(struct nvkm_gsp *gsp)
{
struct nvkm_subdev *subdev = &gsp->subdev;
struct nvkm_device *device = subdev->device;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
index f544afa12b6bb..4a3b771ded255 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
@@ -1749,7 +1749,7 @@ r535_gsp_fini(struct nvkm_gsp *gsp, enum nvkm_suspend_state suspend)
sr->sysmemAddrOfSuspendResumeData = gsp->sr.radix3.lvl0.addr;
sr->sizeOfSuspendResumeData = len;
- ret = rm->api->fbsr->suspend(gsp, suspend == NVKM_RUNTIME_SUSPEND);
+ ret = rm->api->fbsr->suspend(gsp);
if (ret) {
nvkm_gsp_mem_dtor(&gsp->sr.meta);
nvkm_gsp_radix3_dtor(gsp, &gsp->sr.radix3);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
index 8ef8b4f655883..2945d5b4e5707 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
@@ -62,7 +62,7 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
}
static int
-r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtime)
+r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
{
NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS *ctrl;
struct nvkm_gsp_object memlist;
@@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
ctrl->hClient = gsp->internal.client.object.handle;
ctrl->hSysMem = memlist.handle;
ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
- ctrl->bEnteringGcoffState = runtime ? 1 : 0;
+ ctrl->bEnteringGcoffState = 1;
ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
if (ret)
@@ -92,7 +92,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
}
static int
-r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
+r570_fbsr_suspend(struct nvkm_gsp *gsp)
{
struct nvkm_subdev *subdev = &gsp->subdev;
struct nvkm_device *device = subdev->device;
@@ -133,7 +133,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
return ret;
/* Initialise FBSR on RM. */
- ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size, runtime);
+ ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size);
if (ret) {
nvkm_gsp_sg_free(device, &gsp->sr.fbsr);
return ret;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
index a9af94adf9efc..0fb0e67406c67 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -78,7 +78,7 @@ struct nvkm_rm_api {
} *device;
const struct nvkm_rm_api_fbsr {
- int (*suspend)(struct nvkm_gsp *, bool runtime);
+ int (*suspend)(struct nvkm_gsp *);
void (*resume)(struct nvkm_gsp *);
} *fbsr;
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 1/3] Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"
@ 2026-07-27 17:47 ` Lyude Paul
0 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Dave Airlie, Andy Shevchenko, Maarten Lankhorst,
Ben Skeggs, Kees Cook, Simona Vetter, Maxime Ripard,
Danilo Krummrich
This reverts commit 8302d0afeaec0bc57d951dd085e0cffe997d4d18.
It turns out this looked like the right fix on some systems, but it's not -
as this causes runtime PM to actually fail on many a laptop.
Fixes: 8302d0afeaec ("nouveau/gsp: fix suspend/resume regression on r570 firmware")
Cc: <stable@vger.kernel.org> # v6.19+
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 8 ++++----
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
index f128330f30d7b..40bf83ea33ac0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/fbsr.c
@@ -208,7 +208,7 @@ r535_fbsr_resume(struct nvkm_gsp *gsp)
}
static int
-r535_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
+r535_fbsr_suspend(struct nvkm_gsp *gsp)
{
struct nvkm_subdev *subdev = &gsp->subdev;
struct nvkm_device *device = subdev->device;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
index f544afa12b6bb..4a3b771ded255 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c
@@ -1749,7 +1749,7 @@ r535_gsp_fini(struct nvkm_gsp *gsp, enum nvkm_suspend_state suspend)
sr->sysmemAddrOfSuspendResumeData = gsp->sr.radix3.lvl0.addr;
sr->sizeOfSuspendResumeData = len;
- ret = rm->api->fbsr->suspend(gsp, suspend == NVKM_RUNTIME_SUSPEND);
+ ret = rm->api->fbsr->suspend(gsp);
if (ret) {
nvkm_gsp_mem_dtor(&gsp->sr.meta);
nvkm_gsp_radix3_dtor(gsp, &gsp->sr.radix3);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
index 8ef8b4f655883..2945d5b4e5707 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
@@ -62,7 +62,7 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
}
static int
-r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtime)
+r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
{
NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS *ctrl;
struct nvkm_gsp_object memlist;
@@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
ctrl->hClient = gsp->internal.client.object.handle;
ctrl->hSysMem = memlist.handle;
ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
- ctrl->bEnteringGcoffState = runtime ? 1 : 0;
+ ctrl->bEnteringGcoffState = 1;
ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
if (ret)
@@ -92,7 +92,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
}
static int
-r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
+r570_fbsr_suspend(struct nvkm_gsp *gsp)
{
struct nvkm_subdev *subdev = &gsp->subdev;
struct nvkm_device *device = subdev->device;
@@ -133,7 +133,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
return ret;
/* Initialise FBSR on RM. */
- ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size, runtime);
+ ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size);
if (ret) {
nvkm_gsp_sg_free(device, &gsp->sr.fbsr);
return ret;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
index a9af94adf9efc..0fb0e67406c67 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -78,7 +78,7 @@ struct nvkm_rm_api {
} *device;
const struct nvkm_rm_api_fbsr {
- int (*suspend)(struct nvkm_gsp *, bool runtime);
+ int (*suspend)(struct nvkm_gsp *);
void (*resume)(struct nvkm_gsp *);
} *fbsr;
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 1/3] Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"
2026-07-27 17:47 ` Lyude Paul
(?)
@ 2026-07-27 18:03 ` sashiko-bot
-1 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-27 18:03 UTC (permalink / raw)
To: Lyude Paul; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] GSP firmware memory allocation (memlist) is permanently leaked on error paths in r570_fbsr_init() and fbsr_init().
- [High] Incomplete error rollback in r570_fbsr_suspend() leaves hardware disabled and leaks system memory.
--
--- Patch [1]: [PATCH v4 1/3] Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware" ---
commit e90949dff871229c99ce748acd0012d5f702035b
Author: Lyude Paul <lyude@redhat.com>
Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"
This reverts commit 8302d0afeaec0bc57d951dd085e0cffe997d4d18.
It turns out this looked like the right fix on some systems, but it's not -
as this causes runtime PM to actually fail on many a laptop.
Cc: <stable@vger.kernel.org> # v6.19+
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> index 8ef8b4f655883..2945d5b4e5707 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> @@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
> ctrl->hClient = gsp->internal.client.object.handle;
> ctrl->hSysMem = memlist.handle;
> ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
> - ctrl->bEnteringGcoffState = runtime ? 1 : 0;
> + ctrl->bEnteringGcoffState = 1;
>
> ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
> if (ret)
[Severity: High]
This isn't a bug introduced by this patch, but does this error path permanently
leak the memlist object?
In r570_fbsr_init(), if the allocation of the control structure fails:
ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.subdevice,
NV2080_CTRL_CMD_INTERNAL_FBSR_INIT, sizeof(*ctrl));
if (IS_ERR(ctrl))
return PTR_ERR(ctrl);
The function returns directly, leaving the memlist object allocated on the GSP
firmware with a hardcoded handle. Because the handle is hardcoded, would a
subsequent suspend attempt fail with a duplicate handle error, permanently
breaking system suspend until the driver is reloaded?
(A similar pattern also appears to exist in fbsr_init()).
[ ... ]
> @@ -133,7 +133,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
> return ret;
>
> /* Initialise FBSR on RM. */
> - ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size, runtime);
> + ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size);
> if (ret) {
> nvkm_gsp_sg_free(device, &gsp->sr.fbsr);
> return ret;
> }
[Severity: High]
This is a pre-existing issue, but does this error path leave the GPU in an
unusable state and leak system memory?
If r570_fbsr_init() fails, r570_fbsr_suspend() returns the error without
rolling back prior operations:
/* Initialise FBSR on RM. */
ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size);
if (ret) {
nvkm_gsp_sg_free(device, &gsp->sr.fbsr);
return ret;
}
Since PM core aborts suspend and doesn't call resume on failure, does this
leave the GPU unresponsive with channel scheduling suspended and BAR2 disabled?
Additionally, earlier in the same function:
/* Save BAR2 allocations to system memory. */
list_for_each_entry(iobj, &imem->list, head) {
if (iobj->preserve) {
ret = nvkm_instobj_save(iobj);
if (ret)
return ret;
}
}
Would failing here (or later) permanently leak the memory allocated by
nvkm_instobj_save(), since it's only freed during a successful resume?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260727175246.642071-1-lyude@redhat.com?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
2026-07-27 17:47 ` Lyude Paul
@ 2026-07-27 17:47 ` Lyude Paul
-1 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Timur Tabi, Dave Airlie, Andy Shevchenko,
Maarten Lankhorst, Ben Skeggs, Kees Cook, Simona Vetter,
David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
Danilo Krummrich, Lyude Paul
Previously, it looked as if we were able to fix suspend/resume on some
desktops by setting Gcoff based on whether or not we were entering runtime
PM. This was a mistake though - the only time suspend/resume would end up
actually working was if Gcoff = 0.
It seems like it's likely the main reason for this is the FBSR GcOff
argument actually controls GSP's behavior with regards to which buffers it
decides to save across suspend/resume. When GcOff = 1, RM reserved regions
are saved unless they are marked as LOST_ON_SUSPEND, and RM channel-context
and kernel-client buffers are also saved -including- when they are
LOST_ON_SUSPEND. This means with GcOff = 1, we end up having GSP save and
restore buffers that actually need to be reinitialized on resume - causing
the failures we're setting.
In the long term, we do want to have GcOff = 1 as there are still a handful
of edge cases around broken runtime PM that I've seen on laptops - and I
have a strong guess that actually properly implementing the required bits
for enabling GcOff will probably address a number of those issues. For now
though, disabling GcOff results in runtime PM being dramatically more
stable then it was before.
Thanks to John Hubbard from Nvidia for providing some background on what
these options do in the GSP firmware do!
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
Cc: <stable@vger.kernel.org> # v6.16+
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
index 2945d5b4e5707..af5aa5065c3dd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
@@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
ctrl->hClient = gsp->internal.client.object.handle;
ctrl->hSysMem = memlist.handle;
ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
- ctrl->bEnteringGcoffState = 1;
+ ctrl->bEnteringGcoffState = 0;
ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
if (ret)
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
@ 2026-07-27 17:47 ` Lyude Paul
0 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Dave Airlie, Andy Shevchenko, Maarten Lankhorst,
Ben Skeggs, Kees Cook, Simona Vetter, Maxime Ripard,
Danilo Krummrich
Previously, it looked as if we were able to fix suspend/resume on some
desktops by setting Gcoff based on whether or not we were entering runtime
PM. This was a mistake though - the only time suspend/resume would end up
actually working was if Gcoff = 0.
It seems like it's likely the main reason for this is the FBSR GcOff
argument actually controls GSP's behavior with regards to which buffers it
decides to save across suspend/resume. When GcOff = 1, RM reserved regions
are saved unless they are marked as LOST_ON_SUSPEND, and RM channel-context
and kernel-client buffers are also saved -including- when they are
LOST_ON_SUSPEND. This means with GcOff = 1, we end up having GSP save and
restore buffers that actually need to be reinitialized on resume - causing
the failures we're setting.
In the long term, we do want to have GcOff = 1 as there are still a handful
of edge cases around broken runtime PM that I've seen on laptops - and I
have a strong guess that actually properly implementing the required bits
for enabling GcOff will probably address a number of those issues. For now
though, disabling GcOff results in runtime PM being dramatically more
stable then it was before.
Thanks to John Hubbard from Nvidia for providing some background on what
these options do in the GSP firmware do!
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
Cc: <stable@vger.kernel.org> # v6.16+
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
index 2945d5b4e5707..af5aa5065c3dd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
@@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
ctrl->hClient = gsp->internal.client.object.handle;
ctrl->hSysMem = memlist.handle;
ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
- ctrl->bEnteringGcoffState = 1;
+ ctrl->bEnteringGcoffState = 0;
ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
if (ret)
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
2026-07-27 17:47 ` Lyude Paul
@ 2026-07-27 17:54 ` lyude
-1 siblings, 0 replies; 12+ messages in thread
From: lyude @ 2026-07-27 17:54 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Dave Airlie, Andy Shevchenko, Maarten Lankhorst,
Ben Skeggs, Kees Cook, Simona Vetter, Maxime Ripard,
Danilo Krummrich
aaaaand I completely forgot to change this from a 1 to 0 in the patch
title ._.
Reviewers - I will make sure this gets fixed before pushing it upstream
On Mon, 2026-07-27 at 13:47 -0400, Lyude Paul wrote:
> Previously, it looked as if we were able to fix suspend/resume on
> some
> desktops by setting Gcoff based on whether or not we were entering
> runtime
> PM. This was a mistake though - the only time suspend/resume would
> end up
> actually working was if Gcoff = 0.
>
> It seems like it's likely the main reason for this is the FBSR GcOff
> argument actually controls GSP's behavior with regards to which
> buffers it
> decides to save across suspend/resume. When GcOff = 1, RM reserved
> regions
> are saved unless they are marked as LOST_ON_SUSPEND, and RM channel-
> context
> and kernel-client buffers are also saved -including- when they are
> LOST_ON_SUSPEND. This means with GcOff = 1, we end up having GSP save
> and
> restore buffers that actually need to be reinitialized on resume -
> causing
> the failures we're setting.
>
> In the long term, we do want to have GcOff = 1 as there are still a
> handful
> of edge cases around broken runtime PM that I've seen on laptops -
> and I
> have a strong guess that actually properly implementing the required
> bits
> for enabling GcOff will probably address a number of those issues.
> For now
> though, disabling GcOff results in runtime PM being dramatically more
> stable then it was before.
>
> Thanks to John Hubbard from Nvidia for providing some background on
> what
> these options do in the GSP firmware do!
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
> Cc: <stable@vger.kernel.org> # v6.16+
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> index 2945d5b4e5707..af5aa5065c3dd 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> @@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct
> sg_table *sgt, u64 size)
> ctrl->hClient = gsp->internal.client.object.handle;
> ctrl->hSysMem = memlist.handle;
> ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
> - ctrl->bEnteringGcoffState = 1;
> + ctrl->bEnteringGcoffState = 0;
>
> ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice,
> ctrl);
> if (ret)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
@ 2026-07-27 17:54 ` lyude
0 siblings, 0 replies; 12+ messages in thread
From: lyude @ 2026-07-27 17:54 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Timur Tabi, Dave Airlie, Andy Shevchenko,
Maarten Lankhorst, Ben Skeggs, Kees Cook, Simona Vetter,
David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
Danilo Krummrich
aaaaand I completely forgot to change this from a 1 to 0 in the patch
title ._.
Reviewers - I will make sure this gets fixed before pushing it upstream
On Mon, 2026-07-27 at 13:47 -0400, Lyude Paul wrote:
> Previously, it looked as if we were able to fix suspend/resume on
> some
> desktops by setting Gcoff based on whether or not we were entering
> runtime
> PM. This was a mistake though - the only time suspend/resume would
> end up
> actually working was if Gcoff = 0.
>
> It seems like it's likely the main reason for this is the FBSR GcOff
> argument actually controls GSP's behavior with regards to which
> buffers it
> decides to save across suspend/resume. When GcOff = 1, RM reserved
> regions
> are saved unless they are marked as LOST_ON_SUSPEND, and RM channel-
> context
> and kernel-client buffers are also saved -including- when they are
> LOST_ON_SUSPEND. This means with GcOff = 1, we end up having GSP save
> and
> restore buffers that actually need to be reinitialized on resume -
> causing
> the failures we're setting.
>
> In the long term, we do want to have GcOff = 1 as there are still a
> handful
> of edge cases around broken runtime PM that I've seen on laptops -
> and I
> have a strong guess that actually properly implementing the required
> bits
> for enabling GcOff will probably address a number of those issues.
> For now
> though, disabling GcOff results in runtime PM being dramatically more
> stable then it was before.
>
> Thanks to John Hubbard from Nvidia for providing some background on
> what
> these options do in the GSP firmware do!
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
> Cc: <stable@vger.kernel.org> # v6.16+
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> index 2945d5b4e5707..af5aa5065c3dd 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> @@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct
> sg_table *sgt, u64 size)
> ctrl->hClient = gsp->internal.client.object.handle;
> ctrl->hSysMem = memlist.handle;
> ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
> - ctrl->bEnteringGcoffState = 1;
> + ctrl->bEnteringGcoffState = 0;
>
> ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice,
> ctrl);
> if (ret)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
2026-07-27 17:47 ` Lyude Paul
(?)
(?)
@ 2026-07-27 18:02 ` sashiko-bot
-1 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-27 18:02 UTC (permalink / raw)
To: Lyude Paul; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit title states 'Set GcOff = 1' but the patch implementation sets it to 0.
--
--- Patch [2]: [PATCH v4 2/3] drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr ---
commit d069cc762363e23906149f0e5723b3c8cac473a5
Author: Lyude Paul <lyude@redhat.com>
drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
This commit modifies the GcOff state setting during FBSR initialization.
It disables GcOff to prevent GSP from saving and restoring buffers that
need to be reinitialized on resume, working around runtime PM issues.
> drm/nouveau/gsp/r570: Set GcOff = 1 in fbsr
[Severity: Medium]
Is the commit title incorrect? The commit message explains that disabling
GcOff improves stability, and the code in r570_fbsr_init() explicitly sets
bEnteringGcoffState to 0 instead of 1.
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
[ ... ]
> @@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
> ctrl->hClient = gsp->internal.client.object.handle;
> ctrl->hSysMem = memlist.handle;
> ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
> - ctrl->bEnteringGcoffState = 1;
> + ctrl->bEnteringGcoffState = 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260727175246.642071-1-lyude@redhat.com?part=2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/3] drm/nouveau/gsp/r570: Enable S/R Display workaround in GSP
2026-07-27 17:47 ` Lyude Paul
@ 2026-07-27 17:47 ` Lyude Paul
-1 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Timur Tabi, Dave Airlie, Andy Shevchenko,
Maarten Lankhorst, Ben Skeggs, Kees Cook, Simona Vetter,
David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
Danilo Krummrich, Lyude Paul
There's two flags that we've never been setting when asking GSP to suspend
the GPU, which OpenRM does set:
GPU_STATE_FLAGS_PRESERVING
GPU_STATE_FLAGS_PM_TRANSITION
These flags aren't -supposed- to do much in GSP, they're mostly used by
OpenRM itself for state tracking. The only thing they do from GSP's side is
control whether or not a single display related workaround is applied
during suspend.
But as it turns out, that single workaround is actually quite crucial for
getting runtime PM working with nouveau - and without it set we end up
seeing a lot more failures with runtime PM resume. So, let's start setting
it.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
Cc: <stable@vger.kernel.org> # v6.16+
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 3 ++-
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
index 996941c668ba9..3e391646d8f7d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
@@ -198,7 +198,8 @@ r570_gsp_set_rmargs(struct nvkm_gsp *gsp, bool resume)
args->srInitArguments.bInPMTransition = 0;
} else {
args->srInitArguments.oldLevel = NV2080_CTRL_GPU_SET_POWER_STATE_GPU_LEVEL_3;
- args->srInitArguments.flags = 0;
+ args->srInitArguments.flags =
+ GPU_STATE_FLAGS_PRESERVING | GPU_STATE_FLAGS_PM_TRANSITION;
args->srInitArguments.bInPMTransition = 1;
}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
index b6075021e74f5..c458569af9d72 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
@@ -523,6 +523,14 @@ typedef struct
#define NV2080_CTRL_GPU_SET_POWER_STATE_GPU_LEVEL_3 (0x00000003U)
+#define GPU_STATE_FLAGS_PRESERVING BIT(0) // GPU state is preserved
+#define GPU_STATE_FLAGS_VGA_TRANSITION BIT(1) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_PM_TRANSITION BIT(2) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_PM_SUSPEND BIT(3)
+#define GPU_STATE_FLAGS_PM_HIBERNATE BIT(4)
+#define GPU_STATE_FLAGS_GC6_TRANSITION BIT(5) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_FAST_UNLOAD BIT(6) // Used during windows restart, skips stateDestroy steps
+
typedef struct
{
// Magic for verification by secure ucode
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 3/3] drm/nouveau/gsp/r570: Enable S/R Display workaround in GSP
@ 2026-07-27 17:47 ` Lyude Paul
0 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2026-07-27 17:47 UTC (permalink / raw)
To: dri-devel, nouveau, linux-kernel
Cc: stable, Dave Airlie, Andy Shevchenko, Maarten Lankhorst,
Ben Skeggs, Kees Cook, Simona Vetter, Maxime Ripard,
Danilo Krummrich
There's two flags that we've never been setting when asking GSP to suspend
the GPU, which OpenRM does set:
GPU_STATE_FLAGS_PRESERVING
GPU_STATE_FLAGS_PM_TRANSITION
These flags aren't -supposed- to do much in GSP, they're mostly used by
OpenRM itself for state tracking. The only thing they do from GSP's side is
control whether or not a single display related workaround is applied
during suspend.
But as it turns out, that single workaround is actually quite crucial for
getting runtime PM working with nouveau - and without it set we end up
seeing a lot more failures with runtime PM resume. So, let's start setting
it.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 53dac0623853 ("drm/nouveau/gsp: add support for 570.144")
Cc: <stable@vger.kernel.org> # v6.16+
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 3 ++-
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
index 996941c668ba9..3e391646d8f7d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c
@@ -198,7 +198,8 @@ r570_gsp_set_rmargs(struct nvkm_gsp *gsp, bool resume)
args->srInitArguments.bInPMTransition = 0;
} else {
args->srInitArguments.oldLevel = NV2080_CTRL_GPU_SET_POWER_STATE_GPU_LEVEL_3;
- args->srInitArguments.flags = 0;
+ args->srInitArguments.flags =
+ GPU_STATE_FLAGS_PRESERVING | GPU_STATE_FLAGS_PM_TRANSITION;
args->srInitArguments.bInPMTransition = 1;
}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
index b6075021e74f5..c458569af9d72 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gsp.h
@@ -523,6 +523,14 @@ typedef struct
#define NV2080_CTRL_GPU_SET_POWER_STATE_GPU_LEVEL_3 (0x00000003U)
+#define GPU_STATE_FLAGS_PRESERVING BIT(0) // GPU state is preserved
+#define GPU_STATE_FLAGS_VGA_TRANSITION BIT(1) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_PM_TRANSITION BIT(2) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_PM_SUSPEND BIT(3)
+#define GPU_STATE_FLAGS_PM_HIBERNATE BIT(4)
+#define GPU_STATE_FLAGS_GC6_TRANSITION BIT(5) // To be used with GPU_STATE_FLAGS_PRESERVING.
+#define GPU_STATE_FLAGS_FAST_UNLOAD BIT(6) // Used during windows restart, skips stateDestroy steps
+
typedef struct
{
// Magic for verification by secure ucode
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread