Am 13.11.2017 um 15:57 schrieb Andrey Grodzovsky:
> On 11/13/2017 07:39 AM, Christian König wrote:
>
>> 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.
>
> Attached V2 patch, I still don't understand why I experience the
> SIGSEV in the tester when the check fails and the IOCTLs will return
> ENOMEM
>
Reviewed-by: Christian König <christian.koenig at amd.com> for this one,
but please use git send-email to send out patches.
> I will update the libdrm test to correctly handle mem failure, it
> segfaults at the moment.
Sounds like it just tries to use the BO for VM or CPU mapping while the
underlying function has failed (or we have another bug somewhere).
Please commit the kernel patch and leave me a note so that I can push
the libdrm patches. BTW: Do you have the link where you request an
account at hand? I want to ping the admins once more.
Regards,
Christian.
>
>
> Thanks,
> Andey
>>
>> Regards,
>> Christian.
>
