[PATCH 03/34] drm/nouveau: move allocation of root client out of nouveau_cli_init()

[Ben Skeggs]

On 4/6/24 20:53, Danilo Krummrich wrote:

> On 5/27/24 16:19, 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.
>>
>> Another patch series will remove the need for the above-mentioned hack
>> entirely.
>
> How does this patch improve the situation for removing the master 
> instance later on?

I've added a couple more lines to the commit message.


>
>>
>> Signed-off-by: Ben Skeggs <bskeggs at nvidia.com>
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_drm.c | 23 +++++++++++++++--------
>>   1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
>> b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> index e6ed68dcef78..25b9c3b19bc0 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> @@ -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
>> @@ -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);
>> @@ -593,11 +592,16 @@ nouveau_drm_device_init(struct drm_device *dev, 
>> struct nvkm_device *nvkm)
>>       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(dev), &drm->master.base);
>> +    if (ret)
>> +        goto fail_alloc;
>
> Why move it above alloc_workqueue()?
>
>> +
>>       drm->sched_wq = alloc_workqueue("nouveau_sched_wq_shared", 0,
>>                       WQ_MAX_ACTIVE);
>>       if (!drm->sched_wq) {
>>           ret = -ENOMEM;
>> -        goto fail_alloc;
>> +        goto fail_nvif;
>>       }
>>         ret = nouveau_cli_init(drm, "DRM-master", &drm->master);
>> @@ -674,6 +678,8 @@ nouveau_drm_device_init(struct drm_device *dev, 
>> struct nvkm_device *nvkm)
>>       nouveau_cli_fini(&drm->master);
>>   fail_wq:
>>       destroy_workqueue(drm->sched_wq);
>> +fail_nvif:
>> +    nvif_client_dtor(&drm->master.base);
>>   fail_alloc:
>>       nvif_parent_dtor(&drm->parent);
>>       kfree(drm);
>> @@ -728,6 +734,7 @@ nouveau_drm_device_fini(struct drm_device *dev)
>>       nouveau_cli_fini(&drm->client);
>>       nouveau_cli_fini(&drm->master);
>>       destroy_workqueue(drm->sched_wq);
>> +    nvif_client_dtor(&drm->master.base);
>>       nvif_parent_dtor(&drm->parent);
>>       mutex_destroy(&drm->clients_lock);
>>       kfree(drm);
>