Re: [PATCH v3 3/5] drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/nouveau/gsp: wrong RM object in r570_memsys_enable_raw_comp_mode()
--

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

drm/nouveau/gsp/r570: Add comp mode workaround from issue #3172217

This commit adds a workaround to temporarily disable raw compression mode
on the GPU before initiating FBSR during suspend, and re-enable it during
resume. This addresses an issue on Ampere and Ada GPUs where restore
operations could otherwise corrupt compressed surfaces.

> 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 af5aa5065c3dd..fe618a94461dc 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
[ ... ]
> @@ -26,6 +26,35 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
>  	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  }
>  
> +static int
> +r570_memsys_enable_raw_comp_mode(struct nvkm_gsp *gsp, bool enable)
> +{
> +	NV2080_CTRL_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE_PARAMS *ctrl;
> +	int ret;
> +
> +	ctrl = nvkm_gsp_rm_ctrl_get(&gsp->internal.device.object,
> +				    NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE,
> +				    sizeof(*ctrl));

[Severity: High]
Should this use &gsp->internal.device.subdevice instead of .object?

The command being allocated is an 0x2080 Subdevice command
(NV2080_CTRL_CMD_INTERNAL_MEMSYS_PROGRAM_RAW_COMPRESSION_MODE). If this is
requested against the Device object instead of the Subdevice object, the GSP RM
will reject the control command with an error because the target object does
not match the command's expected class.

> +	if (IS_ERR(ctrl))
> +		return PTR_ERR(ctrl);
> +
> +	ctrl->bRawMode = enable;
> +
> +	ret = nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.object, ctrl);

[Severity: High]
Similarly, should this write be issued to the subdevice? 

Since r570_fbsr_suspend() propagates the return value of this function on GA100
and AD100 GPUs, an RM validation failure resulting from the class mismatch will
abort the suspend sequence entirely.

> +	if (!ret)
> +		nvkm_debug(&gsp->subdev, "memsys: Raw compression mode %s\n",
> +			   str_enabled_disabled(enable));
> +
> +	return ret;
> +}

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