Re: [Intel-gfx] [PATCH 02/20] drm: implement top-down allocation method

Robert Beckett
2022-01-26T18:42:11Z

On 26/01/2022 15:21, Matthew Auld wrote:
> From: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
>
> Implemented a function which walk through the order list,
> compares the offset and returns the maximum offset block,
> this method is unpredictable in obtaining the high range
> address blocks which depends on allocation and deallocation.
> for instance, if driver requests address at a low specific
> range, allocator traverses from the root block and splits
> the larger blocks until it reaches the specific block and
> in the process of splitting, lower orders in the freelist
> are occupied with low range address blocks and for the
> subsequent TOPDOWN memory request we may return the low
> range blocks.To overcome this issue, we may go with the
> below approach.
>
> The other approach, sorting each order list entries in
> ascending order and compares the last entry of each
> order list in the freelist and return the max block.
> This creates sorting overhead on every drm_buddy_free()
> request and split up of larger blocks for a single page
> request.

ooc, why did you choose to implement this as an alloc flag?
Seems to me like it would be a good candidate for a new memory region.
That way allocation algorithms wouldn't need exta logic and ttm can
already handle migrations.

>
> v2:
> - Fix alignment issues(Matthew Auld)
> - Remove unnecessary list_empty check(Matthew Auld)
> - merged the below patch to see the feature in action
> - add top-down alloc support to i915 driver
>
> Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
> ---
> drivers/gpu/drm/drm_buddy.c | 36 ++++++++++++++++---
> drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 3 ++
> include/drm/drm_buddy.h | 1 +
> 3 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index 954e31962c74..6aa5c1ce25bf 100644
> --- a/drivers/gpu/drm/drm_buddy.c
> +++ b/drivers/gpu/drm/drm_buddy.c
> @@ -371,6 +371,26 @@ alloc_range_bias(struct drm_buddy *mm,
> return ERR_PTR(err);
> }
>
> +static struct drm_buddy_block *
> +get_maxblock(struct list_head *head)
> +{
> + struct drm_buddy_block *max_block = NULL, *node;
> +
> + max_block = list_first_entry_or_null(head,
> + struct drm_buddy_block,
> + link);
> + if (!max_block)
> + return NULL;
> +
> + list_for_each_entry(node, head, link) {
> + if (drm_buddy_block_offset(node) >
> + drm_buddy_block_offset(max_block))
> + max_block = node;
> + }
> +
> + return max_block;
> +}
> +
> static struct drm_buddy_block *
> alloc_from_freelist(struct drm_buddy *mm,
> unsigned int order,
> @@ -381,11 +401,17 @@ alloc_from_freelist(struct drm_buddy *mm,
> int err;
>
> for (i = order; i <= mm->max_order; ++i) {
> - block = list_first_entry_or_null(&mm->free_list[i],
> - struct drm_buddy_block,
> - link);
> - if (block)
> - break;
> + if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) {
> + block = get_maxblock(&mm->free_list[i]);
> + if (block)
> + break;
> + } else {
> + block = list_first_entry_or_null(&mm->free_list[i],
> + struct drm_buddy_block,
> + link);
> + if (block)
> + break;
> + }
> }
>
> if (!block)
> diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> index 1411f4cf1f21..3662434b64bb 100644
> --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> @@ -53,6 +53,9 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
> INIT_LIST_HEAD(&bman_res->blocks);
> bman_res->mm = mm;
>
> + if (place->flags & TTM_PL_FLAG_TOPDOWN)
> + bman_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION;
> +
> if (place->fpfn || lpfn != man->size)
> bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION;
>
> diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
> index 865664b90a8a..424fc443115e 100644
> --- a/include/drm/drm_buddy.h
> +++ b/include/drm/drm_buddy.h
> @@ -28,6 +28,7 @@
> })
>
> #define DRM_BUDDY_RANGE_ALLOCATION (1 << 0)
> +#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1)
>
> struct drm_buddy_block {
> #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)