Hi guys, I already tried this a few month ago, but since I don't have NVidia hardware its rather hard to test for me (need to get some ordered). Dave brought up the topic that we should probably try to move the handling into Nouveau once more, so I tried to fix the problem Ben reported and rebased on top of current drm-misc-next. Dave can you test this? At least in theory the approach should work, but in practice there can of course be bugs. Thanks, Christian.
Christian König
2020-Aug-20 15:24 UTC
[Nouveau] [PATCH 1/2] drm/ttm: fix broken merge between drm-next and drm-misc-next
drm-next reverted the changes to ttm_tt_create() to do the
NULL check inside the function, but drm-misc-next adds new
users of this approach.
Re-apply the NULL check change inside the function to fix this.
Signed-off-by: Christian K?nig <christian.koenig at amd.com>
Reviewed-by: Dave Airlie <airlied at redhat.com>
Link: https://patchwork.freedesktop.org/patch/386628/
---
drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 97ac662a47cb..e3931e515906 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1180,7 +1180,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
/*
* We might need to add a TTM.
*/
- if (bo->mem.mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
+ if (bo->mem.mem_type == TTM_PL_SYSTEM) {
ret = ttm_tt_create(bo, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 9aa4fbe386e6..1ccf1ef050d6 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -50,6 +50,9 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool
zero_alloc)
dma_resv_assert_held(bo->base.resv);
+ if (bo->ttm)
+ return 0;
+
if (bdev->need_dma32)
page_flags |= TTM_PAGE_FLAG_DMA32;
@@ -67,7 +70,6 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool
zero_alloc)
page_flags |= TTM_PAGE_FLAG_SG;
break;
default:
- bo->ttm = NULL;
pr_err("Illegal buffer object type\n");
return -EINVAL;
}
--
2.17.1
Christian König
2020-Aug-20 15:24 UTC
[Nouveau] [PATCH 2/2] drm/nouveau: move io_reserve_lru handling into the driver v3
From: Christian K?nig <ckoenig.leichtzumerken at gmail.com>
While working on TTM cleanups I've found that the io_reserve_lru used by
Nouveau is actually not working at all.
In general we should remove driver specific handling from the memory
management, so this patch moves the io_reserve_lru handling into Nouveau
instead.
The patch should be functional correct, but is only compile tested!
v2: don't call ttm_bo_unmap_virtual in nouveau_ttm_io_mem_reserve
v3: rebased and use both base and offset in the check
Signed-off-by: Christian K?nig <christian.koenig at amd.com>
Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 100 +++++++++++++++++++++-----
drivers/gpu/drm/nouveau/nouveau_bo.h | 3 +
drivers/gpu/drm/nouveau/nouveau_drv.h | 2 +
drivers/gpu/drm/nouveau/nouveau_ttm.c | 44 +++++++++++-
4 files changed, 128 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 5392e5fea5d4..aa9fceaa79eb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -137,6 +137,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
struct nouveau_bo *nvbo = nouveau_bo(bo);
WARN_ON(nvbo->pin_refcnt > 0);
+ nouveau_bo_del_io_reserve_lru(bo);
nv10_bo_put_tile_region(dev, nvbo->tile, NULL);
/*
@@ -304,6 +305,7 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int
align, u32 flags,
nvbo->bo.mem.num_pages = size >> PAGE_SHIFT;
nouveau_bo_placement_set(nvbo, flags, 0);
+ INIT_LIST_HEAD(&nvbo->io_reserve_lru);
ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT, false,
@@ -574,6 +576,26 @@ nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo)
PAGE_SIZE, DMA_FROM_DEVICE);
}
+void nouveau_bo_add_io_reserve_lru(struct ttm_buffer_object *bo)
+{
+ struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
+ struct nouveau_bo *nvbo = nouveau_bo(bo);
+
+ mutex_lock(&drm->ttm.io_reserve_mutex);
+ list_move_tail(&nvbo->io_reserve_lru, &drm->ttm.io_reserve_lru);
+ mutex_unlock(&drm->ttm.io_reserve_mutex);
+}
+
+void nouveau_bo_del_io_reserve_lru(struct ttm_buffer_object *bo)
+{
+ struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
+ struct nouveau_bo *nvbo = nouveau_bo(bo);
+
+ mutex_lock(&drm->ttm.io_reserve_mutex);
+ list_del_init(&nvbo->io_reserve_lru);
+ mutex_unlock(&drm->ttm.io_reserve_mutex);
+}
+
int
nouveau_bo_validate(struct nouveau_bo *nvbo, bool interruptible,
bool no_wait_gpu)
@@ -888,6 +910,8 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool
evict,
if (bo->destroy != nouveau_bo_del_ttm)
return;
+ nouveau_bo_del_io_reserve_lru(bo);
+
if (mem && new_reg->mem_type != TTM_PL_SYSTEM &&
mem->mem.page == nvbo->page) {
list_for_each_entry(vma, &nvbo->vma_list, head) {
@@ -1018,12 +1042,43 @@ nouveau_bo_verify_access(struct ttm_buffer_object *bo,
struct file *filp)
filp->private_data);
}
+static void
+nouveau_ttm_io_mem_free_locked(struct nouveau_drm *drm,
+ struct ttm_resource *reg)
+{
+ struct nouveau_mem *mem = nouveau_mem(reg);
+
+ if (!reg->bus.base && !reg->bus.offset)
+ return; /* already freed */
+
+ if (drm->client.mem->oclass >= NVIF_CLASS_MEM_NV50) {
+ switch (reg->mem_type) {
+ case TTM_PL_TT:
+ if (mem->kind)
+ nvif_object_unmap_handle(&mem->mem.object);
+ break;
+ case TTM_PL_VRAM:
+ nvif_object_unmap_handle(&mem->mem.object);
+ break;
+ default:
+ break;
+ }
+ }
+ reg->bus.base = 0;
+ reg->bus.offset = 0;
+}
+
static int
nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource
*reg)
{
struct nouveau_drm *drm = nouveau_bdev(bdev);
struct nvkm_device *device = nvxx_device(&drm->client.device);
struct nouveau_mem *mem = nouveau_mem(reg);
+ struct nouveau_bo *nvbo;
+ int ret;
+
+ if (reg->bus.base || reg->bus.offset)
+ return 0; /* already mapped */
reg->bus.addr = NULL;
reg->bus.offset = 0;
@@ -1031,10 +1086,13 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
struct ttm_resource *reg)
reg->bus.base = 0;
reg->bus.is_iomem = false;
+ mutex_lock(&drm->ttm.io_reserve_mutex);
+retry:
switch (reg->mem_type) {
case TTM_PL_SYSTEM:
/* System memory */
- return 0;
+ ret = 0;
+ goto out;
case TTM_PL_TT:
#if IS_ENABLED(CONFIG_AGP)
if (drm->agp.bridge) {
@@ -1058,7 +1116,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
struct ttm_resource *reg)
} args;
u64 handle, length;
u32 argc = 0;
- int ret;
switch (mem->mem.object.oclass) {
case NVIF_CLASS_MEM_NV50:
@@ -1085,7 +1142,7 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
struct ttm_resource *reg)
if (ret != 1) {
if (WARN_ON(ret == 0))
return -EINVAL;
- return ret;
+ goto out;
}
reg->bus.base = 0;
@@ -1093,30 +1150,35 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
struct ttm_resource *reg)
}
break;
default:
- return -EINVAL;
+ ret = -EINVAL;
}
- return 0;
+
+out:
+ if (ret == -ENOSPC) {
+ nvbo = list_first_entry_or_null(&drm->ttm.io_reserve_lru,
+ typeof(*nvbo),
+ io_reserve_lru);
+ if (nvbo) {
+ list_del_init(&nvbo->io_reserve_lru);
+ drm_vma_node_unmap(&nvbo->bo.base.vma_node,
+ bdev->dev_mapping);
+ nouveau_ttm_io_mem_free_locked(drm, &nvbo->bo.mem);
+ goto retry;
+ }
+
+ }
+ mutex_unlock(&drm->ttm.io_reserve_mutex);
+ return ret;
}
static void
nouveau_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_resource *reg)
{
struct nouveau_drm *drm = nouveau_bdev(bdev);
- struct nouveau_mem *mem = nouveau_mem(reg);
- if (drm->client.mem->oclass >= NVIF_CLASS_MEM_NV50) {
- switch (reg->mem_type) {
- case TTM_PL_TT:
- if (mem->kind)
- nvif_object_unmap_handle(&mem->mem.object);
- break;
- case TTM_PL_VRAM:
- nvif_object_unmap_handle(&mem->mem.object);
- break;
- default:
- break;
- }
- }
+ mutex_lock(&drm->ttm.io_reserve_mutex);
+ nouveau_ttm_io_mem_free_locked(drm, reg);
+ mutex_unlock(&drm->ttm.io_reserve_mutex);
}
static int
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h
b/drivers/gpu/drm/nouveau/nouveau_bo.h
index aecb7481df0d..ae90aca33fef 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -18,6 +18,7 @@ struct nouveau_bo {
bool force_coherent;
struct ttm_bo_kmap_obj kmap;
struct list_head head;
+ struct list_head io_reserve_lru;
/* protected by ttm_bo_reserve() */
struct drm_file *reserved_by;
@@ -96,6 +97,8 @@ int nouveau_bo_validate(struct nouveau_bo *, bool
interruptible,
bool no_wait_gpu);
void nouveau_bo_sync_for_device(struct nouveau_bo *nvbo);
void nouveau_bo_sync_for_cpu(struct nouveau_bo *nvbo);
+void nouveau_bo_add_io_reserve_lru(struct ttm_buffer_object *bo);
+void nouveau_bo_del_io_reserve_lru(struct ttm_buffer_object *bo);
/* TODO: submit equivalent to TTM generic API upstream? */
static inline void __iomem *
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index f63ac72aa556..26a2c1090045 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -164,6 +164,8 @@ struct nouveau_drm {
int type_vram;
int type_host[2];
int type_ncoh[2];
+ struct mutex io_reserve_mutex;
+ struct list_head io_reserve_lru;
} ttm;
/* GEM interface support */
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 53c6f8827322..a62f37b1131c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -123,13 +123,51 @@ const struct ttm_resource_manager_func nv04_gart_manager =
{
.free = nouveau_manager_del,
};
+static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
+{
+ struct vm_area_struct *vma = vmf->vma;
+ struct ttm_buffer_object *bo = vma->vm_private_data;
+ pgprot_t prot;
+ vm_fault_t ret;
+
+ ret = ttm_bo_vm_reserve(bo, vmf);
+ if (ret)
+ return ret;
+
+ nouveau_bo_del_io_reserve_lru(bo);
+
+ prot = vm_get_page_prot(vma->vm_flags);
+ ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
+ if (ret == VM_FAULT_RETRY && !(vmf->flags &
FAULT_FLAG_RETRY_NOWAIT))
+ return ret;
+
+ nouveau_bo_add_io_reserve_lru(bo);
+
+ dma_resv_unlock(bo->base.resv);
+
+ return ret;
+}
+
+static struct vm_operations_struct nouveau_ttm_vm_ops = {
+ .fault = nouveau_ttm_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+ .access = ttm_bo_vm_access
+};
+
int
nouveau_ttm_mmap(struct file *filp, struct vm_area_struct *vma)
{
struct drm_file *file_priv = filp->private_data;
struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
+ int ret;
- return ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
+ ret = ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
+ if (ret)
+ return ret;
+
+ vma->vm_ops = &nouveau_ttm_vm_ops;
+ return 0;
}
static int
@@ -173,7 +211,6 @@ nouveau_ttm_init_vram(struct nouveau_drm *drm)
}
man->func = &nouveau_vram_manager;
- man->use_io_reserve_lru = true;
ttm_resource_manager_init(man,
drm->gem.vram_available >> PAGE_SHIFT);
@@ -339,6 +376,9 @@ nouveau_ttm_init(struct nouveau_drm *drm)
return ret;
}
+ mutex_init(&drm->ttm.io_reserve_mutex);
+ INIT_LIST_HEAD(&drm->ttm.io_reserve_lru);
+
NV_INFO(drm, "VRAM: %d MiB\n", (u32)(drm->gem.vram_available
>> 20));
NV_INFO(drm, "GART: %d MiB\n", (u32)(drm->gem.gart_available
>> 20));
return 0;
--
2.17.1
Christian König
2020-Aug-20 15:27 UTC
[Nouveau] [PATCH 1/2] drm/ttm: fix broken merge between drm-next and drm-misc-next
Please ignore this one, it's to hot here and I'm typing to fast :) Christian. Am 20.08.20 um 17:24 schrieb Christian K?nig:> drm-next reverted the changes to ttm_tt_create() to do the > NULL check inside the function, but drm-misc-next adds new > users of this approach. > > Re-apply the NULL check change inside the function to fix this. > > Signed-off-by: Christian K?nig <christian.koenig at amd.com> > Reviewed-by: Dave Airlie <airlied at redhat.com> > Link: https://patchwork.freedesktop.org/patch/386628/ > --- > drivers/gpu/drm/ttm/ttm_bo.c | 2 +- > drivers/gpu/drm/ttm/ttm_tt.c | 4 +++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 97ac662a47cb..e3931e515906 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -1180,7 +1180,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, > /* > * We might need to add a TTM. > */ > - if (bo->mem.mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { > + if (bo->mem.mem_type == TTM_PL_SYSTEM) { > ret = ttm_tt_create(bo, true); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index 9aa4fbe386e6..1ccf1ef050d6 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -50,6 +50,9 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc) > > dma_resv_assert_held(bo->base.resv); > > + if (bo->ttm) > + return 0; > + > if (bdev->need_dma32) > page_flags |= TTM_PAGE_FLAG_DMA32; > > @@ -67,7 +70,6 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc) > page_flags |= TTM_PAGE_FLAG_SG; > break; > default: > - bo->ttm = NULL; > pr_err("Illegal buffer object type\n"); > return -EINVAL; > }