[PATCH 02/34] drm/nouveau: store nvkm_device pointer in nouveau_drm

[Ben Skeggs]

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

> On 5/27/24 16:19, Ben Skeggs wrote:
>> There's various different places in the drm code that get at the
>> nvkm_device via various creative (and not very type-safe) means.
>>
>> One of those being via nvif_device.object.priv.
>>
>> Another patch series is going to entirely remove the ioctl-like
>> interfaces beween the drm code and nvkm, and that field will no
>> longer exist.
>>
>> This provides a safer replacement for accessing the nvkm_device,
>> and will used more in upcoming patches to cleanup other cases.
>>
>> Signed-off-by: Ben Skeggs <bskeggs at nvidia.com>
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_drm.c | 13 +++++--------
>>   drivers/gpu/drm/nouveau/nouveau_drv.h |  1 +
>>   2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
>> b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> index c37798b507ea..e6ed68dcef78 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> @@ -579,7 +579,7 @@ nouveau_parent = {
>>   };
>>     static int
>> -nouveau_drm_device_init(struct drm_device *dev)
>> +nouveau_drm_device_init(struct drm_device *dev, struct nvkm_device 
>> *nvkm)
>>   {
>>       struct nouveau_drm *drm;
>>       int ret;
>> @@ -588,6 +588,7 @@ nouveau_drm_device_init(struct drm_device *dev)
>>           return -ENOMEM;
>>       dev->dev_private = drm;
>>       drm->dev = dev;
>> +    drm->nvkm = nvkm;
>>         nvif_parent_ctor(&nouveau_parent, &drm->parent);
>>       drm->master.base.object.parent = &drm->parent;
>> @@ -830,7 +831,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
>>         pci_set_drvdata(pdev, drm_dev);
>>   -    ret = nouveau_drm_device_init(drm_dev);
>> +    ret = nouveau_drm_device_init(drm_dev, device);
>
> NIT (and actually not directly related to this patch): It seems that we
> randomly switch between 'dev', 'drm_dev' for struct drm_device and
> 'device', 'nvkm' for nvkm_device. Since this is a cleanup series, mind
> adding a patch to get this straight?

Yeah, this was annoying me (again) recently.  I haven't fixed all of it, 
but I pulled in a patch from another series, and added another, to 
address a bunch of the "drm_dev" usage.  I think it'd be good to look at 
this again after landing all the various other cleanups (some of which 
end up removing other cases), and cleaning up whatever's left then.


>
>>       if (ret)
>>           goto fail_pci;
>>   @@ -861,14 +862,10 @@ void
>>   nouveau_drm_device_remove(struct drm_device *dev)
>>   {
>>       struct nouveau_drm *drm = nouveau_drm(dev);
>> -    struct nvkm_client *client;
>> -    struct nvkm_device *device;
>> +    struct nvkm_device *device = drm->nvkm;
>>         drm_dev_unplug(dev);
>>   -    client = nvxx_client(&drm->client.base);
>> -    device = nvkm_device_find(client->device);
>> -
>>       nouveau_drm_device_fini(dev);
>>       drm_dev_put(dev);
>>       nvkm_device_del(&device);
>> @@ -1376,7 +1373,7 @@ nouveau_platform_device_create(const struct 
>> nvkm_device_tegra_func *func,
>>           goto err_free;
>>       }
>>   -    err = nouveau_drm_device_init(drm);
>> +    err = nouveau_drm_device_init(drm, *pdevice);
>
> Same here, 'pdev' for struct platform_device and 'pdevice' for a ** to a
> struct nvkm_device seems confusing.
>
>>       if (err)
>>           goto err_put;
>>   diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h 
>> b/drivers/gpu/drm/nouveau/nouveau_drv.h
>> index e239c6bf4afa..b711e994407b 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
>> @@ -201,6 +201,7 @@ u_memcpya(uint64_t user, unsigned int nmemb, 
>> unsigned int size)
>>   #include <nvif/parent.h>
>>     struct nouveau_drm {
>> +    struct nvkm_device *nvkm;
>>       struct nvif_parent parent;
>>       struct nouveau_cli master;
>>       struct nouveau_cli client;
>