Re: [PATCH v2 34/37] drm/nouveau: remove master

[Ben Skeggs <bskeggs@xxxxxxxxxx>]

On 10/7/24 02:38, Danilo Krummrich wrote:

> On Fri, Jul 05, 2024 at 04:37:18AM +1000, Ben Skeggs wrote:
> > 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.
> >
> > Another patch series will make it possible to also remove the mutex, but
> > it's still required for the moment.
> It would be good to also mention what we're left with to protect and how we get
> rid of that later on.
I already mention what's left to protect?  I've modified the commit 
message to mention the remove-ioctl series regardless.
> > 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 c6c9a528783a..85214d35fd5d 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);
> >   }
> >   
> > @@ -733,14 +723,15 @@ 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;
> > +	mutex_init(&drm->client_mutex);
> > +	drm->_client.object.parent = &drm->parent;
> >   
> >   	ret = nvif_driver_init(NULL, nouveau_config, nouveau_debug, "drm",
> > -			       nouveau_name(parent), &drm->master.base);
> > +			       nouveau_name(parent), &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 2535a50b99f3..630463668a76 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;
> > --
> > 2.45.1