[PATCH] drm/buddy: Add start address support to trim function

[PATCH] drm/buddy: Add start address support to trim function

[Arunpravin Paneer Selvam]

- 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.

Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/drm_buddy.c          | 22 ++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_ttm_vram_mgr.c |  2 +-
 include/drm/drm_buddy.h              |  2 ++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 6a8e45e9d0ec..287b6acb1637 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) - 1;
+
 	if (WARN_ON(!drm_buddy_block_is_allocated(block)))
 		return -EINVAL;
 
@@ -894,6 +900,17 @@ 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 ((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 +921,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 +1082,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 +1100,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);
 
-- 
2.25.1

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

[Matthew Auld]

Hi,

On 21/06/2024 06:29, 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.
> 
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
>   drivers/gpu/drm/drm_buddy.c          | 22 ++++++++++++++++++++--
>   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c |  2 +-
>   include/drm/drm_buddy.h              |  2 ++
>   3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> index 6a8e45e9d0ec..287b6acb1637 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,

I guess just wondering if this should be offset within or address. If it 
offset then zero be the valid default giving the existing behaviour. But 
hard to say without seeing the user for this. Are there some more 
patches to give some context for this usecase?

>   			 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) - 1;
> +
>   	if (WARN_ON(!drm_buddy_block_is_allocated(block)))
>   		return -EINVAL;
>   
> @@ -894,6 +900,17 @@ 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;

In addition should check that the alignment of new_start is at least 
compatible with the min chunk_size. Otherwise I think bad stuff can happen.

> +
> +		if ((new_start + new_size) > block_end)

range_overflows() ?

> +			return -EINVAL;
> +	}
> +
>   	list_del(&block->link);
>   	mark_free(mm, block);
>   	mm->avail += drm_buddy_block_size(mm, block);
> @@ -904,7 +921,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 +1082,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 +1100,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);
>   

[PATCH] drm/buddy: Add start address support to trim function

[Arunpravin Paneer Selvam]

- 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>
---
 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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
 
-- 
2.25.1

[PATCH 2/2] drm/amdgpu: Add address alignment support to DCC buffers

[Arunpravin Paneer Selvam]

Add address alignment support to the DCC VRAM buffers.

v2:
  - adjust size based on the max_texture_channel_caches values
    only for GFX12 DCC buffers.
  - used AMDGPU_GEM_CREATE_GFX12_DCC flag to apply change only
    for DCC buffers.
  - roundup non power of two DCC buffer adjusted size to nearest
    power of two number as the buddy allocator does not support non
    power of two alignments. This applies only to the contiguous
    DCC buffers.

v3:(Alex)
  - rewrite the max texture channel caches comparison code in an
    algorithmic way to determine the alignment size.

v4:(Alex)
  - Move the logic from amdgpu_vram_mgr_dcc_alignment() to gmc_v12_0.c
    and add a new gmc func callback for dcc alignment. If the callback
    is non-NULL, call it to get the alignment, otherwise, use the default.

v5:(Alex)
  - Set the Alignment to a default value if the callback doesn't exist.
  - Add the callback to amdgpu_gmc_funcs.

Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h      |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 36 ++++++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c       | 15 ++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index febca3130497..49dfcf112ac1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -156,6 +156,8 @@ struct amdgpu_gmc_funcs {
 				      uint64_t addr, uint64_t *flags);
 	/* get the amount of memory used by the vbios for pre-OS console */
 	unsigned int (*get_vbios_fb_size)(struct amdgpu_device *adev);
+	/* get the DCC buffer alignment */
+	u64 (*get_dcc_alignment)(struct amdgpu_device *adev);
 
 	enum amdgpu_memory_partition (*query_mem_partition_mode)(
 		struct amdgpu_device *adev);
@@ -363,6 +365,7 @@ struct amdgpu_gmc {
 	(adev)->gmc.gmc_funcs->override_vm_pte_flags			\
 		((adev), (vm), (addr), (pte_flags))
 #define amdgpu_gmc_get_vbios_fb_size(adev) (adev)->gmc.gmc_funcs->get_vbios_fb_size((adev))
+#define amdgpu_gmc_get_dcc_alignment(adev) ((adev)->gmc.gmc_funcs->get_dcc_alignment((adev)))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index eb94f943b28e..756d1c8cbffd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -509,6 +509,16 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
 		vres->flags |= DRM_BUDDY_RANGE_ALLOCATION;
 
 	remaining_size = (u64)vres->base.size;
+	if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
+	    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC) {
+		u64 adjust_size;
+
+		if (adev->gmc.gmc_funcs->get_dcc_alignment) {
+			adjust_size = amdgpu_gmc_get_dcc_alignment(adev);
+			remaining_size = roundup_pow_of_two(remaining_size + adjust_size);
+			vres->flags |= DRM_BUDDY_TRIM_DISABLE;
+		}
+	}
 
 	mutex_lock(&mgr->lock);
 	while (remaining_size) {
@@ -518,8 +528,12 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
 			min_block_size = mgr->default_page_size;
 
 		size = remaining_size;
-		if ((size >= (u64)pages_per_block << PAGE_SHIFT) &&
-		    !(size & (((u64)pages_per_block << PAGE_SHIFT) - 1)))
+
+		if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
+		    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC)
+			min_block_size = size;
+		else if ((size >= (u64)pages_per_block << PAGE_SHIFT) &&
+			 !(size & (((u64)pages_per_block << PAGE_SHIFT) - 1)))
 			min_block_size = (u64)pages_per_block << PAGE_SHIFT;
 
 		BUG_ON(min_block_size < mm->chunk_size);
@@ -550,6 +564,24 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
 	}
 	mutex_unlock(&mgr->lock);
 
+	if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
+	    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC) {
+		struct drm_buddy_block *dcc_block;
+		u64 dcc_start, alignment;
+
+		dcc_block = amdgpu_vram_mgr_first_block(&vres->blocks);
+		dcc_start = amdgpu_vram_mgr_block_start(dcc_block);
+
+		if (adev->gmc.gmc_funcs->get_dcc_alignment) {
+			alignment = amdgpu_gmc_get_dcc_alignment(adev);
+			/* Adjust the start address for DCC buffers only */
+			dcc_start = roundup(dcc_start, alignment);
+			drm_buddy_block_trim(mm, &dcc_start,
+					     (u64)vres->base.size,
+					     &vres->blocks);
+		}
+	}
+
 	vres->base.start = 0;
 	size = max_t(u64, amdgpu_vram_mgr_blocks_size(&vres->blocks),
 		     vres->base.size);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index fd3ac483760e..4259edcdec8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -542,6 +542,20 @@ static unsigned gmc_v12_0_get_vbios_fb_size(struct amdgpu_device *adev)
 	return 0;
 }
 
+static u64 gmc_v12_0_get_dcc_alignment(struct amdgpu_device *adev)
+{
+	u64 max_tex_channel_caches, alignment;
+
+	max_tex_channel_caches = adev->gfx.config.max_texture_channel_caches;
+	if (is_power_of_2(max_tex_channel_caches))
+		alignment = (max_tex_channel_caches / SZ_4) * max_tex_channel_caches;
+	else
+		alignment = roundup_pow_of_two(max_tex_channel_caches) *
+				max_tex_channel_caches;
+
+	return (u64)alignment * SZ_1K;
+}
+
 static const struct amdgpu_gmc_funcs gmc_v12_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v12_0_flush_gpu_tlb,
 	.flush_gpu_tlb_pasid = gmc_v12_0_flush_gpu_tlb_pasid,
@@ -551,6 +565,7 @@ static const struct amdgpu_gmc_funcs gmc_v12_0_gmc_funcs = {
 	.get_vm_pde = gmc_v12_0_get_vm_pde,
 	.get_vm_pte = gmc_v12_0_get_vm_pte,
 	.get_vbios_fb_size = gmc_v12_0_get_vbios_fb_size,
+	.get_dcc_alignment = gmc_v12_0_get_dcc_alignment,
 };
 
 static void gmc_v12_0_set_gmc_funcs(struct amdgpu_device *adev)
-- 
2.25.1

Re: [PATCH 2/2] drm/amdgpu: Add address alignment support to DCC buffers

[Christian König]

Am 04.07.24 um 10:30 schrieb Arunpravin Paneer Selvam:
> Add address alignment support to the DCC VRAM buffers.
>
> v2:
>    - adjust size based on the max_texture_channel_caches values
>      only for GFX12 DCC buffers.
>    - used AMDGPU_GEM_CREATE_GFX12_DCC flag to apply change only
>      for DCC buffers.
>    - roundup non power of two DCC buffer adjusted size to nearest
>      power of two number as the buddy allocator does not support non
>      power of two alignments. This applies only to the contiguous
>      DCC buffers.
>
> v3:(Alex)
>    - rewrite the max texture channel caches comparison code in an
>      algorithmic way to determine the alignment size.
>
> v4:(Alex)
>    - Move the logic from amdgpu_vram_mgr_dcc_alignment() to gmc_v12_0.c
>      and add a new gmc func callback for dcc alignment. If the callback
>      is non-NULL, call it to get the alignment, otherwise, use the default.
>
> v5:(Alex)
>    - Set the Alignment to a default value if the callback doesn't exist.
>    - Add the callback to amdgpu_gmc_funcs.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

Acked-by: Christian König <christian.koenig@amd.com> for the series.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h      |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 36 ++++++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c       | 15 ++++++++
>   3 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index febca3130497..49dfcf112ac1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -156,6 +156,8 @@ struct amdgpu_gmc_funcs {
>   				      uint64_t addr, uint64_t *flags);
>   	/* get the amount of memory used by the vbios for pre-OS console */
>   	unsigned int (*get_vbios_fb_size)(struct amdgpu_device *adev);
> +	/* get the DCC buffer alignment */
> +	u64 (*get_dcc_alignment)(struct amdgpu_device *adev);
>   
>   	enum amdgpu_memory_partition (*query_mem_partition_mode)(
>   		struct amdgpu_device *adev);
> @@ -363,6 +365,7 @@ struct amdgpu_gmc {
>   	(adev)->gmc.gmc_funcs->override_vm_pte_flags			\
>   		((adev), (vm), (addr), (pte_flags))
>   #define amdgpu_gmc_get_vbios_fb_size(adev) (adev)->gmc.gmc_funcs->get_vbios_fb_size((adev))
> +#define amdgpu_gmc_get_dcc_alignment(adev) ((adev)->gmc.gmc_funcs->get_dcc_alignment((adev)))
>   
>   /**
>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index eb94f943b28e..756d1c8cbffd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -509,6 +509,16 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   		vres->flags |= DRM_BUDDY_RANGE_ALLOCATION;
>   
>   	remaining_size = (u64)vres->base.size;
> +	if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
> +	    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC) {
> +		u64 adjust_size;
> +
> +		if (adev->gmc.gmc_funcs->get_dcc_alignment) {
> +			adjust_size = amdgpu_gmc_get_dcc_alignment(adev);
> +			remaining_size = roundup_pow_of_two(remaining_size + adjust_size);
> +			vres->flags |= DRM_BUDDY_TRIM_DISABLE;
> +		}
> +	}
>   
>   	mutex_lock(&mgr->lock);
>   	while (remaining_size) {
> @@ -518,8 +528,12 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   			min_block_size = mgr->default_page_size;
>   
>   		size = remaining_size;
> -		if ((size >= (u64)pages_per_block << PAGE_SHIFT) &&
> -		    !(size & (((u64)pages_per_block << PAGE_SHIFT) - 1)))
> +
> +		if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
> +		    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC)
> +			min_block_size = size;
> +		else if ((size >= (u64)pages_per_block << PAGE_SHIFT) &&
> +			 !(size & (((u64)pages_per_block << PAGE_SHIFT) - 1)))
>   			min_block_size = (u64)pages_per_block << PAGE_SHIFT;
>   
>   		BUG_ON(min_block_size < mm->chunk_size);
> @@ -550,6 +564,24 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   	}
>   	mutex_unlock(&mgr->lock);
>   
> +	if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS &&
> +	    bo->flags & AMDGPU_GEM_CREATE_GFX12_DCC) {
> +		struct drm_buddy_block *dcc_block;
> +		u64 dcc_start, alignment;
> +
> +		dcc_block = amdgpu_vram_mgr_first_block(&vres->blocks);
> +		dcc_start = amdgpu_vram_mgr_block_start(dcc_block);
> +
> +		if (adev->gmc.gmc_funcs->get_dcc_alignment) {
> +			alignment = amdgpu_gmc_get_dcc_alignment(adev);
> +			/* Adjust the start address for DCC buffers only */
> +			dcc_start = roundup(dcc_start, alignment);
> +			drm_buddy_block_trim(mm, &dcc_start,
> +					     (u64)vres->base.size,
> +					     &vres->blocks);
> +		}
> +	}
> +
>   	vres->base.start = 0;
>   	size = max_t(u64, amdgpu_vram_mgr_blocks_size(&vres->blocks),
>   		     vres->base.size);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> index fd3ac483760e..4259edcdec8a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> @@ -542,6 +542,20 @@ static unsigned gmc_v12_0_get_vbios_fb_size(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +static u64 gmc_v12_0_get_dcc_alignment(struct amdgpu_device *adev)
> +{
> +	u64 max_tex_channel_caches, alignment;
> +
> +	max_tex_channel_caches = adev->gfx.config.max_texture_channel_caches;
> +	if (is_power_of_2(max_tex_channel_caches))
> +		alignment = (max_tex_channel_caches / SZ_4) * max_tex_channel_caches;
> +	else
> +		alignment = roundup_pow_of_two(max_tex_channel_caches) *
> +				max_tex_channel_caches;
> +
> +	return (u64)alignment * SZ_1K;
> +}
> +
>   static const struct amdgpu_gmc_funcs gmc_v12_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v12_0_flush_gpu_tlb,
>   	.flush_gpu_tlb_pasid = gmc_v12_0_flush_gpu_tlb_pasid,
> @@ -551,6 +565,7 @@ static const struct amdgpu_gmc_funcs gmc_v12_0_gmc_funcs = {
>   	.get_vm_pde = gmc_v12_0_get_vm_pde,
>   	.get_vm_pte = gmc_v12_0_get_vm_pte,
>   	.get_vbios_fb_size = gmc_v12_0_get_vbios_fb_size,
> +	.get_dcc_alignment = gmc_v12_0_get_dcc_alignment,
>   };
>   
>   static void gmc_v12_0_set_gmc_funcs(struct amdgpu_device *adev)

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

[Alex Deucher]

On Thu, Jul 4, 2024 at 4:40 AM 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>

Series is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

I'd like to take this series through the amdgpu tree if there are no
objections as it's required for display buffers on some chips and I'd
like to make sure it lands in 6.11.

Thanks,

Alex

> ---
>  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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>
> --
> 2.25.1
>

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

[Paneer Selvam, Arunpravin]

Thanks Alex.

Hi Matthew,
Any comments?

Thanks,
Arun.

On 7/9/2024 1:42 AM, Alex Deucher wrote:
> On Thu, Jul 4, 2024 at 4:40 AM 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>
> Series is:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>
> I'd like to take this series through the amdgpu tree if there are no
> objections as it's required for display buffers on some chips and I'd
> like to make sure it lands in 6.11.
>
> Thanks,
>
> Alex
>
>> ---
>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>
>> --
>> 2.25.1
>>

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

[Matthew Auld]

On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
> Thanks Alex.
> 
> Hi Matthew,
> Any comments?

Do we not pass the required address alignment when allocating the pages 
in the first place?

> 
> Thanks,
> Arun.
> 
> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>> Series is:
>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> I'd like to take this series through the amdgpu tree if there are no
>> objections as it's required for display buffers on some chips and I'd
>> like to make sure it lands in 6.11.
>>
>> Thanks,
>>
>> Alex
>>
>>> ---
>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>
>>> -- 
>>> 2.25.1
>>>
> 

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

[Paneer Selvam, Arunpravin]

Hi Matthew,

On 7/10/2024 6:20 PM, Matthew Auld wrote:
> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>> Thanks Alex.
>>
>> Hi Matthew,
>> Any comments?
>
> Do we not pass the required address alignment when allocating the 
> pages in the first place?
If address alignment is really useful, we can add that in the 
drm_buddy_alloc_blocks() function.

Thanks,
Arun.
>
>>
>> Thanks,
>> Arun.
>>
>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>> Series is:
>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>
>>> I'd like to take this series through the amdgpu tree if there are no
>>> objections as it's required for display buffers on some chips and I'd
>>> like to make sure it lands in 6.11.
>>>
>>> Thanks,
>>>
>>> Alex
>>>
>>>> ---
>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>
>>>> -- 
>>>> 2.25.1
>>>>
>>

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

[Matthew Auld]

On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
> Hi Matthew,
> 
> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>> Thanks Alex.
>>>
>>> Hi Matthew,
>>> Any comments?
>>
>> Do we not pass the required address alignment when allocating the 
>> pages in the first place?
> If address alignment is really useful, we can add that in the 
> drm_buddy_alloc_blocks() function.

I mean don't we already pass the min page size, which should give us 
matching physical address alignment?

> 
> Thanks,
> Arun.
>>
>>>
>>> Thanks,
>>> Arun.
>>>
>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>>> Series is:
>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>>
>>>> I'd like to take this series through the amdgpu tree if there are no
>>>> objections as it's required for display buffers on some chips and I'd
>>>> like to make sure it lands in 6.11.
>>>>
>>>> Thanks,
>>>>
>>>> Alex
>>>>
>>>>> ---
>>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>>
>>>>> -- 
>>>>> 2.25.1
>>>>>
>>>
> 

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

[Paneer Selvam, Arunpravin]

On 7/16/2024 3:34 PM, Matthew Auld wrote:
> On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
>> Hi Matthew,
>>
>> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>>> Thanks Alex.
>>>>
>>>> Hi Matthew,
>>>> Any comments?
>>>
>>> Do we not pass the required address alignment when allocating the 
>>> pages in the first place?
>> If address alignment is really useful, we can add that in the 
>> drm_buddy_alloc_blocks() function.
>
> I mean don't we already pass the min page size, which should give us 
> matching physical address alignment?
I think we don't need to align the address to the passed min_block_size 
value for all the contiguous
buffers, so I thought that decision we can leave it to the drivers and 
they can achieve that through trim function
in this kind of a specific request.

https://patchwork.freedesktop.org/series/136150/
We are getting this sparse error from the Intel CI. Do you think these 
errors are introduced with this patches?

Thanks,
Arun.
>
>>
>> Thanks,
>> Arun.
>>>
>>>>
>>>> Thanks,
>>>> Arun.
>>>>
>>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>>>> Series is:
>>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>
>>>>> I'd like to take this series through the amdgpu tree if there are no
>>>>> objections as it's required for display buffers on some chips and I'd
>>>>> like to make sure it lands in 6.11.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Alex
>>>>>
>>>>>> ---
>>>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>>>
>>>>>> -- 
>>>>>> 2.25.1
>>>>>>
>>>>
>>

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

[Matthew Auld]

On 17/07/2024 16:02, Paneer Selvam, Arunpravin wrote:
> 
> 
> On 7/16/2024 3:34 PM, Matthew Auld wrote:
>> On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
>>> Hi Matthew,
>>>
>>> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>>>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>>>> Thanks Alex.
>>>>>
>>>>> Hi Matthew,
>>>>> Any comments?
>>>>
>>>> Do we not pass the required address alignment when allocating the 
>>>> pages in the first place?
>>> If address alignment is really useful, we can add that in the 
>>> drm_buddy_alloc_blocks() function.
>>
>> I mean don't we already pass the min page size, which should give us 
>> matching physical address alignment?
> I think we don't need to align the address to the passed min_block_size 
> value for all the contiguous
> buffers, so I thought that decision we can leave it to the drivers and 
> they can achieve that through trim function
> in this kind of a specific request.

I would have assumed it would be simpler to use min_block_size and then 
trim the size, if it's too big? That would then also take care of the 
try_harder case?

Also how are we dealing with the multi-block try_harder case? AFAICT we 
only allow trimming single block atm, or is it not possible to trigger 
that path here? Or are we handling that somehow?

> 
> https://patchwork.freedesktop.org/series/136150/
> We are getting this sparse error from the Intel CI. Do you think these 
> errors are introduced with this patches?

I think it's safe to ignore, there seem to be other series with the same 
thing.

> 
> Thanks,
> Arun.
>>
>>>
>>> Thanks,
>>> Arun.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Arun.
>>>>>
>>>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>>>>> Series is:
>>>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>>
>>>>>> I'd like to take this series through the amdgpu tree if there are no
>>>>>> objections as it's required for display buffers on some chips and I'd
>>>>>> like to make sure it lands in 6.11.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Alex
>>>>>>
>>>>>>> ---
>>>>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>>>>
>>>>>>> -- 
>>>>>>> 2.25.1
>>>>>>>
>>>>>
>>>
> 

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

[Paneer Selvam, Arunpravin]

Hi Matthew,

On 7/19/2024 4:01 PM, Matthew Auld wrote:
> On 17/07/2024 16:02, Paneer Selvam, Arunpravin wrote:
>>
>>
>> On 7/16/2024 3:34 PM, Matthew Auld wrote:
>>> On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
>>>> Hi Matthew,
>>>>
>>>> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>>>>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>>>>> Thanks Alex.
>>>>>>
>>>>>> Hi Matthew,
>>>>>> Any comments?
>>>>>
>>>>> Do we not pass the required address alignment when allocating the 
>>>>> pages in the first place?
>>>> If address alignment is really useful, we can add that in the 
>>>> drm_buddy_alloc_blocks() function.
>>>
>>> I mean don't we already pass the min page size, which should give us 
>>> matching physical address alignment?
>> I think we don't need to align the address to the passed 
>> min_block_size value for all the contiguous
>> buffers, so I thought that decision we can leave it to the drivers 
>> and they can achieve that through trim function
>> in this kind of a specific request.
>
> I would have assumed it would be simpler to use min_block_size and 
> then trim the size, if it's too big? That would then also take care of 
> the try_harder case?
For example, if the required contiguous size is 1MiB and min_block_size 
is 256KiB, to perform the address alignment of 256KiB, we might need to 
over-allocate at least
to the min_block_size (say 256KiB). Now the size becomes 1280KiB and 
since the contiguous flag is enabled, we will round up the size to the 
next power of two and the size
value becomes 2MiB. Next, in trimming we should round up the block start 
address to the min_block_size. May be we can keep the above mentioned 
operations under the
flag combination DRM_BUDDY_CONTIGUOUS_ALLOCATION && 
DRM_BUDDY_ADDRESS_ALIGNMENT?.

At the moment, we cannot support address alignment for try_harder 
allocations since in case of try_harder allocations we first traverse 
RHS to allocate the maximum possible
and traverse LHS (here we align the LHS size to min_block_size) to 
allocate the remaining size. May be in case of 
DRM_BUDDY_ADDRESS_ALIGNMENT, we should first allocate
LHS satisfying the address alignment requirement and then traverse RHS 
to allocate the remaining size if required?
>
> Also how are we dealing with the multi-block try_harder case? AFAICT 
> we only allow trimming single block atm, or is it not possible to 
> trigger that path here? Or are we handling that somehow?
not possible to trigger that path here. only when we either 
over-allocate the LHS size and pass the multiple blocks to the trim 
function or implement the above mentioned method.
>
>>
>> https://patchwork.freedesktop.org/series/136150/
>> We are getting this sparse error from the Intel CI. Do you think 
>> these errors are introduced with this patches?
>
> I think it's safe to ignore, there seem to be other series with the 
> same thing.
Thanks.
>
>>
>> Thanks,
>> Arun.
>>>
>>>>
>>>> Thanks,
>>>> Arun.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Arun.
>>>>>>
>>>>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>>>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>>>>>> Series is:
>>>>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>>>
>>>>>>> I'd like to take this series through the amdgpu tree if there 
>>>>>>> are no
>>>>>>> objections as it's required for display buffers on some chips 
>>>>>>> and I'd
>>>>>>> like to make sure it lands in 6.11.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Alex
>>>>>>>
>>>>>>>> ---
>>>>>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.25.1
>>>>>>>>
>>>>>>
>>>>
>>

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

[Matthew Auld]

Hi,

On 22/07/2024 12:41, Paneer Selvam, Arunpravin wrote:
> Hi Matthew,
> 
> On 7/19/2024 4:01 PM, Matthew Auld wrote:
>> On 17/07/2024 16:02, Paneer Selvam, Arunpravin wrote:
>>>
>>>
>>> On 7/16/2024 3:34 PM, Matthew Auld wrote:
>>>> On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
>>>>> Hi Matthew,
>>>>>
>>>>> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>>>>>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>>>>>> Thanks Alex.
>>>>>>>
>>>>>>> Hi Matthew,
>>>>>>> Any comments?
>>>>>>
>>>>>> Do we not pass the required address alignment when allocating the 
>>>>>> pages in the first place?
>>>>> If address alignment is really useful, we can add that in the 
>>>>> drm_buddy_alloc_blocks() function.
>>>>
>>>> I mean don't we already pass the min page size, which should give us 
>>>> matching physical address alignment?
>>> I think we don't need to align the address to the passed 
>>> min_block_size value for all the contiguous
>>> buffers, so I thought that decision we can leave it to the drivers 
>>> and they can achieve that through trim function
>>> in this kind of a specific request.
>>
>> I would have assumed it would be simpler to use min_block_size and 
>> then trim the size, if it's too big? That would then also take care of 
>> the try_harder case?
> For example, if the required contiguous size is 1MiB and min_block_size 
> is 256KiB, to perform the address alignment of 256KiB, we might need to 
> over-allocate at least
> to the min_block_size (say 256KiB). Now the size becomes 1280KiB and 

If we have 1M contig request then it should already be aligned to 256K 
and every other power-of-two < 1M. VRAM should start at offset zero, so 
1M block will have 1M address alignment, and so should also be aligned 
to 256K, right?

Or does "address alignment of 256KiB" mean something else here? To me it 
just means IS_ALIGNED(block_start, 256K).

> since the contiguous flag is enabled, we will round up the size to the 
> next power of two and the size
> value becomes 2MiB. Next, in trimming we should round up the block start 
> address to the min_block_size. May be we can keep the above mentioned 
> operations under the
> flag combination DRM_BUDDY_CONTIGUOUS_ALLOCATION && 
> DRM_BUDDY_ADDRESS_ALIGNMENT?.
> 
> At the moment, we cannot support address alignment for try_harder 
> allocations since in case of try_harder allocations we first traverse 
> RHS to allocate the maximum possible
> and traverse LHS (here we align the LHS size to min_block_size) to 
> allocate the remaining size. May be in case of 
> DRM_BUDDY_ADDRESS_ALIGNMENT, we should first allocate
> LHS satisfying the address alignment requirement and then traverse RHS 
> to allocate the remaining size if required?
>>
>> Also how are we dealing with the multi-block try_harder case? AFAICT 
>> we only allow trimming single block atm, or is it not possible to 
>> trigger that path here? Or are we handling that somehow?
> not possible to trigger that path here. only when we either 
> over-allocate the LHS size and pass the multiple blocks to the trim 
> function or implement the above mentioned method.
>>
>>>
>>> https://patchwork.freedesktop.org/series/136150/
>>> We are getting this sparse error from the Intel CI. Do you think 
>>> these errors are introduced with this patches?
>>
>> I think it's safe to ignore, there seem to be other series with the 
>> same thing.
> Thanks.
>>
>>>
>>> Thanks,
>>> Arun.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Arun.
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Arun.
>>>>>>>
>>>>>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>>>>>> On Thu, Jul 4, 2024 at 4:40 AM 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>
>>>>>>>> Series is:
>>>>>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>>>>>>
>>>>>>>> I'd like to take this series through the amdgpu tree if there 
>>>>>>>> are no
>>>>>>>> objections as it's required for display buffers on some chips 
>>>>>>>> and I'd
>>>>>>>> like to make sure it lands in 6.11.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Alex
>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>   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 94f8c34fc293..8cebe1fa4e9d 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 82570f77e817..0c2f735f0265 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);
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> 2.25.1
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>