| Message ID | 20190805140119.7337-9-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: make ttm bo a gem bo subclass | expand |
On Mon, Aug 05, 2019 at 04:01:10PM +0200, Gerd Hoffmann wrote: > Drop vma_node from ttm_buffer_object, use the gem struct > (base.vma_node) instead. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +- > drivers/gpu/drm/qxl/qxl_object.h | 2 +- > drivers/gpu/drm/radeon/radeon_object.h | 2 +- > drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- > include/drm/ttm/ttm_bo_api.h | 4 ---- > drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- > drivers/gpu/drm/ttm/ttm_bo.c | 8 ++++---- > drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 9 +++++---- > drivers/gpu/drm/virtio/virtgpu_prime.c | 3 --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 4 ++-- > 14 files changed, 21 insertions(+), 27 deletions(-) Hi Gerd, I've been seeing a regression on Nouveau with recent linux-next releases and git bisect points at this commit as the first bad one. If I revert it (there's a tiny conflict with a patch that was merged subsequently), things are back to normal. I think the reason for this issue is that Nouveau doesn't use GEM objects for all buffer objects, and even when it uses GEM objects, the code will not initialize the GEM object until after the buffer objects and the backing TTM objects have been created. I tried to fix that by making sure drm_gem_object_init() gets called by Nouveau before ttm_bo_init(), but the changes are fairly involved and I was unable to get the GEM reference counting right. I can look into the proper fix some more, but it might be worth reverting this patch for now to get Nouveau working again. Thierry > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > index 645a189d365c..113fb2feb437 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > @@ -191,7 +191,7 @@ static inline unsigned amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo) > */ > static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo) > { > - return drm_vma_node_offset_addr(&bo->tbo.vma_node); > + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); > } > > /** > diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h > index b812d4ae9d0d..8ae54ba7857c 100644 > --- a/drivers/gpu/drm/qxl/qxl_object.h > +++ b/drivers/gpu/drm/qxl/qxl_object.h > @@ -60,7 +60,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo) > > static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) > { > - return drm_vma_node_offset_addr(&bo->tbo.vma_node); > + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); > } > > static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, > diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h > index 9ffd8215d38a..e5554bf9140e 100644 > --- a/drivers/gpu/drm/radeon/radeon_object.h > +++ b/drivers/gpu/drm/radeon/radeon_object.h > @@ -116,7 +116,7 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo) > */ > static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) > { > - return drm_vma_node_offset_addr(&bo->tbo.vma_node); > + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); > } > > extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index f4ecea6054ba..e28829661724 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -396,7 +396,7 @@ static inline void virtio_gpu_object_unref(struct virtio_gpu_object **bo) > > static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) > { > - return drm_vma_node_offset_addr(&bo->tbo.vma_node); > + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); > } > > static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo, > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index fa050f0328ab..7ffc50a3303d 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++ b/include/drm/ttm/ttm_bo_api.h > @@ -152,7 +152,6 @@ struct ttm_tt; > * @ddestroy: List head for the delayed destroy list. > * @swap: List head for swap LRU list. > * @moving: Fence set when BO is moving > - * @vma_node: Address space manager node. > * @offset: The current GPU offset, which can have different meanings > * depending on the memory type. For SYSTEM type memory, it should be 0. > * @cur_placement: Hint of current placement. > @@ -219,9 +218,6 @@ struct ttm_buffer_object { > */ > > struct dma_fence *moving; > - > - struct drm_vma_offset_node vma_node; > - > unsigned priority; > > /** > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > index fc13920b3cb4..fd751078bae1 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -168,7 +168,7 @@ EXPORT_SYMBOL(drm_gem_vram_put); > */ > u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo) > { > - return drm_vma_node_offset_addr(&gbo->bo.vma_node); > + return drm_vma_node_offset_addr(&gbo->bo.base.vma_node); > } > EXPORT_SYMBOL(drm_gem_vram_mmap_offset); > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c > index fc8f5bb73ca8..98afc50162e9 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > @@ -675,7 +675,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv, > gem = drm_gem_object_lookup(file_priv, handle); > if (gem) { > struct nouveau_bo *bo = nouveau_gem_object(gem); > - *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node); > + *poffset = drm_vma_node_offset_addr(&bo->bo.base.vma_node); > drm_gem_object_put_unlocked(gem); > return 0; > } > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c > index 2f484ab7dbca..b1e4852810ed 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c > @@ -240,7 +240,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, > } > > rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT; > - rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node); > + rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.base.vma_node); > rep->tile_mode = nvbo->mode; > rep->tile_flags = nvbo->contig ? 0 : NOUVEAU_GEM_TILE_NONCONTIG; > if (cli->device.info.family >= NV_DEVICE_INFO_V0_FERMI) > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index ceff153f7e68..3e0a0cbc410e 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -672,7 +672,7 @@ static void ttm_bo_release(struct kref *kref) > struct ttm_bo_device *bdev = bo->bdev; > struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type]; > > - drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node); > + drm_vma_offset_remove(&bdev->vma_manager, &bo->base.vma_node); > ttm_mem_io_lock(man, false); > ttm_mem_io_free_vm(bo); > ttm_mem_io_unlock(man); > @@ -1343,9 +1343,9 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, > * struct elements we want use regardless. > */ > reservation_object_init(&bo->base._resv); > + drm_vma_node_reset(&bo->base.vma_node); > } > atomic_inc(&bo->bdev->glob->bo_count); > - drm_vma_node_reset(&bo->vma_node); > > /* > * For ttm_bo_type_device buffers, allocate > @@ -1353,7 +1353,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, > */ > if (bo->type == ttm_bo_type_device || > bo->type == ttm_bo_type_sg) > - ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node, > + ret = drm_vma_offset_add(&bdev->vma_manager, &bo->base.vma_node, > bo->mem.num_pages); > > /* passed reservation objects should already be locked, > @@ -1781,7 +1781,7 @@ void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo) > { > struct ttm_bo_device *bdev = bo->bdev; > > - drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping); > + drm_vma_node_unmap(&bo->base.vma_node, bdev->dev_mapping); > ttm_mem_io_free_vm(bo); > } > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 05fbcaf6a3f2..f5009c1b6a9c 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -510,7 +510,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, > INIT_LIST_HEAD(&fbo->base.io_reserve_lru); > mutex_init(&fbo->base.wu_mutex); > fbo->base.moving = NULL; > - drm_vma_node_reset(&fbo->base.vma_node); > + drm_vma_node_reset(&fbo->base.base.vma_node); > atomic_set(&fbo->base.cpu_writers, 0); > > kref_init(&fbo->base.list_kref); > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 6dacff49c1cc..fb6875a789b7 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -211,9 +211,9 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) > } > > page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) + > - vma->vm_pgoff - drm_vma_node_start(&bo->vma_node); > + vma->vm_pgoff - drm_vma_node_start(&bo->base.vma_node); > page_last = vma_pages(vma) + vma->vm_pgoff - > - drm_vma_node_start(&bo->vma_node); > + drm_vma_node_start(&bo->base.vma_node); > > if (unlikely(page_offset >= bo->num_pages)) { > ret = VM_FAULT_SIGBUS; > @@ -267,7 +267,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) > } else if (unlikely(!page)) { > break; > } > - page->index = drm_vma_node_start(&bo->vma_node) + > + page->index = drm_vma_node_start(&bo->base.vma_node) + > page_offset; > pfn = page_to_pfn(page); > } > @@ -413,7 +413,8 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, > > node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages); > if (likely(node)) { > - bo = container_of(node, struct ttm_buffer_object, vma_node); > + bo = container_of(node, struct ttm_buffer_object, > + base.vma_node); > bo = ttm_bo_get_unless_zero(bo); > } > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > index 8b3b2caf3364..dc642a884b88 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > @@ -68,8 +68,5 @@ void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) > int virtgpu_gem_prime_mmap(struct drm_gem_object *obj, > struct vm_area_struct *vma) > { > - struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); > - > - bo->gem_base.vma_node.vm_node.start = bo->tbo.vma_node.vm_node.start; > return drm_gem_prime_mmap(obj, vma); > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index 315da41a18b4..5739c6c49c99 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -835,7 +835,7 @@ int vmw_bo_alloc_ioctl(struct drm_device *dev, void *data, > goto out_no_bo; > > rep->handle = handle; > - rep->map_handle = drm_vma_node_offset_addr(&vbo->base.vma_node); > + rep->map_handle = drm_vma_node_offset_addr(&vbo->base.base.vma_node); > rep->cur_gmr_id = handle; > rep->cur_gmr_offset = 0; > > @@ -1077,7 +1077,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv, > if (ret != 0) > return -EINVAL; > > - *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node); > + *offset = drm_vma_node_offset_addr(&out_buf->base.base.vma_node); > vmw_bo_unreference(&out_buf); > return 0; > } > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > index 219471903bc1..3a6da3b66484 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -1669,7 +1669,7 @@ vmw_gb_surface_define_internal(struct drm_device *dev, > rep->backup_size = res->backup_size; > if (res->backup) { > rep->buffer_map_handle = > - drm_vma_node_offset_addr(&res->backup->base.vma_node); > + drm_vma_node_offset_addr(&res->backup->base.base.vma_node); > rep->buffer_size = res->backup->base.num_pages * PAGE_SIZE; > rep->buffer_handle = backup_handle; > } else { > @@ -1745,7 +1745,7 @@ vmw_gb_surface_reference_internal(struct drm_device *dev, > rep->crep.backup_size = srf->res.backup_size; > rep->crep.buffer_handle = backup_handle; > rep->crep.buffer_map_handle = > - drm_vma_node_offset_addr(&srf->res.backup->base.vma_node); > + drm_vma_node_offset_addr(&srf->res.backup->base.base.vma_node); > rep->crep.buffer_size = srf->res.backup->base.num_pages * PAGE_SIZE; > > rep->creq.version = drm_vmw_gb_surface_v1; > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> Hi Gerd, > > I've been seeing a regression on Nouveau with recent linux-next releases > and git bisect points at this commit as the first bad one. If I revert > it (there's a tiny conflict with a patch that was merged subsequently), > things are back to normal. > > I think the reason for this issue is that Nouveau doesn't use GEM > objects for all buffer objects, That shouldn't be a problem ... > and even when it uses GEM objects, the > code will not initialize the GEM object until after the buffer objects > and the backing TTM objects have been created. ... but the initialization order is. ttm_bo_uses_embedded_gem_object() assumes gem gets initialized first. drm_gem_object_init() init calling drm_vma_node_reset() again is probably the root cause for the breakage. > I tried to fix that by making sure drm_gem_object_init() gets called by > Nouveau before ttm_bo_init(), but the changes are fairly involved and I > was unable to get the GEM reference counting right. I can look into the > proper fix some more, but it might be worth reverting this patch for > now to get Nouveau working again. Changing the order doesn't look hard. Patch attached (untested, have no test hardware). But maybe I missed some detail ... The other patch attached works around the issue with a flag, to avoid drm_vma_node_reset() being called twice. cheers, Gerd From af43f933533140e2df58176a68df0c60ba082273 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann <kraxel@redhat.com> Date: Wed, 14 Aug 2019 07:23:31 +0200 Subject: [PATCH 1/2] try unbreak nouveau #1 --- include/drm/drm_gem.h | 11 +++++++++++ drivers/gpu/drm/drm_gem.c | 6 ++++-- drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index ae693c0666cd..24e8fc58a3e1 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -297,6 +297,17 @@ struct drm_gem_object { * */ const struct drm_gem_object_funcs *funcs; + + /** + * @ttm_init: indicate ttm has initialized _resv and vma_node fields. + * + * ttm_bo_uses_embedded_gem_object() assumes gem is + * initialized before ttm, nouveau does it the other way + * around though. + * + * This is a temporary stopgap to handle that case. + */ + bool ttm_init; }; /** diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index afc38cece3f5..0a75d8cf7ac7 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -159,11 +159,13 @@ void drm_gem_private_object_init(struct drm_device *dev, kref_init(&obj->refcount); obj->handle_count = 0; obj->size = size; - reservation_object_init(&obj->_resv); if (!obj->resv) obj->resv = &obj->_resv; - drm_vma_node_reset(&obj->vma_node); + if (!obj->ttm_init) { + reservation_object_init(&obj->_resv); + drm_vma_node_reset(&obj->vma_node); + } } EXPORT_SYMBOL(drm_gem_private_object_init); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 10a861a1690c..83b389fc117e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -160,7 +160,7 @@ static void ttm_bo_release_list(struct kref *list_kref) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->bdev->glob->bo_count); dma_fence_put(bo->moving); - if (!ttm_bo_uses_embedded_gem_object(bo)) + if (bo->base.ttm_init) reservation_object_fini(&bo->base._resv); mutex_destroy(&bo->wu_mutex); bo->destroy(bo); @@ -1344,6 +1344,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, */ reservation_object_init(&bo->base._resv); drm_vma_node_reset(&bo->base.vma_node); + bo->base.ttm_init = true; } atomic_inc(&bo->bdev->glob->bo_count);
On Wed, Aug 14, 2019 at 07:58:27AM +0200, Gerd Hoffmann wrote: > > Hi Gerd, > > > > I've been seeing a regression on Nouveau with recent linux-next releases > > and git bisect points at this commit as the first bad one. If I revert > > it (there's a tiny conflict with a patch that was merged subsequently), > > things are back to normal. > > > > I think the reason for this issue is that Nouveau doesn't use GEM > > objects for all buffer objects, > > That shouldn't be a problem ... > > > and even when it uses GEM objects, the > > code will not initialize the GEM object until after the buffer objects > > and the backing TTM objects have been created. > > ... but the initialization order is. > > ttm_bo_uses_embedded_gem_object() assumes gem gets initialized first. > > drm_gem_object_init() init calling drm_vma_node_reset() again is > probably the root cause for the breakage. > > > I tried to fix that by making sure drm_gem_object_init() gets called by > > Nouveau before ttm_bo_init(), but the changes are fairly involved and I > > was unable to get the GEM reference counting right. I can look into the > > proper fix some more, but it might be worth reverting this patch for > > now to get Nouveau working again. > > Changing the order doesn't look hard. Patch attached (untested, have no > test hardware). But maybe I missed some detail ... > > The other patch attached works around the issue with a flag, to avoid > drm_vma_node_reset() being called twice. I came up with something very similar by splitting up nouveau_bo_new() into allocation and initialization steps, so that when necessary the GEM object can be initialized in between. I think that's slightly more flexible and easier to understand than a boolean flag. Thierry From a1130a6affcb7c00133e89f3e498cb6757f5bb51 Mon Sep 17 00:00:00 2001 From: Thierry Reding <treding@nvidia.com> Date: Wed, 14 Aug 2019 11:00:48 +0200 Subject: [PATCH] drm/nouveau: Initialize GEM object before TTM object TTM assumes that drivers initialize the embedded GEM object before calling the ttm_bo_init() function. This is not currently the case in the Nouveau driver. Fix this by splitting up nouveau_bo_new() into nouveau_bo_alloc() and nouveau_bo_init() so that the GEM can be initialized before TTM BO initialization when necessary. Fixes: b96f3e7c8069 ("drm/ttm: use gem vma_node") Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/nouveau/nouveau_bo.c | 69 ++++++++++++++++--------- drivers/gpu/drm/nouveau/nouveau_bo.h | 4 ++ drivers/gpu/drm/nouveau/nouveau_gem.c | 29 ++++++----- drivers/gpu/drm/nouveau/nouveau_prime.c | 16 ++++-- 4 files changed, 77 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 99e391be9370..b3d3e07de1af 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -185,31 +185,24 @@ nouveau_bo_fixup_align(struct nouveau_bo *nvbo, u32 flags, *size = roundup_64(*size, PAGE_SIZE); } -int -nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, - uint32_t flags, uint32_t tile_mode, uint32_t tile_flags, - struct sg_table *sg, struct reservation_object *robj, - struct nouveau_bo **pnvbo) +struct nouveau_bo * +nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, + u32 tile_flags) { struct nouveau_drm *drm = cli->drm; struct nouveau_bo *nvbo; struct nvif_mmu *mmu = &cli->mmu; struct nvif_vmm *vmm = cli->svm.cli ? &cli->svm.vmm : &cli->vmm.vmm; - size_t acc_size; - int type = ttm_bo_type_device; - int ret, i, pi = -1; + int i, pi = -1; if (!size) { NV_WARN(drm, "skipped size %016llx\n", size); - return -EINVAL; + return ERR_PTR(-EINVAL); } - if (sg) - type = ttm_bo_type_sg; - nvbo = kzalloc(sizeof(struct nouveau_bo), GFP_KERNEL); if (!nvbo) - return -ENOMEM; + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(&nvbo->head); INIT_LIST_HEAD(&nvbo->entry); INIT_LIST_HEAD(&nvbo->vma_list); @@ -231,7 +224,7 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, nvbo->kind = (tile_flags & 0x0000ff00) >> 8; if (!nvif_mmu_kind_valid(mmu, nvbo->kind)) { kfree(nvbo); - return -EINVAL; + return ERR_PTR(-EINVAL); } nvbo->comp = mmu->kind[nvbo->kind] != nvbo->kind; @@ -241,7 +234,7 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, nvbo->comp = (tile_flags & 0x00030000) >> 16; if (!nvif_mmu_kind_valid(mmu, nvbo->kind)) { kfree(nvbo); - return -EINVAL; + return ERR_PTR(-EINVAL); } } else { nvbo->zeta = (tile_flags & 0x00000007); @@ -278,7 +271,7 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, } if (WARN_ON(pi < 0)) - return -EINVAL; + return ERR_PTR(-EINVAL); /* Disable compression if suitable settings couldn't be found. */ if (nvbo->comp && !vmm->page[pi].comp) { @@ -288,23 +281,51 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, } nvbo->page = vmm->page[pi].shift; + return nvbo; +} + +int +nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 flags, + struct sg_table *sg, struct reservation_object *robj) +{ + int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; + size_t acc_size; + int ret; + + acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo)); + nouveau_bo_fixup_align(nvbo, flags, &align, &size); nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; nouveau_bo_placement_set(nvbo, flags, 0); - acc_size = ttm_bo_dma_acc_size(&drm->ttm.bdev, size, - sizeof(struct nouveau_bo)); - - ret = ttm_bo_init(&drm->ttm.bdev, &nvbo->bo, size, - type, &nvbo->placement, - align >> PAGE_SHIFT, false, acc_size, sg, - robj, nouveau_bo_del_ttm); - + ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type, + &nvbo->placement, align >> PAGE_SHIFT, false, + acc_size, sg, robj, nouveau_bo_del_ttm); if (ret) { /* ttm will call nouveau_bo_del_ttm if it fails.. */ return ret; } + return 0; +} + +int +nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, + uint32_t flags, uint32_t tile_mode, uint32_t tile_flags, + struct sg_table *sg, struct reservation_object *robj, + struct nouveau_bo **pnvbo) +{ + struct nouveau_bo *nvbo; + int ret; + + nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); + if (IS_ERR(nvbo)) + return PTR_ERR(nvbo); + + ret = nouveau_bo_init(nvbo, size, align, flags, sg, robj); + if (ret) + return ret; + *pnvbo = nvbo; return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h index d675efe8e7f9..7529035b971f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.h +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h @@ -71,6 +71,10 @@ nouveau_bo_ref(struct nouveau_bo *ref, struct nouveau_bo **pnvbo) extern struct ttm_bo_driver nouveau_bo_driver; void nouveau_bo_move_init(struct nouveau_drm *); +struct nouveau_bo *nouveau_bo_alloc(struct nouveau_cli *, u64 size, u32 flags, + u32 tile_mode, u32 tile_flags); +int nouveau_bo_init(struct nouveau_bo *, u64 size, int align, u32 flags, + struct sg_table *sg, struct reservation_object *robj); int nouveau_bo_new(struct nouveau_cli *, u64 size, int align, u32 flags, u32 tile_mode, u32 tile_flags, struct sg_table *sg, struct reservation_object *robj, diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index c7368aa0bdec..e9c772e07789 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -188,11 +188,23 @@ nouveau_gem_new(struct nouveau_cli *cli, u64 size, int align, uint32_t domain, if (domain & NOUVEAU_GEM_DOMAIN_COHERENT) flags |= TTM_PL_FLAG_UNCACHED; - ret = nouveau_bo_new(cli, size, align, flags, tile_mode, - tile_flags, NULL, NULL, pnvbo); - if (ret) + nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); + if (IS_ERR(nvbo)) + return PTR_ERR(nvbo); + + /* Initialize the embedded gem-object. We return a single gem-reference + * to the caller, instead of a normal nouveau_bo ttm reference. */ + ret = drm_gem_object_init(drm->dev, &nvbo->bo.base, size); + if (ret) { + nouveau_bo_ref(NULL, &nvbo); + return ret; + } + + ret = nouveau_bo_init(nvbo, size, align, flags, NULL, NULL); + if (ret) { + nouveau_bo_ref(NULL, &nvbo); return ret; - nvbo = *pnvbo; + } /* we restrict allowed domains on nv50+ to only the types * that were requested at creation time. not possibly on @@ -203,15 +215,8 @@ nouveau_gem_new(struct nouveau_cli *cli, u64 size, int align, uint32_t domain, if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA) nvbo->valid_domains &= domain; - /* Initialize the embedded gem-object. We return a single gem-reference - * to the caller, instead of a normal nouveau_bo ttm reference. */ - ret = drm_gem_object_init(drm->dev, &nvbo->bo.base, nvbo->bo.mem.size); - if (ret) { - nouveau_bo_ref(NULL, pnvbo); - return -ENOMEM; - } - nvbo->bo.persistent_swap_storage = nvbo->bo.base.filp; + *pnvbo = nvbo; return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index e86ad7ae622b..0ca71a84e23a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -63,28 +63,34 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_bo *nvbo; struct reservation_object *robj = attach->dmabuf->resv; + size_t size = attach->dmabuf->size; u32 flags = 0; int ret; flags = TTM_PL_FLAG_TT; reservation_object_lock(robj, NULL); - ret = nouveau_bo_new(&drm->client, attach->dmabuf->size, 0, flags, 0, 0, - sg, robj, &nvbo); + nvbo = nouveau_bo_alloc(&drm->client, size, flags, 0, 0); reservation_object_unlock(robj); - if (ret) - return ERR_PTR(ret); + if (IS_ERR(nvbo)) + return ERR_CAST(nvbo); nvbo->valid_domains = NOUVEAU_GEM_DOMAIN_GART; /* Initialize the embedded gem-object. We return a single gem-reference * to the caller, instead of a normal nouveau_bo ttm reference. */ - ret = drm_gem_object_init(dev, &nvbo->bo.base, nvbo->bo.mem.size); + ret = drm_gem_object_init(dev, &nvbo->bo.base, size); if (ret) { nouveau_bo_ref(NULL, &nvbo); return ERR_PTR(-ENOMEM); } + ret = nouveau_bo_init(nvbo, size, 0, flags, sg, robj); + if (ret) { + nouveau_bo_ref(NULL, &nvbo); + return ERR_PTR(ret); + } + return &nvbo->bo.base; }
Hi, > > Changing the order doesn't look hard. Patch attached (untested, have no > > test hardware). But maybe I missed some detail ... > > I came up with something very similar by splitting up nouveau_bo_new() > into allocation and initialization steps, so that when necessary the GEM > object can be initialized in between. I think that's slightly more > flexible and easier to understand than a boolean flag. Yes, that should work too. Acked-by: Gerd Hoffmann <kraxel@redhat.com> cheers, Gerd
On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > > Changing the order doesn't look hard. Patch attached (untested, have no > > > test hardware). But maybe I missed some detail ... > > > > I came up with something very similar by splitting up nouveau_bo_new() > > into allocation and initialization steps, so that when necessary the GEM > > object can be initialized in between. I think that's slightly more > > flexible and easier to understand than a boolean flag. > > Yes, that should work too. > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Ben Skeggs <bskeggs@redhat.com> > > cheers, > Gerd > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote: > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > Hi, > > > > > > Changing the order doesn't look hard. Patch attached (untested, have no > > > > test hardware). But maybe I missed some detail ... > > > > > > I came up with something very similar by splitting up nouveau_bo_new() > > > into allocation and initialization steps, so that when necessary the GEM > > > object can be initialized in between. I think that's slightly more > > > flexible and easier to understand than a boolean flag. > > > > Yes, that should work too. > > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > Acked-by: Ben Skeggs <bskeggs@redhat.com> Thanks guys, applied to drm-misc-next. Thierry
On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote: > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > Hi, > > > > > > > > Changing the order doesn't look hard. Patch attached (untested, have no > > > > > test hardware). But maybe I missed some detail ... > > > > > > > > I came up with something very similar by splitting up nouveau_bo_new() > > > > into allocation and initialization steps, so that when necessary the GEM > > > > object can be initialized in between. I think that's slightly more > > > > flexible and easier to understand than a boolean flag. > > > > > > Yes, that should work too. > > > > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > Acked-by: Ben Skeggs <bskeggs@redhat.com> > > Thanks guys, applied to drm-misc-next. Hi Thierry, Initial investigations suggest that this commit currently in drm-next commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5 Author: Thierry Reding <treding@nvidia.com> Date: Wed Aug 14 11:00:48 2019 +0200 drm/nouveau: Initialize GEM object before TTM object breaks nouveau userspace which tries to allocate GEM objects with a non-page-aligned size. Previously nouveau_gem_new would just call nouveau_bo_init which would call nouveau_bo_fixup_align before initializing the GEM object. With this change, it is done after. What do you think -- OK to just move that bit of logic into the new nouveau_bo_alloc() (and make size/align be pointers so that they can be fixed up?) Cheers, -ilia
On Sat, Sep 07, 2019 at 09:58:46PM -0400, Ilia Mirkin wrote: > On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote: > > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > > > Hi, > > > > > > > > > > Changing the order doesn't look hard. Patch attached (untested, have no > > > > > > test hardware). But maybe I missed some detail ... > > > > > > > > > > I came up with something very similar by splitting up nouveau_bo_new() > > > > > into allocation and initialization steps, so that when necessary the GEM > > > > > object can be initialized in between. I think that's slightly more > > > > > flexible and easier to understand than a boolean flag. > > > > > > > > Yes, that should work too. > > > > > > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > Acked-by: Ben Skeggs <bskeggs@redhat.com> > > > > Thanks guys, applied to drm-misc-next. > > Hi Thierry, > > Initial investigations suggest that this commit currently in drm-next > > commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5 > Author: Thierry Reding <treding@nvidia.com> > Date: Wed Aug 14 11:00:48 2019 +0200 > > drm/nouveau: Initialize GEM object before TTM object > > breaks nouveau userspace which tries to allocate GEM objects with a > non-page-aligned size. Previously nouveau_gem_new would just call > nouveau_bo_init which would call nouveau_bo_fixup_align before > initializing the GEM object. With this change, it is done after. What > do you think -- OK to just move that bit of logic into the new > nouveau_bo_alloc() (and make size/align be pointers so that they can > be fixed up?) Hi Ilia, sorry, got side-tracked earlier and forgot to send this out. I'll turn this into a proper patch, but if you manage to find the time to test this while I work out the userspace issues that are preventing me from testing this more thoroughly, that'd be great. Thierry --- >8 --- diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index e918b437af17..7d5ede756711 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -186,8 +186,8 @@ nouveau_bo_fixup_align(struct nouveau_bo *nvbo, u32 flags, } struct nouveau_bo * -nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, - u32 tile_flags) +nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int *align, u32 flags, + u32 tile_mode, u32 tile_flags) { struct nouveau_drm *drm = cli->drm; struct nouveau_bo *nvbo; @@ -195,8 +195,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, struct nvif_vmm *vmm = cli->svm.cli ? &cli->svm.vmm : &cli->vmm.vmm; int i, pi = -1; - if (!size) { - NV_WARN(drm, "skipped size %016llx\n", size); + if (!*size) { + NV_WARN(drm, "skipped size %016llx\n", *size); return ERR_PTR(-EINVAL); } @@ -266,7 +266,7 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, pi = i; /* Stop once the buffer is larger than the current page size. */ - if (size >= 1ULL << vmm->page[i].shift) + if (*size >= 1ULL << vmm->page[i].shift) break; } @@ -281,6 +281,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, } nvbo->page = vmm->page[pi].shift; + nouveau_bo_fixup_align(nvbo, flags, align, size); + return nvbo; } @@ -292,12 +294,11 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 flags, size_t acc_size; int ret; - acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo)); - - nouveau_bo_fixup_align(nvbo, flags, &align, &size); nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; nouveau_bo_placement_set(nvbo, flags, 0); + acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo)); + ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type, &nvbo->placement, align >> PAGE_SHIFT, false, acc_size, sg, robj, nouveau_bo_del_ttm); @@ -318,7 +319,8 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, struct nouveau_bo *nvbo; int ret; - nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); + nvbo = nouveau_bo_alloc(cli, &size, &align, flags, tile_mode, + tile_flags); if (IS_ERR(nvbo)) return PTR_ERR(nvbo); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h index 62930d834fba..38f9d8350963 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.h +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h @@ -71,8 +71,8 @@ nouveau_bo_ref(struct nouveau_bo *ref, struct nouveau_bo **pnvbo) extern struct ttm_bo_driver nouveau_bo_driver; void nouveau_bo_move_init(struct nouveau_drm *); -struct nouveau_bo *nouveau_bo_alloc(struct nouveau_cli *, u64 size, u32 flags, - u32 tile_mode, u32 tile_flags); +struct nouveau_bo *nouveau_bo_alloc(struct nouveau_cli *, u64 *size, int *align, + u32 flags, u32 tile_mode, u32 tile_flags); int nouveau_bo_init(struct nouveau_bo *, u64 size, int align, u32 flags, struct sg_table *sg, struct dma_resv *robj); int nouveau_bo_new(struct nouveau_cli *, u64 size, int align, u32 flags, diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index c2bfc0591909..1bdffd714456 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -188,7 +188,8 @@ nouveau_gem_new(struct nouveau_cli *cli, u64 size, int align, uint32_t domain, if (domain & NOUVEAU_GEM_DOMAIN_COHERENT) flags |= TTM_PL_FLAG_UNCACHED; - nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); + nvbo = nouveau_bo_alloc(cli, &size, &align, flags, tile_mode, + tile_flags); if (IS_ERR(nvbo)) return PTR_ERR(nvbo); diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 84658d434225..656c334ee7d9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -62,14 +62,15 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_bo *nvbo; struct dma_resv *robj = attach->dmabuf->resv; - size_t size = attach->dmabuf->size; + u64 size = attach->dmabuf->size; u32 flags = 0; + int align = 0; int ret; flags = TTM_PL_FLAG_TT; dma_resv_lock(robj, NULL); - nvbo = nouveau_bo_alloc(&drm->client, size, flags, 0, 0); + nvbo = nouveau_bo_alloc(&drm->client, &size, &align, flags, 0, 0); dma_resv_unlock(robj); if (IS_ERR(nvbo)) return ERR_CAST(nvbo); @@ -84,7 +85,7 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, return ERR_PTR(-ENOMEM); } - ret = nouveau_bo_init(nvbo, size, 0, flags, sg, robj); + ret = nouveau_bo_init(nvbo, size, align, flags, sg, robj); if (ret) { nouveau_bo_ref(NULL, &nvbo); return ERR_PTR(ret);
On Wed, 11 Sep 2019 at 07:53, Thierry Reding <thierry.reding@gmail.com> wrote: > > On Sat, Sep 07, 2019 at 09:58:46PM -0400, Ilia Mirkin wrote: > > On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote: > > > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > Changing the order doesn't look hard. Patch attached (untested, have no > > > > > > > test hardware). But maybe I missed some detail ... > > > > > > > > > > > > I came up with something very similar by splitting up nouveau_bo_new() > > > > > > into allocation and initialization steps, so that when necessary the GEM > > > > > > object can be initialized in between. I think that's slightly more > > > > > > flexible and easier to understand than a boolean flag. > > > > > > > > > > Yes, that should work too. > > > > > > > > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > > Acked-by: Ben Skeggs <bskeggs@redhat.com> > > > > > > Thanks guys, applied to drm-misc-next. > > > > Hi Thierry, > > > > Initial investigations suggest that this commit currently in drm-next > > > > commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5 > > Author: Thierry Reding <treding@nvidia.com> > > Date: Wed Aug 14 11:00:48 2019 +0200 > > > > drm/nouveau: Initialize GEM object before TTM object > > > > breaks nouveau userspace which tries to allocate GEM objects with a > > non-page-aligned size. Previously nouveau_gem_new would just call > > nouveau_bo_init which would call nouveau_bo_fixup_align before > > initializing the GEM object. With this change, it is done after. What > > do you think -- OK to just move that bit of logic into the new > > nouveau_bo_alloc() (and make size/align be pointers so that they can > > be fixed up?) > > Hi Ilia, > > sorry, got side-tracked earlier and forgot to send this out. I'll turn > this into a proper patch, but if you manage to find the time to test > this while I work out the userspace issues that are preventing me from > testing this more thoroughly, that'd be great. I can confirm both I can reproduce the bug, and that the fix here appears to do the trick nicely. Ben. > > Thierry > > --- >8 --- > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c > index e918b437af17..7d5ede756711 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -186,8 +186,8 @@ nouveau_bo_fixup_align(struct nouveau_bo *nvbo, u32 flags, > } > > struct nouveau_bo * > -nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, > - u32 tile_flags) > +nouveau_bo_alloc(struct nouveau_cli *cli, u64 *size, int *align, u32 flags, > + u32 tile_mode, u32 tile_flags) > { > struct nouveau_drm *drm = cli->drm; > struct nouveau_bo *nvbo; > @@ -195,8 +195,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, > struct nvif_vmm *vmm = cli->svm.cli ? &cli->svm.vmm : &cli->vmm.vmm; > int i, pi = -1; > > - if (!size) { > - NV_WARN(drm, "skipped size %016llx\n", size); > + if (!*size) { > + NV_WARN(drm, "skipped size %016llx\n", *size); > return ERR_PTR(-EINVAL); > } > > @@ -266,7 +266,7 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, > pi = i; > > /* Stop once the buffer is larger than the current page size. */ > - if (size >= 1ULL << vmm->page[i].shift) > + if (*size >= 1ULL << vmm->page[i].shift) > break; > } > > @@ -281,6 +281,8 @@ nouveau_bo_alloc(struct nouveau_cli *cli, u64 size, u32 flags, u32 tile_mode, > } > nvbo->page = vmm->page[pi].shift; > > + nouveau_bo_fixup_align(nvbo, flags, align, size); > + > return nvbo; > } > > @@ -292,12 +294,11 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 flags, > size_t acc_size; > int ret; > > - acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo)); > - > - nouveau_bo_fixup_align(nvbo, flags, &align, &size); > nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; > nouveau_bo_placement_set(nvbo, flags, 0); > > + acc_size = ttm_bo_dma_acc_size(nvbo->bo.bdev, size, sizeof(*nvbo)); > + > ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type, > &nvbo->placement, align >> PAGE_SHIFT, false, > acc_size, sg, robj, nouveau_bo_del_ttm); > @@ -318,7 +319,8 @@ nouveau_bo_new(struct nouveau_cli *cli, u64 size, int align, > struct nouveau_bo *nvbo; > int ret; > > - nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); > + nvbo = nouveau_bo_alloc(cli, &size, &align, flags, tile_mode, > + tile_flags); > if (IS_ERR(nvbo)) > return PTR_ERR(nvbo); > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h > index 62930d834fba..38f9d8350963 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.h > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.h > @@ -71,8 +71,8 @@ nouveau_bo_ref(struct nouveau_bo *ref, struct nouveau_bo **pnvbo) > extern struct ttm_bo_driver nouveau_bo_driver; > > void nouveau_bo_move_init(struct nouveau_drm *); > -struct nouveau_bo *nouveau_bo_alloc(struct nouveau_cli *, u64 size, u32 flags, > - u32 tile_mode, u32 tile_flags); > +struct nouveau_bo *nouveau_bo_alloc(struct nouveau_cli *, u64 *size, int *align, > + u32 flags, u32 tile_mode, u32 tile_flags); > int nouveau_bo_init(struct nouveau_bo *, u64 size, int align, u32 flags, > struct sg_table *sg, struct dma_resv *robj); > int nouveau_bo_new(struct nouveau_cli *, u64 size, int align, u32 flags, > diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c > index c2bfc0591909..1bdffd714456 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_gem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c > @@ -188,7 +188,8 @@ nouveau_gem_new(struct nouveau_cli *cli, u64 size, int align, uint32_t domain, > if (domain & NOUVEAU_GEM_DOMAIN_COHERENT) > flags |= TTM_PL_FLAG_UNCACHED; > > - nvbo = nouveau_bo_alloc(cli, size, flags, tile_mode, tile_flags); > + nvbo = nouveau_bo_alloc(cli, &size, &align, flags, tile_mode, > + tile_flags); > if (IS_ERR(nvbo)) > return PTR_ERR(nvbo); > > diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c > index 84658d434225..656c334ee7d9 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_prime.c > +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c > @@ -62,14 +62,15 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, > struct nouveau_drm *drm = nouveau_drm(dev); > struct nouveau_bo *nvbo; > struct dma_resv *robj = attach->dmabuf->resv; > - size_t size = attach->dmabuf->size; > + u64 size = attach->dmabuf->size; > u32 flags = 0; > + int align = 0; > int ret; > > flags = TTM_PL_FLAG_TT; > > dma_resv_lock(robj, NULL); > - nvbo = nouveau_bo_alloc(&drm->client, size, flags, 0, 0); > + nvbo = nouveau_bo_alloc(&drm->client, &size, &align, flags, 0, 0); > dma_resv_unlock(robj); > if (IS_ERR(nvbo)) > return ERR_CAST(nvbo); > @@ -84,7 +85,7 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, > return ERR_PTR(-ENOMEM); > } > > - ret = nouveau_bo_init(nvbo, size, 0, flags, sg, robj); > + ret = nouveau_bo_init(nvbo, size, align, flags, sg, robj); > if (ret) { > nouveau_bo_ref(NULL, &nvbo); > return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index 645a189d365c..113fb2feb437 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -191,7 +191,7 @@ static inline unsigned amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo) */ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } /** diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index b812d4ae9d0d..8ae54ba7857c 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -60,7 +60,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo) static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index 9ffd8215d38a..e5554bf9140e 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -116,7 +116,7 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo) */ static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index f4ecea6054ba..e28829661724 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -396,7 +396,7 @@ static inline void virtio_gpu_object_unref(struct virtio_gpu_object **bo) static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo, diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index fa050f0328ab..7ffc50a3303d 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -152,7 +152,6 @@ struct ttm_tt; * @ddestroy: List head for the delayed destroy list. * @swap: List head for swap LRU list. * @moving: Fence set when BO is moving - * @vma_node: Address space manager node. * @offset: The current GPU offset, which can have different meanings * depending on the memory type. For SYSTEM type memory, it should be 0. * @cur_placement: Hint of current placement. @@ -219,9 +218,6 @@ struct ttm_buffer_object { */ struct dma_fence *moving; - - struct drm_vma_offset_node vma_node; - unsigned priority; /** diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index fc13920b3cb4..fd751078bae1 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -168,7 +168,7 @@ EXPORT_SYMBOL(drm_gem_vram_put); */ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo) { - return drm_vma_node_offset_addr(&gbo->bo.vma_node); + return drm_vma_node_offset_addr(&gbo->bo.base.vma_node); } EXPORT_SYMBOL(drm_gem_vram_mmap_offset); diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index fc8f5bb73ca8..98afc50162e9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -675,7 +675,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv, gem = drm_gem_object_lookup(file_priv, handle); if (gem) { struct nouveau_bo *bo = nouveau_gem_object(gem); - *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node); + *poffset = drm_vma_node_offset_addr(&bo->bo.base.vma_node); drm_gem_object_put_unlocked(gem); return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 2f484ab7dbca..b1e4852810ed 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -240,7 +240,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, } rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT; - rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node); + rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.base.vma_node); rep->tile_mode = nvbo->mode; rep->tile_flags = nvbo->contig ? 0 : NOUVEAU_GEM_TILE_NONCONTIG; if (cli->device.info.family >= NV_DEVICE_INFO_V0_FERMI) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index ceff153f7e68..3e0a0cbc410e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -672,7 +672,7 @@ static void ttm_bo_release(struct kref *kref) struct ttm_bo_device *bdev = bo->bdev; struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type]; - drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node); + drm_vma_offset_remove(&bdev->vma_manager, &bo->base.vma_node); ttm_mem_io_lock(man, false); ttm_mem_io_free_vm(bo); ttm_mem_io_unlock(man); @@ -1343,9 +1343,9 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, * struct elements we want use regardless. */ reservation_object_init(&bo->base._resv); + drm_vma_node_reset(&bo->base.vma_node); } atomic_inc(&bo->bdev->glob->bo_count); - drm_vma_node_reset(&bo->vma_node); /* * For ttm_bo_type_device buffers, allocate @@ -1353,7 +1353,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, */ if (bo->type == ttm_bo_type_device || bo->type == ttm_bo_type_sg) - ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node, + ret = drm_vma_offset_add(&bdev->vma_manager, &bo->base.vma_node, bo->mem.num_pages); /* passed reservation objects should already be locked, @@ -1781,7 +1781,7 @@ void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; - drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping); + drm_vma_node_unmap(&bo->base.vma_node, bdev->dev_mapping); ttm_mem_io_free_vm(bo); } diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 05fbcaf6a3f2..f5009c1b6a9c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -510,7 +510,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, INIT_LIST_HEAD(&fbo->base.io_reserve_lru); mutex_init(&fbo->base.wu_mutex); fbo->base.moving = NULL; - drm_vma_node_reset(&fbo->base.vma_node); + drm_vma_node_reset(&fbo->base.base.vma_node); atomic_set(&fbo->base.cpu_writers, 0); kref_init(&fbo->base.list_kref); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 6dacff49c1cc..fb6875a789b7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -211,9 +211,9 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) } page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) + - vma->vm_pgoff - drm_vma_node_start(&bo->vma_node); + vma->vm_pgoff - drm_vma_node_start(&bo->base.vma_node); page_last = vma_pages(vma) + vma->vm_pgoff - - drm_vma_node_start(&bo->vma_node); + drm_vma_node_start(&bo->base.vma_node); if (unlikely(page_offset >= bo->num_pages)) { ret = VM_FAULT_SIGBUS; @@ -267,7 +267,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) } else if (unlikely(!page)) { break; } - page->index = drm_vma_node_start(&bo->vma_node) + + page->index = drm_vma_node_start(&bo->base.vma_node) + page_offset; pfn = page_to_pfn(page); } @@ -413,7 +413,8 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, node = drm_vma_offset_lookup_locked(&bdev->vma_manager, offset, pages); if (likely(node)) { - bo = container_of(node, struct ttm_buffer_object, vma_node); + bo = container_of(node, struct ttm_buffer_object, + base.vma_node); bo = ttm_bo_get_unless_zero(bo); } diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 8b3b2caf3364..dc642a884b88 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -68,8 +68,5 @@ void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) int virtgpu_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { - struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); - - bo->gem_base.vma_node.vm_node.start = bo->tbo.vma_node.vm_node.start; return drm_gem_prime_mmap(obj, vma); } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 315da41a18b4..5739c6c49c99 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -835,7 +835,7 @@ int vmw_bo_alloc_ioctl(struct drm_device *dev, void *data, goto out_no_bo; rep->handle = handle; - rep->map_handle = drm_vma_node_offset_addr(&vbo->base.vma_node); + rep->map_handle = drm_vma_node_offset_addr(&vbo->base.base.vma_node); rep->cur_gmr_id = handle; rep->cur_gmr_offset = 0; @@ -1077,7 +1077,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv, if (ret != 0) return -EINVAL; - *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node); + *offset = drm_vma_node_offset_addr(&out_buf->base.base.vma_node); vmw_bo_unreference(&out_buf); return 0; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c index 219471903bc1..3a6da3b66484 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c @@ -1669,7 +1669,7 @@ vmw_gb_surface_define_internal(struct drm_device *dev, rep->backup_size = res->backup_size; if (res->backup) { rep->buffer_map_handle = - drm_vma_node_offset_addr(&res->backup->base.vma_node); + drm_vma_node_offset_addr(&res->backup->base.base.vma_node); rep->buffer_size = res->backup->base.num_pages * PAGE_SIZE; rep->buffer_handle = backup_handle; } else { @@ -1745,7 +1745,7 @@ vmw_gb_surface_reference_internal(struct drm_device *dev, rep->crep.backup_size = srf->res.backup_size; rep->crep.buffer_handle = backup_handle; rep->crep.buffer_map_handle = - drm_vma_node_offset_addr(&srf->res.backup->base.vma_node); + drm_vma_node_offset_addr(&srf->res.backup->base.base.vma_node); rep->crep.buffer_size = srf->res.backup->base.num_pages * PAGE_SIZE; rep->creq.version = drm_vmw_gb_surface_v1;