Re: [PATCH v2 01/37] drm/nouveau: move nouveau_drm_device_fini() above init()

[Danilo Krummrich <dakr@redhat.com>]

On Fri, Jul 05, 2024 at 04:36:45AM +1000, Ben Skeggs wrote:
> The next commit wants to be able to call fini() from an init() failure
> path to remove the need to duplicate a bunch of cleanup.
> 
> Moving fini() above init() avoids the need for a forward-declaration.

Hm, I don't really like that this makes git-blame less useful for this function,
moving it up is clearly cleaner though - your call.

> 
> Signed-off-by: Ben Skeggs <bskeggs@nvidia.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 106 +++++++++++++-------------
>  1 file changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index a58c31089613..eae48c87e3d5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -578,6 +578,59 @@ nouveau_parent = {
>  	.errorf = nouveau_drm_errorf,
>  };
>  
> +static void
> +nouveau_drm_device_fini(struct drm_device *dev)
> +{
> +	struct nouveau_cli *cli, *temp_cli;
> +	struct nouveau_drm *drm = nouveau_drm(dev);
> +
> +	if (nouveau_pmops_runtime()) {
> +		pm_runtime_get_sync(dev->dev);
> +		pm_runtime_forbid(dev->dev);
> +	}
> +
> +	nouveau_led_fini(dev);
> +	nouveau_dmem_fini(drm);
> +	nouveau_svm_fini(drm);
> +	nouveau_hwmon_fini(dev);
> +	nouveau_debugfs_fini(drm);
> +
> +	if (dev->mode_config.num_crtc)
> +		nouveau_display_fini(dev, false, false);
> +	nouveau_display_destroy(dev);
> +
> +	nouveau_accel_fini(drm);
> +	nouveau_bios_takedown(dev);
> +
> +	nouveau_ttm_fini(drm);
> +	nouveau_vga_fini(drm);
> +
> +	/*
> +	 * There may be existing clients from as-yet unclosed files. For now,
> +	 * clean them up here rather than deferring until the file is closed,
> +	 * but this likely not correct if we want to support hot-unplugging
> +	 * properly.
> +	 */
> +	mutex_lock(&drm->clients_lock);
> +	list_for_each_entry_safe(cli, temp_cli, &drm->clients, head) {
> +		list_del(&cli->head);
> +		mutex_lock(&cli->mutex);
> +		if (cli->abi16)
> +			nouveau_abi16_fini(cli->abi16);
> +		mutex_unlock(&cli->mutex);
> +		nouveau_cli_fini(cli);
> +		kfree(cli);
> +	}
> +	mutex_unlock(&drm->clients_lock);
> +
> +	nouveau_cli_fini(&drm->client);
> +	nouveau_cli_fini(&drm->master);
> +	destroy_workqueue(drm->sched_wq);
> +	nvif_parent_dtor(&drm->parent);
> +	mutex_destroy(&drm->clients_lock);
> +	kfree(drm);
> +}
> +
>  static int
>  nouveau_drm_device_init(struct drm_device *dev)
>  {
> @@ -679,59 +732,6 @@ nouveau_drm_device_init(struct drm_device *dev)
>  	return ret;
>  }
>  
> -static void
> -nouveau_drm_device_fini(struct drm_device *dev)
> -{
> -	struct nouveau_cli *cli, *temp_cli;
> -	struct nouveau_drm *drm = nouveau_drm(dev);
> -
> -	if (nouveau_pmops_runtime()) {
> -		pm_runtime_get_sync(dev->dev);
> -		pm_runtime_forbid(dev->dev);
> -	}
> -
> -	nouveau_led_fini(dev);
> -	nouveau_dmem_fini(drm);
> -	nouveau_svm_fini(drm);
> -	nouveau_hwmon_fini(dev);
> -	nouveau_debugfs_fini(drm);
> -
> -	if (dev->mode_config.num_crtc)
> -		nouveau_display_fini(dev, false, false);
> -	nouveau_display_destroy(dev);
> -
> -	nouveau_accel_fini(drm);
> -	nouveau_bios_takedown(dev);
> -
> -	nouveau_ttm_fini(drm);
> -	nouveau_vga_fini(drm);
> -
> -	/*
> -	 * There may be existing clients from as-yet unclosed files. For now,
> -	 * clean them up here rather than deferring until the file is closed,
> -	 * but this likely not correct if we want to support hot-unplugging
> -	 * properly.
> -	 */
> -	mutex_lock(&drm->clients_lock);
> -	list_for_each_entry_safe(cli, temp_cli, &drm->clients, head) {
> -		list_del(&cli->head);
> -		mutex_lock(&cli->mutex);
> -		if (cli->abi16)
> -			nouveau_abi16_fini(cli->abi16);
> -		mutex_unlock(&cli->mutex);
> -		nouveau_cli_fini(cli);
> -		kfree(cli);
> -	}
> -	mutex_unlock(&drm->clients_lock);
> -
> -	nouveau_cli_fini(&drm->client);
> -	nouveau_cli_fini(&drm->master);
> -	destroy_workqueue(drm->sched_wq);
> -	nvif_parent_dtor(&drm->parent);
> -	mutex_destroy(&drm->clients_lock);
> -	kfree(drm);
> -}
> -
>  /*
>   * On some Intel PCIe bridge controllers doing a
>   * D0 -> D3hot -> D3cold -> D0 sequence causes Nvidia GPUs to not reappear.
> -- 
> 2.45.1
>