[PATCH] drm/amdgpu: consider user preference when pinning for SG display

[Michel Dänzer]

On 2018-05-19 05:59 PM, Christian König wrote:
> Am 18.05.2018 um 21:16 schrieb Alex Deucher:
>> On Fri, May 18, 2018 at 2:22 PM, Samuel Li <samuel.li at amd.com> wrote:
>>>
>>> On 2018-05-18 04:21 AM, Michel Dänzer wrote:
>>>> On 2018-05-17 06:55 PM, Alex Deucher wrote:
>>>>> If the pin domain is set to GTT | VRAM, look at the preferred domains
>>>>> for the bo and respect that if it's been set explicitly.
>>>>>
>>>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++++++--
>>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> index 6a9e46ae7f0a..16192f17653e 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> @@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
>>>>> *bo, u32 domain,
>>>>>        * See function amdgpu_display_supported_domains()
>>>>>        */
>>>>>       if (domain == (AMDGPU_GEM_DOMAIN_VRAM |
>>>>> AMDGPU_GEM_DOMAIN_GTT)) {
>>>>> -            domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>> -            if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>>>> +            if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
>>>>> +                    domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user
>>>>> really wants vram, respect it */
>>>>> +            else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
>>>>> +                    domain = AMDGPU_GEM_DOMAIN_GTT; /* if user
>>>>> really wants gtt, respect it */
>>>> I'd spell VRAM and GTT in capital letters in the comments.
>>>>
>>>>
>>>>> +            else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>>>>                       domain = AMDGPU_GEM_DOMAIN_GTT;
>>>>> +            else
>>>>> +                    domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>>       }
>>>> Is everything in place to deal with any issues that might occur when
>>>> flipping between buffers in VRAM and GTT?
>>>>
>>> Besides this question, I am also wondering how to deal with the first
>>> display buffer created in kernel at amdgpufb_create_pinned_object()?
>> amdgpufb_create_pinned_object() calls
>> amdgpu_display_supported_domains() for the initial domain and
>> amdgpu_gem_object_create() uses that for the preferred domain so it
>> won't change the behavior there.
> 
> That was actually something I was not sure about when I initially
> created the patch.
> 
> See the FB emulation expects some pointer into a linear PCI resource
> (VRAM), I'm not sure if that can actually deal with something kmapped
> into the vmap (GTT).
> 
> Since this is seldom used and only with low resolution

It's not that low resolution these days (normally the smallest native
resolution of all connected displays, i.e. generally >= 1920x1080), i.e.
on the order of ~10 MB of VRAM that can't be used for anything else.

> it might be better to do the conservative approach and use VRAM here.

Or somebody could try forcing GTT for this, and see what happens. :) If
it works, I'd say GTT is actually preferable for this.


> And we do a full mode set when switching away from FB emulation AFAIK,
> so the move from VRAM to GTT shouldn't be a problem.

Agreed.


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