[PATCH 4/5] drm/amdgpu: cleanup amdgpu_bo_pin_restricted
Liu, Monk
Monk.Liu at amd.com
Wed Sep 13 09:14:39 UTC 2017
SRIOV need to reserve a memory at an offset that set by GIM/hypervisor side, but I'm not sure how to do it perfectly, currently we call bo_create to allocate a VRAM BO, and call pin_restrict with "offset" as parameter for "min" and "offset + size" as "max",
I feel strange of above approach frankly speaking (unless the new offset equals to the original offset from bo_create),
Because the original gpu offset (from the bo_create) is different with the new "offset" provided by GIM, what will TTM/DRM do on the range of <original offset, new offset> after we pin the bo to <new offset, new offset+ size> ???
BR Monk
________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Deucher, Alexander <Alexander.Deucher at amd.com>
Sent: Tuesday, September 12, 2017 11:59:35 PM
To: 'Christian König'; amd-gfx at lists.freedesktop.org
Subject: RE: [PATCH 4/5] drm/amdgpu: cleanup amdgpu_bo_pin_restricted
> -----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 4/5] drm/amdgpu: cleanup amdgpu_bo_pin_restricted
>
> From: Christian König <christian.koenig at amd.com>
>
> Nobody is using the min/max interface any more.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
I'm not sure it's a good idea to get rid of this. I can see a need to reserve memory at specific offsets in memory. Specifically I think SR-IOV will be placing structures in memory to communicate configuration details from the host to the guest. Also, we should be reserving the vbios scratch area, but we don't currently.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 39 +++++------------------
> -------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 ---
> 2 files changed, 6 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 726a662..8a8add3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -629,20 +629,15 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
> *bo = NULL;
> }
>
> -int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
> - u64 min_offset, u64 max_offset,
> - u64 *gpu_addr)
> +int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain, u64 *gpu_addr)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> + unsigned lpfn;
> int r, i;
> - unsigned fpfn, lpfn;
>
> if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm))
> return -EPERM;
>
> - if (WARN_ON_ONCE(min_offset > max_offset))
> - return -EINVAL;
> -
> /* A shared bo cannot be migrated to VRAM */
> if (bo->prime_shared_count && (domain ==
> AMDGPU_GEM_DOMAIN_VRAM))
> return -EINVAL;
> @@ -657,12 +652,6 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
> *bo, u32 domain,
> if (gpu_addr)
> *gpu_addr = amdgpu_bo_gpu_offset(bo);
>
> - if (max_offset != 0) {
> - u64 domain_start = bo->tbo.bdev-
> >man[mem_type].gpu_offset;
> - WARN_ON_ONCE(max_offset <
> - (amdgpu_bo_gpu_offset(bo) -
> domain_start));
> - }
> -
> return 0;
> }
>
> @@ -671,23 +660,12 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
> *bo, u32 domain,
> for (i = 0; i < bo->placement.num_placement; i++) {
> /* force to pin into visible video ram */
> if ((bo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
> - !(bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
> &&
> - (!max_offset || max_offset >
> - adev->mc.visible_vram_size)) {
> - if (WARN_ON_ONCE(min_offset >
> - adev->mc.visible_vram_size))
> - return -EINVAL;
> - fpfn = min_offset >> PAGE_SHIFT;
> + !(bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS))
> {
> lpfn = adev->mc.visible_vram_size >> PAGE_SHIFT;
> - } else {
> - fpfn = min_offset >> PAGE_SHIFT;
> - lpfn = max_offset >> PAGE_SHIFT;
> + if (!bo->placements[i].lpfn ||
> + (lpfn && lpfn < bo->placements[i].lpfn))
> + bo->placements[i].lpfn = lpfn;
> }
> - if (fpfn > bo->placements[i].fpfn)
> - bo->placements[i].fpfn = fpfn;
> - if (!bo->placements[i].lpfn ||
> - (lpfn && lpfn < bo->placements[i].lpfn))
> - bo->placements[i].lpfn = lpfn;
> bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
> }
>
> @@ -718,11 +696,6 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
> *bo, u32 domain,
> return r;
> }
>
> -int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain, u64 *gpu_addr)
> -{
> - return amdgpu_bo_pin_restricted(bo, domain, 0, 0, gpu_addr);
> -}
> -
> int amdgpu_bo_unpin(struct amdgpu_bo *bo)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 39b6bf6..4b2c042 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -211,9 +211,6 @@ void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
> struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
> void amdgpu_bo_unref(struct amdgpu_bo **bo);
> int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain, u64 *gpu_addr);
> -int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
> - u64 min_offset, u64 max_offset,
> - u64 *gpu_addr);
> int amdgpu_bo_unpin(struct amdgpu_bo *bo);
> int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
> int amdgpu_bo_init(struct amdgpu_device *adev);
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170913/fe16a084/attachment-0001.html>
More information about the amd-gfx
mailing list