Re: [Intel-gfx] [PATCH 3/7] drm/i915/ttm, drm/ttm: Initialize the ttm device and memory managers.

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Mon, 2021-05-17 at 09:18 +0200, Thomas Hellström wrote:
> On Wed, 2021-05-12 at 09:57 +0100, Matthew Auld wrote:
> > On Tue, 11 May 2021 at 14:26, Thomas Hellström
> > <thomas.hellstrom@linux.intel.com> wrote:
> > > 
> > > Temporarily remove the buddy allocator and related selftests
> > > and hook up the TTM range manager for i915 regions.
> > > 
> > > In order to support some of the mock region-related selftests, we
> > > need to
> > > be able to initialize the TTM range-manager standalone without a
> > > struct
> > > ttm_device. Add two functions to allow that to the TTM api.
> > > 
> > > Finally modify the mock region selftests somewhat to account for
> > > a
> > > fragmenting manager.
> > > 
> > > Cc: Christian König <christian.koenig@amd.com>
> > > Signed-off-by: Thomas Hellström
> > > <thomas.hellstrom@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/Kconfig                  |   1 +
> > >  drivers/gpu/drm/i915/Makefile                 |   2 +-
> > >  drivers/gpu/drm/i915/gem/i915_gem_lmem.c      |  58 +-
> > >  .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +-
> > >  drivers/gpu/drm/i915/gem/i915_gem_pages.c     |   3 +-
> > >  drivers/gpu/drm/i915/gem/i915_gem_region.c    | 120 ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_region.h    |   4 -
> > >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |   4 +-
> > >  drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  10 +-
> > >  drivers/gpu/drm/i915/gem/i915_gem_stolen.h    |   9 +-
> > >  drivers/gpu/drm/i915/gt/intel_gt.c            |   2 -
> > >  drivers/gpu/drm/i915/gt/intel_region_lmem.c   |  27 +-
> > >  drivers/gpu/drm/i915/i915_buddy.c             | 435 ----------
> > >  drivers/gpu/drm/i915/i915_buddy.h             | 131 ---
> > >  drivers/gpu/drm/i915/i915_drv.c               |   8 +
> > >  drivers/gpu/drm/i915/i915_drv.h               |   7 +-
> > >  drivers/gpu/drm/i915/i915_gem.c               |   1 +
> > >  drivers/gpu/drm/i915/i915_globals.c           |   1 -
> > >  drivers/gpu/drm/i915/i915_globals.h           |   1 -
> > >  drivers/gpu/drm/i915/i915_scatterlist.c       |  70 ++
> > >  drivers/gpu/drm/i915/i915_scatterlist.h       |  35 +
> > >  drivers/gpu/drm/i915/intel_memory_region.c    | 180 ++--
> > >  drivers/gpu/drm/i915/intel_memory_region.h    |  44 +-
> > >  drivers/gpu/drm/i915/intel_region_ttm.c       | 246 ++++++
> > >  drivers/gpu/drm/i915/intel_region_ttm.h       |  29 +
> > >  drivers/gpu/drm/i915/selftests/i915_buddy.c   | 789 ------------
> > > --
> > > ----
> > >  .../drm/i915/selftests/i915_mock_selftests.h  |   1 -
> > >  .../drm/i915/selftests/intel_memory_region.c  | 133 +--
> > >  drivers/gpu/drm/i915/selftests/mock_region.c  |  51 +-
> > >  drivers/gpu/drm/ttm/ttm_range_manager.c       |  55 +-
> > >  include/drm/ttm/ttm_bo_driver.h               |  23 +
> > >  31 files changed, 715 insertions(+), 1771 deletions(-)
> > >  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c
> > >  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h
> > >  create mode 100644 drivers/gpu/drm/i915/intel_region_ttm.c
> > >  create mode 100644 drivers/gpu/drm/i915/intel_region_ttm.h
> > >  delete mode 100644 drivers/gpu/drm/i915/selftests/i915_buddy.c
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Kconfig
> > > b/drivers/gpu/drm/i915/Kconfig
> > > index 1e1cb245fca7..b63d374dff23 100644
> > > --- a/drivers/gpu/drm/i915/Kconfig
> > > +++ b/drivers/gpu/drm/i915/Kconfig
> > > @@ -26,6 +26,7 @@ config DRM_I915
> > >         select SND_HDA_I915 if SND_HDA_CORE
> > >         select CEC_CORE if CEC_NOTIFIER
> > >         select VMAP_PFN
> > > +       select DRM_TTM
> > >         help
> > >           Choose this option if you have a system that has "Intel
> > > Graphics
> > >           Media Accelerator" or "HD Graphics" integrated
> > > graphics,
> > > diff --git a/drivers/gpu/drm/i915/Makefile
> > > b/drivers/gpu/drm/i915/Makefile
> > > index d0d936d9137b..cb8823570996 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -50,6 +50,7 @@ i915-y += i915_drv.o \
> > >           intel_memory_region.o \
> > >           intel_pch.o \
> > >           intel_pm.o \
> > > +         intel_region_ttm.o \
> > >           intel_runtime_pm.o \
> > >           intel_sideband.o \
> > >           intel_step.o \
> > > @@ -160,7 +161,6 @@ gem-y += \
> > >  i915-y += \
> > >           $(gem-y) \
> > >           i915_active.o \
> > > -         i915_buddy.o \
> > >           i915_cmd_parser.o \
> > >           i915_gem_evict.o \
> > >           i915_gem_gtt.o \
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > index f44bdd08f7cb..f42803ea48f2 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > @@ -4,16 +4,70 @@
> > >   */
> > > 
> > >  #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);
> > 
> > error unwind?
> > 
> > > +
> > > +       __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);
> > > +
> > > +               if (!vaddr) {
> > > +                       struct sg_table *pages =
> > > +                              
> > > __i915_gem_object_unset_pages(obj);
> > > +
> > > +                       if (!IS_ERR_OR_NULL(pages))
> > > +                               lmem_put_pages(obj, pages);
> > > +               }
> > > +
> > > +               memset_io(vaddr, 0, obj->base.size);
> > > +               io_mapping_unmap(vaddr);
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > 
> > <snip>
> > 
> > > +/**
> > > + * i915_sg_dma_page_sizes - Calculate page sizes from a
> > > scatterlist
> > > + * @sg: The scatterlist from which to calculate page sizes
> > > + *
> > > + * Return: a value with bits sets for relevant page sizes.
> > > + */
> > > +static inline unsigned int i915_sg_dma_page_sizes(struct
> > > scatterlist *sg)
> > > +{
> > > +       unsigned int page_sizes;
> > > +
> > > +       page_sizes = 0;
> > > +       while (sg) {
> > > +               GEM_BUG_ON(sg->offset);
> > > +               GEM_BUG_ON(!IS_ALIGNED(sg->dma_length,
> > > PAGE_SIZE));
> > > +               page_sizes |= sg->dma_length;
> > > +               sg = __sg_next(sg);
> > > +       }
> > > +
> > > +       /*
> > > +        * Is this necessary to support building large GPU pages
> > > from
> > > +        * even larger segments?
> > > +        */
> > 
> > We just need to know the above sg page_sizes, which is the mask of
> > sg->dma_length for each sg. Later in __i915_gem_object_set_pages()
> > we
> > use this to determine the potential GPU page sizes for the object,
> > depending on what the device supports. So the below stuff shouldn't
> > be
> > needed. Also we can just use i915_sg_page_sizes() here.
> 
> I noticed that i915_sg_page_sizes() uses sg->length() instead of sg-
> > dma_length(). That doesn't look like what we're after here. Do you
> know if there is a reason for that?

Actually, looking at the users, that seems like a bug. I'll make a
separate patch out of it.

/Thomas

> 
> Thanks,
> Thomas
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx