Re: [PATCH v7 1/2] drm/buddy: Add start address support to trim function

[Matthew Auld <matthew.auld@intel.com>]

On 24/07/2024 02:35, Marek Olšák wrote:
> The reason is that our DCC requires 768K alignment in some cases. I 
> haven't read this patch series, but one way to do that is to align to 
> 256K, overallocate by 512K, and then not use either 0, 256K, or 512K at 
> the beginning to get to 768K alignment.

Ah, so we need a non power-of-two alignment. That makes sense, thanks.

> 
> Marek
> 
> On Tue, Jul 23, 2024, 11:04 Matthew Auld <matthew.auld@intel.com 
> <mailto:matthew.auld@intel.com>> wrote:
> 
>     On 23/07/2024 14:43, Paneer Selvam, Arunpravin wrote:
>      > Hi Matthew,
>      >
>      > Can we push this version for now as we need to mainline the DCC
>     changes
>      > ASAP,
>      > while we continue our discussion and proceed to implement the
>     permanent
>      > solution
>      > for address alignment?
> 
>     Yeah, we can always merge now and circle back around later, if this for
>     sure helps your usecase and is needed asap. I just didn't fully get the
>     idea for needing this interface, but likely I am missing something.
> 
>      >
>      > Thanks,
>      > Arun.
>      >
>      > On 7/23/2024 6:55 PM, Arunpravin Paneer Selvam wrote:
>      >> - Add a new start parameter in trim function to specify exact
>      >>    address from where to start the trimming. This would help us
>      >>    in situations like if drivers would like to do address alignment
>      >>    for specific requirements.
>      >>
>      >> - Add a new flag DRM_BUDDY_TRIM_DISABLE. Drivers can use this
>      >>    flag to disable the allocator trimming part. This patch enables
>      >>    the drivers control trimming and they can do it themselves
>      >>    based on the application requirements.
>      >>
>      >> v1:(Matthew)
>      >>    - check new_start alignment with min chunk_size
>      >>    - use range_overflows()
>      >>
>      >> Signed-off-by: Arunpravin Paneer Selvam
>     <Arunpravin.PaneerSelvam@amd.com
>     <mailto:Arunpravin.PaneerSelvam@amd.com>>
>      >> Acked-by: Alex Deucher <alexander.deucher@amd.com
>     <mailto:alexander.deucher@amd.com>>
>      >> Acked-by: Christian König <christian.koenig@amd.com
>     <mailto:christian.koenig@amd.com>>
>      >> ---
>      >>   drivers/gpu/drm/drm_buddy.c          | 25
>     +++++++++++++++++++++++--
>      >>   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c |  2 +-
>      >>   include/drm/drm_buddy.h              |  2 ++
>      >>   3 files changed, 26 insertions(+), 3 deletions(-)
>      >>
>      >> diff --git a/drivers/gpu/drm/drm_buddy.c
>     b/drivers/gpu/drm/drm_buddy.c
>      >> index 6a8e45e9d0ec..103c185bb1c8 100644
>      >> --- a/drivers/gpu/drm/drm_buddy.c
>      >> +++ b/drivers/gpu/drm/drm_buddy.c
>      >> @@ -851,6 +851,7 @@ static int __alloc_contig_try_harder(struct
>      >> drm_buddy *mm,
>      >>    * drm_buddy_block_trim - free unused pages
>      >>    *
>      >>    * @mm: DRM buddy manager
>      >> + * @start: start address to begin the trimming.
>      >>    * @new_size: original size requested
>      >>    * @blocks: Input and output list of allocated blocks.
>      >>    * MUST contain single block as input to be trimmed.
>      >> @@ -866,11 +867,13 @@ static int __alloc_contig_try_harder(struct
>      >> drm_buddy *mm,
>      >>    * 0 on success, error code on failure.
>      >>    */
>      >>   int drm_buddy_block_trim(struct drm_buddy *mm,
>      >> +             u64 *start,
>      >>                u64 new_size,
>      >>                struct list_head *blocks)
>      >>   {
>      >>       struct drm_buddy_block *parent;
>      >>       struct drm_buddy_block *block;
>      >> +    u64 block_start, block_end;
>      >>       LIST_HEAD(dfs);
>      >>       u64 new_start;
>      >>       int err;
>      >> @@ -882,6 +885,9 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>      >>                    struct drm_buddy_block,
>      >>                    link);
>      >> +    block_start = drm_buddy_block_offset(block);
>      >> +    block_end = block_start + drm_buddy_block_size(mm, block);
>      >> +
>      >>       if (WARN_ON(!drm_buddy_block_is_allocated(block)))
>      >>           return -EINVAL;
>      >> @@ -894,6 +900,20 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>      >>       if (new_size == drm_buddy_block_size(mm, block))
>      >>           return 0;
>      >> +    new_start = block_start;
>      >> +    if (start) {
>      >> +        new_start = *start;
>      >> +
>      >> +        if (new_start < block_start)
>      >> +            return -EINVAL;
>      >> +
>      >> +        if (!IS_ALIGNED(new_start, mm->chunk_size))
>      >> +            return -EINVAL;
>      >> +
>      >> +        if (range_overflows(new_start, new_size, block_end))
>      >> +            return -EINVAL;
>      >> +    }
>      >> +
>      >>       list_del(&block->link);
>      >>       mark_free(mm, block);
>      >>       mm->avail += drm_buddy_block_size(mm, block);
>      >> @@ -904,7 +924,6 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>      >>       parent = block->parent;
>      >>       block->parent = NULL;
>      >> -    new_start = drm_buddy_block_offset(block);
>      >>       list_add(&block->tmp_link, &dfs);
>      >>       err =  __alloc_range(mm, &dfs, new_start, new_size,
>     blocks, NULL);
>      >>       if (err) {
>      >> @@ -1066,7 +1085,8 @@ int drm_buddy_alloc_blocks(struct
>     drm_buddy *mm,
>      >>       } while (1);
>      >>       /* Trim the allocated block to the required size */
>      >> -    if (original_size != size) {
>      >> +    if (!(flags & DRM_BUDDY_TRIM_DISABLE) &&
>      >> +        original_size != size) {
>      >>           struct list_head *trim_list;
>      >>           LIST_HEAD(temp);
>      >>           u64 trim_size;
>      >> @@ -1083,6 +1103,7 @@ int drm_buddy_alloc_blocks(struct
>     drm_buddy *mm,
>      >>           }
>      >>           drm_buddy_block_trim(mm,
>      >> +                     NULL,
>      >>                        trim_size,
>      >>                        trim_list);
>      >> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>      >> b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>      >> index fe3779fdba2c..423b261ea743 100644
>      >> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>      >> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>      >> @@ -150,7 +150,7 @@ static int xe_ttm_vram_mgr_new(struct
>      >> ttm_resource_manager *man,
>      >>       } while (remaining_size);
>      >>       if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>      >> -        if (!drm_buddy_block_trim(mm, vres->base.size,
>     &vres->blocks))
>      >> +        if (!drm_buddy_block_trim(mm, NULL, vres->base.size,
>      >> &vres->blocks))
>      >>               size = vres->base.size;
>      >>       }
>      >> diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
>      >> index 2a74fa9d0ce5..9689a7c5dd36 100644
>      >> --- a/include/drm/drm_buddy.h
>      >> +++ b/include/drm/drm_buddy.h
>      >> @@ -27,6 +27,7 @@
>      >>   #define DRM_BUDDY_CONTIGUOUS_ALLOCATION        BIT(2)
>      >>   #define DRM_BUDDY_CLEAR_ALLOCATION        BIT(3)
>      >>   #define DRM_BUDDY_CLEARED            BIT(4)
>      >> +#define DRM_BUDDY_TRIM_DISABLE            BIT(5)
>      >>   struct drm_buddy_block {
>      >>   #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>      >> @@ -155,6 +156,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>      >>                  unsigned long flags);
>      >>   int drm_buddy_block_trim(struct drm_buddy *mm,
>      >> +             u64 *start,
>      >>                u64 new_size,
>      >>                struct list_head *blocks);
>      >>
>      >> base-commit: b27d70e1042bf6a31ba7e5acf58b61c9cd28f95b
>      >
>