On Fri, Jul 05, 2024 at 04:36:50AM +1000, Ben Skeggs wrote:
> drm->master isn't really a nouveau_cli, and is mostly just used to get
> at an nvif_mmu object to implement a hack around issues with the ioctl
> interface to nvkm.
>
> Later patches in this series will allocate nvif_device/mmu objects in
> nouveau_drm directly, removing the need for master.
Please don't use future tense.
>
> Another patch series will remove the need for the above-mentioned hack
> entirely.
Do you have those patches already?
>
> The only other member of drm->master that's needed is the nvif_client,
> and is a dependency of device/mmu. So the first step is to move its
> allocation out of code handling nouveau_cli init.
>
> v2:
> - modified slightly due to the addition of tegra/pci cleanup patches
>
> Signed-off-by: Ben Skeggs <bskeggs@xxxxxxxxxx>
> ---
> drivers/gpu/drm/nouveau/nouveau_drm.c | 46 ++++++++++++++-------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 140e27af0d64..a942d2c03d44 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -129,12 +129,12 @@ nouveau_platform_name(struct platform_device *platformdev)
> }
>
> static u64
> -nouveau_name(struct drm_device *dev)
> +nouveau_name(struct device *dev)
> {
> - if (dev_is_pci(dev->dev))
> - return nouveau_pci_name(to_pci_dev(dev->dev));
> + if (dev_is_pci(dev))
> + return nouveau_pci_name(to_pci_dev(dev));
> else
> - return nouveau_platform_name(to_platform_device(dev->dev));
> + return nouveau_platform_name(to_platform_device(dev));
This looks like it should be a separate patch.
> }
>
> static inline bool
> @@ -209,9 +209,11 @@ nouveau_cli_fini(struct nouveau_cli *cli)
> nouveau_vmm_fini(&cli->vmm);
> nvif_mmu_dtor(&cli->mmu);
> nvif_device_dtor(&cli->device);
> - mutex_lock(&cli->drm->master.lock);
> - nvif_client_dtor(&cli->base);
> - mutex_unlock(&cli->drm->master.lock);
> + if (cli != &cli->drm->master) {
> + mutex_lock(&cli->drm->master.lock);
> + nvif_client_dtor(&cli->base);
> + mutex_unlock(&cli->drm->master.lock);
> + }
> }
>
> static int
> @@ -241,7 +243,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char *sname,
> { NVIF_CLASS_VMM_NV04 , -1 },
> {}
> };
> - u64 device = nouveau_name(drm->dev);
> + u64 device = nouveau_name(drm->dev->dev);
> int ret;
>
> snprintf(cli->name, sizeof(cli->name), "%s", sname);
> @@ -253,10 +255,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char *sname,
> INIT_LIST_HEAD(&cli->worker);
> mutex_init(&cli->lock);
>
> - if (cli == &drm->master) {
> - ret = nvif_driver_init(NULL, nouveau_config, nouveau_debug,
> - cli->name, device, &cli->base);
> - } else {
> + if (cli != &drm->master) {
> mutex_lock(&drm->master.lock);
> ret = nvif_client_ctor(&drm->master.base, cli->name, device,
> &cli->base);
> @@ -626,7 +625,6 @@ nouveau_drm_device_fini(struct nouveau_drm *drm)
> 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);
> }
>
> @@ -636,15 +634,10 @@ nouveau_drm_device_init(struct nouveau_drm *drm)
> struct drm_device *dev = drm->dev;
> int ret;
>
> - nvif_parent_ctor(&nouveau_parent, &drm->parent);
> - drm->master.base.object.parent = &drm->parent;
Moving this to nouveau_drm_device_new(), plus the resulting changes in error
handling, don't seem to be related to this commit either.
> -
> drm->sched_wq = alloc_workqueue("nouveau_sched_wq_shared", 0,
> WQ_MAX_ACTIVE);
> - if (!drm->sched_wq) {
> - ret = -ENOMEM;
> - goto fail_alloc;
> - }
> + if (!drm->sched_wq)
> + return -ENOMEM;
>
> ret = nouveau_cli_init(drm, "DRM-master", &drm->master);
> if (ret)
> @@ -726,8 +719,6 @@ nouveau_drm_device_init(struct nouveau_drm *drm)
> nouveau_cli_fini(&drm->master);
> fail_wq:
> destroy_workqueue(drm->sched_wq);
> -fail_alloc:
> - nvif_parent_dtor(&drm->parent);
> return ret;
> }
>
> @@ -737,6 +728,9 @@ nouveau_drm_device_del(struct nouveau_drm *drm)
> if (drm->dev)
> drm_dev_put(drm->dev);
>
> + nvif_client_dtor(&drm->master.base);
> + nvif_parent_dtor(&drm->parent);
> +
> kfree(drm);
> }
>
> @@ -753,6 +747,14 @@ nouveau_drm_device_new(const struct drm_driver *drm_driver, struct device *paren
>
> drm->nvkm = device;
>
> + nvif_parent_ctor(&nouveau_parent, &drm->parent);
> + drm->master.base.object.parent = &drm->parent;
> +
> + ret = nvif_driver_init(NULL, nouveau_config, nouveau_debug, "drm",
> + nouveau_name(parent), &drm->master.base);
> + if (ret)
> + goto done;
> +
> drm->dev = drm_dev_alloc(drm_driver, parent);
> if (IS_ERR(drm->dev)) {
> ret = PTR_ERR(drm->dev);
> --
> 2.45.1
>
