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
I will update the libdrm test to correctly handle mem failure, it
segfaults at the moment.
Thanks,
Andey
>
> Regards,
> Christian.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-Implement-BO-size-validation-V2.patch
Type: text/x-patch
Size: 2252 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20171113/393a18a3/attachment-0001.bin>
