[PATCH libdrm 0/4] Dynamicly disable suites and tets.

Christian König christian.koenig at amd.com
Mon Nov 13 12:39:35 UTC 2017


Am 13.11.2017 um 12:32 schrieb Michel Dänzer:
> On 12/11/17 10:35 AM, Christian König wrote:
>> A few comments on the code:
>>
>>> +/* Validate bo size is bit bigger then the request domain */
>>> +static inline bool amdgpu_bo_validate_bo_size(struct amdgpu_device
>>> *adev,
>>> +                      unsigned long size, u32 domain)
>> Drop the inline keyword and the second _bo_ in the name here.
>>
>>> +{
>>> +    struct ttm_mem_type_manager *man = NULL;
>>> +
>>> +    if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
>>> +        man = &adev->mman.bdev.man[TTM_PL_VRAM];
>>> +
>>> +        if (man && size < (man->size << PAGE_SHIFT))
>> Drop the extra check that man is not NULL. We get the pointer to an
>> array element, that can't be NULL.
>>
>>> +            return true;
>> Mhm, domain is a bitmask of allowed domains.
>>
>> So we should check all valid domains if the size fit, not just the first
>> one.
> Assuming VRAM <-> system migration of BOs larger than the GTT domain
> works, I'd say we should only require that the BO can fit in any of the
> allowed domains. Otherwise it must also always fit in GTT.
Good point, and yes VRAM <-> system migration of BOs larger than the GTT 
domain works now.

I can agree on that VRAM should probably be optional, otherwise we can't 
allocate anything large when the driver uses only very low amounts of 
stolen VRAM on APUs.

But I think when userspace requests VRAM and GTT at the same time we 
still should be able to fall back to GTT.

Regards,
Christian.


More information about the amd-gfx mailing list