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.
