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

[Michel Dänzer]

On 2018-05-22 11:11 AM, Christian König wrote:
> Am 22.05.2018 um 09:22 schrieb 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.
> 
> Hui, what? Isn't the fb freed after X takes over?

Nope, that isn't possible (not even un-pinning it from VRAM), because
the console layer can try accessing the buffer at any time from any context.


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