Re: [PATCH 5/5] drm/amdgpu: implement amdgpu_gem_prime_move_notify

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 29, 2019 at 11:40:49AM +0100, Christian König wrote:
> Implement the importer side of unpinned DMA-buf handling.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 28 ++++++++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 +++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index 3629cfe53aad..af39553c51ad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -456,7 +456,33 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
>  	return ERR_PTR(ret);
>  }
>  
> +/**
> + * amdgpu_dma_buf_move_notify - &attach.move_notify implementation
> + *
> + * @attach: the DMA-buf attachment
> + *
> + * Invalidate the DMA-buf attachment, making sure that the we re-create the
> + * mapping before the next use.
> + */
> +static void
> +amdgpu_dma_buf_move_notify(struct dma_buf_attachment *attach)
> +{
> +	struct ttm_operation_ctx ctx = { false, false };
> +	struct drm_gem_object *obj = attach->importer_priv;
> +	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> +	struct ttm_placement placement = {};
> +	int r;
> +
> +	if (bo->tbo.mem.mem_type == TTM_PL_SYSTEM)
> +		return;
> +
> +	r = ttm_bo_validate(&bo->tbo, &placement, &ctx);
> +	if (r)
> +		DRM_ERROR("Failed to invalidate DMA-buf import (%d))\n", r);

Where do you update pagetables?

The only thing I've found is in the amdgpu CS code, which is way to late
for this stuff here. Plus TTM doesn't handle virtual memory at all (aside
from the gart tt), so clearly you need to call into amdgpu code somewhere
for this. But I didn't find it, neither in your ->move_notify nor the
->move callback in ttm_bo_driver.

How does this work?
-Daniel

> +}
> +
>  static const struct dma_buf_attach_ops amdgpu_dma_buf_attach_ops = {
> +	.move_notify = amdgpu_dma_buf_move_notify
>  };
>  
>  /**
> @@ -492,7 +518,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
>  		return obj;
>  
>  	attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
> -					&amdgpu_dma_buf_attach_ops, NULL);
> +					&amdgpu_dma_buf_attach_ops, obj);
>  	if (IS_ERR(attach)) {
>  		drm_gem_object_put(obj);
>  		return ERR_CAST(attach);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ac776d2620eb..cfa46341c9a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -861,6 +861,9 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>  		return 0;
>  	}
>  
> +	if (bo->tbo.base.import_attach)
> +		dma_buf_pin(bo->tbo.base.import_attach);
> +
>  	bo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
>  	/* force to pin into visible video ram */
>  	if (!(bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS))
> @@ -944,6 +947,9 @@ int amdgpu_bo_unpin(struct amdgpu_bo *bo)
>  
>  	amdgpu_bo_subtract_pin_size(bo);
>  
> +	if (bo->tbo.base.import_attach)
> +		dma_buf_unpin(bo->tbo.base.import_attach);
> +
>  	for (i = 0; i < bo->placement.num_placement; i++) {
>  		bo->placements[i].lpfn = 0;
>  		bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel