Re: [RFC PATCH 5/7] drm/ttm: add range busy check for range manager

[Christian König]

Am 16.03.22 um 15:26 schrieb Robert Beckett:
> [SNIP]
> this is where I replace an existing range check via drm_mm with the 
> range check I added in this patch.

Mhm, I still don't get the use case from the code, but I don't think it 
matters any more.

> > > I suppose we could add another drm_mm range tracker just for testing 
> > > and shadow track each allocation in the range, but that seemed like 
> > > a lot of extra infrastructure for no general runtime use.
> >
> > I have no idea what you mean with that.
>
> I meant as a potential solution to tracking allocations without a 
> range check, we would need to add something external. e.g. adding a 
> shadow drm_mm range tracker, or a bitmask across the range, or stick 
> objects in a list etc.

Ah! So you are trying to get access to the drm_mm inside the 
ttm_range_manager and not add some additional range check function! Now 
I got your use case.

> > > would you mind explaining the rationale for removing range checks? 
> > > It seems to me like a natural fit for a memory manager
> >
> > TTM manages buffer objects and resources, not address space. The 
> > lpfn/fpfn parameter for the resource allocators are actually used as 
> > just two independent parameters and not define any range. We just 
> > keep the names for historical reasons.
> >
> > The only places we still use and compare them as ranges are 
> > ttm_resource_compat() and ttm_bo_eviction_valuable() and I already 
> > have patches to clean up those and move them into the backend 
> > resource handling.
>
> except the ttm_range_manager seems to still use them as a range specifier.

Yeah, because the range manager is the backend which handles ranges 
using the drm_mm :)

> If the general design going forward is to not consider ranges, how 
> would you recommend constructing buffers around pre-allocated regions 
> e.g. uefi frame buffers who's range is dictated externally?

Call ttm_bo_mem_space() with the fpfn/lpfn filled in as required. See 
function amdgpu_bo_create_kernel_at() for an example.

Regards,
Christian.

> > Regards,
> > Christian.
> >
> > > > Regards,
> > > > Christian.
> > > >
> > > > > Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>
> > > > > ---
> > > > >   drivers/gpu/drm/ttm/ttm_range_manager.c | 21 +++++++++++++++++++++
> > > > >   include/drm/ttm/ttm_range_manager.h     |  3 +++
> > > > >   2 files changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c 
> > > > > b/drivers/gpu/drm/ttm/ttm_range_manager.c
> > > > > index 8cd4f3fb9f79..5662627bb933 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_range_manager.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
> > > > > @@ -206,3 +206,24 @@ int ttm_range_man_fini_nocheck(struct 
> > > > > ttm_device *bdev,
> > > > >       return 0;
> > > > >   }
> > > > >   EXPORT_SYMBOL(ttm_range_man_fini_nocheck);
> > > > > +
> > > > > +/**
> > > > > + * ttm_range_man_range_busy - Check whether anything is allocated 
> > > > > with a range
> > > > > + *
> > > > > + * @man: memory manager to check
> > > > > + * @fpfn: first page number to check
> > > > > + * @lpfn: last page number to check
> > > > > + *
> > > > > + * Return: true if anything allocated within the range, false 
> > > > > otherwise.
> > > > > + */
> > > > > +bool ttm_range_man_range_busy(struct ttm_resource_manager *man,
> > > > > +                  unsigned fpfn, unsigned lpfn)
> > > > > +{
> > > > > +    struct ttm_range_manager *rman = to_range_manager(man);
> > > > > +    struct drm_mm *mm = &rman->mm;
> > > > > +
> > > > > +    if (__drm_mm_interval_first(mm, PFN_PHYS(fpfn), PFN_PHYS(lpfn 
> > > > > + 1) - 1))
> > > > > +        return true;
> > > > > +    return false;
> > > > > +}
> > > > > +EXPORT_SYMBOL(ttm_range_man_range_busy);
> > > > > diff --git a/include/drm/ttm/ttm_range_manager.h 
> > > > > b/include/drm/ttm/ttm_range_manager.h
> > > > > index 7963b957e9ef..86794a3f9101 100644
> > > > > --- a/include/drm/ttm/ttm_range_manager.h
> > > > > +++ b/include/drm/ttm/ttm_range_manager.h
> > > > > @@ -53,4 +53,7 @@ static __always_inline int 
> > > > > ttm_range_man_fini(struct ttm_device *bdev,
> > > > >       BUILD_BUG_ON(__builtin_constant_p(type) && type >= 
> > > > > TTM_NUM_MEM_TYPES);
> > > > >       return ttm_range_man_fini_nocheck(bdev, type);
> > > > >   }
> > > > > +
> > > > > +bool ttm_range_man_range_busy(struct ttm_resource_manager *man,
> > > > > +                  unsigned fpfn, unsigned lpfn);
> > > > >   #endif