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@xxxxxxxxxxxxxxx> 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@xxxxxxx>
> > > Signed-off-by: Thomas Hellström
> > > <thomas.hellstrom@xxxxxxxxxxxxxxx>
> > > ---
> > > 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
>
