On 7/8/26 4:18 PM, [email protected] wrote: > On Thu, 2026-07-02 at 12:46 -1000, John Hubbard wrote: >> On 7/1/26 2:47 PM, Danilo Krummrich wrote: >>> On Thu Jul 2, 2026 at 2:30 AM CEST, David Airlie wrote: >>>> On Thu, Jul 2, 2026 at 10:27 AM Danilo Krummrich >>>> <[email protected]> wrote: >>>>> >>>>> (Cc: John) >> >> >> Also Cc: Aaron Plattner. I've provided answers below, but Aaron >> has actual experience in debugging suspend-resume on our Linux >> drivers. >> >> These answers are the result of my moderately long session with >> our best AI tools, using Open RM, GSP-RM, and Nouveau sources >> as a reference. I'm not actually experienced in this suspend-resume >> area, much, but this makes sense from what anecdotal things I've >> seen before. > > Would definitely be good to get human eyes on this, see down below
Re-adding Aaron to Cc. Suspend/resume is hard, we need an actual expert here. > >> >>>>> >>>>> On Wed Jul 1, 2026 at 8:17 PM CEST, Lyude Paul wrote: >>>>>> It turns out that the only reason our previous fixes looked >>>>>> like they >>>>>> worked for this was because we would occasionally set the >>>>>> Gcoff state to 0 >>>>>> in the normal S3 path, which fixed suspend/resume on desktops >>>>>> - but not on >>>>>> machines using runtime suspend. >>>>>> >>>>>> The proper fix is to just never set this flag. Our current >>>>>> guess for the >>>>>> reasoning behind this is that Gcoff likely coincides with >>>>>> GC6, and not >>>>>> literally power off. >>>>> >>>>> I don't think GcOff coincides with GC6, it should actually be a >>>>> power off. >> >> You're right, it's the other way around from the commit message >> guess. >> In the RM sources GC6 and GCOFF are two distinct GCx targets. GC6 >> keeps >> video memory alive in self-refresh. GCOFF is a full power-off where >> video memory content is lost, so RM copies the used framebuffer out >> to >> sysmem before entering it, and it reports vidmem power as off while >> in >> GCOFF. GCOFF is the power-off case, GC6 is not. >> >>>>> >>>>> From a quick glance in OpenRM, it seems that with >>>>> bEnteringGcoffState = 1 it >>>>> also saves off buffers flagged as >>>>> MEMDESC_FLAGS_LOST_ON_SUSPEND. >> >> That matches what I see, and it's the key point. bEnteringGcoffState >> is >> not a GC6-versus-off selector at the FBSR layer. It becomes the >> PDB_PROP_GPU_GCOFF_STATE_ENTERING property on the RM side, and that >> property widens the set of allocations RM saves and restores across >> suspend (memmgrAddMemNodes, through its bSaveAllRmAllocations >> argument). >> >> With it set: >> * RM reserved regions get saved, unless they are LOST_ON_SUSPEND. >> * RM channel-context and kernel-client buffers get saved even when >> they are LOST_ON_SUSPEND. >> >> With it clear, the reserved regions are skipped and the channel and >> kernel-client buffers are saved only when they are not >> LOST_ON_SUSPEND. >> So =1 is a strict superset of =0, and it does include the >> LOST_ON_SUSPEND buffers you found. >> >> The part that matters for nouveau: in the full driver that property >> is >> never just a standalone flag. RM sets it only when it has decided to >> do >> a GCOFF as part of its own RTD3 policy, after it has reserved >> correctly >> sized sysmem for the save and turned on comptag backing-store >> preservation for the state unload and load. Setting the flag in the >> FBSR init RPC on its own, the way nouveau does, gives GSP the wider >> save >> and restore set without any of that surrounding GCOFF handling. >> >> So I would adjust the guess slightly. It is not that nouveau never >> saved those buffers or never had them. nouveau provides the sysmem >> and >> GSP-RM does the copy into it. The problem is the reverse: with =1, >> GSP >> saves and then restores buffers that were meant to be reinitialized >> on >> resume, and it does so without the comptag and state-load handling a >> real GCOFF pairs with them. So the accurate framing is "buffers that >> should have been reinitialized get restored instead", not "buffers >> nouveau never saved". >> >>>>> >>>>> My guess would be that with bEnteringGcoffState = 1, GSP's >>>>> resume path expects >>>>> certain kernel-driver-allocated buffers to still be in place >>>>> that nouveau didn't >>>>> save off, or rather never had in the first place. >>>>> >>>>> John, do you have some details about this? >>>>> >>>> >>>> In nouveau we have the INST_SR_LOST target, for buffers that >>>> aren't >>>> preserved, I wonder did something change between 535 and 570 >>>> around >>>> what needs to be kept around. >>> >>> The r535 code never set bEnteringGcoffState in the first place. In >>> r535 OpenRM >>> seems to do the exact same thing. >> >> The set of buffers did not change. The FBSR client ABI did. In 535 >> nouveau enumerates the exact VRAM regions and sends them to RM one at >> a >> time, and it never sets the gcoff field, so the flag is a no-op on >> 535. >> >> In 570 nouveau passes RM a single sysmem buffer for the whole heap >> and >> lets GSP build the region list itself, and the gcoff flag is the only >> control nouveau has over which regions GSP picks. Forcing it to 0 >> makes >> the 570 GSP-built set match what 535 effectively saved, which is why >> 535 >> looks like it does the same thing. So 0 is the right value for how >> nouveau drives suspend today. RM derives this per transition from its >> RTD3 policy, and 570 setting it to 1 was the deviation, not 0. >> >> On patch 3 (the resume state flags), I looked at that as well, and >> here >> is what the firmware actually does with it. In the 570 GSP firmware >> the >> resume state load already runs with GPU_STATE_FLAGS_PRESERVING | >> GPU_STATE_FLAGS_PM_TRANSITION. That is set unconditionally in the >> resume >> path, and it is gated on the bInPMTransition field of the SR init >> arguments, which nouveau already sets on resume. The firmware does >> not >> derive those flags from srInitArguments.flags. That field is read in >> only one place on the resume path, an unrelated display workaround >> gated >> on the PM_SUSPEND bit. Neither 0 nor PRESERVING | PM_TRANSITION sets This "unrelated display workaround" might be related after all, perhaps. >> that bit. And the value the open driver itself puts in that field on >> a >> standby or RTD3 resume is GPU_STATE_FLAGS_PM_SUSPEND, which is a PM- >> type >> indicator, not the state-load flags. >> >> So from the 570 sources I do not see a path by which patch 3 changes >> what the firmware does on resume. That points to patches 1 and 2, the >> revert plus never entering the gcoff save path, as what actually >> fixes >> the push-buffer timeouts. Your 100-cycle RTD3 result is consistent >> with >> that: those two are what stop GSP from doing the wide GCOFF-style >> save >> and restore. >> >> I want to be clear about the limits of what I checked. I confirmed >> the >> resume-side firmware behavior against the 570 release (latest) >> sources >> rather >> than the exact 570.144 build, so I am not claiming patch 3 is >> provably >> inert on 570.144, only that I do not see how it changes behavior. And >> I >> have the mechanism for the =1 breakage but not the single allocation >> behind the timeout. I can see that =1 restores LOST_ON_SUSPEND RM >> buffers that should have been reinitialized, without the matching >> state-load handling, but I have not isolated the exact buffer that >> produces the failure. > > Mhm - the AI must be missing something, mainly because I went back and > double checked - and at least with runtime PM on this ampere machine > I'm immediately able to reproduce issues if I drop patch 3 (in > particular - job timeouts after runtime resume). The actual > suspend/resume process succeeds, but it leaves us with a GPU that > doesn't seem to be able to render anything: > > [ 93.167997] nouveau 0000:01:00.0: vkcube[11028]: job timeout, channel 4 > killed! > [ 100.365899] nouveau 0000:01:00.0: gsp: rc engn:00000001 chid:4 gfid:0 > level:2 type:38 scope:1 part:233 fault_addr:0000000000000000 > fault_type:00000000 > [ 100.365907] nouveau 0000:01:00.0: fifo:c00000:0004:0004:[vkcube[11028]] > errored - disabling channel > >> >> My bottom line: patch 2 (=0) is correct and is the right value for >> how >> nouveau drives suspend today, and patch 1 is needed with it. Patch 3 >> is >> harmless, and from the sources I do not expect it to change anything >> on >> 570.144. >> >> Assisted-by: Cursor :) >> >> thanks, > thanks, -- John Hubbard
