Re: [PATCH v2 2/4] drm/nouveau/gsp/r570: Never enter Gcoff state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but GSP active channel scheduling is not resumed on suspend error paths, leading to a permanent GPU hang.
--

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

drm/nouveau/gsp/r570: Never enter Gcoff state

This commit disables the Gcoff state transition for the r570 platform
during suspend operations. It resolves an issue where the Gcoff state
caused runtime suspend failures by ensuring the bEnteringGcoffState
flag is permanently set to 0.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> index 2945d5b4e5707..af5aa5065c3dd 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> @@ -81,7 +81,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size)
>  	ctrl->hClient = gsp->internal.client.object.handle;
>  	ctrl->hSysMem = memlist.handle;
>  	ctrl->sysmemAddrOfSuspendResumeData = gsp->sr.meta.addr;
> -	ctrl->bEnteringGcoffState = 1;
> +	ctrl->bEnteringGcoffState = 0;
>  
>  	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  	if (ret)

[Severity: High]
This is a pre-existing issue, but I have a question about the error
handling paths during suspend in this file.

If r570_fbsr_init() or other operations like nvkm_instobj_save() return
an error, those errors propagate out of r570_fbsr_suspend():

drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c:r570_fbsr_suspend() {
    ...
	/* Stop channel scheduling. */
	r570_fbsr_suspend_channels(gsp, true);

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

If any of these early returns are taken, r570_fbsr_suspend_channels(gsp,
false) is never called. Will this leave the GSP active channel scheduling
permanently disabled and result in a GPU hang if the PM core aborts the
suspend process?

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