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

[John Hubbard <jhubbard@nvidia.com>]

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,
-- 
John Hubbard