[PATCH 1/1] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 18.04.2018 um 17:29 schrieb Samuel Li:
>
> On 2018-04-18 12:14 AM, Alex Deucher wrote:
>> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li at amd.com> wrote:
>>> It's auto by default. For CZ/ST, auto setting enables sg display
>>> when vram size is small; otherwise still uses vram.
>>> This patch fixed some potention hang issue introduced by change
>>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>>> limitation.
>>>
>>> v2: Change default setting to auto.
>>> v3: Move some logic from amdgpu_display_framebuffer_domains()
>>>      to pin function, suggested by Christian.
>>> Signed-off-by: Samuel Li <Samuel.Li at amd.com>
>>> [...]
>>> @@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
>>>          .create_handle = drm_gem_fb_create_handle,
>>>   };
>>>
>>> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
>> This change should be a separate patch,
>>
> OK.
>
> [...]
>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 24f582c..f0f1f8a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>>                  return -EINVAL;
>>>
>>>          /* A shared bo cannot be migrated to VRAM */
>>> -       if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
>>> -               return -EINVAL;
>>> +       if (bo->prime_shared_count) {
>>> +               if (domain & AMDGPU_GEM_DOMAIN_GTT)
>>> +                       domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +               else
>>> +                       return -EINVAL;
>>> +       }
>> This is a bug fix and should be split out into a separate patch.
>>
> OK.
>
>>> +
>>> +       /* display buffer */
>>> +#if defined(CONFIG_DRM_AMD_DC)
>>> +       if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
>>> +           adev->flags & AMD_IS_APU &&
>>> +           amdgpu_device_asic_has_dc_support(adev->asic_type) &&
>>> +           domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>>> +               if (amdgpu_sg_display == 1)
>>> +                       domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +               else if (amdgpu_sg_display == -1) {
>>> +                       if (adev->gmc.real_vram_size < AMDGPU_SG_THRESHOLD)
>>> +                               domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +                       else
>>> +                               domain = AMDGPU_GEM_DOMAIN_VRAM;
>>> +               }
>> I thought we were dropping the module parameter.  Also, we had talked
>> about taking preferred domains into account here as well, but that can
>> be a follow on patch.
> As per the documents, SG display feature can affect other features. The option has been used for debugging on Windows by development, testing and supported teams. So I prefer to keep it.

Mhm, for developer testing we can easily modify 
amdgpu_display_supported_domains().

The real question is should we give an end user the ability to modify 
the behavior? I currently can't think of a reason for that.

Regards,
Christian.

>
> Regards,
> Samuel Li
>
>> Alex
>>
>>> +       }
>>> +#endif
>>>
>>>          if (bo->pin_count) {
>>>                  uint32_t mem_type = bo->tbo.mem.mem_type;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> index 4b584cb7..cf0749f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>>>          struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>>>          struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>          struct ttm_operation_ctx ctx = { true, false };
>>> -       u32 domain = amdgpu_display_framebuffer_domains(adev);
>>> +       u32 domain = amdgpu_display_supported_domains(adev);
>>>          int ret;
>>>          bool reads = (direction == DMA_BIDIRECTIONAL ||
>>>                        direction == DMA_FROM_DEVICE);
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index 6f92a19..1f5603a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>>                  return r;
>>>
>>>          if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>> -               domain = amdgpu_display_framebuffer_domains(adev);
>>> +               domain = amdgpu_display_supported_domains(adev);
>>>          else
>>>                  domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>
>>>          r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>> -
>>>          amdgpu_bo_unreserve(rbo);
>>>
>>>          if (unlikely(r != 0)) {
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx