[PATCH 1/1] drm/amdgpu: Use GTT for dumb buffer if sg display enabled

[Michel Dänzer]

On 2018-05-22 12:22 PM, Christian König wrote:
> Am 22.05.2018 um 12:14 schrieb Michel Dänzer:
>> On 2018-05-22 12:11 PM, Christian König wrote:
>>> Am 22.05.2018 um 09:09 schrieb Michel Dänzer:
>>>> On 2018-05-22 01:22 AM, Deepak Sharma wrote:
>>>>> When vram size <= THRESHOLD(256M) lets use GTT for dumb buffer
>>>>> allocation. As SG will be enabled with vram size <= 256M
>>>>> scan out will not be an issue.
>>>>>
>>>>> Signed-off-by: Deepak Sharma <Deepak.Sharma at amd.com>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 7 +++++--
>>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> index 2c8e27370284..c547d92dbf4e 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> @@ -749,15 +749,18 @@ int amdgpu_mode_dumb_create(struct drm_file
>>>>> *file_priv,
>>>>>        struct amdgpu_device *adev = dev->dev_private;
>>>>>        struct drm_gem_object *gobj;
>>>>>        uint32_t handle;
>>>>> +    u32 domain;
>>>>>        int r;
>>>>>          args->pitch = amdgpu_align_pitch(adev, args->width,
>>>>>                         DIV_ROUND_UP(args->bpp, 8), 0);
>>>>>        args->size = (u64)args->pitch * args->height;
>>>>>        args->size = ALIGN(args->size, PAGE_SIZE);
>>>>> +    domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>> +    if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>>>> +        domain = AMDGPU_GEM_DOMAIN_GTT;
>>>> This could choose GTT even in cases where scanout from that isn't
>>>> supported.
>>>>
>>>> It should call amdgpu_display_supported_domains, and then use the same
>>>> logic as in amdgpu_bo_pin_restricted:
>>>>
>>>>           if (domain == (AMDGPU_GEM_DOMAIN_VRAM |
>>>> AMDGPU_GEM_DOMAIN_GTT)) {
>>>>                   domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>                   if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>>>                           domain = AMDGPU_GEM_DOMAIN_GTT;
>>>>           }
>>>>
>>>> Maybe this logic could be refactored into a helper function.
>>> Actually you don't even need that logic here. All you need to do is to
>>> use amdgpu_display_supported_domains and you're done.
>>>
>>> The BO is moved to the correct place when it is pinned for scanout or
>>> use with the fb interface.
>> Right, however, if the buffer is going to be pinned to GTT for scanout,
>> it would be nice to create it there in the first place, otherwise we
>> incur some overhead for moving it (and possibly for the slower CPU
>> access while it's in VRAM).
> 
> If the BO is moved from VRAM to VRAM (to make it linear) or from VRAM to
> GTT for scanout doesn't really matter.

Not sure what you mean. Dumb BOs are only used for CPU access and
scanout, and should be created accordingly.


> And BTW GTT BOs used for scanout should be created USWC, so the argument
> that it is faster for CPU access isn't really true either.

You're saying scanout doesn't work from cacheable GTT BOs? If it does
work, it would be preferable for dumb BOs.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer