[PATCH 2/3] drm/amdgpu: Implement new dummy vram manager
Felix Kuehling
felix.kuehling at amd.com
Thu Jun 15 14:52:40 UTC 2023
Am 2023-06-15 um 03:37 schrieb Christian König:
> Am 14.06.23 um 17:42 schrieb Felix Kuehling:
>> Am 2023-06-14 um 06:38 schrieb Christian König:
>>> Am 10.05.23 um 00:01 schrieb Alex Deucher:
>>>> From: Rajneesh Bhardwaj <rajneesh.bhardwaj at amd.com>
>>>>
>>>> This adds dummy vram manager to support ASICs that do not have a
>>>> dedicated or carvedout vram domain.
>>>
>>> Well that doesn't seem to make much sense. Why we should have that?
>>
>> TTM always expects a resource manager for VRAM. There are no NULL
>> pointer checks in TTM for not having a resource manager for VRAM. The
>> existing amdgpu_vram_mgr gets confused if there is no VRAM. It seemed
>> cleaner to add a dummy manager than to scatter conditions for a
>> memory-less GPU corner case through the regular VRAM manager.
>
> Well no that's absolutely *not* cleaner. TTM has a predefined manager
> if you need to use a dummy.
I think you are referring to ttm_range_manager. ttm_range_man_alloc does
a bunch of useless stuff when there is no hope of succeeding:
* kzalloc a node struct
* ttm_resource_init
o add the node to an LRU
* drm_mm_insert_node_in_range (which fails because the drm_mm was
created with 0 size)
* ttm_resource_fini
o remove the node from an LRU
* kfree the node struct
In that process it also takes 3 spin_locks. All of that for TTM to
figure out that VRAM is not a feasible placement. All we need to do here
in the dummy manager is to return -ENOSPC.
I really don't get why this bothers you so much, or why this is even
controversial.
Regards,
Felix
>
> Why the heck didn't you ask me before doing stuff like that?
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Felix
>>
>>
>>>
>>> Christian.
>>>
>>>>
>>>> Reviewed-by: Felix Kuehling <felix.kuehling at amd.com>
>>>> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj at amd.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 67
>>>> ++++++++++++++++++--
>>>> 1 file changed, 60 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>> index 43d6a9d6a538..89d35d194f2c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>>> @@ -370,6 +370,45 @@ int amdgpu_vram_mgr_query_page_status(struct
>>>> amdgpu_vram_mgr *mgr,
>>>> return ret;
>>>> }
>>>> +static void amdgpu_dummy_vram_mgr_debug(struct
>>>> ttm_resource_manager *man,
>>>> + struct drm_printer *printer)
>>>> +{
>>>> + DRM_DEBUG_DRIVER("Dummy vram mgr debug\n");
>>>> +}
>>>> +
>>>> +static bool amdgpu_dummy_vram_mgr_compatible(struct
>>>> ttm_resource_manager *man,
>>>> + struct ttm_resource *res,
>>>> + const struct ttm_place *place,
>>>> + size_t size)
>>>> +{
>>>> + DRM_DEBUG_DRIVER("Dummy vram mgr compatible\n");
>>>> + return false;
>>>> +}
>>>> +
>>>> +static bool amdgpu_dummy_vram_mgr_intersects(struct
>>>> ttm_resource_manager *man,
>>>> + struct ttm_resource *res,
>>>> + const struct ttm_place *place,
>>>> + size_t size)
>>>> +{
>>>> + DRM_DEBUG_DRIVER("Dummy vram mgr intersects\n");
>>>> + return true;
>>>> +}
>>>> +
>>>> +static void amdgpu_dummy_vram_mgr_del(struct ttm_resource_manager
>>>> *man,
>>>> + struct ttm_resource *res)
>>>> +{
>>>> + DRM_DEBUG_DRIVER("Dummy vram mgr deleted\n");
>>>> +}
>>>> +
>>>> +static int amdgpu_dummy_vram_mgr_new(struct ttm_resource_manager
>>>> *man,
>>>> + struct ttm_buffer_object *tbo,
>>>> + const struct ttm_place *place,
>>>> + struct ttm_resource **res)
>>>> +{
>>>> + DRM_DEBUG_DRIVER("Dummy vram mgr new\n");
>>>> + return -ENOSPC;
>>>> +}
>>>> +
>>>> /**
>>>> * amdgpu_vram_mgr_new - allocate new ranges
>>>> *
>>>> @@ -817,6 +856,14 @@ static void amdgpu_vram_mgr_debug(struct
>>>> ttm_resource_manager *man,
>>>> mutex_unlock(&mgr->lock);
>>>> }
>>>> +static const struct ttm_resource_manager_func
>>>> amdgpu_dummy_vram_mgr_func = {
>>>> + .alloc = amdgpu_dummy_vram_mgr_new,
>>>> + .free = amdgpu_dummy_vram_mgr_del,
>>>> + .intersects = amdgpu_dummy_vram_mgr_intersects,
>>>> + .compatible = amdgpu_dummy_vram_mgr_compatible,
>>>> + .debug = amdgpu_dummy_vram_mgr_debug
>>>> +};
>>>> +
>>>> static const struct ttm_resource_manager_func
>>>> amdgpu_vram_mgr_func = {
>>>> .alloc = amdgpu_vram_mgr_new,
>>>> .free = amdgpu_vram_mgr_del,
>>>> @@ -841,17 +888,22 @@ int amdgpu_vram_mgr_init(struct amdgpu_device
>>>> *adev)
>>>> ttm_resource_manager_init(man, &adev->mman.bdev,
>>>> adev->gmc.real_vram_size);
>>>> - man->func = &amdgpu_vram_mgr_func;
>>>> -
>>>> - err = drm_buddy_init(&mgr->mm, man->size, PAGE_SIZE);
>>>> - if (err)
>>>> - return err;
>>>> -
>>>> mutex_init(&mgr->lock);
>>>> INIT_LIST_HEAD(&mgr->reservations_pending);
>>>> INIT_LIST_HEAD(&mgr->reserved_pages);
>>>> mgr->default_page_size = PAGE_SIZE;
>>>> + if (!adev->gmc.is_app_apu) {
>>>> + man->func = &amdgpu_vram_mgr_func;
>>>> +
>>>> + err = drm_buddy_init(&mgr->mm, man->size, PAGE_SIZE);
>>>> + if (err)
>>>> + return err;
>>>> + } else {
>>>> + man->func = &amdgpu_dummy_vram_mgr_func;
>>>> + DRM_INFO("Setup dummy vram mgr\n");
>>>> + }
>>>> +
>>>> ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_VRAM,
>>>> &mgr->manager);
>>>> ttm_resource_manager_set_used(man, true);
>>>> return 0;
>>>> @@ -886,7 +938,8 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device
>>>> *adev)
>>>> drm_buddy_free_list(&mgr->mm, &rsv->allocated);
>>>> kfree(rsv);
>>>> }
>>>> - drm_buddy_fini(&mgr->mm);
>>>> + if (!adev->gmc.is_app_apu)
>>>> + drm_buddy_fini(&mgr->mm);
>>>> mutex_unlock(&mgr->lock);
>>>> ttm_resource_manager_cleanup(man);
>>>
>
More information about the amd-gfx
mailing list