Re: [PATCH 01/13] drm/amdgpu: move suballoc defines into own header

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. május 29., péntek 13:24:03 közép-európai nyári idő Christian König 
wrote:
> Just some code cleanup, while at it remove outdated comment.
> 
> No functional change.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Looks good, makes good sense.

Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 32 +--------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 40 -----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_sa.h     | 77 ++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 71 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 5d7bfa59424a..4b6c9d9e6773
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -112,6 +112,7 @@
>  #include "amdgpu_userq.h"
>  #include "amdgpu_eviction_fence.h"
>  #include "amdgpu_ip.h"
> +#include "amdgpu_sa.h"
>  #if defined(CONFIG_DRM_AMD_ISP)
>  #include "amdgpu_isp.h"
>  #endif
> @@ -386,37 +387,6 @@ struct amdgpu_clock {
>  	uint32_t max_pixel_clock;
>  };
> 
> -/* sub-allocation manager, it has to be protected by another lock.
> - * By conception this is an helper for other part of the driver
> - * like the indirect buffer or semaphore, which both have their
> - * locking.
> - *
> - * Principe is simple, we keep a list of sub allocation in offset
> - * order (first entry has offset == 0, last entry has the highest
> - * offset).
> - *
> - * When allocating new object we first check if there is room at
> - * the end total_size - (last_object_offset + last_object_size) >=
> - * alloc_size. If so we allocate new object there.
> - *
> - * When there is not enough room at the end, we start waiting for
> - * each sub object until we reach object_offset+object_size >=
> - * alloc_size, this object then become the sub object we return.
> - *
> - * Alignment can't be bigger than page size.
> - *
> - * Hole are not considered for allocation to keep things simple.
> - * Assumption is that there won't be hole (all object on same
> - * alignment).
> - */
> -
> -struct amdgpu_sa_manager {
> -	struct drm_suballoc_manager	base;
> -	struct amdgpu_bo		*bo;
> -	uint64_t			gpu_addr;
> -	void				*cpu_ptr;
> -};
> -
>  /*
>   * IRQS.
>   */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index
> 4d68732d6223..ff11a0903499 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -312,46 +312,6 @@ uint32_t amdgpu_bo_mem_stats_placement(struct amdgpu_bo
> *bo); uint32_t amdgpu_bo_get_preferred_domain(struct amdgpu_device *adev,
> uint32_t domain);
> 
> -/*
> - * sub allocation
> - */
> -static inline struct amdgpu_sa_manager *
> -to_amdgpu_sa_manager(struct drm_suballoc_manager *manager)
> -{
> -	return container_of(manager, struct amdgpu_sa_manager, base);
> -}
> -
> -static inline uint64_t amdgpu_sa_bo_gpu_addr(struct drm_suballoc *sa_bo)
> -{
> -	return to_amdgpu_sa_manager(sa_bo->manager)->gpu_addr +
> -		drm_suballoc_soffset(sa_bo);
> -}
> -
> -static inline void *amdgpu_sa_bo_cpu_addr(struct drm_suballoc *sa_bo)
> -{
> -	return to_amdgpu_sa_manager(sa_bo->manager)->cpu_ptr +
> -		drm_suballoc_soffset(sa_bo);
> -}
> -
> -int amdgpu_sa_bo_manager_init(struct amdgpu_device *adev,
> -				     struct amdgpu_sa_manager 
*sa_manager,
> -				     unsigned size, u32 align, 
u32 domain);
> -void amdgpu_sa_bo_manager_fini(struct amdgpu_device *adev,
> -				      struct amdgpu_sa_manager 
*sa_manager);
> -int amdgpu_sa_bo_manager_start(struct amdgpu_device *adev,
> -				      struct amdgpu_sa_manager 
*sa_manager);
> -int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager,
> -		     struct drm_suballoc **sa_bo,
> -		     unsigned int size);
> -void amdgpu_sa_bo_free(struct drm_suballoc **sa_bo,
> -		       struct dma_fence *fence);
> -#if defined(CONFIG_DEBUG_FS)
> -void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
> -					 struct seq_file *m);
> -u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m);
> -#endif
> -void amdgpu_debugfs_sa_init(struct amdgpu_device *adev);
> -
>  bool amdgpu_bo_support_uswc(u64 bo_flags);
> 
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.h new file mode 100644
> index 000000000000..8c85c80fc119
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright 2026 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"), + * to deal in the Software without restriction, including
> without limitation + * the rights to use, copy, modify, merge, publish,
> distribute, sublicense, + * and/or sell copies of the Software, and to
> permit persons to whom the + * Software is furnished to do so, subject to
> the following conditions: + *
> + * The above copyright notice and this permission notice shall be included
> in + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. 
> IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR
> ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF
> CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH
> THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef AMDGPU_SA_H_
> +#define AMDGPU_SA_H_
> +
> +#include <drm/drm_suballoc.h>
> +
> +struct amdgpu_device;
> +struct amdgpu_bo;
> +
> +struct amdgpu_sa_manager {
> +	struct drm_suballoc_manager	base;
> +	struct amdgpu_bo		*bo;
> +	uint64_t			gpu_addr;
> +	void				*cpu_ptr;
> +};
> +
> +static inline struct amdgpu_sa_manager *
> +to_amdgpu_sa_manager(struct drm_suballoc_manager *manager)
> +{
> +	return container_of(manager, struct amdgpu_sa_manager, base);
> +}
> +
> +static inline uint64_t amdgpu_sa_bo_gpu_addr(struct drm_suballoc *sa_bo)
> +{
> +	return to_amdgpu_sa_manager(sa_bo->manager)->gpu_addr +
> +		drm_suballoc_soffset(sa_bo);
> +}
> +
> +static inline void *amdgpu_sa_bo_cpu_addr(struct drm_suballoc *sa_bo)
> +{
> +	return to_amdgpu_sa_manager(sa_bo->manager)->cpu_ptr +
> +		drm_suballoc_soffset(sa_bo);
> +}
> +
> +int amdgpu_sa_bo_manager_init(struct amdgpu_device *adev,
> +			      struct amdgpu_sa_manager *sa_manager,
> +			      unsigned size, u32 align, u32 domain);
> +void amdgpu_sa_bo_manager_fini(struct amdgpu_device *adev,
> +			       struct amdgpu_sa_manager *sa_manager);
> +int amdgpu_sa_bo_manager_start(struct amdgpu_device *adev,
> +			       struct amdgpu_sa_manager *sa_manager);
> +int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager,
> +		     struct drm_suballoc **sa_bo,
> +		     unsigned int size);
> +void amdgpu_sa_bo_free(struct drm_suballoc **sa_bo,
> +		       struct dma_fence *fence);
> +#if defined(CONFIG_DEBUG_FS)
> +void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
> +				  struct seq_file *m);
> +u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m);
> +#endif
> +void amdgpu_debugfs_sa_init(struct amdgpu_device *adev);
> +
> +#endif