Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/ttm: switch over to ttm_buddy_man

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

On Tue, 2021-06-08 at 11:08 +0100, Matthew Auld wrote:
> On 08/06/2021 10:53, Thomas Hellström wrote:
> > 
> > On 6/8/21 10:44 AM, Matthew Auld wrote:
> > > Move back to the buddy allocator for managing device local
> > > memory, and
> > > restore the lost mock selftests. Keep around the range manager
> > > related
> > > bits, since we likely need this for managing stolen at some
> > > point. For
> > > stolen we also don't need to reserve anything so no need to
> > > support a
> > > generic reserve interface.
> > > 
> > > v2(Thomas):
> > >      - bo->page_alignment is in page units, not bytes
> > > 
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gem/i915_gem_ttm.c       |  26 +--
> > >   drivers/gpu/drm/i915/intel_memory_region.c    |  55 +-----
> > >   drivers/gpu/drm/i915/intel_memory_region.h    |  17 --
> > >   drivers/gpu/drm/i915/intel_region_ttm.c       | 100 +++--------
> > >   .../drm/i915/selftests/intel_memory_region.c  | 170
> > > ++++++++++++------
> > >   drivers/gpu/drm/i915/selftests/mock_region.c  |  15 +-
> > >   6 files changed, 168 insertions(+), 215 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
> > > b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > index c612275c36c9..5bf1d1945dd6 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > > @@ -181,11 +181,7 @@ static bool
> > > i915_ttm_eviction_valuable(struct 
> > > ttm_buffer_object *bo,
> > >       struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> > >       /* Will do for now. Our pinned objects are still on TTM's
> > > LRU 
> > > lists */
> > > -    if (!i915_gem_object_evictable(obj))
> > > -        return false;
> > > -
> > > -    /* This isn't valid with a buddy allocator */
> > > -    return ttm_bo_eviction_valuable(bo, place);
> > > +    return i915_gem_object_evictable(obj);
> > >   }
> > >   static void i915_ttm_evict_flags(struct ttm_buffer_object *bo,
> > > @@ -328,11 +324,15 @@ i915_ttm_resource_get_st(struct 
> > > drm_i915_gem_object *obj,
> > >       struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
> > >       struct ttm_resource_manager *man =
> > >           ttm_manager_type(bo->bdev, res->mem_type);
> > > +    struct intel_memory_region *mr = obj->mm.region;
> > >       if (man->use_tt)
> > >           return i915_ttm_tt_get_st(bo->ttm);
> > > -    return intel_region_ttm_node_to_st(obj->mm.region, res-
> > > >mm_node);
> > > +    if (mr->is_range_manager)
> > 
> > Did you look at moving this test into
> > intel_region_ttm_node_to_st())
> 
> I guess I didn't like the _node since that seems to suggest
> drm_mm_node 
> to me.
> 
> What about something like:
> i915_ttm_resource_to_st(res, mr)
> intel_region_ttm_resource_to_st(mr, res)

intel_region_ttm_resource_to_st() would be nice I think. I think it
would be nice if the region ttm code could hide the manager selection.

/Thomas


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