[PATCH 3/5] drm/amdgpu: fix and cleanup amdgpu_bo_create
Deucher, Alexander
Alexander.Deucher at amd.com
Tue Sep 12 15:55:58 UTC 2017
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Tuesday, September 12, 2017 5:09 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: [PATCH 3/5] drm/amdgpu: fix and cleanup amdgpu_bo_create
>
> From: Christian König <christian.koenig at amd.com>
>
> Fix USWC handling by cleaning up the function and removing
> quite a bit of unused code.
Can you clarify what was broken?
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 83 +++++++++------------
> ---------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 ---
> 2 files changed, 23 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 52d0109..726a662 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -64,11 +64,12 @@ bool amdgpu_ttm_bo_is_amdgpu_bo(struct
> ttm_buffer_object *bo)
> return false;
> }
>
> -static void amdgpu_ttm_placement_init(struct amdgpu_device *adev,
> - struct ttm_placement *placement,
> - struct ttm_place *places,
> - u32 domain, u64 flags)
> +void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32
> domain)
> {
> + struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
> + struct ttm_placement *placement = &abo->placement;
> + struct ttm_place *places = abo->placements;
> + u64 flags = abo->flags;
> u32 c = 0;
>
> if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
> @@ -151,27 +152,6 @@ static void amdgpu_ttm_placement_init(struct
> amdgpu_device *adev,
> placement->busy_placement = places;
> }
>
> -void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32
> domain)
> -{
> - struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
> -
> - amdgpu_ttm_placement_init(adev, &abo->placement, abo-
> >placements,
> - domain, abo->flags);
> -}
> -
> -static void amdgpu_fill_placement_to_bo(struct amdgpu_bo *bo,
> - struct ttm_placement *placement)
> -{
> - BUG_ON(placement->num_placement >
> (AMDGPU_GEM_DOMAIN_MAX + 1));
> -
> - memcpy(bo->placements, placement->placement,
> - placement->num_placement * sizeof(struct ttm_place));
> - bo->placement.num_placement = placement->num_placement;
> - bo->placement.num_busy_placement = placement-
> >num_busy_placement;
> - bo->placement.placement = bo->placements;
> - bo->placement.busy_placement = bo->placements;
> -}
> -
> /**
> * amdgpu_bo_create_reserved - create reserved BO for kernel use
> *
> @@ -303,14 +283,13 @@ void amdgpu_bo_free_kernel(struct amdgpu_bo
> **bo, u64 *gpu_addr,
> *cpu_addr = NULL;
> }
>
> -int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
> - unsigned long size, int byte_align,
> - bool kernel, u32 domain, u64 flags,
> - struct sg_table *sg,
> - struct ttm_placement *placement,
> - struct reservation_object *resv,
> - uint64_t init_value,
> - struct amdgpu_bo **bo_ptr)
> +static int amdgpu_bo_do_create(struct amdgpu_device *adev,
> + unsigned long size, int byte_align,
> + bool kernel, u32 domain, u64 flags,
> + struct sg_table *sg,
> + struct reservation_object *resv,
> + uint64_t init_value,
> + struct amdgpu_bo **bo_ptr)
Still seems like amdgpu_bo_create_restricted is a better name than do_create.
> {
> struct amdgpu_bo *bo;
> enum ttm_bo_type type;
> @@ -384,10 +363,11 @@ int amdgpu_bo_create_restricted(struct
> amdgpu_device *adev,
> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> #endif
>
> - amdgpu_fill_placement_to_bo(bo, placement);
> - /* Kernel allocation are uninterruptible */
> + bo->tbo.bdev = &adev->mman.bdev;
> + amdgpu_ttm_placement_from_domain(bo, domain);
>
> initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
> + /* Kernel allocation are uninterruptible */
> r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size,
> type,
> &bo->placement, page_align, !kernel, NULL,
> acc_size, sg, resv,
> &amdgpu_ttm_bo_destroy);
> @@ -442,27 +422,17 @@ static int amdgpu_bo_create_shadow(struct
> amdgpu_device *adev,
> unsigned long size, int byte_align,
> struct amdgpu_bo *bo)
> {
> - struct ttm_placement placement = {0};
> - struct ttm_place placements[AMDGPU_GEM_DOMAIN_MAX + 1];
> int r;
>
> if (bo->shadow)
> return 0;
>
> - memset(&placements, 0, sizeof(placements));
> - amdgpu_ttm_placement_init(adev, &placement, placements,
> - AMDGPU_GEM_DOMAIN_GTT,
> - AMDGPU_GEM_CREATE_CPU_GTT_USWC
> |
> - AMDGPU_GEM_CREATE_SHADOW);
> -
> - r = amdgpu_bo_create_restricted(adev, size, byte_align, true,
> - AMDGPU_GEM_DOMAIN_GTT,
> -
> AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> - AMDGPU_GEM_CREATE_SHADOW,
> - NULL, &placement,
> - bo->tbo.resv,
> - 0,
> - &bo->shadow);
> + r = amdgpu_bo_do_create(adev, size, byte_align, true,
> + AMDGPU_GEM_DOMAIN_GTT,
> + AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> + AMDGPU_GEM_CREATE_SHADOW,
> + NULL, bo->tbo.resv, 0,
> + &bo->shadow);
> if (!r) {
> bo->shadow->parent = amdgpu_bo_ref(bo);
> mutex_lock(&adev->shadow_list_lock);
> @@ -484,18 +454,11 @@ int amdgpu_bo_create(struct amdgpu_device
> *adev,
> uint64_t init_value,
> struct amdgpu_bo **bo_ptr)
> {
> - struct ttm_placement placement = {0};
> - struct ttm_place placements[AMDGPU_GEM_DOMAIN_MAX + 1];
> uint64_t parent_flags = flags & ~AMDGPU_GEM_CREATE_SHADOW;
> int r;
>
> - memset(&placements, 0, sizeof(placements));
> - amdgpu_ttm_placement_init(adev, &placement, placements,
> - domain, parent_flags);
> -
> - r = amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
> domain,
> - parent_flags, sg, &placement, resv,
> - init_value, bo_ptr);
> + r = amdgpu_bo_do_create(adev, size, byte_align, kernel, domain,
> + parent_flags, sg, resv, init_value, bo_ptr);
> if (r)
> return r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index a4891be..39b6bf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -195,14 +195,6 @@ int amdgpu_bo_create(struct amdgpu_device
> *adev,
> struct reservation_object *resv,
> uint64_t init_value,
> struct amdgpu_bo **bo_ptr);
> -int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
> - unsigned long size, int byte_align,
> - bool kernel, u32 domain, u64 flags,
> - struct sg_table *sg,
> - struct ttm_placement *placement,
> - struct reservation_object *resv,
> - uint64_t init_value,
> - struct amdgpu_bo **bo_ptr);
> int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
> unsigned long size, int align,
> u32 domain, struct amdgpu_bo **bo_ptr,
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list