Re: [PATCH 1/2] Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Hardware channels and BAR2 are left permanently disabled when r570_fbsr_suspend fails midway.
--

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

Revert "nouveau/gsp: fix suspend/resume regression on r570 firmware"

This reverts commit 8302d0afeaec0bc57d951dd085e0cffe997d4d18, which was
intended to fix a suspend/resume regression on r570 firmware but instead
caused runtime PM to fail on many laptops.

Cc: <stable@vger.kernel.org> # v6.19+

> 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

[ ... ]

> @@ -92,7 +92,7 @@ r570_fbsr_init(struct nvkm_gsp *gsp, struct sg_table *sgt, u64 size, bool runtim
>  }
>  
>  static int
> -r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
> +r570_fbsr_suspend(struct nvkm_gsp *gsp)
>  {
>  	struct nvkm_subdev *subdev = &gsp->subdev;
>  	struct nvkm_device *device = subdev->device;

[ ... ]

> @@ -133,7 +133,7 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp, bool runtime)
>  		return ret;
>  
>  	/* Initialise FBSR on RM. */
> -	ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size, runtime);
> +	ret = r570_fbsr_init(gsp, &gsp->sr.fbsr, size);
>  	if (ret) {
>  		nvkm_gsp_sg_free(device, &gsp->sr.fbsr);
>  		return ret;

[Severity: High]
This is a pre-existing issue, but does this error path leave the GPU
unusable if it fails?

During the suspend sequence, r570_fbsr_suspend() disables channel
scheduling and BAR2 early on. If a subsequent operation like
r570_fbsr_init() (or nvkm_gsp_sg() earlier) fails, it returns an error
here without restoring the hardware state.

Since the PM core will abort the suspend on error and skip the resume
callback for this device, does this mean channels and BAR2 are never
re-enabled, leaving the device in an unrecoverable state until reboot?

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