On Fri, 26 Jul 2024 14:38:25 +1000
Ben Skeggs <bskeggs@xxxxxxxxxx> wrote:
Can you paste a link that shows how the workaround got removed?
> The only remaining nouveau_drm.master struct member that's being used
> is the mutex that protects its object tree. Move that into
> nouveau_drm and remove nouveau_drm.master entirely.
>
> A pending series to remove the "ioctl" layer between DRM and NVKM also
> removes the need for object handle lookups, and hence this mutex, but
> it's still required for the moment.
>
> Signed-off-by: Ben Skeggs <bskeggs@xxxxxxxxxx>
> ---
> drivers/gpu/drm/nouveau/nouveau_drm.c | 39
> +++++++++++---------------- drivers/gpu/drm/nouveau/nouveau_drv.h |
> 3 ++- drivers/gpu/drm/nouveau/nouveau_mem.c | 12 ++++-----
> 3 files changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c index
> 8b9c463d21ac..ca63d13efb25 100644 ---
> a/drivers/gpu/drm/nouveau/nouveau_drm.c +++
> b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -208,11 +208,9 @@
> nouveau_cli_fini(struct nouveau_cli *cli) nvif_mmu_dtor(&cli->mmu);
> cli->device.object.map.ptr = NULL;
> nvif_device_dtor(&cli->device);
> - if (cli != &cli->drm->master) {
> - mutex_lock(&cli->drm->master.lock);
> - nvif_client_dtor(&cli->base);
> - mutex_unlock(&cli->drm->master.lock);
> - }
> + mutex_lock(&cli->drm->client_mutex);
> + nvif_client_dtor(&cli->base);
> + mutex_unlock(&cli->drm->client_mutex);
> }
>
> static int
> @@ -245,11 +243,9 @@ nouveau_cli_init(struct nouveau_drm *drm, const
> char *sname, INIT_LIST_HEAD(&cli->worker);
> mutex_init(&cli->lock);
>
> - if (cli != &drm->master) {
> - mutex_lock(&drm->master.lock);
> - ret = nvif_client_ctor(&drm->master.base, cli->name,
> &cli->base);
> - mutex_unlock(&drm->master.lock);
> - }
> + mutex_lock(&drm->client_mutex);
> + ret = nvif_client_ctor(&drm->_client, cli->name, &cli->base);
> + mutex_unlock(&drm->client_mutex);
> if (ret) {
> NV_PRINTK(err, cli, "Client allocation failed:
> %d\n", ret); goto done;
> @@ -602,7 +598,6 @@ nouveau_drm_device_fini(struct nouveau_drm *drm)
> mutex_unlock(&drm->clients_lock);
>
> nouveau_cli_fini(&drm->client);
> - nouveau_cli_fini(&drm->master);
> destroy_workqueue(drm->sched_wq);
> mutex_destroy(&drm->clients_lock);
> }
> @@ -618,13 +613,9 @@ nouveau_drm_device_init(struct nouveau_drm *drm)
> if (!drm->sched_wq)
> return -ENOMEM;
>
> - ret = nouveau_cli_init(drm, "DRM-master", &drm->master);
> - if (ret)
> - goto fail_wq;
> -
> ret = nouveau_cli_init(drm, "DRM", &drm->client);
> if (ret)
> - goto fail_master;
> + goto fail_wq;
>
> INIT_LIST_HEAD(&drm->clients);
> mutex_init(&drm->clients_lock);
> @@ -691,8 +682,6 @@ nouveau_drm_device_init(struct nouveau_drm *drm)
> fail_ttm:
> nouveau_vga_fini(drm);
> nouveau_cli_fini(&drm->client);
> -fail_master:
> - nouveau_cli_fini(&drm->master);
> fail_wq:
> destroy_workqueue(drm->sched_wq);
> return ret;
> @@ -706,9 +695,10 @@ nouveau_drm_device_del(struct nouveau_drm *drm)
>
> nvif_mmu_dtor(&drm->mmu);
> nvif_device_dtor(&drm->device);
> - nvif_client_dtor(&drm->master.base);
> + nvif_client_dtor(&drm->_client);
> nvif_parent_dtor(&drm->parent);
>
> + mutex_destroy(&drm->client_mutex);
> kfree(drm);
> }
>
> @@ -742,14 +732,15 @@ nouveau_drm_device_new(const struct drm_driver
> *drm_driver, struct device *paren dev_set_drvdata(parent, drm);
>
> nvif_parent_ctor(&nouveau_parent, &drm->parent);
> - drm->master.base.object.parent = &drm->parent;
> + mutex_init(&drm->client_mutex);
> + drm->_client.object.parent = &drm->parent;
>
> ret = nvif_driver_init(NULL, nouveau_config, nouveau_debug,
> "drm",
> - nouveau_name(drm->dev),
> &drm->master.base);
> + nouveau_name(drm->dev),
> &drm->_client); if (ret)
> goto done;
>
> - ret = nvif_device_ctor(&drm->master.base, "drmDevice",
> &drm->device);
> + ret = nvif_device_ctor(&drm->_client, "drmDevice",
> &drm->device); if (ret) {
> NV_ERROR(drm, "Device allocation failed: %d\n", ret);
> goto done;
> @@ -966,7 +957,7 @@ nouveau_do_suspend(struct nouveau_drm *drm, bool
> runtime) }
>
> NV_DEBUG(drm, "suspending object tree...\n");
> - ret = nvif_client_suspend(&drm->master.base);
> + ret = nvif_client_suspend(&drm->_client);
> if (ret)
> goto fail_client;
>
> @@ -991,7 +982,7 @@ nouveau_do_resume(struct nouveau_drm *drm, bool
> runtime) int ret = 0;
>
> NV_DEBUG(drm, "resuming object tree...\n");
> - ret = nvif_client_resume(&drm->master.base);
> + ret = nvif_client_resume(&drm->_client);
> if (ret) {
> NV_ERROR(drm, "Client resume failed with error:
> %d\n", ret); return ret;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
> b/drivers/gpu/drm/nouveau/nouveau_drv.h index
> c55665faf2c1..f3da66a12dd3 100644 ---
> a/drivers/gpu/drm/nouveau/nouveau_drv.h +++
> b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -203,10 +203,11 @@
> u_memcpya(uint64_t user, unsigned int nmemb, unsigned int size)
> struct nouveau_drm { struct nvkm_device *nvkm;
> struct nvif_parent parent;
> + struct mutex client_mutex;
> + struct nvif_client _client;
> struct nvif_device device;
> struct nvif_mmu mmu;
>
> - struct nouveau_cli master;
> struct nouveau_cli client;
> struct drm_device *dev;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.c
> b/drivers/gpu/drm/nouveau/nouveau_mem.c index
> b112b62dca3c..fac92fdbf9cc 100644 ---
> a/drivers/gpu/drm/nouveau/nouveau_mem.c +++
> b/drivers/gpu/drm/nouveau/nouveau_mem.c @@ -80,9 +80,9 @@
> nouveau_mem_fini(struct nouveau_mem *mem) {
> nvif_vmm_put(&mem->drm->client.vmm.vmm, &mem->vma[1]);
> nvif_vmm_put(&mem->drm->client.vmm.vmm, &mem->vma[0]);
> - mutex_lock(&mem->drm->master.lock);
> + mutex_lock(&mem->drm->client_mutex);
> nvif_mem_dtor(&mem->mem);
> - mutex_unlock(&mem->drm->master.lock);
> + mutex_unlock(&mem->drm->client_mutex);
> }
>
> int
> @@ -113,11 +113,11 @@ nouveau_mem_host(struct ttm_resource *reg,
> struct ttm_tt *tt) else
> args.dma = tt->dma_address;
>
> - mutex_lock(&drm->master.lock);
> + mutex_lock(&drm->client_mutex);
> ret = nvif_mem_ctor_type(mmu, "ttmHostMem", mmu->mem, type,
> PAGE_SHIFT, reg->size,
> &args, sizeof(args), &mem->mem);
> - mutex_unlock(&drm->master.lock);
> + mutex_unlock(&drm->client_mutex);
> return ret;
> }
>
> @@ -130,7 +130,7 @@ nouveau_mem_vram(struct ttm_resource *reg, bool
> contig, u8 page) u64 size = ALIGN(reg->size, 1 << page);
> int ret;
>
> - mutex_lock(&drm->master.lock);
> + mutex_lock(&drm->client_mutex);
> switch (mmu->mem) {
> case NVIF_CLASS_MEM_GF100:
> ret = nvif_mem_ctor_type(mmu, "ttmVram", mmu->mem,
> @@ -154,7 +154,7 @@ nouveau_mem_vram(struct ttm_resource *reg, bool
> contig, u8 page) WARN_ON(1);
> break;
> }
> - mutex_unlock(&drm->master.lock);
> + mutex_unlock(&drm->client_mutex);
>
> reg->start = mem->mem.addr >> PAGE_SHIFT;
> return ret;
