On Tue, 11 May 2021 at 14:26, Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx> wrote:
>
> Most logical place to introduce TTM buffer objects is as an i915
> gem object backend. We need to add some ops to account for added
> functionality like delayed delete and LRU list manipulation.
>
> Initially we support only LMEM and SYSTEM memory, but SYSTEM
> (which in this case means evicted LMEM objects) is not
> visible to i915 GEM yet. The plan is to move the i915 gem system region
> over to the TTM system memory type in upcoming patches.
>
> We set up GPU bindings directly both from LMEM and from the system region,
> as there is no need to use the legacy TTM_TT memory type. We reserve
> that for future porting of GGTT bindings to TTM.
>
> There are some changes to TTM to allow for purging system memory buffer
> objects and to refuse swapping of some objects: Unfortunately i915 gem
> still relies heavily on short-term object pinning, and we've chosen to
> keep short-term-pinned buffer objects on the TTM LRU lists for now,
> meaning that we need some sort of mechanism to tell TTM they are not
> swappable. A longer term goal is to get rid of the short-term pinning.
>
> Remove the old lmem backend.
>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 83 ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 5 -
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 126 +++--
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 18 +
> drivers/gpu/drm/i915/gem/i915_gem_region.c | 6 +-
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 534 ++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 48 ++
> drivers/gpu/drm/i915/gt/intel_region_lmem.c | 3 +-
> drivers/gpu/drm/i915/i915_gem.c | 5 +-
> drivers/gpu/drm/i915/intel_memory_region.c | 1 -
> drivers/gpu/drm/i915/intel_memory_region.h | 1 -
> drivers/gpu/drm/i915/intel_region_ttm.c | 5 +-
> drivers/gpu/drm/i915/intel_region_ttm.h | 7 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 12 +
> include/drm/ttm/ttm_device.h | 9 +
> 17 files changed, 733 insertions(+), 140 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 958ccc1edfed..ef0d884a9e2d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -155,6 +155,7 @@ gem-y += \
> gem/i915_gem_stolen.o \
> gem/i915_gem_throttle.o \
> gem/i915_gem_tiling.o \
> + gem/i915_gem_ttm.o \
> gem/i915_gem_ttm_bo_util.o \
> gem/i915_gem_userptr.o \
> gem/i915_gem_wait.o \
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index f42803ea48f2..2b8cd15de1d9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -4,73 +4,10 @@
> */
>
> #include "intel_memory_region.h"
> -#include "intel_region_ttm.h"
> #include "gem/i915_gem_region.h"
> #include "gem/i915_gem_lmem.h"
> #include "i915_drv.h"
>
> -static void lmem_put_pages(struct drm_i915_gem_object *obj,
> - struct sg_table *pages)
> -{
> - intel_region_ttm_node_free(obj->mm.region, obj->mm.st_mm_node);
> - obj->mm.dirty = false;
> - sg_free_table(pages);
> - kfree(pages);
> -}
> -
> -static int lmem_get_pages(struct drm_i915_gem_object *obj)
> -{
> - unsigned int flags;
> - struct sg_table *pages;
> -
> - flags = I915_ALLOC_MIN_PAGE_SIZE;
> - if (obj->flags & I915_BO_ALLOC_CONTIGUOUS)
> - flags |= I915_ALLOC_CONTIGUOUS;
> -
> - obj->mm.st_mm_node = intel_region_ttm_node_alloc(obj->mm.region,
> - obj->base.size,
> - flags);
> - if (IS_ERR(obj->mm.st_mm_node))
> - return PTR_ERR(obj->mm.st_mm_node);
> -
> - /* Range manager is always contigous */
> - if (obj->mm.region->is_range_manager)
> - obj->flags |= I915_BO_ALLOC_CONTIGUOUS;
> - pages = intel_region_ttm_node_to_st(obj->mm.region, obj->mm.st_mm_node);
> - if (IS_ERR(pages))
> - return PTR_ERR(pages);
> -
> - __i915_gem_object_set_pages(obj, pages,
> - i915_sg_dma_page_sizes(pages->sgl));
> -
> - if (obj->flags & I915_BO_ALLOC_CPU_CLEAR) {
> - void __iomem *vaddr =
> - i915_gem_object_lmem_io_map(obj, 0, obj->base.size);
Where did the object clearing go? I'm not seeing it in the new code.
<snip>
> +
> +static struct sg_table *
> +i915_ttm_resource_get_st(struct drm_i915_gem_object *obj,
> + struct ttm_resource *res)
> +{
> + struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
> + struct ttm_resource_manager *man =
> + ttm_manager_type(bo->bdev, res->mem_type);
> + struct sg_table *st;
> +
> + if (man->use_tt)
> + return i915_ttm_tt_get_st(bo->ttm);
> +
> + st = kzalloc(sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return ERR_PTR(-ENOMEM);
The st is already allocated below.
<snip>
> +
> +/**
> + * __i915_gem_ttm_object_init - Initialize a ttm-backed i915 gem object
> + * @mem: The initial memory region for the object.
> + * @obj: The gem object.
> + * @size: Object size in bytes.
> + * @flags: gem object flags.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int __i915_gem_ttm_object_init(struct intel_memory_region *mem,
> + struct drm_i915_gem_object *obj,
> + resource_size_t size,
> + unsigned int flags)
> +{
> + static struct lock_class_key lock_class;
> + struct drm_i915_private *i915 = mem->i915;
> + size_t alignment = 0;
> + int ret;
> +
> + /* Adjust alignment to GPU- and CPU huge page sizes. */
> +
> + if (mem->is_range_manager) {
> + if (size >= SZ_1G)
> + alignment = SZ_1G >> PAGE_SHIFT;
> + else if (size >= SZ_2M)
> + alignment = SZ_2M >> PAGE_SHIFT;
> + else if (size >= SZ_64K)
> + alignment = SZ_64K >> PAGE_SHIFT;
> + }
> +
> + drm_gem_private_object_init(&i915->drm, &obj->base, size);
> + i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class, flags);
> + i915_gem_object_init_memory_region(obj, mem);
> + i915_gem_object_make_unshrinkable(obj);
> +
> + ret = ttm_bo_init(&i915->bdev, i915_gem_to_ttm(obj), size,
> + ttm_bo_type_kernel, &i915_sys_placement, alignment,
> + true, NULL, NULL, i915_ttm_bo_destroy);
Handle the error?
> +
> + obj->read_domains = I915_GEM_DOMAIN_WC | I915_GEM_DOMAIN_GTT;
> +
> + i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE);
> +
> + return 0;
> +}
