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