Re: [PATCH v3 2/3] drm/nouveau/gsp/r570: Never enter Gcoff state

[lyude@redhat.com]

FWIW - I think for the time being I'm going to fixup these patches and
send out another version so we can get this pushed upstream sooner then
later. I am still planning on digging into this further - running this
patch series on my laptop got things much more stable then previously
but it definitely doesn't seem to have fixed everything (unfortunately
the remaining issues I am seeing don't have any clear reproducer yet)
So I get the feeling the next step we should probably be following is
ensuring we actually handle the comptag storage stuff properly on
runtime suspend.

On Wed, 2026-07-08 at 16:25 -0700, John Hubbard wrote:
> On 7/8/26 4:18 PM, lyude@redhat.com 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
> > > > > <dakr@kernel.org> 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,