diff mbox series

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

Message ID 20240723132525.31294-1-Arunpravin.PaneerSelvam@amd.com (mailing list archive)
State New, archived
Headers show
Series [v7,1/2] drm/buddy: Add start address support to trim function | expand

Commit Message

Paneer Selvam, Arunpravin July 23, 2024, 1:25 p.m. UTC
- 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>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Christian König <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(-)


base-commit: b27d70e1042bf6a31ba7e5acf58b61c9cd28f95b

Comments

Paneer Selvam, Arunpravin July 23, 2024, 1:43 p.m. UTC | #1
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?

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>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Acked-by: Christian König <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
Matthew Auld July 23, 2024, 3:02 p.m. UTC | #2
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>
>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>> Acked-by: Christian König <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
>
Marek Olšák July 24, 2024, 1:35 a.m. UTC | #3
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.

Marek

On Tue, Jul 23, 2024, 11:04 Matthew Auld <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>
> >> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> >> Acked-by: Christian König <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
> >
>
Matthew Auld July 24, 2024, 9:37 a.m. UTC | #4
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
>      >
>
Matthew Auld July 24, 2024, 9:48 a.m. UTC | #5
On 23/07/2024 14:25, 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>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>

Given the comment from Marek that this is about non power-of-two 
alignment, this makes sense (although might be good to mention that 
somewhere in the commit message to make that clearer),
Reviewed-by: Matthew Auld <matthew.auld@intel.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
Christian König July 24, 2024, 11:20 a.m. UTC | #6
Am 24.07.24 um 11:37 schrieb Matthew Auld:
> 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.

Well actually the requirement is that memory reads for scanout needs to 
be distributed over the memory channels in a certain way.

Our hw guys just expressed that as a rather strange non-power of two 
alignment :)

Christian.

>
>>
>> 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
>>      >
>>
Jani Nikula July 24, 2024, 3:12 p.m. UTC | #7
On Tue, 23 Jul 2024, Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> 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>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Acked-by: Christian König <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,

Is this a pointer only to allow passing NULL as "don't specify"? It's
not used for returning anything in *start. Maybe it should be a const
pointer?

Not the prettiest of interfaces, and the kernel-doc does not reflect any
of this.

BR,
Jani.


>  			 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
Paneer Selvam, Arunpravin July 26, 2024, 2:02 p.m. UTC | #8
On 7/24/2024 8:42 PM, Jani Nikula wrote:
> On Tue, 23 Jul 2024, Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> 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>
>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>> Acked-by: Christian König <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,
> Is this a pointer only to allow passing NULL as "don't specify"? It's
> not used for returning anything in *start. Maybe it should be a const
> pointer?
>
> Not the prettiest of interfaces, and the kernel-doc does not reflect any
> of this.
We have a plan to improve the interface and add multiple block trim 
support as well.
I will modify in the follow-up patch.

Thanks,
Arun.
>
> BR,
> Jani.
>
>
>>   			 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
diff mbox series

Patch

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);