Great to have this!
Reviewed-by: Martin Krastev <martin.krastev@xxxxxxxxxxxx>
Regards,
Martin
On Wed, Apr 3, 2024 at 2:28 AM Zack Rusin <zack.rusin@xxxxxxxxxxxx> wrote:
>
> vmwgfx never supported prime import of external buffers. Furthermore the
> driver exposes two different objects to userspace: vmw_surface's and
> gem buffers but prime import/export only worked with vmw_surfaces.
>
> Because gem buffers are used through the dumb_buffer interface this meant
> that the driver created buffers couldn't have been prime exported or
> imported.
>
> Fix prime import/export. Makes IGT's kms_prime pass.
>
> Signed-off-by: Zack Rusin <zack.rusin@xxxxxxxxxxxx>
> Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
> Cc: <stable@xxxxxxxxxxxxxxx> # v6.6+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 35 +++++++++++++++--
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 7 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 2 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 +
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 32 ++++++++++++++++
> drivers/gpu/drm/vmwgfx/vmwgfx_prime.c | 15 +++++++-
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 44 +++++++++++++++-------
> 8 files changed, 117 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> index c52c7bf1485b..717d624e9a05 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> @@ -456,8 +456,10 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst,
> .no_wait_gpu = false
> };
> u32 j, initial_line = dst_offset / dst_stride;
> - struct vmw_bo_blit_line_data d;
> + struct vmw_bo_blit_line_data d = {0};
> int ret = 0;
> + struct page **dst_pages = NULL;
> + struct page **src_pages = NULL;
>
> /* Buffer objects need to be either pinned or reserved: */
> if (!(dst->pin_count))
> @@ -477,12 +479,35 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst,
> return ret;
> }
>
> + if (!src->ttm->pages && src->ttm->sg) {
> + src_pages = kvmalloc_array(src->ttm->num_pages,
> + sizeof(struct page *), GFP_KERNEL);
> + if (!src_pages)
> + return -ENOMEM;
> + ret = drm_prime_sg_to_page_array(src->ttm->sg, src_pages,
> + src->ttm->num_pages);
> + if (ret)
> + goto out;
> + }
> + if (!dst->ttm->pages && dst->ttm->sg) {
> + dst_pages = kvmalloc_array(dst->ttm->num_pages,
> + sizeof(struct page *), GFP_KERNEL);
> + if (!dst_pages) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + ret = drm_prime_sg_to_page_array(dst->ttm->sg, dst_pages,
> + dst->ttm->num_pages);
> + if (ret)
> + goto out;
> + }
> +
> d.mapped_dst = 0;
> d.mapped_src = 0;
> d.dst_addr = NULL;
> d.src_addr = NULL;
> - d.dst_pages = dst->ttm->pages;
> - d.src_pages = src->ttm->pages;
> + d.dst_pages = dst->ttm->pages ? dst->ttm->pages : dst_pages;
> + d.src_pages = src->ttm->pages ? src->ttm->pages : src_pages;
> d.dst_num_pages = PFN_UP(dst->resource->size);
> d.src_num_pages = PFN_UP(src->resource->size);
> d.dst_prot = ttm_io_prot(dst, dst->resource, PAGE_KERNEL);
> @@ -504,6 +529,10 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst,
> kunmap_atomic(d.src_addr);
> if (d.dst_addr)
> kunmap_atomic(d.dst_addr);
> + if (src_pages)
> + kvfree(src_pages);
> + if (dst_pages)
> + kvfree(dst_pages);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index bfd41ce3c8f4..e5eb21a471a6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -377,7 +377,8 @@ static int vmw_bo_init(struct vmw_private *dev_priv,
> {
> struct ttm_operation_ctx ctx = {
> .interruptible = params->bo_type != ttm_bo_type_kernel,
> - .no_wait_gpu = false
> + .no_wait_gpu = false,
> + .resv = params->resv,
> };
> struct ttm_device *bdev = &dev_priv->bdev;
> struct drm_device *vdev = &dev_priv->drm;
> @@ -394,8 +395,8 @@ static int vmw_bo_init(struct vmw_private *dev_priv,
>
> vmw_bo_placement_set(vmw_bo, params->domain, params->busy_domain);
> ret = ttm_bo_init_reserved(bdev, &vmw_bo->tbo, params->bo_type,
> - &vmw_bo->placement, 0, &ctx, NULL,
> - NULL, destroy);
> + &vmw_bo->placement, 0, &ctx,
> + params->sg, params->resv, destroy);
> if (unlikely(ret))
> return ret;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> index 0d496dc9c6af..f349642e6190 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> @@ -55,6 +55,8 @@ struct vmw_bo_params {
> enum ttm_bo_type bo_type;
> size_t size;
> bool pin;
> + struct dma_resv *resv;
> + struct sg_table *sg;
> };
>
> /**
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 89d3679d2608..41ad13e45554 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1631,6 +1631,7 @@ static const struct drm_driver driver = {
>
> .prime_fd_to_handle = vmw_prime_fd_to_handle,
> .prime_handle_to_fd = vmw_prime_handle_to_fd,
> + .gem_prime_import_sg_table = vmw_prime_import_sg_table,
>
> .fops = &vmwgfx_driver_fops,
> .name = VMWGFX_DRIVER_NAME,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index ddbceaa31b59..4ecaea0026fc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -1107,6 +1107,9 @@ extern int vmw_prime_handle_to_fd(struct drm_device *dev,
> struct drm_file *file_priv,
> uint32_t handle, uint32_t flags,
> int *prime_fd);
> +struct drm_gem_object *vmw_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *table);
>
> /*
> * MemoryOBject management - vmwgfx_mob.c
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> index 186150f41fbc..2132a8ad8c0c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> @@ -136,6 +136,38 @@ int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
> return ret;
> }
>
> +struct drm_gem_object *vmw_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *table)
> +{
> + int ret;
> + struct vmw_private *dev_priv = vmw_priv(dev);
> + struct drm_gem_object *gem = NULL;
> + struct vmw_bo *vbo;
> + struct vmw_bo_params params = {
> + .domain = (dev_priv->has_mob) ? VMW_BO_DOMAIN_SYS : VMW_BO_DOMAIN_VRAM,
> + .busy_domain = VMW_BO_DOMAIN_SYS,
> + .bo_type = ttm_bo_type_sg,
> + .size = attach->dmabuf->size,
> + .pin = false,
> + .resv = attach->dmabuf->resv,
> + .sg = table,
> +
> + };
> +
> + dma_resv_lock(params.resv, NULL);
> +
> + ret = vmw_bo_create(dev_priv, ¶ms, &vbo);
> + if (ret != 0)
> + goto out_no_bo;
> +
> + vbo->tbo.base.funcs = &vmw_gem_object_funcs;
> +
> + gem = &vbo->tbo.base;
> +out_no_bo:
> + dma_resv_unlock(params.resv);
> + return gem;
> +}
>
> int vmw_gem_object_create_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
> index 2d72a5ee7c0c..c99cad444991 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
> @@ -75,8 +75,12 @@ int vmw_prime_fd_to_handle(struct drm_device *dev,
> int fd, u32 *handle)
> {
> struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
> + int ret = ttm_prime_fd_to_handle(tfile, fd, handle);
>
> - return ttm_prime_fd_to_handle(tfile, fd, handle);
> + if (ret)
> + ret = drm_gem_prime_fd_to_handle(dev, file_priv, fd, handle);
> +
> + return ret;
> }
>
> int vmw_prime_handle_to_fd(struct drm_device *dev,
> @@ -85,5 +89,12 @@ int vmw_prime_handle_to_fd(struct drm_device *dev,
> int *prime_fd)
> {
> struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
> - return ttm_prime_handle_to_fd(tfile, handle, flags, prime_fd);
> + int ret;
> +
> + if (handle > VMWGFX_NUM_MOB)
> + ret = ttm_prime_handle_to_fd(tfile, handle, flags, prime_fd);
> + else
> + ret = drm_gem_prime_handle_to_fd(dev, file_priv, handle, flags, prime_fd);
> +
> + return ret;
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 4d23d0a70bcb..621d98b376bb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -188,13 +188,18 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
> switch (dev_priv->map_mode) {
> case vmw_dma_map_bind:
> case vmw_dma_map_populate:
> - vsgt->sgt = &vmw_tt->sgt;
> - ret = sg_alloc_table_from_pages_segment(
> - &vmw_tt->sgt, vsgt->pages, vsgt->num_pages, 0,
> - (unsigned long)vsgt->num_pages << PAGE_SHIFT,
> - dma_get_max_seg_size(dev_priv->drm.dev), GFP_KERNEL);
> - if (ret)
> - goto out_sg_alloc_fail;
> + if (vmw_tt->dma_ttm.page_flags & TTM_TT_FLAG_EXTERNAL) {
> + vsgt->sgt = vmw_tt->dma_ttm.sg;
> + } else {
> + vsgt->sgt = &vmw_tt->sgt;
> + ret = sg_alloc_table_from_pages_segment(&vmw_tt->sgt,
> + vsgt->pages, vsgt->num_pages, 0,
> + (unsigned long)vsgt->num_pages << PAGE_SHIFT,
> + dma_get_max_seg_size(dev_priv->drm.dev),
> + GFP_KERNEL);
> + if (ret)
> + goto out_sg_alloc_fail;
> + }
>
> ret = vmw_ttm_map_for_dma(vmw_tt);
> if (unlikely(ret != 0))
> @@ -209,8 +214,9 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
> return 0;
>
> out_map_fail:
> - sg_free_table(vmw_tt->vsgt.sgt);
> - vmw_tt->vsgt.sgt = NULL;
> + drm_warn(&dev_priv->drm, "VSG table map failed!");
> + sg_free_table(vsgt->sgt);
> + vsgt->sgt = NULL;
> out_sg_alloc_fail:
> return ret;
> }
> @@ -356,15 +362,17 @@ static void vmw_ttm_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
> static int vmw_ttm_populate(struct ttm_device *bdev,
> struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
> {
> - int ret;
> + bool external = (ttm->page_flags & TTM_TT_FLAG_EXTERNAL) != 0;
>
> - /* TODO: maybe completely drop this ? */
> if (ttm_tt_is_populated(ttm))
> return 0;
>
> - ret = ttm_pool_alloc(&bdev->pool, ttm, ctx);
> + if (external && ttm->sg)
> + return drm_prime_sg_to_dma_addr_array(ttm->sg,
> + ttm->dma_address,
> + ttm->num_pages);
>
> - return ret;
> + return ttm_pool_alloc(&bdev->pool, ttm, ctx);
> }
>
> static void vmw_ttm_unpopulate(struct ttm_device *bdev,
> @@ -372,6 +380,10 @@ static void vmw_ttm_unpopulate(struct ttm_device *bdev,
> {
> struct vmw_ttm_tt *vmw_tt = container_of(ttm, struct vmw_ttm_tt,
> dma_ttm);
> + bool external = (ttm->page_flags & TTM_TT_FLAG_EXTERNAL) != 0;
> +
> + if (external)
> + return;
>
> vmw_ttm_unbind(bdev, ttm);
>
> @@ -390,6 +402,7 @@ static struct ttm_tt *vmw_ttm_tt_create(struct ttm_buffer_object *bo,
> {
> struct vmw_ttm_tt *vmw_be;
> int ret;
> + bool external = bo->type == ttm_bo_type_sg;
>
> vmw_be = kzalloc(sizeof(*vmw_be), GFP_KERNEL);
> if (!vmw_be)
> @@ -398,7 +411,10 @@ static struct ttm_tt *vmw_ttm_tt_create(struct ttm_buffer_object *bo,
> vmw_be->dev_priv = vmw_priv_from_ttm(bo->bdev);
> vmw_be->mob = NULL;
>
> - if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)
> + if (external)
> + page_flags |= TTM_TT_FLAG_EXTERNAL | TTM_TT_FLAG_EXTERNAL_MAPPABLE;
> +
> + if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent || external)
> ret = ttm_sg_tt_init(&vmw_be->dma_ttm, bo, page_flags,
> ttm_cached);
> else
> --
> 2.40.1
>
