Re: [RFC 4/8] drm/nouveau: scrub the FB memory when scrubber firmware is loaded

[Danilo Krummrich <dakr@kernel.org>]

On Fri, Nov 22, 2024 at 04:57:08AM -0800, Zhi Wang wrote:
> When WPR2 heap size > 256MB, the FB memory needs to be scrubbed
> before use.
> 
> If not, the GSP firmware hangs when booting.
> 
> If the scrubber firmware presents, execute it to scrub the FB memory
> before executing any other ucode images.
> 
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/ad102.c   | 35 +++++++++++++++++++
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/priv.h    |  1 +
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 12 +++++--
>  3 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/ad102.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/ad102.c
> index bd8bd37955fa..596ccd758e66 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/ad102.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/ad102.c
> @@ -19,8 +19,42 @@
>   * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
> +
> +#include <engine/sec2.h>
>  #include "priv.h"
>  
> +static bool is_scrubber_completed(struct nvkm_gsp *gsp)
> +{
> +	return ((nvkm_rd32(gsp->subdev.device, 0x001180fc) >> 29) >= 0x3);

Please, no more magic values.

Please add proper defines for the base address, register offset, shift and
value.

I think include/nvkm/regs/ would be a good place for that.

> +}
> +
> +static int
> +ad102_execute_scrubber(struct nvkm_gsp *gsp)
> +{
> +	struct nvkm_falcon_fw fw = {0};
> +	struct nvkm_subdev *subdev = &gsp->subdev;
> +	struct nvkm_device *device = subdev->device;
> +	int ret;
> +
> +	if (!gsp->fws.scrubber || is_scrubber_completed(gsp))
> +		return 0;
> +
> +	ret = gsp->func->booter.ctor(gsp, "scrubber", gsp->fws.scrubber,
> +				     &device->sec2->falcon, &fw);
> +	if (ret)
> +		return ret;
> +
> +	ret = nvkm_falcon_fw_boot(&fw, subdev, true, NULL, NULL, 0, 0);
> +	nvkm_falcon_fw_dtor(&fw);
> +	if (ret)
> +		return ret;
> +
> +	if (WARN_ON(!is_scrubber_completed(gsp)))
> +		return -ENOSPC;
> +
> +	return 0;
> +}
> +
>  static int
>  ad102_gsp_init_fw_heap(struct nvkm_gsp *gsp)
>  {
> @@ -51,6 +85,7 @@ ad102_gsp_r535_113_01 = {
>  	.wpr_heap.base_size = 8 << 20,
>  	.wpr_heap.min_size = 84 << 20,
>  	.wpr_heap.init_fw_heap = ad102_gsp_init_fw_heap,
> +	.wpr_heap.execute_scrubber = ad102_execute_scrubber,
>  
>  	.booter.ctor = ga102_gsp_booter_ctor,
>  
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> index a89ab7b22263..fe56ced9b369 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/priv.h
> @@ -31,6 +31,7 @@ struct nvkm_gsp_func {
>  		u32 base_size;
>  		u64 min_size;
>  		int (*init_fw_heap)(struct nvkm_gsp *gsp);
> +		int (*execute_scrubber)(struct nvkm_gsp *gsp);
>  	} wpr_heap;
>  
>  	struct {
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> index ef867eb20cff..d5d6d0df863e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> @@ -2618,13 +2618,19 @@ r535_gsp_oneinit(struct nvkm_gsp *gsp)
>  	if (ret)
>  		return ret;
>  
> -	/* Release FW images - we've copied them to DMA buffers now. */
> -	r535_gsp_dtor_fws(gsp);
> -
>  	ret = gsp->func->wpr_heap.init_fw_heap(gsp);
>  	if (WARN_ON(ret))
>  		return ret;
>  
> +	if (gsp->func->wpr_heap.execute_scrubber) {
> +		ret = gsp->func->wpr_heap.execute_scrubber(gsp);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Release FW images - we've copied them to DMA buffers now. */
> +	r535_gsp_dtor_fws(gsp);
> +
>  	ret = nvkm_gsp_fwsec_frts(gsp);
>  	if (WARN_ON(ret))
>  		return ret;
> -- 
> 2.34.1
>