[PATCH 4/5] drm/amdgpu: cleanup amdgpu_bo_pin_restricted
Christian König
deathsimple at vodafone.de
Wed Sep 13 09:30:43 UTC 2017
> but we use VRAM domain in the first bo_create stage, so this BO is
> allocated directly ...
>
Yeah, but just stop doing so. amdgpu_bo_create can just be used to
allocate the BO structure without any backing storage.
So what you do is calling amdgpu_bo_create() with AMDGPU_GEM_DOMAIN_CPU
(or just 0 might work as well).
And then when you call amdgpu_bo_pin_restricted() you give
AMDGPU_GEM_DOMAIN_VRAM and your offset and size as min/max value for the
placement.
This way you can avoid the move and allocate the BO directly where you
want it.
Regards,
Christian.
Am 13.09.2017 um 11:26 schrieb Liu, Monk:
>
> but we use VRAM domain in the first bo_create stage, so this BO is
> allocated directly ...
>
>
> I think our current approach is incorrect ... because the space from
> bo_create(VRAM_DOMAIN) is missed somehow
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple at vodafone.de>
> *Sent:* Wednesday, September 13, 2017 5:21:13 PM
> *To:* Liu, Monk; Deucher, Alexander; amd-gfx at lists.freedesktop.org
> *Subject:* Re: [PATCH 4/5] drm/amdgpu: cleanup amdgpu_bo_pin_restricted
> amdgpu_bo_create() doesn't necessarily allocate anything, it just
> creates the BO structure.
>
> The backing memory for GTT and CPU domain is only allocated on first
> use, only VRAM is allocated directly.
>
> So just call amdgpu_bo_create() with AMDGPU_GEM_DOMAIN_CPU and then
> the pin with AMDGPU_GEM_DOMAIN_VRAM and your desired offset.
>
> Regards,
> Christian.
>
> Am 13.09.2017 um 11:14 schrieb Liu, Monk:
>>
>> 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
>> <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
>
>
>
>
> _______________________________________________
> 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/f6c5957e/attachment.html>
More information about the amd-gfx
mailing list