On Fri, 26 Jul 2024 14:38:15 +1000
Ben Skeggs <bskeggs at nvidia.com> wrote:
Reviewed-by: Zhi Wang <zhiw at nvidia.com>
I agree that to always map the device so that we can remove extra rd/wr
callbacks in patch 25 and patch 26.
Would you mind to point me the patch you mentioned to clean up this
later as well? I am interested to see the fate of this newly added
function.
> The next commit removes the nvif rd/wr methods from nvif_device, which
> were probably a bad idea, and mostly intended as a fallback if
> ioremap() failed (or wasn't available, as was the case in some tools
> I once used).
>
> The nv04 KMS driver already mapped the device, because it's mostly
> been kept alive on life-support for many years and still directly
> bashes PRI a lot for modesetting.
>
> Post-nv50, I tried pretty hard to keep PRI accesses out of the DRM
> code, but there's still a few random places where we do, and those
> were using the rd/wr paths prior to this commit.
>
> This allocates and maps a new nvif_device (which will replace the
> usage of nouveau_drm.master.device later on), and replicates this
> pointer to all other possible users.
>
> This will be cleaned up by the end of another patch series, after it's
> been made safe to do so.
>
> Signed-off-by: Ben Skeggs <bskeggs at nvidia.com>
> ---
> drivers/gpu/drm/nouveau/dispnv04/disp.c | 5 -----
> drivers/gpu/drm/nouveau/include/nvif/device.h | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 16 ++++++++++++++++
> drivers/gpu/drm/nouveau/nouveau_drv.h | 2 ++
> drivers/gpu/drm/nouveau/nvif/device.c | 6 ++++++
> 5 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c index
> 13705c5f1497..e8b27bb135e7 100644 ---
> a/drivers/gpu/drm/nouveau/dispnv04/disp.c +++
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c @@ -189,7 +189,6 @@ static
> void nv04_display_destroy(struct drm_device *dev)
> {
> struct nv04_display *disp = nv04_display(dev);
> - struct nouveau_drm *drm = nouveau_drm(dev);
> struct nouveau_encoder *encoder;
> struct nouveau_crtc *nv_crtc;
>
> @@ -206,8 +205,6 @@ nv04_display_destroy(struct drm_device *dev)
>
> nouveau_display(dev)->priv = NULL;
> vfree(disp);
> -
> - nvif_object_unmap(&drm->client.device.object);
> }
>
> int
> @@ -229,8 +226,6 @@ nv04_display_create(struct drm_device *dev)
>
> disp->drm = drm;
>
> - nvif_object_map(&drm->client.device.object, NULL, 0);
> -
> nouveau_display(dev)->priv = disp;
> nouveau_display(dev)->dtor = nv04_display_destroy;
> nouveau_display(dev)->init = nv04_display_init;
> diff --git a/drivers/gpu/drm/nouveau/include/nvif/device.h
> b/drivers/gpu/drm/nouveau/include/nvif/device.h index
> f7c1b3a43c84..fec76f4733a4 100644 ---
> a/drivers/gpu/drm/nouveau/include/nvif/device.h +++
> b/drivers/gpu/drm/nouveau/include/nvif/device.h @@ -20,6 +20,7 @@
> struct nvif_device {
> int nvif_device_ctor(struct nvif_client *, const char *name, struct
> nvif_device *); void nvif_device_dtor(struct nvif_device *);
> +int nvif_device_map(struct nvif_device *);
> u64 nvif_device_time(struct nvif_device *);
>
> /*XXX*/
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c index
> 22cdd987dd2f..316f7877047d 100644 ---
> a/drivers/gpu/drm/nouveau/nouveau_drm.c +++
> b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -206,6 +206,7 @@
> nouveau_cli_fini(struct nouveau_cli *cli) nouveau_vmm_fini(&cli->svm);
> nouveau_vmm_fini(&cli->vmm);
> 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);
> @@ -267,6 +268,8 @@ nouveau_cli_init(struct nouveau_drm *drm, const
> char *sname, goto done;
> }
>
> + cli->device.object.map.ptr = drm->device.object.map.ptr;
> +
> ret = nvif_mclass(&cli->device.object, mmus);
> if (ret < 0) {
> NV_PRINTK(err, cli, "No supported MMU class\n");
> @@ -715,6 +718,7 @@ nouveau_drm_device_del(struct nouveau_drm *drm)
> if (drm->dev)
> drm_dev_put(drm->dev);
>
> + nvif_device_dtor(&drm->device);
> nvif_client_dtor(&drm->master.base);
> nvif_parent_dtor(&drm->parent);
>
> @@ -751,6 +755,18 @@ nouveau_drm_device_new(const struct drm_driver
> *drm_driver, struct device *paren if (ret)
> goto done;
>
> + ret = nvif_device_ctor(&drm->master.base, "drmDevice",
> &drm->device);
> + if (ret) {
> + NV_ERROR(drm, "Device allocation failed: %d\n", ret);
> + goto done;
> + }
> +
> + ret = nvif_device_map(&drm->device);
> + if (ret) {
> + NV_ERROR(drm, "Failed to map PRI: %d\n", ret);
> + goto done;
> + }
> +
> done:
> if (ret) {
> nouveau_drm_device_del(drm);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
> b/drivers/gpu/drm/nouveau/nouveau_drv.h index
> e7d072a9a477..80ffe15ba76b 100644 ---
> a/drivers/gpu/drm/nouveau/nouveau_drv.h +++
> b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -203,6 +203,8 @@
> u_memcpya(uint64_t user, unsigned int nmemb, unsigned int size)
> struct nouveau_drm { struct nvkm_device *nvkm;
> struct nvif_parent parent;
> + struct nvif_device device;
> +
> struct nouveau_cli master;
> struct nouveau_cli client;
> struct drm_device *dev;
> diff --git a/drivers/gpu/drm/nouveau/nvif/device.c
> b/drivers/gpu/drm/nouveau/nvif/device.c index
> b14bccb9a93f..24880931039f 100644 ---
> a/drivers/gpu/drm/nouveau/nvif/device.c +++
> b/drivers/gpu/drm/nouveau/nvif/device.c @@ -38,6 +38,12 @@
> nvif_device_time(struct nvif_device *device) return
> device->user.func->time(&device->user); }
>
> +int
> +nvif_device_map(struct nvif_device *device)
> +{
> + return nvif_object_map(&device->object, NULL, 0);
> +}
> +
> void
> nvif_device_dtor(struct nvif_device *device)
> {