Re: [PATCH 6/7] drm/i915/ttm, drm/ttm: Introduce a TTM i915 gem object backend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux