[PATCH 2/3] drm/amdgpu: Implement new dummy vram manager

[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.

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);
>>