[PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2018å¹´04æ??16æ?¥ 17:36, Christian König wrote:
> Am 16.04.2018 um 11:21 schrieb zhoucm1:
>>
>>
>> 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.
>
> Oh, good idea! That would be a good solution as well I think.
>
> Then we can provide amdgpu_bo_do_create() with both the preferred and 
> the allowed domain.
>
> Do you want to keep working on this or should I?
you can push your reverting patch#1 and #2 first. we can re-do this 
patch#3 again after I pass my patches of collecting parameters for 
amdgpu_bo_create.

Regards,
David Zhou
>
> Thanks,
> Christian.
>
>>
>> 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
>>
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux