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 <[email protected]> >>> --- >>> 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 :) - 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 <[email protected]> >> >> - Shashank >> >>> if (domain & AMDGPU_GEM_DOMAIN_GTT) >>> domain = AMDGPU_GEM_DOMAIN_GTT; >>> else _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
