Hi guys,
this patch set cleans up the handling of TTM buffer objects quite a bit by allowing to create them without allocating a ttm_resource as well.
That's not only cleaner in general, but also a necessary prerequisite for quite a number of related work.
Please review and comment, Christian.
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index b827b87aefe2..7afafbbc4ea0 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, struct dma_resv *resv, struct radeon_bo **bo_ptr) { - struct radeon_bo *bo; - enum ttm_bo_type type; unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; + + /* Kernel allocation are uninterruptible */ + struct ttm_operation_ctx ctx = { !kernel, false }; + enum ttm_bo_type type; + struct radeon_bo *bo; int r;
size = ALIGN(size, PAGE_SIZE); @@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, #endif
radeon_ttm_placement_from_domain(bo, domain); - /* Kernel allocation are uninterruptible */ down_read(&rdev->pm.mclk_lock); - r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, - &bo->placement, page_align, !kernel, sg, resv, - &radeon_ttm_bo_destroy); + r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, + &bo->placement, page_align, &ctx, sg, resv, + &radeon_ttm_bo_destroy); + if (!r) + ttm_bo_unreserve(&bo->tbo); + up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { return r;
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index fa73fe57f97b..ceac591a7c01 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -302,19 +302,23 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, struct sg_table *sg, struct dma_resv *robj) { int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; + struct ttm_operation_ctx ctx = { false, false }; int ret;
nouveau_bo_placement_set(nvbo, domain, 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, sg, - robj, nouveau_bo_del_ttm); + ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type, + &nvbo->placement, align >> PAGE_SHIFT, &ctx, + sg, robj, nouveau_bo_del_ttm); if (ret) { /* ttm will call nouveau_bo_del_ttm if it fails.. */ return ret; }
+ if (!robj) + ttm_bo_unreserve(&nvbo->bo); + return 0; }
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 3f00192215d1..0bd46a138ded 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -186,6 +186,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, size_t size, unsigned long pg_align) { + struct ttm_operation_ctx ctx = { false, false }; struct drm_gem_vram_object *gbo; struct drm_gem_object *gem; struct drm_vram_mm *vmm = dev->vram_mm; @@ -225,12 +226,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, * A failing ttm_bo_init will call ttm_buffer_object_destroy * to release gbo->bo.base and kfree gbo. */ - ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, - &gbo->placement, pg_align, false, NULL, NULL, - ttm_buffer_object_destroy); - if (ret) + ret = ttm_bo_init_reserved(bdev, &gbo->bo, size, ttm_bo_type_device, + &gbo->placement, pg_align, &ctx, NULL, NULL, + ttm_buffer_object_destroy); + if (ret) return ERR_PTR(ret);
+ ttm_bo_unreserve(&gbo->bo); return gbo; } EXPORT_SYMBOL(drm_gem_vram_create);
It's the only driver using this.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 9 +-------- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e5fd0f2c0299..7598d59423bf 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -44,12 +44,6 @@
#include "ttm_module.h"
-/* default destructor */ -static void ttm_bo_default_destroy(struct ttm_buffer_object *bo) -{ - kfree(bo); -} - static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, struct ttm_placement *placement) { @@ -938,8 +932,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, bool locked; int ret;
- bo->destroy = destroy ? destroy : ttm_bo_default_destroy; - + bo->destroy = destroy; kref_init(&bo->kref); INIT_LIST_HEAD(&bo->ddestroy); bo->bdev = bdev; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 31aecc46624b..60dcc6a75248 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -378,6 +378,12 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo) kfree(vmw_bo); }
+/* default destructor */ +static void vmw_bo_default_destroy(struct ttm_buffer_object *bo) +{ + kfree(bo); +} + /** * vmw_bo_create_kernel - Create a pinned BO for internal kernel use. * @@ -410,7 +416,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size, ttm_bo_type_kernel, placement, 0, - &ctx, NULL, NULL, NULL); + &ctx, NULL, NULL, vmw_bo_default_destroy); if (unlikely(ret)) goto error_free;
@@ -439,6 +445,9 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; }
+ if (!bo_free) + bo_free = vmw_bo_default_destroy; + ret = vmw_bo_init(vmw, *p_bo, size, placement, interruptible, pin, bo_free);
On Tue, 2022-03-29 at 13:02 +0200, Christian König wrote:
⚠ External Email
It's the only driver using this.
Signed-off-by: Christian König christian.koenig@amd.com
Looks good. A small suggestion underneath.
Reviewed-by: Zack Rusin zackr@vmware.com
drivers/gpu/drm/ttm/ttm_bo.c | 9 +-------- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e5fd0f2c0299..7598d59423bf 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -44,12 +44,6 @@
#include "ttm_module.h"
-/* default destructor */ -static void ttm_bo_default_destroy(struct ttm_buffer_object *bo) -{ - kfree(bo); -}
static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo, struct ttm_placement *placement) { @@ -938,8 +932,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, bool locked; int ret;
- bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
+ bo->destroy = destroy; kref_init(&bo->kref); INIT_LIST_HEAD(&bo->ddestroy); bo->bdev = bdev; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 31aecc46624b..60dcc6a75248 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -378,6 +378,12 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo) kfree(vmw_bo); }
+/* default destructor */ +static void vmw_bo_default_destroy(struct ttm_buffer_object *bo) +{ + kfree(bo); +}
/** * vmw_bo_create_kernel - Create a pinned BO for internal kernel use. * @@ -410,7 +416,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size, ttm_bo_type_kernel, placement, 0, - &ctx, NULL, NULL, NULL); + &ctx, NULL, NULL, vmw_bo_default_destroy); if (unlikely(ret)) goto error_free;
@@ -439,6 +445,9 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; }
+ if (!bo_free) + bo_free = vmw_bo_default_destroy;
If you could change this to just BUG_ON(!bo_free) that'd be great. bo_free == NULL should never happen here.
z
Not used any more.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 26 --------------------- include/drm/ttm/ttm_bo_api.h | 44 ------------------------------------ 2 files changed, 70 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 7598d59423bf..37f68e710247 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -988,32 +988,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_bo_init_reserved);
-int ttm_bo_init(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, - enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - bool interruptible, - struct sg_table *sg, - struct dma_resv *resv, - void (*destroy) (struct ttm_buffer_object *)) -{ - struct ttm_operation_ctx ctx = { interruptible, false }; - int ret; - - ret = ttm_bo_init_reserved(bdev, bo, size, type, placement, - page_alignment, &ctx, sg, resv, destroy); - if (ret) - return ret; - - if (!resv) - ttm_bo_unreserve(bo); - - return 0; -} -EXPORT_SYMBOL(ttm_bo_init); - /* * buffer object vm functions. */ diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c61e1e5ceb83..51fbbd035c11 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -363,50 +363,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *));
-/** - * ttm_bo_init - * - * @bdev: Pointer to a ttm_device struct. - * @bo: Pointer to a ttm_buffer_object to be initialized. - * @size: Requested size of buffer object. - * @type: Requested type of buffer object. - * @placement: Initial placement for buffer object. - * @page_alignment: Data alignment in pages. - * @interruptible: If needing to sleep to wait for GPU resources, - * sleep interruptible. - * pinned in physical memory. If this behaviour is not desired, this member - * holds a pointer to a persistent shmem object. Typically, this would - * point to the shmem object backing a GEM object if TTM is used to back a - * GEM user interface. - * @sg: Scatter-gather table. - * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. - * @destroy: Destroy function. Use NULL for kfree(). - * - * This function initializes a pre-allocated struct ttm_buffer_object. - * As this object may be part of a larger structure, this function, - * together with the @destroy function, - * enables driver-specific objects derived from a ttm_buffer_object. - * - * On successful return, the caller owns an object kref to @bo. The kref and - * list_kref are usually set to 1, but note that in some situations, other - * tasks may already be holding references to @bo as well. - * - * If a failure occurs, the function will call the @destroy function, or - * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is - * illegal and will likely cause memory corruption. - * - * Returns - * -ENOMEM: Out of memory. - * -EINVAL: Invalid placement flags. - * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. - */ -int ttm_bo_init(struct ttm_device *bdev, struct ttm_buffer_object *bo, - size_t size, enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, bool interrubtible, - struct sg_table *sg, struct dma_resv *resv, - void (*destroy) (struct ttm_buffer_object *)); - /** * ttm_kmap_obj_virtual *
Rename ttm_bo_init_reserved to ttm_bo_init_validate since that better matches what the function is actually doing.
Remove the unused size parameter, move the function's kerneldoc to the implementation and cleanup the whole error handling.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 2 +- drivers/gpu/drm/radeon/radeon_object.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 92 ++++++++++++++-------- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 12 ++- include/drm/ttm/ttm_bo_api.h | 44 +---------- 9 files changed, 75 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index ea0cde4904f0..4cb3afbdf6b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -587,7 +587,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (!bp->destroy) bp->destroy = &amdgpu_bo_destroy;
- r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type, + r = ttm_bo_init_validate(&adev->mman.bdev, &bo->tbo, bp->type, &bo->placement, page_align, &ctx, NULL, bp->resv, bp->destroy); if (unlikely(r != 0)) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 0bd46a138ded..3e678d49265c 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -226,7 +226,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, * A failing ttm_bo_init will call ttm_buffer_object_destroy * to release gbo->bo.base and kfree gbo. */ - ret = ttm_bo_init_reserved(bdev, &gbo->bo, size, ttm_bo_type_device, + ret = ttm_bo_init_validate(bdev, &gbo->bo, ttm_bo_type_device, &gbo->placement, pg_align, &ctx, NULL, NULL, ttm_buffer_object_destroy); if (ret) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 6fc192082d8c..10d89bde7dd3 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1075,9 +1075,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, * Similarly, in delayed_destroy, we can't call ttm_bo_put() * until successful initialization. */ - ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), size, - bo_type, &i915_sys_placement, - page_size >> PAGE_SHIFT, + ret = ttm_bo_init_validate(&i915->bdev, i915_gem_to_ttm(obj), bo_type, + &i915_sys_placement, page_size >> PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); if (ret) return i915_ttm_err_to_gem(ret); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index ceac591a7c01..43c862bbbbfb 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -308,7 +308,7 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type, + ret = ttm_bo_init_validate(nvbo->bo.bdev, &nvbo->bo, type, &nvbo->placement, align >> PAGE_SHIFT, &ctx, sg, robj, nouveau_bo_del_ttm); if (ret) { diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index fbb36e3e8564..e4dbce4afdd0 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -141,7 +141,7 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned long size, qxl_ttm_placement_from_domain(bo, domain);
bo->tbo.priority = priority; - r = ttm_bo_init_reserved(&qdev->mman.bdev, &bo->tbo, size, type, + r = ttm_bo_init_validate(&qdev->mman.bdev, &bo->tbo, type, &bo->placement, 0, &ctx, NULL, NULL, &qxl_ttm_bo_destroy); if (unlikely(r != 0)) { diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 7afafbbc4ea0..9d255170b771 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -204,7 +204,7 @@ int radeon_bo_create(struct radeon_device *rdev,
radeon_ttm_placement_from_domain(bo, domain); down_read(&rdev->pm.mclk_lock); - r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, + r = ttm_bo_init_validate(&rdev->mman.bdev, &bo->tbo, type, &bo->placement, page_align, &ctx, sg, resv, &radeon_ttm_bo_destroy); if (!r) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 37f68e710247..b7e259245f82 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -917,37 +917,62 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, } EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init_reserved(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, - enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - struct ttm_operation_ctx *ctx, - struct sg_table *sg, - struct dma_resv *resv, +/** + * ttm_bo_init_validate + * + * @bdev: Pointer to a ttm_device struct. + * @bo: Pointer to a ttm_buffer_object to be initialized. + * @type: Requested type of buffer object. + * @placement: Initial placement for buffer object. + * @alignment: Data alignment in pages. + * @ctx: TTM operation context for memory allocation. + * @sg: Scatter-gather table. + * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. + * @destroy: Destroy function. Use NULL for kfree(). + * + * This function initializes a pre-allocated struct ttm_buffer_object. + * As this object may be part of a larger structure, this function, + * together with the @destroy function, enables driver-specific objects + * derived from a ttm_buffer_object. + * + * On successful return, the caller owns an object kref to @bo. The kref and + * list_kref are usually set to 1, but note that in some situations, other + * tasks may already be holding references to @bo as well. + * Furthermore, if resv == NULL, the buffer's reservation lock will be held, + * and it is the caller's responsibility to call ttm_bo_unreserve. + * + * If a failure occurs, the function will call the @destroy function. Thus, + * after a failure, dereferencing @bo is illegal and will likely cause memory + * corruption. + * + * Returns + * -ENOMEM: Out of memory. + * -EINVAL: Invalid placement flags. + * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. + */ +int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, + enum ttm_bo_type type, struct ttm_placement *placement, + uint32_t alignment, struct ttm_operation_ctx *ctx, + struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; - bool locked; int ret;
- bo->destroy = destroy; kref_init(&bo->kref); INIT_LIST_HEAD(&bo->ddestroy); bo->bdev = bdev; bo->type = type; - bo->page_alignment = page_alignment; + bo->page_alignment = alignment; + bo->destroy = destroy; bo->moving = NULL; bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL; - if (resv) { + if (resv) bo->base.resv = resv; - dma_resv_assert_held(bo->base.resv); - } else { + else bo->base.resv = &bo->base._resv; - } atomic_inc(&ttm_glob.bo_count);
ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); @@ -960,33 +985,36 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, * For ttm_bo_type_device buffers, allocate * address space from the device. */ - if (bo->type == ttm_bo_type_device || - bo->type == ttm_bo_type_sg) + if (bo->type == ttm_bo_type_device || bo->type == ttm_bo_type_sg) { ret = drm_vma_offset_add(bdev->vma_manager, &bo->base.vma_node, - bo->resource->num_pages); + PFN_UP(bo->base.size)); + if (ret) + goto err_put; + }
/* passed reservation objects should already be locked, * since otherwise lockdep will be angered in radeon. */ - if (!resv) { - locked = dma_resv_trylock(bo->base.resv); - WARN_ON(!locked); - } + if (!resv) + WARN_ON(!dma_resv_trylock(bo->base.resv)); + else + dma_resv_assert_held(resv);
- if (likely(!ret)) - ret = ttm_bo_validate(bo, placement, ctx); + ret = ttm_bo_validate(bo, placement, ctx); + if (unlikely(ret)) + goto err_unlock;
- if (unlikely(ret)) { - if (!resv) - ttm_bo_unreserve(bo); + return 0;
- ttm_bo_put(bo); - return ret; - } +err_unlock: + if (!resv) + dma_resv_unlock(bo->base.resv);
+err_put: + ttm_bo_put(bo); return ret; } -EXPORT_SYMBOL(ttm_bo_init_reserved); +EXPORT_SYMBOL(ttm_bo_init_validate);
/* * buffer object vm functions. diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 60dcc6a75248..052a68408b85 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -414,9 +414,9 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
drm_gem_private_object_init(vdev, &bo->base, size);
- ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size, - ttm_bo_type_kernel, placement, 0, - &ctx, NULL, NULL, vmw_bo_default_destroy); + ret = ttm_bo_init_validate(&dev_priv->bdev, bo, ttm_bo_type_kernel, + placement, 0, &ctx, NULL, NULL, + vmw_bo_default_destroy); if (unlikely(ret)) goto error_free;
@@ -498,10 +498,8 @@ int vmw_bo_init(struct vmw_private *dev_priv, size = ALIGN(size, PAGE_SIZE); drm_gem_private_object_init(vdev, &vmw_bo->base.base, size);
- ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size, - ttm_bo_type_device, - placement, - 0, &ctx, NULL, NULL, bo_free); + ret = ttm_bo_init_validate(bdev, &vmw_bo->base, ttm_bo_type_device, + placement, 0, &ctx, NULL, NULL, bo_free); if (unlikely(ret)) { return ret; } diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 51fbbd035c11..4b3bb6cfca39 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -319,47 +319,9 @@ void ttm_bo_unlock_delayed_workqueue(struct ttm_device *bdev, int resched); bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, const struct ttm_place *place);
-/** - * ttm_bo_init_reserved - * - * @bdev: Pointer to a ttm_device struct. - * @bo: Pointer to a ttm_buffer_object to be initialized. - * @size: Requested size of buffer object. - * @type: Requested type of buffer object. - * @placement: Initial placement for buffer object. - * @page_alignment: Data alignment in pages. - * @ctx: TTM operation context for memory allocation. - * @sg: Scatter-gather table. - * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. - * @destroy: Destroy function. Use NULL for kfree(). - * - * This function initializes a pre-allocated struct ttm_buffer_object. - * As this object may be part of a larger structure, this function, - * together with the @destroy function, - * enables driver-specific objects derived from a ttm_buffer_object. - * - * On successful return, the caller owns an object kref to @bo. The kref and - * list_kref are usually set to 1, but note that in some situations, other - * tasks may already be holding references to @bo as well. - * Furthermore, if resv == NULL, the buffer's reservation lock will be held, - * and it is the caller's responsibility to call ttm_bo_unreserve. - * - * If a failure occurs, the function will call the @destroy function, or - * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is - * illegal and will likely cause memory corruption. - * - * Returns - * -ENOMEM: Out of memory. - * -EINVAL: Invalid placement flags. - * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. - */ - -int ttm_bo_init_reserved(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - struct ttm_operation_ctx *ctx, +int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, + enum ttm_bo_type type, struct ttm_placement *placement, + uint32_t alignment, struct ttm_operation_ctx *ctx, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *));
Make sure we can at least move and release BOs without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 4cb3afbdf6b9..49e8ba118322 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -1299,7 +1299,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo) if (bo->base.resv == &bo->base._resv) amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
- if (bo->resource->mem_type != TTM_PL_VRAM || + if (!bo->resource || bo->resource->mem_type != TTM_PL_VRAM || !(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE)) return;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 57ac118fc266..22954c84df39 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -476,7 +476,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
adev = amdgpu_ttm_adev(bo->bdev);
- if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { + if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && + bo->ttm == NULL)) { ttm_bo_move_null(bo, new_mem); goto out; }
Make sure we can at least move and release BOs without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 43c862bbbbfb..5b937d68a291 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1003,7 +1003,8 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, }
/* Fake bo copy. */ - if (old_reg->mem_type == TTM_PL_SYSTEM && !bo->ttm) { + if (!old_reg || (old_reg->mem_type == TTM_PL_SYSTEM && + !bo->ttm)) { ttm_bo_move_null(bo, new_reg); goto out; }
Allow BOs to exist without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index b7e259245f82..bd001fdde9fb 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -117,12 +117,13 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, struct ttm_place *hop) { - struct ttm_resource_manager *old_man, *new_man; struct ttm_device *bdev = bo->bdev; + bool old_use_tt, new_use_tt; int ret;
- old_man = ttm_manager_type(bdev, bo->resource->mem_type); - new_man = ttm_manager_type(bdev, mem->mem_type); + old_use_tt = bo->resource && + ttm_manager_type(bdev, bo->resource->mem_type)->use_tt; + new_use_tt = ttm_manager_type(bdev, mem->mem_type)->use_tt;
ttm_bo_unmap_virtual(bo);
@@ -130,11 +131,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, * Create and bind a ttm if required. */
- if (new_man->use_tt) { + if (new_use_tt) { /* Zero init the new TTM structure if the old location should * have used one as well. */ - ret = ttm_tt_create(bo, old_man->use_tt); + ret = ttm_tt_create(bo, old_use_tt); if (ret) goto out_err;
@@ -156,8 +157,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, return 0;
out_err: - new_man = ttm_manager_type(bdev, bo->resource->mem_type); - if (!new_man->use_tt) + if (!old_use_tt) ttm_bo_tt_destroy(bo);
return ret; @@ -900,7 +900,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, /* * Check whether we need to move buffer. */ - if (!ttm_resource_compat(bo->resource, placement)) { + if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { ret = ttm_bo_move_buffer(bo, placement, ctx); if (ret) return ret;
That should not be necessary any more when drivers should at least be able to handle the move without a resource.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bd001fdde9fb..9d40641dcf39 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -956,7 +956,6 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { - static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; int ret;
kref_init(&bo->kref); @@ -975,12 +974,6 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->base.resv = &bo->base._resv; atomic_inc(&ttm_glob.bo_count);
- ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); - if (unlikely(ret)) { - ttm_bo_put(bo); - return ret; - } - /* * For ttm_bo_type_device buffers, allocate * address space from the device.
That should not be necessary any more when drivers should at least be able to handle a move without a resource.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 0163e92b57af..979abe095f42 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -585,16 +585,10 @@ EXPORT_SYMBOL(ttm_bo_move_accel_cleanup); */ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { - static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost; - struct ttm_resource *sys_res; struct ttm_tt *ttm; int ret;
- ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); - if (ret) - return ret; - /* If already idle, no need for ghost object dance. */ ret = ttm_bo_wait(bo, false, true); if (ret != -EBUSY) { @@ -602,14 +596,13 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) /* See comment below about clearing. */ ret = ttm_tt_create(bo, true); if (ret) - goto error_free_sys_mem; + return ret; } else { ttm_tt_unpopulate(bo->bdev, bo->ttm); if (bo->type == ttm_bo_type_device) ttm_tt_mark_for_clear(bo->ttm); } ttm_resource_free(bo, &bo->resource); - ttm_bo_assign_mem(bo, sys_res); return 0; }
@@ -626,7 +619,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) ret = ttm_tt_create(bo, true); swap(bo->ttm, ttm); if (ret) - goto error_free_sys_mem; + return ret;
ret = ttm_buffer_object_transfer(bo, &ghost); if (ret) @@ -640,13 +633,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; - ttm_bo_assign_mem(bo, sys_res); return 0;
error_destroy_tt: ttm_tt_destroy(bo->bdev, ttm); - -error_free_sys_mem: - ttm_resource_free(bo, &sys_res); return ret; }
On Tue, Mar 29, 2022 at 01:02:32PM +0200, Christian König wrote:
Hi guys,
this patch set cleans up the handling of TTM buffer objects quite a bit by allowing to create them without allocating a ttm_resource as well.
That's not only cleaner in general, but also a necessary prerequisite for quite a number of related work.
Maybe there's some threads I missed, but I can't really guess what this could be useful for without even a hint. -Daniel
Am 29.03.22 um 16:02 schrieb Daniel Vetter:
On Tue, Mar 29, 2022 at 01:02:32PM +0200, Christian König wrote:
Hi guys,
this patch set cleans up the handling of TTM buffer objects quite a bit by allowing to create them without allocating a ttm_resource as well.
That's not only cleaner in general, but also a necessary prerequisite for quite a number of related work.
Maybe there's some threads I missed, but I can't really guess what this could be useful for without even a hint.
Well about a week ago Bob send me a partial implementation of this and said he needs it for i915. What exactly i915 needs here I'm not sure about either.
I have this cleanup in the pipeline because amdgpu wants to improve his page table handling with this and independent of those driver use case patches #10 and #11 drop allocating dummy resources for two use cases in TTM.
Christian.
-Daniel
dri-devel@lists.freedesktop.org
-
Christian König -
Daniel Vetter -
Zack Rusin