> -----Original Message----- > From: amd-gfx [mailto:[email protected]] On Behalf > Of Christian König > Sent: Tuesday, September 12, 2017 5:09 AM > To: [email protected] > Subject: [PATCH 3/5] drm/amdgpu: fix and cleanup amdgpu_bo_create > > From: Christian König <[email protected]> > > 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 <[email protected]> > --- > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
