[PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely
@ 2026-09-16 22:25 ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

While we have runtime PM working a lot more reliably then in the past
with R570, we're still doing things a lot differently then OpenRM. For
instance, we don't save/restore comptag backing stores, and we don't ask
GSP whether it's OK to enter GcOff.

This series fixes this, and gets our runtime PM process to look a lot
closer to OpenRM's by implementing some missing pieces and workarounds.

Tested on my daily-driver desktop with an AD102, a ThinkPad P17 Gen1
with a GA103GLM, a Dell Precision 7780 with an AD103, and my test
machine with a GB206.

Patch series based on drm-misc/drm-misc-next. Branch with patches here:

	https://gitlab.freedesktop.org/lyudess/linux/-/commits/nouveau/rpm-openrm-sync

Previous version:

	https://patchwork.freedesktop.org/series/173901/

Lyude Paul (5):
  drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
  drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
  drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
  drm/nouveau/gsp/r570: Start saving comptag backing stores
  drm/nouveau/gsp/r570: Enable Gcoff in fbsr again

 drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 +++
 drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  6 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 ++++
 drivers/gpu/drm/nouveau/nvif/device.c         | 13 ++++
 .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 +++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 ++++
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 52 +++++++++++++
 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 77 ++++++++++++++++++-
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 36 +++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 29 +++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  3 +
 15 files changed, 308 insertions(+), 2 deletions(-)


base-commit: 5589ef09f93b85b6f506e279a502d4cf1654f232
-- 
2.55.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely
@ 2026-09-16 22:25 ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

While we have runtime PM working a lot more reliably then in the past
with R570, we're still doing things a lot differently then OpenRM. For
instance, we don't save/restore comptag backing stores, and we don't ask
GSP whether it's OK to enter GcOff.

This series fixes this, and gets our runtime PM process to look a lot
closer to OpenRM's by implementing some missing pieces and workarounds.

Tested on my daily-driver desktop with an AD102, a ThinkPad P17 Gen1
with a GA103GLM, a Dell Precision 7780 with an AD103, and my test
machine with a GB206.

Patch series based on drm-misc/drm-misc-next. Branch with patches here:

	https://gitlab.freedesktop.org/lyudess/linux/-/commits/nouveau/rpm-openrm-sync

Previous version:

	https://patchwork.freedesktop.org/series/173901/

Lyude Paul (5):
  drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
  drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
  drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
  drm/nouveau/gsp/r570: Start saving comptag backing stores
  drm/nouveau/gsp/r570: Enable Gcoff in fbsr again

 drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 +++
 drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  6 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 ++++
 drivers/gpu/drm/nouveau/nvif/device.c         | 13 ++++
 .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 +++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 ++++
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 52 +++++++++++++
 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 77 ++++++++++++++++++-
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 36 +++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 29 +++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  3 +
 15 files changed, 308 insertions(+), 2 deletions(-)


base-commit: 5589ef09f93b85b6f506e279a502d4cf1654f232
-- 
2.55.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 22:25   ` Lyude Paul
  -1 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

OpenRM's runtime PM handling looks a bit different then nouveau's, one part
in particular that differs from us: OpenRM actually consults GSP to ask
whether the GPU should be allowed to enter Gc6 and/or GcOff before runtime
suspending the GPU. In the event the card isn't ready, runtime suspend is
simply delayed for a few seconds before retrying.

Implement the command used for querying GSP about this,
NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE, and check to ensure that
the GPU is ready for runtime suspend in nouveau_pmops_runtime_suspend()
using this query. If the GPU can't be runtime suspended, update the last
busy counter of the device and then return -EBUSY from
nouveau_pmops_runtime_suspend() - essentially delaying the runtime suspend
process by whatever autosuspend_delay_ms is set to.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Remove accidental return of bitmask from nvkm_udevice_gcx_ready()
* Use gsp variable in nvkm_udevice_gcx_ready() more.
* Remove unused GSP_GCX_ENTRY defines
V3:
* Fix error checking
V4:
* Fix layering violation
* Remove structs from r535, this hadn't been added yet!

 drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 ++++++
 drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 +++++++
 drivers/gpu/drm/nouveau/nvif/device.c         | 13 +++++++
 .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++++++++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 ++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        |  7 ++++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 28 +++++++++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  2 ++
 12 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
index ea8267e0d8da2..9e639df1da467 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
@@ -4,6 +4,7 @@
 
 #define NV_DEVICE_V0_INFO                                                  0x00
 #define NV_DEVICE_V0_TIME                                                  0x01
+#define NV_DEVICE_V0_GCX_READY						   0x02
 
 struct nv_device_info_v0 {
 	__u8  version;
@@ -55,6 +56,15 @@ struct nv_device_time_v0 {
 	__u64 time;
 };
 
+#define NV_DEVICE_GC6_READY	BIT(0)
+#define NV_DEVICE_GCOFF_READY	BIT(1)
+
+struct nv_device_gcx_ready_v0 {
+	__u8 version;
+	__u8 pad01[6];
+	__u8 ready;
+};
+
 #define NV_DEVICE_INFO_UNIT                               (0xffffffffULL << 32)
 #define NV_DEVICE_INFO(n)                          ((n) | (0x00000000ULL << 32))
 #define NV_DEVICE_HOST(n)                          ((n) | (0x00000001ULL << 32))
diff --git a/drivers/gpu/drm/nouveau/include/nvif/device.h b/drivers/gpu/drm/nouveau/include/nvif/device.h
index 7877a2a79da9b..ce2fadcb05d9e 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/device.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/device.h
@@ -22,4 +22,5 @@ int  nvif_device_ctor(struct nvif_client *, const char *name, struct nvif_device
 void nvif_device_dtor(struct nvif_device *);
 int  nvif_device_map(struct nvif_device *);
 u64  nvif_device_time(struct nvif_device *);
+int  nvif_device_gcx_ready(struct nvif_device *);
 #endif
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 64fed208e4cf8..cd10c37053695 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -495,6 +495,8 @@ nvkm_gsp_event_dtor(struct nvkm_gsp_event *event)
 int nvkm_gsp_intr_stall(struct nvkm_gsp *, enum nvkm_subdev_type, int);
 int nvkm_gsp_intr_nonstall(struct nvkm_gsp *, enum nvkm_subdev_type, int);
 
+int nvkm_gsp_gcx_ready(struct nvkm_gsp *gsp);
+
 int gv100_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
 int tu102_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
 int tu116_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index b0f9fb10a74dd..c2e2f430deded 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1157,6 +1157,7 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct nouveau_drm *drm = pci_get_drvdata(pdev);
+	struct nvif_device *nvif = &drm->client.device;
 	int ret;
 
 	if (!nouveau_pmops_runtime()) {
@@ -1164,6 +1165,18 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 		return -EBUSY;
 	}
 
+	// Check if the GPU itself is ready for runtime suspend, otherwise mark as busy and check
+	// again in a bit.
+	ret = nvif_device_gcx_ready(nvif);
+	if (ret < 0) {
+		NV_ERROR(drm, "Failed to query GCX readiness (returned %d)\n", ret);
+		return -EBUSY;
+	} else if (!(ret & NV_DEVICE_GCOFF_READY)) {
+		NV_DEBUG(drm, "GPU isn't ready for suspend yet, delaying...\n");
+		pm_runtime_mark_last_busy(dev);
+		return -EBUSY;
+	}
+
 	nouveau_switcheroo_optimus_dsm();
 	ret = nouveau_do_suspend(drm, true);
 	pci_save_state(pdev);
diff --git a/drivers/gpu/drm/nouveau/nvif/device.c b/drivers/gpu/drm/nouveau/nvif/device.c
index 24880931039f6..1be9fbe6cb703 100644
--- a/drivers/gpu/drm/nouveau/nvif/device.c
+++ b/drivers/gpu/drm/nouveau/nvif/device.c
@@ -38,6 +38,19 @@ nvif_device_time(struct nvif_device *device)
 	return device->user.func->time(&device->user);
 }
 
+int
+nvif_device_gcx_ready(struct nvif_device *device)
+{
+	struct nv_device_gcx_ready_v0 args = {};
+	int ret;
+
+	ret = nvif_object_mthd(&device->object, NV_DEVICE_V0_GCX_READY, &args, sizeof(args));
+	if (ret)
+		return ret;
+
+	return args.ready;
+}
+
 int
 nvif_device_map(struct nvif_device *device)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
index 23d11d8221cb6..f78e6b9b42923 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
@@ -27,6 +27,7 @@
 
 #include <core/client.h>
 #include <subdev/fb.h>
+#include <subdev/gsp.h>
 #include <subdev/instmem.h>
 #include <subdev/timer.h>
 
@@ -189,6 +190,38 @@ nvkm_udevice_time(struct nvkm_udevice *udev, void *data, u32 size)
 	return ret;
 }
 
+static int
+nvkm_udevice_gcx_ready(struct nvkm_udevice *udev, void *data, u32 size)
+{
+	struct nvkm_object *object = &udev->object;
+	struct nvkm_device *device = udev->device;
+	struct nvkm_gsp *gsp = device->gsp;
+	union {
+		struct nv_device_gcx_ready_v0 v0;
+	} *args = data;
+	int ret = -ENOSYS;
+
+	if (!gsp) {
+		args->v0.ready = NV_DEVICE_GC6_READY | NV_DEVICE_GCOFF_READY;
+		return 0;
+	}
+
+	nvif_ioctl(object, "device gcx ready size %d\n", size);
+	ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, false);
+	if (!ret) {
+		nvif_ioctl(object, "device gcx ready vers %d\n", args->v0.version);
+
+		ret = nvkm_gsp_gcx_ready(gsp);
+		if (ret < 0)
+			return ret;
+
+		args->v0.ready = ret;
+		ret = 0;
+	}
+
+	return ret;
+}
+
 static int
 nvkm_udevice_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size)
 {
@@ -199,6 +232,8 @@ nvkm_udevice_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size)
 		return nvkm_udevice_info(udev, data, size);
 	case NV_DEVICE_V0_TIME:
 		return nvkm_udevice_time(udev, data, size);
+	case NV_DEVICE_V0_GCX_READY:
+		return nvkm_udevice_gcx_ready(udev, data, size);
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
index 9ba1316831e7e..e475d0e8fa7bf 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
@@ -20,6 +20,7 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 #include "priv.h"
+#include <nvif/cl0080.h>
 
 int
 nvkm_gsp_intr_nonstall(struct nvkm_gsp *gsp, enum nvkm_subdev_type type, int inst)
@@ -47,6 +48,15 @@ nvkm_gsp_intr_stall(struct nvkm_gsp *gsp, enum nvkm_subdev_type type, int inst)
 	return -ENOENT;
 }
 
+int
+nvkm_gsp_gcx_ready(struct nvkm_gsp *gsp)
+{
+	if (!gsp->rm->api->gsp->gcx_ready)
+		return NV_DEVICE_GC6_READY | NV_DEVICE_GCOFF_READY;
+
+	return gsp->rm->api->gsp->gcx_ready(gsp);
+}
+
 static int
 nvkm_gsp_fini(struct nvkm_subdev *subdev, enum nvkm_suspend_state suspend)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
index 71b7203bef507..b07797813b049 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
@@ -93,6 +93,7 @@ void r535_gsp_dtor(struct nvkm_gsp *);
 int r535_gsp_oneinit(struct nvkm_gsp *);
 int r535_gsp_init(struct nvkm_gsp *);
 int r535_gsp_fini(struct nvkm_gsp *, enum nvkm_suspend_state suspend);
+int r535_gsp_gcx_ready(struct nvkm_gsp *gsp);
 
 int nvkm_gsp_new_(const struct nvkm_gsp_fwif *, struct nvkm_device *, enum nvkm_subdev_type, int,
 		  struct nvkm_gsp **);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
index b6683a5bf870c..fd1170037c030 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
@@ -782,6 +782,13 @@ typedef struct NV2080_CTRL_INTERNAL_INTR_GET_KERNEL_TABLE_PARAMS {
 
 #define GSP_FW_HEAP_PARAM_CLIENT_ALLOC_SIZE      ((48 << 10) * 2048)   // Support 2048 channels
 
+#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
+
+typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
+    NvBool bIsGC6Satisfied;
+    NvBool bIsGCOFFSatisfied;
+} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
+
 typedef union rpc_message_rpc_union_field_v03_00
 {
     NvU32      spare;
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 b45781cd0dfdc..dfda6b0b910a2 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
@@ -10,6 +10,7 @@
 #include "nvrm/gsp.h"
 #include "nvrm/rpcfn.h"
 #include "nvrm/msgfn.h"
+#include "nvif/cl0080.h"
 
 #include <core/pci.h>
 #include <subdev/pci/priv.h>
@@ -215,6 +216,32 @@ r570_gsp_set_rmargs(struct nvkm_gsp *gsp, bool resume)
 	args->bDmemStack = 1;
 }
 
+int
+r570_gsp_gcx_ready(struct nvkm_gsp *gsp)
+{
+	NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS *ctrl;
+	int ret = 0;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+				   NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE,
+				   sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	if (ctrl->bIsGC6Satisfied)
+		ret |= NV_DEVICE_GC6_READY;
+	if (ctrl->bIsGCOFFSatisfied)
+		ret |= NV_DEVICE_GCOFF_READY;
+
+	nvkm_debug(&gsp->subdev,
+		   "GCX ready status: GC6=%s GCOFF=%s\n",
+		   str_yes_no(ctrl->bIsGC6Satisfied), str_yes_no(ctrl->bIsGCOFFSatisfied));
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.client.object, ctrl);
+	return ret;
+}
+
+
 const struct nvkm_rm_api_gsp
 r570_gsp = {
 	.set_rmargs = r570_gsp_set_rmargs,
@@ -223,4 +250,5 @@ r570_gsp = {
 	.xlat_mc_engine_idx = r570_gsp_xlat_mc_engine_idx,
 	.drop_post_nocat_record = r570_gsp_drop_post_nocat_record,
 	.sr_data_size = r570_gsp_sr_data_size,
+	.gcx_ready = r570_gsp_gcx_ready,
 };
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 c458569af9d72..2814629fddd22 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
@@ -639,4 +639,11 @@ typedef struct GSP_FMC_BOOT_PARAMS
 } GSP_FMC_BOOT_PARAMS;
 
 #define GSP_FW_HEAP_PARAM_BASE_RM_SIZE_GH100              (14 << 20)   // Hopper+
+
+#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
+
+typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
+    NvBool bIsGC6Satisfied;
+    NvBool bIsGCOFFSatisfied;
+} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
 #endif
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 e9ac47d86b69a..86970129ad96a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -40,6 +40,7 @@ struct nvkm_rm_api {
 		void (*drop_send_user_shared_data)(struct nvkm_gsp *);
 		void (*drop_post_nocat_record)(struct nvkm_gsp *);
 		u32 (*sr_data_size)(struct nvkm_gsp *);
+		int (*gcx_ready)(struct nvkm_gsp *gsp);
 	} *gsp;
 
 	const struct nvkm_rm_api_rpc {
@@ -174,6 +175,7 @@ int r535_gr_chan_new(struct nvkm_gr *, struct nvkm_chan *, const struct nvkm_ocl
 int r535_gr_promote_ctx(struct r535_gr *, bool golden, struct nvkm_vmm *,
 			struct nvkm_memory **pctxbuf_mem, struct nvkm_vma **pctxbuf_vma,
 			struct nvkm_gsp_object *chan);
+int r570_gsp_gcx_ready(struct nvkm_gsp *gsp);
 extern const struct nvkm_rm_api_engine r535_nvdec;
 extern const struct nvkm_rm_api_engine r535_nvenc;
 extern const struct nvkm_rm_api_engine r535_nvjpg;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
@ 2026-09-16 22:25   ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

OpenRM's runtime PM handling looks a bit different then nouveau's, one part
in particular that differs from us: OpenRM actually consults GSP to ask
whether the GPU should be allowed to enter Gc6 and/or GcOff before runtime
suspending the GPU. In the event the card isn't ready, runtime suspend is
simply delayed for a few seconds before retrying.

Implement the command used for querying GSP about this,
NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE, and check to ensure that
the GPU is ready for runtime suspend in nouveau_pmops_runtime_suspend()
using this query. If the GPU can't be runtime suspended, update the last
busy counter of the device and then return -EBUSY from
nouveau_pmops_runtime_suspend() - essentially delaying the runtime suspend
process by whatever autosuspend_delay_ms is set to.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Remove accidental return of bitmask from nvkm_udevice_gcx_ready()
* Use gsp variable in nvkm_udevice_gcx_ready() more.
* Remove unused GSP_GCX_ENTRY defines
V3:
* Fix error checking
V4:
* Fix layering violation
* Remove structs from r535, this hadn't been added yet!

 drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 ++++++
 drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  2 ++
 drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 +++++++
 drivers/gpu/drm/nouveau/nvif/device.c         | 13 +++++++
 .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++++++++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 ++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        |  7 ++++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 28 +++++++++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  2 ++
 12 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
index ea8267e0d8da2..9e639df1da467 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
@@ -4,6 +4,7 @@
 
 #define NV_DEVICE_V0_INFO                                                  0x00
 #define NV_DEVICE_V0_TIME                                                  0x01
+#define NV_DEVICE_V0_GCX_READY						   0x02
 
 struct nv_device_info_v0 {
 	__u8  version;
@@ -55,6 +56,15 @@ struct nv_device_time_v0 {
 	__u64 time;
 };
 
+#define NV_DEVICE_GC6_READY	BIT(0)
+#define NV_DEVICE_GCOFF_READY	BIT(1)
+
+struct nv_device_gcx_ready_v0 {
+	__u8 version;
+	__u8 pad01[6];
+	__u8 ready;
+};
+
 #define NV_DEVICE_INFO_UNIT                               (0xffffffffULL << 32)
 #define NV_DEVICE_INFO(n)                          ((n) | (0x00000000ULL << 32))
 #define NV_DEVICE_HOST(n)                          ((n) | (0x00000001ULL << 32))
diff --git a/drivers/gpu/drm/nouveau/include/nvif/device.h b/drivers/gpu/drm/nouveau/include/nvif/device.h
index 7877a2a79da9b..ce2fadcb05d9e 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/device.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/device.h
@@ -22,4 +22,5 @@ int  nvif_device_ctor(struct nvif_client *, const char *name, struct nvif_device
 void nvif_device_dtor(struct nvif_device *);
 int  nvif_device_map(struct nvif_device *);
 u64  nvif_device_time(struct nvif_device *);
+int  nvif_device_gcx_ready(struct nvif_device *);
 #endif
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 64fed208e4cf8..cd10c37053695 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -495,6 +495,8 @@ nvkm_gsp_event_dtor(struct nvkm_gsp_event *event)
 int nvkm_gsp_intr_stall(struct nvkm_gsp *, enum nvkm_subdev_type, int);
 int nvkm_gsp_intr_nonstall(struct nvkm_gsp *, enum nvkm_subdev_type, int);
 
+int nvkm_gsp_gcx_ready(struct nvkm_gsp *gsp);
+
 int gv100_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
 int tu102_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
 int tu116_gsp_new(struct nvkm_device *, enum nvkm_subdev_type, int, struct nvkm_gsp **);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index b0f9fb10a74dd..c2e2f430deded 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1157,6 +1157,7 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct nouveau_drm *drm = pci_get_drvdata(pdev);
+	struct nvif_device *nvif = &drm->client.device;
 	int ret;
 
 	if (!nouveau_pmops_runtime()) {
@@ -1164,6 +1165,18 @@ nouveau_pmops_runtime_suspend(struct device *dev)
 		return -EBUSY;
 	}
 
+	// Check if the GPU itself is ready for runtime suspend, otherwise mark as busy and check
+	// again in a bit.
+	ret = nvif_device_gcx_ready(nvif);
+	if (ret < 0) {
+		NV_ERROR(drm, "Failed to query GCX readiness (returned %d)\n", ret);
+		return -EBUSY;
+	} else if (!(ret & NV_DEVICE_GCOFF_READY)) {
+		NV_DEBUG(drm, "GPU isn't ready for suspend yet, delaying...\n");
+		pm_runtime_mark_last_busy(dev);
+		return -EBUSY;
+	}
+
 	nouveau_switcheroo_optimus_dsm();
 	ret = nouveau_do_suspend(drm, true);
 	pci_save_state(pdev);
diff --git a/drivers/gpu/drm/nouveau/nvif/device.c b/drivers/gpu/drm/nouveau/nvif/device.c
index 24880931039f6..1be9fbe6cb703 100644
--- a/drivers/gpu/drm/nouveau/nvif/device.c
+++ b/drivers/gpu/drm/nouveau/nvif/device.c
@@ -38,6 +38,19 @@ nvif_device_time(struct nvif_device *device)
 	return device->user.func->time(&device->user);
 }
 
+int
+nvif_device_gcx_ready(struct nvif_device *device)
+{
+	struct nv_device_gcx_ready_v0 args = {};
+	int ret;
+
+	ret = nvif_object_mthd(&device->object, NV_DEVICE_V0_GCX_READY, &args, sizeof(args));
+	if (ret)
+		return ret;
+
+	return args.ready;
+}
+
 int
 nvif_device_map(struct nvif_device *device)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
index 23d11d8221cb6..f78e6b9b42923 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
@@ -27,6 +27,7 @@
 
 #include <core/client.h>
 #include <subdev/fb.h>
+#include <subdev/gsp.h>
 #include <subdev/instmem.h>
 #include <subdev/timer.h>
 
@@ -189,6 +190,38 @@ nvkm_udevice_time(struct nvkm_udevice *udev, void *data, u32 size)
 	return ret;
 }
 
+static int
+nvkm_udevice_gcx_ready(struct nvkm_udevice *udev, void *data, u32 size)
+{
+	struct nvkm_object *object = &udev->object;
+	struct nvkm_device *device = udev->device;
+	struct nvkm_gsp *gsp = device->gsp;
+	union {
+		struct nv_device_gcx_ready_v0 v0;
+	} *args = data;
+	int ret = -ENOSYS;
+
+	if (!gsp) {
+		args->v0.ready = NV_DEVICE_GC6_READY | NV_DEVICE_GCOFF_READY;
+		return 0;
+	}
+
+	nvif_ioctl(object, "device gcx ready size %d\n", size);
+	ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, false);
+	if (!ret) {
+		nvif_ioctl(object, "device gcx ready vers %d\n", args->v0.version);
+
+		ret = nvkm_gsp_gcx_ready(gsp);
+		if (ret < 0)
+			return ret;
+
+		args->v0.ready = ret;
+		ret = 0;
+	}
+
+	return ret;
+}
+
 static int
 nvkm_udevice_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size)
 {
@@ -199,6 +232,8 @@ nvkm_udevice_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size)
 		return nvkm_udevice_info(udev, data, size);
 	case NV_DEVICE_V0_TIME:
 		return nvkm_udevice_time(udev, data, size);
+	case NV_DEVICE_V0_GCX_READY:
+		return nvkm_udevice_gcx_ready(udev, data, size);
 	default:
 		break;
 	}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
index 9ba1316831e7e..e475d0e8fa7bf 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
@@ -20,6 +20,7 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 #include "priv.h"
+#include <nvif/cl0080.h>
 
 int
 nvkm_gsp_intr_nonstall(struct nvkm_gsp *gsp, enum nvkm_subdev_type type, int inst)
@@ -47,6 +48,15 @@ nvkm_gsp_intr_stall(struct nvkm_gsp *gsp, enum nvkm_subdev_type type, int inst)
 	return -ENOENT;
 }
 
+int
+nvkm_gsp_gcx_ready(struct nvkm_gsp *gsp)
+{
+	if (!gsp->rm->api->gsp->gcx_ready)
+		return NV_DEVICE_GC6_READY | NV_DEVICE_GCOFF_READY;
+
+	return gsp->rm->api->gsp->gcx_ready(gsp);
+}
+
 static int
 nvkm_gsp_fini(struct nvkm_subdev *subdev, enum nvkm_suspend_state suspend)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
index 71b7203bef507..b07797813b049 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
@@ -93,6 +93,7 @@ void r535_gsp_dtor(struct nvkm_gsp *);
 int r535_gsp_oneinit(struct nvkm_gsp *);
 int r535_gsp_init(struct nvkm_gsp *);
 int r535_gsp_fini(struct nvkm_gsp *, enum nvkm_suspend_state suspend);
+int r535_gsp_gcx_ready(struct nvkm_gsp *gsp);
 
 int nvkm_gsp_new_(const struct nvkm_gsp_fwif *, struct nvkm_device *, enum nvkm_subdev_type, int,
 		  struct nvkm_gsp **);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
index b6683a5bf870c..fd1170037c030 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
@@ -782,6 +782,13 @@ typedef struct NV2080_CTRL_INTERNAL_INTR_GET_KERNEL_TABLE_PARAMS {
 
 #define GSP_FW_HEAP_PARAM_CLIENT_ALLOC_SIZE      ((48 << 10) * 2048)   // Support 2048 channels
 
+#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
+
+typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
+    NvBool bIsGC6Satisfied;
+    NvBool bIsGCOFFSatisfied;
+} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
+
 typedef union rpc_message_rpc_union_field_v03_00
 {
     NvU32      spare;
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 b45781cd0dfdc..dfda6b0b910a2 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
@@ -10,6 +10,7 @@
 #include "nvrm/gsp.h"
 #include "nvrm/rpcfn.h"
 #include "nvrm/msgfn.h"
+#include "nvif/cl0080.h"
 
 #include <core/pci.h>
 #include <subdev/pci/priv.h>
@@ -215,6 +216,32 @@ r570_gsp_set_rmargs(struct nvkm_gsp *gsp, bool resume)
 	args->bDmemStack = 1;
 }
 
+int
+r570_gsp_gcx_ready(struct nvkm_gsp *gsp)
+{
+	NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS *ctrl;
+	int ret = 0;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+				   NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE,
+				   sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	if (ctrl->bIsGC6Satisfied)
+		ret |= NV_DEVICE_GC6_READY;
+	if (ctrl->bIsGCOFFSatisfied)
+		ret |= NV_DEVICE_GCOFF_READY;
+
+	nvkm_debug(&gsp->subdev,
+		   "GCX ready status: GC6=%s GCOFF=%s\n",
+		   str_yes_no(ctrl->bIsGC6Satisfied), str_yes_no(ctrl->bIsGCOFFSatisfied));
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.client.object, ctrl);
+	return ret;
+}
+
+
 const struct nvkm_rm_api_gsp
 r570_gsp = {
 	.set_rmargs = r570_gsp_set_rmargs,
@@ -223,4 +250,5 @@ r570_gsp = {
 	.xlat_mc_engine_idx = r570_gsp_xlat_mc_engine_idx,
 	.drop_post_nocat_record = r570_gsp_drop_post_nocat_record,
 	.sr_data_size = r570_gsp_sr_data_size,
+	.gcx_ready = r570_gsp_gcx_ready,
 };
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 c458569af9d72..2814629fddd22 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
@@ -639,4 +639,11 @@ typedef struct GSP_FMC_BOOT_PARAMS
 } GSP_FMC_BOOT_PARAMS;
 
 #define GSP_FW_HEAP_PARAM_BASE_RM_SIZE_GH100              (14 << 20)   // Hopper+
+
+#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
+
+typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
+    NvBool bIsGC6Satisfied;
+    NvBool bIsGCOFFSatisfied;
+} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
 #endif
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 e9ac47d86b69a..86970129ad96a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -40,6 +40,7 @@ struct nvkm_rm_api {
 		void (*drop_send_user_shared_data)(struct nvkm_gsp *);
 		void (*drop_post_nocat_record)(struct nvkm_gsp *);
 		u32 (*sr_data_size)(struct nvkm_gsp *);
+		int (*gcx_ready)(struct nvkm_gsp *gsp);
 	} *gsp;
 
 	const struct nvkm_rm_api_rpc {
@@ -174,6 +175,7 @@ int r535_gr_chan_new(struct nvkm_gr *, struct nvkm_chan *, const struct nvkm_ocl
 int r535_gr_promote_ctx(struct r535_gr *, bool golden, struct nvkm_vmm *,
 			struct nvkm_memory **pctxbuf_mem, struct nvkm_vma **pctxbuf_vma,
 			struct nvkm_gsp_object *chan);
+int r570_gsp_gcx_ready(struct nvkm_gsp *gsp);
 extern const struct nvkm_rm_api_engine r535_nvdec;
 extern const struct nvkm_rm_api_engine r535_nvenc;
 extern const struct nvkm_rm_api_engine r535_nvjpg;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
  2026-09-16 22:25   ` Lyude Paul
  (?)
@ 2026-09-16 22:51   ` sashiko-bot
  -1 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-16 22:51 UTC (permalink / raw)
  To: Lyude Paul; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] nouveau/gsp: NULL pointer dereference when RM is disabled
- [Medium] pm: autosuspend stall on transient GCX readiness query error
- [Low] nouveau/gsp: API mismatch in gcx_ready using different object for alloc/free
--

commit 71e096484486fcd07150f6da48fcb331a8eae639
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE

OpenRM consults GSP to ask whether the GPU should be allowed to enter Gc6
and/or GcOff before runtime suspending. This patch implements the
NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE command query to check for
this readiness and delay runtime suspend if necessary.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index b0f9fb10a74dd..c2e2f430deded 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c

[ ... ]

> @@ -1164,6 +1165,18 @@ nouveau_pmops_runtime_suspend(struct device *dev)
>  		return -EBUSY;
>  	}
>  
> +	// Check if the GPU itself is ready for runtime suspend, otherwise mark as busy and check
> +	// again in a bit.
> +	ret = nvif_device_gcx_ready(nvif);
> +	if (ret < 0) {
> +		NV_ERROR(drm, "Failed to query GCX readiness (returned %d)\n", ret);
> +		return -EBUSY;

[Severity: Medium]
When nvif_device_gcx_ready() fails, this error path returns -EBUSY without
calling pm_runtime_mark_last_busy(dev).

Will this stall the autosuspend timer? Without advancing last_busy, the PM
core will not automatically reschedule the timer, leaving the device in
the RPM_ACTIVE state.

> +	} else if (!(ret & NV_DEVICE_GCOFF_READY)) {
> +		NV_DEBUG(drm, "GPU isn't ready for suspend yet, delaying...\n");
> +		pm_runtime_mark_last_busy(dev);
> +		return -EBUSY;
> +	}
> +
>  	nouveau_switcheroo_optimus_dsm();
>  	ret = nouveau_do_suspend(drm, true);

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
> index 9ba1316831e7e..e475d0e8fa7bf 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/base.c

[ ... ]

> @@ -47,6 +48,15 @@ nvkm_gsp_intr_stall(struct nvkm_gsp *gsp, enum nvkm_subdev_type type, int inst)
>  	return -ENOENT;
>  }
>  
> +int
> +nvkm_gsp_gcx_ready(struct nvkm_gsp *gsp)
> +{
> +	if (!gsp->rm->api->gsp->gcx_ready)

[Severity: Critical]
Does this code cause a NULL pointer dereference when RM is disabled?

On devices that use GSP but do not load GSP-RM (such as GV100 or Turing
without experimental RM enabled, or when disabled via the module parameter),
gsp->rm is never allocated and remains NULL.

Since nouveau_pmops_runtime_suspend() invokes this path for any GPU with a
GSP, won't this result in a kernel panic upon entering runtime suspend?

> +		return NV_DEVICE_GC6_READY | NV_DEVICE_GCOFF_READY;
> +
> +	return gsp->rm->api->gsp->gcx_ready(gsp);
> +}
> +

> 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 b45781cd0dfdc..dfda6b0b910a2 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

[ ... ]

> @@ -215,6 +216,32 @@ r570_gsp_set_rmargs(struct nvkm_gsp *gsp, bool resume)
>  	args->bDmemStack = 1;
>  }
>  
> +int
> +r570_gsp_gcx_ready(struct nvkm_gsp *gsp)
> +{
> +	NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS *ctrl;
> +	int ret = 0;
> +
> +	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
> +				   NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE,
> +				   sizeof(*ctrl));
> +	if (IS_ERR(ctrl))
> +		return PTR_ERR(ctrl);
> +
> +	if (ctrl->bIsGC6Satisfied)
> +		ret |= NV_DEVICE_GC6_READY;
> +	if (ctrl->bIsGCOFFSatisfied)
> +		ret |= NV_DEVICE_GCOFF_READY;
> +
> +	nvkm_debug(&gsp->subdev,
> +		   "GCX ready status: GC6=%s GCOFF=%s\n",
> +		   str_yes_no(ctrl->bIsGC6Satisfied), str_yes_no(ctrl->bIsGCOFFSatisfied));
> +
> +	nvkm_gsp_rm_ctrl_done(&gsp->internal.client.object, ctrl);

[Severity: Low]
This isn't a bug, but is there an object mismatch here?

The memory for ctrl is allocated earlier using the device.subdevice object:

	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice, ...);

but it is released here using the mismatched client.object.

While this avoids a crash because both objects' client fields resolve to
the same GSP context internally, does this violate the API contract?

> +	return ret;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916223358.507351-1-lyude@redhat.com?part=1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
  2026-09-16 22:25   ` Lyude Paul
@ 2026-09-16 23:48     ` David Airlie
  -1 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:48 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, Maxime Ripard,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:35 AM Lyude Paul <lyude@redhat.com> wrote:
>
> OpenRM's runtime PM handling looks a bit different then nouveau's, one part
> in particular that differs from us: OpenRM actually consults GSP to ask
> whether the GPU should be allowed to enter Gc6 and/or GcOff before runtime
> suspending the GPU. In the event the card isn't ready, runtime suspend is
> simply delayed for a few seconds before retrying.
>
> Implement the command used for querying GSP about this,
> NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE, and check to ensure that
> the GPU is ready for runtime suspend in nouveau_pmops_runtime_suspend()
> using this query. If the GPU can't be runtime suspended, update the last
> busy counter of the device and then return -EBUSY from
> nouveau_pmops_runtime_suspend() - essentially delaying the runtime suspend
> process by whatever autosuspend_delay_ms is set to.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
>
> ---

>  static int
>  nvkm_gsp_fini(struct nvkm_subdev *subdev, enum nvkm_suspend_state suspend)
>  {
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> index 71b7203bef507..b07797813b049 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> @@ -93,6 +93,7 @@ void r535_gsp_dtor(struct nvkm_gsp *);
>  int r535_gsp_oneinit(struct nvkm_gsp *);
>  int r535_gsp_init(struct nvkm_gsp *);
>  int r535_gsp_fini(struct nvkm_gsp *, enum nvkm_suspend_state suspend);
> +int r535_gsp_gcx_ready(struct nvkm_gsp *gsp);
>
>  int nvkm_gsp_new_(const struct nvkm_gsp_fwif *, struct nvkm_device *, enum nvkm_subdev_type, int,
>                   struct nvkm_gsp **);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> index b6683a5bf870c..fd1170037c030 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> @@ -782,6 +782,13 @@ typedef struct NV2080_CTRL_INTERNAL_INTR_GET_KERNEL_TABLE_PARAMS {
>
>  #define GSP_FW_HEAP_PARAM_CLIENT_ALLOC_SIZE      ((48 << 10) * 2048)   // Support 2048 channels
>
> +#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
> +
> +typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
> +    NvBool bIsGC6Satisfied;
> +    NvBool bIsGCOFFSatisfied;
> +} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
> +
>  typedef union rpc_message_rpc_union_field_v03_00

you can drop this hunk if r535 doesn't have it.

Dave.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
@ 2026-09-16 23:48     ` David Airlie
  0 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:48 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Timur Tabi, Andy Shevchenko,
	Mohamed Ahmed, Maarten Lankhorst, Kees Cook, Simona Vetter,
	David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:35 AM Lyude Paul <lyude@redhat.com> wrote:
>
> OpenRM's runtime PM handling looks a bit different then nouveau's, one part
> in particular that differs from us: OpenRM actually consults GSP to ask
> whether the GPU should be allowed to enter Gc6 and/or GcOff before runtime
> suspending the GPU. In the event the card isn't ready, runtime suspend is
> simply delayed for a few seconds before retrying.
>
> Implement the command used for querying GSP about this,
> NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE, and check to ensure that
> the GPU is ready for runtime suspend in nouveau_pmops_runtime_suspend()
> using this query. If the GPU can't be runtime suspended, update the last
> busy counter of the device and then return -EBUSY from
> nouveau_pmops_runtime_suspend() - essentially delaying the runtime suspend
> process by whatever autosuspend_delay_ms is set to.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
>
> ---

>  static int
>  nvkm_gsp_fini(struct nvkm_subdev *subdev, enum nvkm_suspend_state suspend)
>  {
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> index 71b7203bef507..b07797813b049 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> @@ -93,6 +93,7 @@ void r535_gsp_dtor(struct nvkm_gsp *);
>  int r535_gsp_oneinit(struct nvkm_gsp *);
>  int r535_gsp_init(struct nvkm_gsp *);
>  int r535_gsp_fini(struct nvkm_gsp *, enum nvkm_suspend_state suspend);
> +int r535_gsp_gcx_ready(struct nvkm_gsp *gsp);
>
>  int nvkm_gsp_new_(const struct nvkm_gsp_fwif *, struct nvkm_device *, enum nvkm_subdev_type, int,
>                   struct nvkm_gsp **);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> index b6683a5bf870c..fd1170037c030 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
> @@ -782,6 +782,13 @@ typedef struct NV2080_CTRL_INTERNAL_INTR_GET_KERNEL_TABLE_PARAMS {
>
>  #define GSP_FW_HEAP_PARAM_CLIENT_ALLOC_SIZE      ((48 << 10) * 2048)   // Support 2048 channels
>
> +#define NV2080_CTRL_CMD_INTERNAL_GCX_ENTRY_PREREQUISITE (0x2080a7d7)
> +
> +typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
> +    NvBool bIsGC6Satisfied;
> +    NvBool bIsGCOFFSatisfied;
> +} NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
> +
>  typedef union rpc_message_rpc_union_field_v03_00

you can drop this hunk if r535 doesn't have it.

Dave.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 2/5] drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 22:25   ` Lyude Paul
  -1 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

This is a GSP structure describing various characteristics of the memory
management system that GSP provides. Start by fetching it during driver
load, but don't do anything with the information we get from it just yet.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V3
* s/nvkm_gsp_rm_ctrl_get()/nvkm_gsp_rm_ctrl_rd()/
V4:
* Remove mistakenly duplicated struct

 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  4 ++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 +++++++
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 45 +++++++++++++++++++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c |  8 ++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  1 +
 5 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index cd10c37053695..ed5c6e0e68d3c 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -156,6 +156,10 @@ struct nvkm_gsp {
 		struct sg_table fbsr;
 	} sr;
 
+	struct {
+		bool use_raw_mode_comptagline_alloc;
+	} memsys;
+
 	struct {
 		struct nvkm_gsp_mem mem;
 
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 94925f1590ea4..63aa30f947471 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
@@ -1782,6 +1782,23 @@ r535_gsp_fini(struct nvkm_gsp *gsp, enum nvkm_suspend_state suspend)
 	return 0;
 }
 
+int
+r535_gsp_get_static_memsys_info(struct nvkm_gsp *gsp)
+{
+	NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS *ctrl;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_GET_STATIC_CONFIG,
+				    sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	gsp->memsys.use_raw_mode_comptagline_alloc = ctrl->bUseRawModeComptaglineAllocation;
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, ctrl);
+	return 0;
+}
+
 int
 r535_gsp_init(struct nvkm_gsp *gsp)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
index fd1170037c030..fee5418a72a68 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
@@ -789,6 +789,51 @@ typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
     NvBool bIsGCOFFSatisfied;
 } NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
 
+typedef struct NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS {
+    /*! Determines if RM should use 1 to 1 Comptagline allocation policy */
+    NvBool bOneToOneComptagLineAllocation;
+
+    /*! Determines if RM should use 1 to 4 Comptagline allocation policy */
+    NvBool bUseOneToFourComptagLineAllocation;
+
+    /*! Determines if RM should use raw Comptagline allocation policy */
+    NvBool bUseRawModeComptaglineAllocation;
+
+    /*! Has COMPBIT_BACKING_SIZE been overridden to zero (i.e. disabled)? */
+    NvBool bDisableCompbitBacking;
+
+    /*! Determine if we need to disable post L2 compression */
+    NvBool bDisablePostL2Compression;
+
+    /*! Is ECC DRAM feature supported? */
+    NvBool bEnabledEccFBPA;
+
+    NvBool bL2PreFill;
+
+    /*! L2 cache size */
+    NV_DECLARE_ALIGNED(NvU64 l2CacheSize, 8);
+
+    /*! Indicate whether fpba is present or not */
+    NvBool bFbpaPresent;
+
+    /*! Size covered by one comptag */
+    NvU32  comprPageSize;
+
+    /*! log32(comprPageSize) */
+    NvU32  comprPageShift;
+
+    /*! RAM type */
+    NvU32  ramType;
+
+    /*! LTC count */
+    NvU32  ltcCount;
+
+    /*! LTS per LTC count */
+    NvU32  ltsPerLtcCount;
+} NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS;
+
+#define NV2080_CTRL_CMD_INTERNAL_MEMSYS_GET_STATIC_CONFIG        (0x20800a1c) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS_MESSAGE_ID" */
+
 typedef union rpc_message_rpc_union_field_v03_00
 {
     NvU32      spare;
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 dfda6b0b910a2..dff2081ed764f 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
@@ -138,6 +138,14 @@ r570_gsp_get_static_info(struct nvkm_gsp *gsp)
 		}
 	}
 
+	ret = r535_gsp_get_static_memsys_info(gsp);
+	if (ret) {
+		nvkm_error(&gsp->subdev, "Retrieving static memsys info failed\n");
+		return ret;
+	}
+	nvkm_debug(&gsp->subdev, "memsys: Use raw mode for comptag allocations? %s\n",
+		   str_yes_no(gsp->memsys.use_raw_mode_comptagline_alloc));
+
 	return 0;
 }
 
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 86970129ad96a..17480d4e527ac 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -176,6 +176,7 @@ int r535_gr_promote_ctx(struct r535_gr *, bool golden, struct nvkm_vmm *,
 			struct nvkm_memory **pctxbuf_mem, struct nvkm_vma **pctxbuf_vma,
 			struct nvkm_gsp_object *chan);
 int r570_gsp_gcx_ready(struct nvkm_gsp *gsp);
+int r535_gsp_get_static_memsys_info(struct nvkm_gsp *gsp);
 extern const struct nvkm_rm_api_engine r535_nvdec;
 extern const struct nvkm_rm_api_engine r535_nvenc;
 extern const struct nvkm_rm_api_engine r535_nvjpg;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 2/5] drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
@ 2026-09-16 22:25   ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

This is a GSP structure describing various characteristics of the memory
management system that GSP provides. Start by fetching it during driver
load, but don't do anything with the information we get from it just yet.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V3
* s/nvkm_gsp_rm_ctrl_get()/nvkm_gsp_rm_ctrl_rd()/
V4:
* Remove mistakenly duplicated struct

 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  4 ++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 +++++++
 .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 45 +++++++++++++++++++
 .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c |  8 ++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  1 +
 5 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index cd10c37053695..ed5c6e0e68d3c 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -156,6 +156,10 @@ struct nvkm_gsp {
 		struct sg_table fbsr;
 	} sr;
 
+	struct {
+		bool use_raw_mode_comptagline_alloc;
+	} memsys;
+
 	struct {
 		struct nvkm_gsp_mem mem;
 
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 94925f1590ea4..63aa30f947471 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
@@ -1782,6 +1782,23 @@ r535_gsp_fini(struct nvkm_gsp *gsp, enum nvkm_suspend_state suspend)
 	return 0;
 }
 
+int
+r535_gsp_get_static_memsys_info(struct nvkm_gsp *gsp)
+{
+	NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS *ctrl;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_GET_STATIC_CONFIG,
+				    sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	gsp->memsys.use_raw_mode_comptagline_alloc = ctrl->bUseRawModeComptaglineAllocation;
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, ctrl);
+	return 0;
+}
+
 int
 r535_gsp_init(struct nvkm_gsp *gsp)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
index fd1170037c030..fee5418a72a68 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/nvrm/gsp.h
@@ -789,6 +789,51 @@ typedef struct NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS {
     NvBool bIsGCOFFSatisfied;
 } NV2080_CTRL_INTERNAL_GCX_ENTRY_PREREQUISITE_PARAMS;
 
+typedef struct NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS {
+    /*! Determines if RM should use 1 to 1 Comptagline allocation policy */
+    NvBool bOneToOneComptagLineAllocation;
+
+    /*! Determines if RM should use 1 to 4 Comptagline allocation policy */
+    NvBool bUseOneToFourComptagLineAllocation;
+
+    /*! Determines if RM should use raw Comptagline allocation policy */
+    NvBool bUseRawModeComptaglineAllocation;
+
+    /*! Has COMPBIT_BACKING_SIZE been overridden to zero (i.e. disabled)? */
+    NvBool bDisableCompbitBacking;
+
+    /*! Determine if we need to disable post L2 compression */
+    NvBool bDisablePostL2Compression;
+
+    /*! Is ECC DRAM feature supported? */
+    NvBool bEnabledEccFBPA;
+
+    NvBool bL2PreFill;
+
+    /*! L2 cache size */
+    NV_DECLARE_ALIGNED(NvU64 l2CacheSize, 8);
+
+    /*! Indicate whether fpba is present or not */
+    NvBool bFbpaPresent;
+
+    /*! Size covered by one comptag */
+    NvU32  comprPageSize;
+
+    /*! log32(comprPageSize) */
+    NvU32  comprPageShift;
+
+    /*! RAM type */
+    NvU32  ramType;
+
+    /*! LTC count */
+    NvU32  ltcCount;
+
+    /*! LTS per LTC count */
+    NvU32  ltsPerLtcCount;
+} NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS;
+
+#define NV2080_CTRL_CMD_INTERNAL_MEMSYS_GET_STATIC_CONFIG        (0x20800a1c) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_GET_STATIC_CONFIG_PARAMS_MESSAGE_ID" */
+
 typedef union rpc_message_rpc_union_field_v03_00
 {
     NvU32      spare;
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 dfda6b0b910a2..dff2081ed764f 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
@@ -138,6 +138,14 @@ r570_gsp_get_static_info(struct nvkm_gsp *gsp)
 		}
 	}
 
+	ret = r535_gsp_get_static_memsys_info(gsp);
+	if (ret) {
+		nvkm_error(&gsp->subdev, "Retrieving static memsys info failed\n");
+		return ret;
+	}
+	nvkm_debug(&gsp->subdev, "memsys: Use raw mode for comptag allocations? %s\n",
+		   str_yes_no(gsp->memsys.use_raw_mode_comptagline_alloc));
+
 	return 0;
 }
 
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 86970129ad96a..17480d4e527ac 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -176,6 +176,7 @@ int r535_gr_promote_ctx(struct r535_gr *, bool golden, struct nvkm_vmm *,
 			struct nvkm_memory **pctxbuf_mem, struct nvkm_vma **pctxbuf_vma,
 			struct nvkm_gsp_object *chan);
 int r570_gsp_gcx_ready(struct nvkm_gsp *gsp);
+int r535_gsp_get_static_memsys_info(struct nvkm_gsp *gsp);
 extern const struct nvkm_rm_api_engine r535_nvdec;
 extern const struct nvkm_rm_api_engine r535_nvenc;
 extern const struct nvkm_rm_api_engine r535_nvjpg;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 22:25   ` Lyude Paul
  -1 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

One of the things that OpenRM does right before initiating fbsr is apply a
special workaround (nvidia issue #3172217) which temporarily disables raw
compression mode on the GPU. It is later re-enabled after resuming with
fbsr completes.

Since we don't currently save the compbit backing with fbsr, this shouldn't
currently make any functional difference in the suspend/resume process. But
it will be required for implementing support for saving and restoring
compbit backings from the GPU.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Fix gsp object mismatch in r570_memsys_enable_raw_comp_mode()
* Clarify debug message a bit in r570_memsys_enable_raw_comp_mode()
V3:
* Enable raw mode before rescheduling FIFO channels

 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 48 +++++++++++++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       |  6 +++
 2 files changed, 54 insertions(+)

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 af5aa5065c3dd..fe618a94461dc 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
@@ -26,6 +26,35 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
 	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 }
 
+static int
+r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
+{
+	NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
+	int ret;
+
+	ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
+				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
+				    sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	ctrl->bRawMode = enable;
+
+	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.object, ctrl);
+	if (!ret)
+		nvkm_debug(&gsp->subdev, "memsys: Raw compression mode %s\n",
+			   str_enabled_disabled(enable));
+
+	return ret;
+}
+
+static bool
+r570_need_raw_comp_war(struct nvkm_gsp *gsp, struct nvkm_device *device)
+{
+	return (device->card_type == GA100 || device->card_type == AD100) &&
+	    gsp->memsys.use_raw_mode_comptagline_alloc;
+}
+
 static void
 r570_fbsr_resume(struct nvkm_gsp *gsp)
 {
@@ -33,6 +62,7 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
 	struct nvkm_instmem *imem = device->imem;
 	struct nvkm_instobj *iobj;
 	struct nvkm_vmm *vmm;
+	int ret;
 
 	/* Restore BAR2 page tables via BAR0 window, and re-enable BAR2. */
 	list_for_each_entry(iobj, &imem->boot, head) {
@@ -54,6 +84,13 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
 	vmm = nvkm_bar_bar1_vmm(device);
 	vmm->func->flush(vmm, 0);
 
+	/* Re-enable raw mode if it was previously disabled */
+	if (r570_need_raw_comp_war(gsp, device)) {
+		ret = r570_memsys_enable_raw_comp_mode(gsp, true);
+		if (ret)
+			nvkm_error(&gsp->subdev, "Failed to re-enable raw comp mode\n");
+	}
+
 	/* Resume channel scheduling. */
 	r570_fbsr_suspend_channels(device->gsp, false);
 
@@ -104,6 +141,17 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	/* Stop channel scheduling. */
 	r570_fbsr_suspend_channels(gsp, true);
 
+	/* Temporarily disable raw mode to prevent FBSR restore operations from corrupting
+	 * compressed surfaces. Required for ampere and ada.
+	 *
+	 * Nvidia bug #3172217
+	 */
+	if (r570_need_raw_comp_war(gsp, device)) {
+		ret = r570_memsys_enable_raw_comp_mode(gsp, false);
+		if (ret)
+			return ret;
+	}
+
 	/* Save BAR2 allocations to system memory. */
 	list_for_each_entry(iobj, &imem->list, head) {
 		if (iobj->preserve) {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
index 8af432375f7a2..9050a8274b273 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
@@ -16,4 +16,10 @@ typedef struct NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS {
     NV_DECLARE_ALIGNED(NvU64 sysmemAddrOfSuspendResumeData, 8);
 } NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS;
 
+#define NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE (0x20800a6f) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS_MESSAGE_ID" */
+
+typedef struct NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS {
+    NvBool bRawMode;
+} NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS;
+
 #endif
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
@ 2026-09-16 22:25   ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

One of the things that OpenRM does right before initiating fbsr is apply a
special workaround (nvidia issue #3172217) which temporarily disables raw
compression mode on the GPU. It is later re-enabled after resuming with
fbsr completes.

Since we don't currently save the compbit backing with fbsr, this shouldn't
currently make any functional difference in the suspend/resume process. But
it will be required for implementing support for saving and restoring
compbit backings from the GPU.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Fix gsp object mismatch in r570_memsys_enable_raw_comp_mode()
* Clarify debug message a bit in r570_memsys_enable_raw_comp_mode()
V3:
* Enable raw mode before rescheduling FIFO channels

 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 48 +++++++++++++++++++
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       |  6 +++
 2 files changed, 54 insertions(+)

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 af5aa5065c3dd..fe618a94461dc 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
@@ -26,6 +26,35 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
 	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 }
 
+static int
+r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
+{
+	NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
+	int ret;
+
+	ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
+				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
+				    sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	ctrl->bRawMode = enable;
+
+	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.object, ctrl);
+	if (!ret)
+		nvkm_debug(&gsp->subdev, "memsys: Raw compression mode %s\n",
+			   str_enabled_disabled(enable));
+
+	return ret;
+}
+
+static bool
+r570_need_raw_comp_war(struct nvkm_gsp *gsp, struct nvkm_device *device)
+{
+	return (device->card_type == GA100 || device->card_type == AD100) &&
+	    gsp->memsys.use_raw_mode_comptagline_alloc;
+}
+
 static void
 r570_fbsr_resume(struct nvkm_gsp *gsp)
 {
@@ -33,6 +62,7 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
 	struct nvkm_instmem *imem = device->imem;
 	struct nvkm_instobj *iobj;
 	struct nvkm_vmm *vmm;
+	int ret;
 
 	/* Restore BAR2 page tables via BAR0 window, and re-enable BAR2. */
 	list_for_each_entry(iobj, &imem->boot, head) {
@@ -54,6 +84,13 @@ r570_fbsr_resume(struct nvkm_gsp *gsp)
 	vmm = nvkm_bar_bar1_vmm(device);
 	vmm->func->flush(vmm, 0);
 
+	/* Re-enable raw mode if it was previously disabled */
+	if (r570_need_raw_comp_war(gsp, device)) {
+		ret = r570_memsys_enable_raw_comp_mode(gsp, true);
+		if (ret)
+			nvkm_error(&gsp->subdev, "Failed to re-enable raw comp mode\n");
+	}
+
 	/* Resume channel scheduling. */
 	r570_fbsr_suspend_channels(device->gsp, false);
 
@@ -104,6 +141,17 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	/* Stop channel scheduling. */
 	r570_fbsr_suspend_channels(gsp, true);
 
+	/* Temporarily disable raw mode to prevent FBSR restore operations from corrupting
+	 * compressed surfaces. Required for ampere and ada.
+	 *
+	 * Nvidia bug #3172217
+	 */
+	if (r570_need_raw_comp_war(gsp, device)) {
+		ret = r570_memsys_enable_raw_comp_mode(gsp, false);
+		if (ret)
+			return ret;
+	}
+
 	/* Save BAR2 allocations to system memory. */
 	list_for_each_entry(iobj, &imem->list, head) {
 		if (iobj->preserve) {
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
index 8af432375f7a2..9050a8274b273 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
@@ -16,4 +16,10 @@ typedef struct NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS {
     NV_DECLARE_ALIGNED(NvU64 sysmemAddrOfSuspendResumeData, 8);
 } NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS;
 
+#define NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE (0x20800a6f) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS_MESSAGE_ID" */
+
+typedef struct NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS {
+    NvBool bRawMode;
+} NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS;
+
 #endif
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
  2026-09-16 22:25   ` Lyude Paul
  (?)
@ 2026-09-16 22:49   ` sashiko-bot
  -1 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-16 22:49 UTC (permalink / raw)
  To: Lyude Paul; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] nouveau/gsp: guaranteed RPC failure in r570_memsys_enable_raw_comp_mode due to object mismatch
- [High] nouveau/gsp: missing hardware state rollback on fbsr suspend error paths
--

commit f701d812b32be8bdc5f923ff8d18f26bd8108911
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217

This commit adds a workaround to temporarily disable raw compression mode on
the GPU right before initiating fbsr, and re-enables it upon resuming. This
change is intended to prepare for saving and restoring compbit backings from
the GPU.

> 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 af5aa5065c3dd..fe618a94461dc 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
[ ... ]
> +static int
> +r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
> +{
> +	NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
> +	int ret;
> +
> +	ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
> +				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
> +				    sizeof(*ctrl));
> +	if (IS_ERR(ctrl))
> +		return PTR_ERR(ctrl);
> +
> +	ctrl->bRawMode = enable;
> +
> +	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.object, ctrl);

[Severity: High]
Does this command need to be sent to the subdevice object rather than the
device object?

The NV2080 prefix indicates that
NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE
is a subdevice-level command. If it is sent to the device object
(&gsp->internal.device.object) instead of the subdevice object
(&gsp->internal.device.subdevice), won't the GSP firmware reject the RPC
due to a class mismatch?

If this fails unconditionally on Ampere and Ada GPUs, it would abort the
suspend/resume sequence.

[ ... ]
> @@ -104,6 +141,17 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
>  	/* Stop channel scheduling. */
>  	r570_fbsr_suspend_channels(gsp, true);
>  
> +	/* Temporarily disable raw mode to prevent FBSR restore operations from corrupting
> +	 * compressed surfaces. Required for ampere and ada.
> +	 *
> +	 * Nvidia bug #3172217
> +	 */
> +	if (r570_need_raw_comp_war(gsp, device)) {
> +		ret = r570_memsys_enable_raw_comp_mode(gsp, false);
> +		if (ret)
> +			return ret;

[Severity: High]
Should we resume the channels before returning an error here?

If r570_memsys_enable_raw_comp_mode() fails, we exit r570_fbsr_suspend()
with an error, bypassing the rest of the suspend. This leaves the channels
permanently suspended since we don't roll back the call to
r570_fbsr_suspend_channels(gsp, true) made just above this block.

> +	}
> +
>  	/* 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;

[Severity: High]
Similarly, if nvkm_instobj_save() fails, do we need to re-enable the raw
compression mode and resume the channels?

Returning directly here leaves the hardware state modified without a rollback
path, which might cause the GPU to be stuck with suspended channels and
disabled compression until a system reboot or driver reload.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916223358.507351-1-lyude@redhat.com?part=3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
  2026-09-16 22:25   ` Lyude Paul
@ 2026-09-16 23:52     ` David Airlie
  -1 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:52 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, Maxime Ripard,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:35 AM Lyude Paul <lyude@redhat.com> wrote:
>
> One of the things that OpenRM does right before initiating fbsr is apply a
> special workaround (nvidia issue #3172217) which temporarily disables raw
> compression mode on the GPU. It is later re-enabled after resuming with
> fbsr completes.
>
> Since we don't currently save the compbit backing with fbsr, this shouldn't
> currently make any functional difference in the suspend/resume process. But
> it will be required for implementing support for saving and restoring
> compbit backings from the GPU.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
>
> ---
> V2:
> * Fix gsp object mismatch in r570_memsys_enable_raw_comp_mode()
> * Clarify debug message a bit in r570_memsys_enable_raw_comp_mode()
> V3:
> * Enable raw mode before rescheduling FIFO channels
>
>  .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 48 +++++++++++++++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       |  6 +++
>  2 files changed, 54 insertions(+)
>
> 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 af5aa5065c3dd..fe618a94461dc 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
> @@ -26,6 +26,35 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
>         return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  }
>
> +static int
> +r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
> +{
> +       NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
> +       int ret;
> +
> +       ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
> +                                   NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
> +                                   sizeof(*ctrl));

I think all 2080 need to be on gsp->internal.device.subdevice

Dave.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
@ 2026-09-16 23:52     ` David Airlie
  0 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:52 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Timur Tabi, Andy Shevchenko,
	Mohamed Ahmed, Maarten Lankhorst, Kees Cook, Simona Vetter,
	David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:35 AM Lyude Paul <lyude@redhat.com> wrote:
>
> One of the things that OpenRM does right before initiating fbsr is apply a
> special workaround (nvidia issue #3172217) which temporarily disables raw
> compression mode on the GPU. It is later re-enabled after resuming with
> fbsr completes.
>
> Since we don't currently save the compbit backing with fbsr, this shouldn't
> currently make any functional difference in the suspend/resume process. But
> it will be required for implementing support for saving and restoring
> compbit backings from the GPU.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
>
> ---
> V2:
> * Fix gsp object mismatch in r570_memsys_enable_raw_comp_mode()
> * Clarify debug message a bit in r570_memsys_enable_raw_comp_mode()
> V3:
> * Enable raw mode before rescheduling FIFO channels
>
>  .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 48 +++++++++++++++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       |  6 +++
>  2 files changed, 54 insertions(+)
>
> 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 af5aa5065c3dd..fe618a94461dc 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
> @@ -26,6 +26,35 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
>         return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  }
>
> +static int
> +r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
> +{
> +       NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
> +       int ret;
> +
> +       ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
> +                                   NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
> +                                   sizeof(*ctrl));

I think all 2080 need to be on gsp->internal.device.subdevice

Dave.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 4/5] drm/nouveau/gsp/r570: Start saving comptag backing stores
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 22:25   ` Lyude Paul
  -1 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

One of the portions of OpenRM's fbsr process that we never implemented is
the saving and restoring of comptag backing stores. This isn't strictly
necessary for fbsr to work (as long as we don't specify bEnteringGcOff =
1), but implementing it brings us much closer to matching OpenRM's fbsr
process - which means we can rely on things being well tested on Nvidia's
side.

Now that we have the required driver workarounds in place and fetch the
required information from GSP's memsys on driver load, let's implement
support for this by fetching the required space for the compbit backing
stores and adding it to the amount of memory that we allocate for fbsr.

With this, we should be able to safely enable bEnteringGcOff in fbsr.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Fix object mismatch in r570_fb_get_compbit_store_size() (rm_ctrl_done
  and rm_ctrl_rd() were using different objects by mistake)
* Fix GSP object mismatchs
* Don't return the size of the compbit store directly from
  r570_fb_get_compbit_store_size(), this makes it impossible for us to
  return errors since it's a u64.

 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 27 ++++++++++++++++++-
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 23 ++++++++++++++++
 2 files changed, 49 insertions(+), 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 fe618a94461dc..d3684bc4cf187 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
@@ -26,6 +26,23 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
 	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 }
 
+static int
+r570_fb_get_compbit_store_size(struct nvkm_gsp *gsp, u64 *size)
+{
+	NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS *ctrl;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.object,
+				   NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO,
+				   sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	*size = ctrl->Size;
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.object, ctrl);
+	return 0;
+}
+
 static int
 r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
 {
@@ -135,7 +152,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	struct nvkm_device *device = subdev->device;
 	struct nvkm_instmem *imem = device->imem;
 	struct nvkm_instobj *iobj;
-	u64 size;
+	u64 size, compbit_store_size;
 	int ret;
 
 	/* Stop channel scheduling. */
@@ -152,6 +169,12 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 			return ret;
 	}
 
+	ret = r570_fb_get_compbit_store_size(gsp, &compbit_store_size);
+	if (ret < 0)
+		return ret;
+	nvkm_debug(&gsp->subdev, "fbsr: Compbit backing store size: 0x%llx bytes\n",
+		   compbit_store_size);
+
 	/* Save BAR2 allocations to system memory. */
 	list_for_each_entry(iobj, &imem->list, head) {
 		if (iobj->preserve) {
@@ -174,6 +197,8 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	size  = gsp->fb.heap.size;
 	size += gsp->fb.rsvd_size;
 	size += gsp->fb.bios.vga_workspace.size;
+	size += compbit_store_size;
+
 	nvkm_debug(subdev, "fbsr: size: 0x%llx bytes\n", size);
 
 	ret = nvkm_gsp_sg(device, size, &gsp->sr.fbsr);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
index 9050a8274b273..cb3e448415b42 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
@@ -16,6 +16,29 @@ typedef struct NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS {
     NV_DECLARE_ALIGNED(NvU64 sysmemAddrOfSuspendResumeData, 8);
 } NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS;
 
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO (0x801306) /* finn: Evaluated from "(FINN_NV01_DEVICE_0_FB_INTERFACE_ID << 8) | NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS_MESSAGE_ID" */
+
+typedef struct NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS {
+    NV_DECLARE_ALIGNED(NvU64 Size, 8);
+    NV_DECLARE_ALIGNED(NvU64 Address, 8);
+    NvU32 AddressSpace;
+    NvU32 MaxCompbitLine;
+    NvU32 comptagsPerCacheLine;
+    NvU32 cacheLineSize;
+    NvU32 cacheLineSizePerSlice;
+    NvU32 cacheLineFetchAlignment;
+    NV_DECLARE_ALIGNED(NvU64 backingStoreBase, 8);
+    NvU32 gobsPerComptagPerSlice;
+    NvU32 backingStoreCbcBase;
+    NvU32 comptaglineAllocationPolicy;
+    NV_DECLARE_ALIGNED(NvU64 privRegionStartOffset, 8);
+    NvU32 cbcCoveragePerSlice;
+} NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS;
+
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_UNKNOWN 0 // ADDR_UNKNOWN
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_SYSMEM  1 // ADDR_SYSMEM
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_FBMEM   2 // ADDR_FBMEM
+
 #define NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE (0x20800a6f) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS_MESSAGE_ID" */
 
 typedef struct NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 4/5] drm/nouveau/gsp/r570: Start saving comptag backing stores
@ 2026-09-16 22:25   ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

One of the portions of OpenRM's fbsr process that we never implemented is
the saving and restoring of comptag backing stores. This isn't strictly
necessary for fbsr to work (as long as we don't specify bEnteringGcOff =
1), but implementing it brings us much closer to matching OpenRM's fbsr
process - which means we can rely on things being well tested on Nvidia's
side.

Now that we have the required driver workarounds in place and fetch the
required information from GSP's memsys on driver load, let's implement
support for this by fetching the required space for the compbit backing
stores and adding it to the amount of memory that we allocate for fbsr.

With this, we should be able to safely enable bEnteringGcOff in fbsr.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Fix object mismatch in r570_fb_get_compbit_store_size() (rm_ctrl_done
  and rm_ctrl_rd() were using different objects by mistake)
* Fix GSP object mismatchs
* Don't return the size of the compbit store directly from
  r570_fb_get_compbit_store_size(), this makes it impossible for us to
  return errors since it's a u64.

 .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 27 ++++++++++++++++++-
 .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 23 ++++++++++++++++
 2 files changed, 49 insertions(+), 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 fe618a94461dc..d3684bc4cf187 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
@@ -26,6 +26,23 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
 	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 }
 
+static int
+r570_fb_get_compbit_store_size(struct nvkm_gsp *gsp, u64 *size)
+{
+	NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS *ctrl;
+
+	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.object,
+				   NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO,
+				   sizeof(*ctrl));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	*size = ctrl->Size;
+
+	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.object, ctrl);
+	return 0;
+}
+
 static int
 r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
 {
@@ -135,7 +152,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	struct nvkm_device *device = subdev->device;
 	struct nvkm_instmem *imem = device->imem;
 	struct nvkm_instobj *iobj;
-	u64 size;
+	u64 size, compbit_store_size;
 	int ret;
 
 	/* Stop channel scheduling. */
@@ -152,6 +169,12 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 			return ret;
 	}
 
+	ret = r570_fb_get_compbit_store_size(gsp, &compbit_store_size);
+	if (ret < 0)
+		return ret;
+	nvkm_debug(&gsp->subdev, "fbsr: Compbit backing store size: 0x%llx bytes\n",
+		   compbit_store_size);
+
 	/* Save BAR2 allocations to system memory. */
 	list_for_each_entry(iobj, &imem->list, head) {
 		if (iobj->preserve) {
@@ -174,6 +197,8 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
 	size  = gsp->fb.heap.size;
 	size += gsp->fb.rsvd_size;
 	size += gsp->fb.bios.vga_workspace.size;
+	size += compbit_store_size;
+
 	nvkm_debug(subdev, "fbsr: size: 0x%llx bytes\n", size);
 
 	ret = nvkm_gsp_sg(device, size, &gsp->sr.fbsr);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
index 9050a8274b273..cb3e448415b42 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h
@@ -16,6 +16,29 @@ typedef struct NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS {
     NV_DECLARE_ALIGNED(NvU64 sysmemAddrOfSuspendResumeData, 8);
 } NV2080_CTRL_INTERNAL_FBSR_INIT_PARAMS;
 
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO (0x801306) /* finn: Evaluated from "(FINN_NV01_DEVICE_0_FB_INTERFACE_ID << 8) | NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS_MESSAGE_ID" */
+
+typedef struct NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS {
+    NV_DECLARE_ALIGNED(NvU64 Size, 8);
+    NV_DECLARE_ALIGNED(NvU64 Address, 8);
+    NvU32 AddressSpace;
+    NvU32 MaxCompbitLine;
+    NvU32 comptagsPerCacheLine;
+    NvU32 cacheLineSize;
+    NvU32 cacheLineSizePerSlice;
+    NvU32 cacheLineFetchAlignment;
+    NV_DECLARE_ALIGNED(NvU64 backingStoreBase, 8);
+    NvU32 gobsPerComptagPerSlice;
+    NvU32 backingStoreCbcBase;
+    NvU32 comptaglineAllocationPolicy;
+    NV_DECLARE_ALIGNED(NvU64 privRegionStartOffset, 8);
+    NvU32 cbcCoveragePerSlice;
+} NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS;
+
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_UNKNOWN 0 // ADDR_UNKNOWN
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_SYSMEM  1 // ADDR_SYSMEM
+#define NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO_ADDRESS_SPACE_FBMEM   2 // ADDR_FBMEM
+
 #define NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE (0x20800a6f) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS_MESSAGE_ID" */
 
 typedef struct NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/5] drm/nouveau/gsp/r570: Start saving comptag backing stores
  2026-09-16 22:25   ` Lyude Paul
  (?)
@ 2026-09-16 22:51   ` sashiko-bot
  -1 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-09-16 22:51 UTC (permalink / raw)
  To: Lyude Paul; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nouveau: integer truncation allows heap overflow via GSP compbit size
--

commit 9c4d2d77f428ab0f70d6ac1a683c604ade884fd2
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/gsp/r570: Start saving comptag backing stores

This commit implements support for saving and restoring comptag backing
stores during the OpenRM fbsr process. It fetches the required space for the
compbit backing stores from the GSP and adds it to the memory allocated for
fbsr.

> 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
[ ... ]
> @@ -26,6 +26,23 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
>  	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  }
>  
> +static int
> +r570_fb_get_compbit_store_size(struct nvkm_gsp *gsp, u64 *size)
> +{
> +	NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS *ctrl;
> +
> +	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.object,
> +				   NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO,
> +				   sizeof(*ctrl));
> +	if (IS_ERR(ctrl))
> +		return PTR_ERR(ctrl);
> +
> +	*size = ctrl->Size;

[Severity: High]
Does this code need to validate the untrusted 64-bit value returned from the
GSP firmware?

If a compromised or malicious GSP firmware provides a crafted value, it can be
used to exploit integer truncations during allocation.

> +
> +	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.object, ctrl);
> +	return 0;
> +}
[ ... ]
> @@ -174,6 +197,8 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
>  	size  = gsp->fb.heap.size;
>  	size += gsp->fb.rsvd_size;
>  	size += gsp->fb.bios.vga_workspace.size;
> +	size += compbit_store_size;

[Severity: High]
Can this unvalidated addition allow an attacker to craft a specific size
that perfectly aligns for integer truncation in the allocator?

When nvkm_gsp_sg() receives this size, it computes pages and passes it to
sg_alloc_table():

nvkm_gsp_sg(device, size, &gsp->sr.fbsr)
   const u64 pages = DIV_ROUND_UP(size, PAGE_SIZE);
   ...
   ret = sg_alloc_table(sgt, pages, GFP_KERNEL);

Since sg_alloc_table() takes a 32-bit unsigned int nents, a crafted size
(like 4TB + 1 bytes) would truncate to a tiny non-zero value, allowing a
massive allocation request to succeed by allocating only a few pages.

Additionally, when this size reaches r535_fbsr_memlist():

r535_fbsr_memlist()
   const u32 pages = size / GSP_PAGE_SIZE;
   ...
   rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY,
                          sizeof(*rpc) + pages * sizeof(rpc->pteDesc.pte_pde[0]));

The 32-bit truncation results in a 0-PTE buffer allocation. The subsequent loop
then writes PTEs based on the actual SG table length, causing an out-of-bounds
heap write.

> +
>  	nvkm_debug(subdev, "fbsr: size: 0x%llx bytes\n", size);
>  
>  	ret = nvkm_gsp_sg(device, size, &gsp->sr.fbsr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916223358.507351-1-lyude@redhat.com?part=4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 5/5] drm/nouveau/gsp/r570: Enable Gcoff in fbsr again
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 22:25   ` Lyude Paul
  -1 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Andy Shevchenko, Mohamed Ahmed, Maarten Lankhorst,
	Kees Cook, Simona Vetter, Maxime Ripard, Hongling Zeng,
	Danilo Krummrich

Now that we're properly saving the compbit backing stores on fbsr init, we
can start setting bEnteringGcOff = 1 again without things breaking, which
brings us closer to following the exact same code-paths OpenRM does for
fbsr.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 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 d3684bc4cf187..f27f19612a80d 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
@@ -135,7 +135,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 = 0;
+	ctrl->bEnteringGcoffState = 1;
 
 	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 	if (ret)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 5/5] drm/nouveau/gsp/r570: Enable Gcoff in fbsr again
@ 2026-09-16 22:25   ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2026-09-16 22:25 UTC (permalink / raw)
  To: dri-devel, nouveau, linux-kernel
  Cc: Dave Airlie, Timur Tabi, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, David Airlie,
	Thomas Zimmermann, Maxime Ripard, Mel Henning, Hongling Zeng,
	Danilo Krummrich, Lyude Paul

Now that we're properly saving the compbit backing stores on fbsr init, we
can start setting bEnteringGcOff = 1 again without things breaking, which
brings us closer to following the exact same code-paths OpenRM does for
fbsr.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 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 d3684bc4cf187..f27f19612a80d 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
@@ -135,7 +135,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 = 0;
+	ctrl->bEnteringGcoffState = 1;
 
 	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
 	if (ret)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely
  2026-09-16 22:25 ` Lyude Paul
@ 2026-09-16 23:53   ` David Airlie
  -1 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:53 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Andy Shevchenko, Mohamed Ahmed,
	Maarten Lankhorst, Kees Cook, Simona Vetter, Maxime Ripard,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:34 AM Lyude Paul <lyude@redhat.com> wrote:
>
> While we have runtime PM working a lot more reliably then in the past
> with R570, we're still doing things a lot differently then OpenRM. For
> instance, we don't save/restore comptag backing stores, and we don't ask
> GSP whether it's OK to enter GcOff.
>
> This series fixes this, and gets our runtime PM process to look a lot
> closer to OpenRM's by implementing some missing pieces and workarounds.
>
> Tested on my daily-driver desktop with an AD102, a ThinkPad P17 Gen1
> with a GA103GLM, a Dell Precision 7780 with an AD103, and my test
> machine with a GB206.

I've replied with a couple of minor fixes but with those applied.

Reviewed-by: Dave Airlie <airlied@redhat.com>

The rules for devices are 0080 ctrls on device.object and 2080 ctrls
on device.subdevice.

Dave.

>
> Patch series based on drm-misc/drm-misc-next. Branch with patches here:
>
>         https://gitlab.freedesktop.org/lyudess/linux/-/commits/nouveau/rpm-openrm-sync
>
> Previous version:
>
>         https://patchwork.freedesktop.org/series/173901/
>
> Lyude Paul (5):
>   drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
>   drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
>   drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
>   drm/nouveau/gsp/r570: Start saving comptag backing stores
>   drm/nouveau/gsp/r570: Enable Gcoff in fbsr again
>
>  drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 +++
>  drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
>  .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  6 ++
>  drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 ++++
>  drivers/gpu/drm/nouveau/nvif/device.c         | 13 ++++
>  .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 +++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
>  .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 ++++
>  .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 52 +++++++++++++
>  .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 77 ++++++++++++++++++-
>  .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 36 +++++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 29 +++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  3 +
>  15 files changed, 308 insertions(+), 2 deletions(-)
>
>
> base-commit: 5589ef09f93b85b6f506e279a502d4cf1654f232
> --
> 2.55.0
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely
@ 2026-09-16 23:53   ` David Airlie
  0 siblings, 0 replies; 21+ messages in thread
From: David Airlie @ 2026-09-16 23:53 UTC (permalink / raw)
  To: Lyude Paul
  Cc: dri-devel, nouveau, linux-kernel, Timur Tabi, Andy Shevchenko,
	Mohamed Ahmed, Maarten Lankhorst, Kees Cook, Simona Vetter,
	David Airlie, Thomas Zimmermann, Maxime Ripard, Mel Henning,
	Hongling Zeng, Danilo Krummrich

On Thu, Sep 17, 2026 at 8:34 AM Lyude Paul <lyude@redhat.com> wrote:
>
> While we have runtime PM working a lot more reliably then in the past
> with R570, we're still doing things a lot differently then OpenRM. For
> instance, we don't save/restore comptag backing stores, and we don't ask
> GSP whether it's OK to enter GcOff.
>
> This series fixes this, and gets our runtime PM process to look a lot
> closer to OpenRM's by implementing some missing pieces and workarounds.
>
> Tested on my daily-driver desktop with an AD102, a ThinkPad P17 Gen1
> with a GA103GLM, a Dell Precision 7780 with an AD103, and my test
> machine with a GB206.

I've replied with a couple of minor fixes but with those applied.

Reviewed-by: Dave Airlie <airlied@redhat.com>

The rules for devices are 0080 ctrls on device.object and 2080 ctrls
on device.subdevice.

Dave.

>
> Patch series based on drm-misc/drm-misc-next. Branch with patches here:
>
>         https://gitlab.freedesktop.org/lyudess/linux/-/commits/nouveau/rpm-openrm-sync
>
> Previous version:
>
>         https://patchwork.freedesktop.org/series/173901/
>
> Lyude Paul (5):
>   drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE
>   drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG
>   drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217
>   drm/nouveau/gsp/r570: Start saving comptag backing stores
>   drm/nouveau/gsp/r570: Enable Gcoff in fbsr again
>
>  drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 10 +++
>  drivers/gpu/drm/nouveau/include/nvif/device.h |  1 +
>  .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  6 ++
>  drivers/gpu/drm/nouveau/nouveau_drm.c         | 13 ++++
>  drivers/gpu/drm/nouveau/nvif/device.c         | 13 ++++
>  .../gpu/drm/nouveau/nvkm/engine/device/user.c | 35 +++++++++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/base.c    | 10 +++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
>  .../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 17 ++++
>  .../nvkm/subdev/gsp/rm/r535/nvrm/gsp.h        | 52 +++++++++++++
>  .../nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c    | 77 ++++++++++++++++++-
>  .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 36 +++++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/fbsr.h       | 29 +++++++
>  .../nvkm/subdev/gsp/rm/r570/nvrm/gsp.h        |  7 ++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   |  3 +
>  15 files changed, 308 insertions(+), 2 deletions(-)
>
>
> base-commit: 5589ef09f93b85b6f506e279a502d4cf1654f232
> --
> 2.55.0
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-09-16 23:54 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 22:25 [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely Lyude Paul
2026-09-16 22:25 ` Lyude Paul
2026-09-16 22:25 ` [PATCH v4 1/5] drm/nouveau/gsp/r535: Add support for INTERNAL_GCX_ENTRY_PREREQUISITE Lyude Paul
2026-09-16 22:25   ` Lyude Paul
2026-09-16 22:51   ` sashiko-bot
2026-09-16 23:48   ` David Airlie
2026-09-16 23:48     ` David Airlie
2026-09-16 22:25 ` [PATCH v4 2/5] drm/nouveau/gsp/r535: Add support for MEMSYS_GET_STATIC_CONFIG Lyude Paul
2026-09-16 22:25   ` Lyude Paul
2026-09-16 22:25 ` [PATCH v4 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217 Lyude Paul
2026-09-16 22:25   ` Lyude Paul
2026-09-16 22:49   ` sashiko-bot
2026-09-16 23:52   ` David Airlie
2026-09-16 23:52     ` David Airlie
2026-09-16 22:25 ` [PATCH v4 4/5] drm/nouveau/gsp/r570: Start saving comptag backing stores Lyude Paul
2026-09-16 22:25   ` Lyude Paul
2026-09-16 22:51   ` sashiko-bot
2026-09-16 22:25 ` [PATCH v4 5/5] drm/nouveau/gsp/r570: Enable Gcoff in fbsr again Lyude Paul
2026-09-16 22:25   ` Lyude Paul
2026-09-16 23:53 ` [PATCH v4 0/5] drm/nouveau/gsp/r570: Follow OpenRM's runtime PM process more closely David Airlie
2026-09-16 23:53   ` David Airlie

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.