Re: [RFC PATCH 10/18] drm/amdgpu: Don't add GTT to initial domains after failing to allocate VRAM

["Christian König" <christian.koenig@amd.com>]

Am 25.04.24 um 09:39 schrieb Friedrich Vock:
> On 25.04.24 08:25, Christian König wrote:
>> Am 24.04.24 um 18:57 schrieb Friedrich Vock:
>>> This adds GTT to the "preferred domains" of this buffer object, which
>>> will also prevent any attempts at moving the buffer back to VRAM if
>>> there is space. If VRAM is full, GTT will already be chosen as a
>>> fallback.
>>
>> Big NAK to that one, this is mandatory for correct operation.
>>
> Hm, how is correctness affected here? We still fall back to GTT if
> allocating in VRAM doesn't work, I don't see a difference except that
> now we'll actually try moving it back into VRAM again.

Well this is the fallback. Only during CS we try to allocate from GTT if 
allocating in VRAM doesn't work.

When you remove this here then any failed allocation from VRAM would be 
fatal.

What could be is that the handling is buggy and when we update the 
initial domain we also add GTT to the preferred domain, but that should 
then be fixed.

Regards,
Christian.

>
> Regards,
> Friedrich
>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 4 ----
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
>>>   2 files changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 6bbab141eaaeb..aea3770d3ea2e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -378,10 +378,6 @@ int amdgpu_gem_create_ioctl(struct drm_device
>>> *dev, void *data,
>>>               goto retry;
>>>           }
>>>
>>> -        if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
>>> -            initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
>>> -            goto retry;
>>> -        }
>>>           DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
>>> %d)\n",
>>>                   size, initial_domain, args->in.alignment, r);
>>>       }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 85c10d8086188..9978b85ed6f40 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -619,7 +619,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>>                 AMDGPU_GEM_DOMAIN_GDS))
>>>           amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
>>>       else
>>> -        amdgpu_bo_placement_from_domain(bo, bp->domain);
>>> +        amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
>>>       if (bp->type == ttm_bo_type_kernel)
>>>           bo->tbo.priority = 2;
>>>       else if (!(bp->flags & AMDGPU_GEM_CREATE_DISCARDABLE))
>>> -- 
>>> 2.44.0
>>>
>>