On Sat, 22 Aug 2020 at 02:01, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> 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.
>
> 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
> v4: fix small typos and test the patch
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/nouveau/nouveau_bo.c | 111 ++++++++++++++++++++------
> 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, 135 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 5392e5fea5d4..ee0e135ddcbb 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,23 +1042,54 @@ 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);
> + int ret;
> +
> + if (reg->bus.base || reg->bus.offset)
> + return 0; /* already mapped */
>
> reg->bus.addr = NULL;
> - reg->bus.offset = 0;
> reg->bus.size = reg->num_pages << PAGE_SHIFT;
> - 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) {
> @@ -1043,9 +1098,12 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *reg)
> reg->bus.is_iomem = !drm->agp.cma;
> }
> #endif
> - if (drm->client.mem->oclass < NVIF_CLASS_MEM_NV50 || !mem->kind)
> + if (drm->client.mem->oclass < NVIF_CLASS_MEM_NV50 ||
> + !mem->kind) {
> /* untiled */
> + ret = 0;
> break;
> + }
> fallthrough; /* tiled memory */
> case TTM_PL_VRAM:
> reg->bus.offset = reg->start << PAGE_SHIFT;
> @@ -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:
> @@ -1084,39 +1141,47 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_resource *reg)
> &handle, &length);
> if (ret != 1) {
> if (WARN_ON(ret == 0))
> - return -EINVAL;
> - return ret;
> + ret = -EINVAL;
> + goto out;
> }
>
> reg->bus.base = 0;
> reg->bus.offset = handle;
> + ret = 0;
> }
> break;
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> }
> - return 0;
> +
> +out:
> + if (ret == -ENOSPC) {
> + struct nouveau_bo *nvbo;
> +
> + 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 = {
should this be const?
Dave.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
