[PATCH 1/3] drm/amdgpu: do not pass ttm_resource_manager to gtt_mgr
Das, Nirmoy
nirmoy.das at amd.com
Wed Oct 20 11:12:14 UTC 2021
On 10/20/2021 12:49 PM, Christian König wrote:
> Am 20.10.21 um 11:19 schrieb Lazar, Lijo:
>>
>>
>> On 10/20/2021 2:18 PM, Das, Nirmoy wrote:
>>>
>>> On 10/20/2021 8:49 AM, Christian König wrote:
>>>> Am 19.10.21 um 20:14 schrieb Nirmoy Das:
>>>>> Do not allow exported amdgpu_gtt_mgr_*() to accept
>>>>> any ttm_resource_manager pointer. Also there is no need
>>>>> to force other module to call a ttm function just to
>>>>> eventually call gtt_mgr functions.
>>>>
>>>> That's a rather bad idea I think.
>>>>
>>>> The GTT and VRAM manager work on their respective objects and not
>>>> on the adev directly.
>>>
>>>
>>> What is bothering me is : it is obvious that the
>>> amdgpu_gtt_mgr_usage() for example should only calculate
>>>
>>> usages for TTM_PL_TT type resource manager, why to pass that
>>> explicitly. I am trying to leverage the fact that
>>>
>>> we only have one gtt/vram manager for a adev and the functions that
>>> I changed work on whole gtt/vram manager
>>>
>>> as a unit.
>>>
>>
>> Don't know about the functional aspects. From a sofware perspective,
>> amdgpu_gtt_mgr_*() operating on struct amdgpu_gtt_mgr *mgr seems more
>> logical.
>
> What we could do is to pass in amdgpu_gtt_mgr instead of
> ttm_resource_manager and then use &adev->mman.gtt_mgr.
Sounds good, I will try this way then.
Regards,
Nirmoy
>
> Regards,
> Christian.
>
>>
>>
>> Thanks,
>> Lijo
>>
>>>
>>> Regards,
>>>
>>> Nirmoy
>>>
>>>
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +--
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 31
>>>>> ++++++++++++---------
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +--
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 +--
>>>>> 4 files changed, 24 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> index 41ce86244144..5807df52031c 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -4287,7 +4287,7 @@ static int amdgpu_device_reset_sriov(struct
>>>>> amdgpu_device *adev,
>>>>> amdgpu_virt_init_data_exchange(adev);
>>>>> /* we need recover gart prior to run SMC/CP/SDMA resume */
>>>>> - amdgpu_gtt_mgr_recover(ttm_manager_type(&adev->mman.bdev,
>>>>> TTM_PL_TT));
>>>>> + amdgpu_gtt_mgr_recover(adev);
>>>>> r = amdgpu_device_fw_loading(adev);
>>>>> if (r)
>>>>> @@ -4604,7 +4604,7 @@ int amdgpu_do_asic_reset(struct list_head
>>>>> *device_list_handle,
>>>>> amdgpu_inc_vram_lost(tmp_adev);
>>>>> }
>>>>> - r =
>>>>> amdgpu_gtt_mgr_recover(ttm_manager_type(&tmp_adev->mman.bdev,
>>>>> TTM_PL_TT));
>>>>> + r = amdgpu_gtt_mgr_recover(tmp_adev);
>>>>> if (r)
>>>>> goto out;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> index c18f16b3be9c..5e41f8ef743a 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> @@ -77,10 +77,8 @@ static ssize_t
>>>>> amdgpu_mem_info_gtt_used_show(struct device *dev,
>>>>> {
>>>>> struct drm_device *ddev = dev_get_drvdata(dev);
>>>>> struct amdgpu_device *adev = drm_to_adev(ddev);
>>>>> - struct ttm_resource_manager *man;
>>>>> - man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>>>>> - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(man));
>>>>> + return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(adev));
>>>>> }
>>>>> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
>>>>> @@ -206,14 +204,19 @@ static void amdgpu_gtt_mgr_del(struct
>>>>> ttm_resource_manager *man,
>>>>> /**
>>>>> * amdgpu_gtt_mgr_usage - return usage of GTT domain
>>>>> *
>>>>> - * @man: TTM memory type manager
>>>>> + * @adev: amdgpu_device pointer
>>>>> *
>>>>> * Return how many bytes are used in the GTT domain
>>>>> */
>>>>> -uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man)
>>>>> +uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_device *adev)
>>>>> {
>>>>> - struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
>>>>> - s64 result = man->size - atomic64_read(&mgr->available);
>>>>> + struct ttm_resource_manager *man;
>>>>> + struct amdgpu_gtt_mgr *mgr;
>>>>> + s64 result;
>>>>> +
>>>>> + man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>>>>> + mgr = to_gtt_mgr(man);
>>>>> + result = man->size - atomic64_read(&mgr->available);
>>>>> return (result > 0 ? result : 0) * PAGE_SIZE;
>>>>> }
>>>>> @@ -221,19 +224,20 @@ uint64_t amdgpu_gtt_mgr_usage(struct
>>>>> ttm_resource_manager *man)
>>>>> /**
>>>>> * amdgpu_gtt_mgr_recover - re-init gart
>>>>> *
>>>>> - * @man: TTM memory type manager
>>>>> + * @adev: amdgpu_device pointer
>>>>> *
>>>>> * Re-init the gart for each known BO in the GTT.
>>>>> */
>>>>> -int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man)
>>>>> +int amdgpu_gtt_mgr_recover(struct amdgpu_device *adev)
>>>>> {
>>>>> - struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
>>>>> - struct amdgpu_device *adev;
>>>>> + struct ttm_resource_manager *man;
>>>>> + struct amdgpu_gtt_mgr *mgr;
>>>>> struct amdgpu_gtt_node *node;
>>>>> struct drm_mm_node *mm_node;
>>>>> int r = 0;
>>>>> - adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
>>>>> + man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>>>>> + mgr = to_gtt_mgr(man);
>>>>> spin_lock(&mgr->lock);
>>>>> drm_mm_for_each_node(mm_node, &mgr->mm) {
>>>>> node = container_of(mm_node, typeof(*node),
>>>>> base.mm_nodes[0]);
>>>>> @@ -260,6 +264,7 @@ static void amdgpu_gtt_mgr_debug(struct
>>>>> ttm_resource_manager *man,
>>>>> struct drm_printer *printer)
>>>>> {
>>>>> struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
>>>>> + struct amdgpu_device *adev = container_of(mgr, typeof(*adev),
>>>>> mman.gtt_mgr);
>>>>> spin_lock(&mgr->lock);
>>>>> drm_mm_print(&mgr->mm, printer);
>>>>> @@ -267,7 +272,7 @@ static void amdgpu_gtt_mgr_debug(struct
>>>>> ttm_resource_manager *man,
>>>>> drm_printf(printer, "man size:%llu pages, gtt
>>>>> available:%lld pages, usage:%lluMB\n",
>>>>> man->size, (u64)atomic64_read(&mgr->available),
>>>>> - amdgpu_gtt_mgr_usage(man) >> 20);
>>>>> + amdgpu_gtt_mgr_usage(adev) >> 20);
>>>>> }
>>>>> static const struct ttm_resource_manager_func
>>>>> amdgpu_gtt_mgr_func = {
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> index d2955ea4a62b..b9b38f70e416 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> @@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev,
>>>>> void *data, struct drm_file *filp)
>>>>> ui64 =
>>>>> amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev,
>>>>> TTM_PL_VRAM));
>>>>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT
>>>>> : 0;
>>>>> case AMDGPU_INFO_GTT_USAGE:
>>>>> - ui64 =
>>>>> amdgpu_gtt_mgr_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_TT));
>>>>> + ui64 = amdgpu_gtt_mgr_usage(adev);
>>>>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT
>>>>> : 0;
>>>>> case AMDGPU_INFO_GDS_CONFIG: {
>>>>> struct drm_amdgpu_info_gds gds_info;
>>>>> @@ -738,7 +738,7 @@ int amdgpu_info_ioctl(struct drm_device *dev,
>>>>> void *data, struct drm_file *filp)
>>>>> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
>>>>> atomic64_read(&adev->gart_pin_size);
>>>>> mem.gtt.heap_usage =
>>>>> - amdgpu_gtt_mgr_usage(gtt_man);
>>>>> + amdgpu_gtt_mgr_usage(adev);
>>>>> mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
>>>>> return copy_to_user(out, &mem,
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> index 91a087f9dc7c..6e337cacef51 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> @@ -114,8 +114,8 @@ int amdgpu_vram_mgr_init(struct amdgpu_device
>>>>> *adev);
>>>>> void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>>>>> bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
>>>>> -uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man);
>>>>> -int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man);
>>>>> +uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_device *adev);
>>>>> +int amdgpu_gtt_mgr_recover(struct amdgpu_device *adev);
>>>>> uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager
>>>>> *man);
>>>>
>
More information about the amd-gfx
mailing list