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.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
