[PATCH] drm/amdgpu: enable freesync for A+A configs

[Christian König]

Am 01.02.21 um 16:13 schrieb Shashank Sharma:
> On 01/02/21 8:39 pm, Christian König wrote:
>> Am 01.02.21 um 16:06 schrieb Shashank Sharma:
>>> Hello Christian,
>>>
>>> On 01/02/21 8:04 pm, Christian König wrote:
>>>> Some newer APUs can scanout directly from GTT, that saves us from
>>>> allocating another bounce buffer in VRAM and enables freesync in such
>>>> configurations.
>>> Shall we add some more details about how this patch helps with VRR, like "Without this patch, it won't be possible for the IGPU to flip buffers which are composed on an external GPU" or something in those lines ?
>> How about:
>>
>> "Without this patch creating a framebuffer from the imported BO will
>> fail and userspace will fall back to a copy".
> Yep, looks good enough. There is just one more tiny comment below, please check that out too.
>
>> Thanks,
>> Christian.
>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++--
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 2 +-
>>>>    2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> index a638709e9c92..823bc25d87de 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> @@ -930,8 +930,10 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>>>>    				       struct drm_file *file_priv,
>>>>    				       const struct drm_mode_fb_cmd2 *mode_cmd)
>>>>    {
>>>> -	struct drm_gem_object *obj;
>>>>    	struct amdgpu_framebuffer *amdgpu_fb;
>>>> +	struct drm_gem_object *obj;
>>>> +	struct amdgpu_bo *bo;
>>>> +	uint32_t domains;
>>>>    	int ret;
>>>>    
>>>>    	obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
>>>> @@ -942,7 +944,9 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>>>>    	}
>>>>    
>>>>    	/* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
>>>> -	if (obj->import_attach) {
>>>> +	bo = gem_to_amdgpu_bo(obj);
>>> Is it possible that the bo can be NULL ? I mean do we need a NULL check here ?
> This one :)

No, this is just a type cast.

Christian.

>
> - Shashank
>
>>>> +	domains = amdgpu_display_supported_domains(drm_to_adev(dev), bo->flags);
>>>> +	if (obj->import_attach && !(domains & AMDGPU_GEM_DOMAIN_GTT)) {
>>>>    		drm_dbg_kms(dev, "Cannot create framebuffer from imported dma_buf\n");
>>>>    		return ERR_PTR(-EINVAL);
>>>>    	}
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index 56854a3ee19c..0bd22ed1dacf 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -900,7 +900,7 @@ 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) {
>>>> +	if (bo->prime_shared_count || bo->tbo.base.import_attach) {
>>> With the above concerns addressed (or reasoned :)), please feel free to use:
>>>
>>> Reviewed-by: Shashank Sharma <shashank.sharma at amd.com>
>>>
>>> - Shashank
>>>
>>>>    		if (domain & AMDGPU_GEM_DOMAIN_GTT)
>>>>    			domain = AMDGPU_GEM_DOMAIN_GTT;
>>>>    		else