On 2018å¹´04æ??16æ?¥ 17:04, zhoucm1 wrote:
>
>
> On 2018å¹´04æ??16æ?¥ 16:57, Christian König wrote:
>> Am 11.04.2018 um 11:22 schrieb Christian König:
>>> Am 11.04.2018 um 04:38 schrieb zhoucm1:
>>>>
>>>>
>>>> On 2018å¹´04æ??10æ?¥ 21:42, Christian König wrote:
>>>>> When GEM needs to fallback to GTT for VRAM BOs we still want the
>>>>> preferred domain to be untouched so that the BO has a cance to
>>>>> move back
>>>>> to VRAM in the future.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>> ---
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   | 14 +++++++++++---
>>>>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++-----------
>>>>> Â 2 files changed, 13 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> index 46b9ea4e6103..9dc0a190413c 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> @@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct
>>>>> amdgpu_device *adev, unsigned long size,
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct reservation_object *resv,
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct drm_gem_object **obj)
>>>>> Â {
>>>>> +Â Â Â uint32_t domain = initial_domain;
>>>>> Â Â Â Â Â struct amdgpu_bo *bo;
>>>>> Â Â Â Â Â int r;
>>>>> Â @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct
>>>>> amdgpu_device *adev, unsigned long size,
>>>>> Â Â Â Â Â }
>>>>> Â Â retry:
>>>>> -Â Â Â r = amdgpu_bo_create(adev, size, alignment, initial_domain,
>>>>> +Â Â Â r = amdgpu_bo_create(adev, size, alignment, domain,
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â flags, type, resv, &bo);
>>>>> Â Â Â Â Â if (r) {
>>>>> Â Â Â Â Â Â Â Â Â if (r != -ERESTARTSYS) {
>>>>> @@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct
>>>>> amdgpu_device *adev, unsigned long size,
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto retry;
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â }
>>>>> Â -Â Â Â Â Â Â Â Â Â Â Â if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> +Â Â Â Â Â Â Â Â Â Â Â if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
>>>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto retry;
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â }
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â DRM_DEBUG("Failed to allocate GEM object (%ld, %d,
>>>>> %u, %d)\n",
>>>>> @@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct
>>>>> amdgpu_device *adev, unsigned long size,
>>>>> Â Â Â Â Â Â Â Â Â }
>>>>> Â Â Â Â Â Â Â Â Â return r;
>>>>> Â Â Â Â Â }
>>>>> +
>>>>> +Â Â Â bo->preferred_domains = initial_domain;
>>>>> +Â Â Â bo->allowed_domains = initial_domain;
>>>>> +Â Â Â if (type != ttm_bo_type_kernel &&
>>>>> +Â Â Â Â Â Â Â bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>>>>> +Â Â Â Â Â Â Â bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>>>> It's an ugly change back after bo creation! Do you think it's
>>>> better than previous?
>>>
>>> Well it's better than moving the fallback handling into the core
>>> functions and then adding a flag to disable it again because we
>>> don't want it in the core function.
>>>
>>>>
>>>> Alternative way, you can add one parameter to amdgpu_bo_create,
>>>> directly pass preferred_domains and allowed_domains to
>>>> amdgpu_bo_create.
>>>
>>> Then we would need three parameters, one where to create the BO now
>>> and two for preferred/allowed domains.
>>>
>>> I think that in this case overwriting the placement from the initial
>>> value looks cleaner to me.
>>
>> Ping? Any more opinions on how to proceed?
> No, I keep my opinion on this. Michel already gave your RB I remember.
>
> Regards,
> David Zhou
>>
>> I agree that neither solution is perfect, but updating the placement
>> after we created the BO still sounds like the least painful to me.
And I'm going to have patch for amdgpu_bo_create paramters, collect many
paramters to one param struct like done in vm.
Regards,
David Zhou
>>
>> Christian.
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> David Zhou
>>>>> +
>>>>> Â Â Â Â Â *obj = &bo->gem_base;
>>>>> Â Â Â Â Â Â return 0;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> index 6d08cde8443c..854d18d5cff4 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> @@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct
>>>>> amdgpu_device *adev, unsigned long size,
>>>>> Â Â Â Â Â drm_gem_private_object_init(adev->ddev, &bo->gem_base, size);
>>>>> Â Â Â Â Â INIT_LIST_HEAD(&bo->shadow_list);
>>>>> Â Â Â Â Â INIT_LIST_HEAD(&bo->va);
>>>>> -Â Â Â bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AMDGPU_GEM_DOMAIN_GTT |
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AMDGPU_GEM_DOMAIN_CPU |
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AMDGPU_GEM_DOMAIN_GDS |
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AMDGPU_GEM_DOMAIN_GWS |
>>>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â AMDGPU_GEM_DOMAIN_OA);
>>>>> -Â Â Â bo->allowed_domains = bo->preferred_domains;
>>>>> -Â Â Â if (type != ttm_bo_type_kernel &&
>>>>> -Â Â Â Â Â Â Â bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>>>>> -Â Â Â Â Â Â Â bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> -
>>>>> +Â Â Â bo->preferred_domains = domain;
>>>>> +Â Â Â bo->allowed_domains = domain;
>>>>> Â Â Â Â Â bo->flags = flags;
>>>>> Â Â #ifdef CONFIG_X86_32
>>>>
>>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
