Re: [PATCH v5 2/6] drm/amdgpu: Handle sg size limit for contiguous allocation

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 23.04.24 um 17:28 schrieb Philip Yang:
> Define macro MAX_SG_SEGMENT_SIZE 2GB, because struct scatterlist length
> is unsigned int, and some users of it cast to a signed int, so every
> segment of sg table is limited to size 2GB maximum.
>
> For contiguous VRAM allocation, don't limit the max buddy block size in
> order to get contiguous VRAM memory. To workaround the sg table segment
> size limit, allocate multiple segments if contiguous size is bigger than
> MAX_SG_SEGMENT_SIZE.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 4be8b091099a..ebffb58ea53a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -31,6 +31,8 @@
>   #include "amdgpu_atomfirmware.h"
>   #include "atom.h"
>   
> +#define AMDGPU_MAX_SG_SEGMENT_SIZE	(2UL << 30)
> +
>   struct amdgpu_vram_reservation {
>   	u64 start;
>   	u64 size;
> @@ -532,9 +534,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
>   
>   		BUG_ON(min_block_size < mm->chunk_size);
>   
> -		/* Limit maximum size to 2GiB due to SG table limitations */
> -		size = min(remaining_size, 2ULL << 30);
> -
> +		size = remaining_size;
>   		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;
> @@ -675,7 +675,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
>   	amdgpu_res_first(res, offset, length, &cursor);
>   	while (cursor.remaining) {
>   		num_entries++;
> -		amdgpu_res_next(&cursor, cursor.size);
> +		amdgpu_res_next(&cursor, min(cursor.size, AMDGPU_MAX_SG_SEGMENT_SIZE));
>   	}
>   
>   	r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
> @@ -695,7 +695,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
>   	amdgpu_res_first(res, offset, length, &cursor);
>   	for_each_sgtable_sg((*sgt), sg, i) {
>   		phys_addr_t phys = cursor.start + adev->gmc.aper_base;
> -		size_t size = cursor.size;
> +		unsigned long size = min(cursor.size, AMDGPU_MAX_SG_SEGMENT_SIZE);
>   		dma_addr_t addr;
>   
>   		addr = dma_map_resource(dev, phys, size, dir,
> @@ -708,7 +708,7 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
>   		sg_dma_address(sg) = addr;
>   		sg_dma_len(sg) = size;
>   
> -		amdgpu_res_next(&cursor, cursor.size);
> +		amdgpu_res_next(&cursor, size);
>   	}
>   
>   	return 0;