Re: [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo (v3)

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 30.07.2018 um 12:02 schrieb Junwei Zhang:
> From: Chunming Zhou <David1.Zhou@amd.com>
>
> v2: get original gem handle from gobj
> v3: update find bo data structure as union(in, out)
>      simply some code logic

Do we now have an open source user for this, so that we can upstream it? 
One more point below.

>
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> (v3)
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 63 +++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  3 +-
>   include/uapi/drm/amdgpu_drm.h           | 21 +++++++++++
>   4 files changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4cd20e7..46c370b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1213,6 +1213,8 @@ int amdgpu_gem_info_ioctl(struct drm_device *dev, void *data,
>   			  struct drm_file *filp);
>   int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>   			struct drm_file *filp);
> +int amdgpu_gem_find_bo_by_cpu_mapping_ioctl(struct drm_device *dev, void *data,
> +					    struct drm_file *filp);
>   int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   			  struct drm_file *filp);
>   int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 71792d8..bae8417 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -288,6 +288,69 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   	return 0;
>   }
>   
> +static int amdgpu_gem_get_handle_from_object(struct drm_file *filp,
> +					     struct drm_gem_object *obj)
> +{
> +	int i;
> +	struct drm_gem_object *tmp;
> +
> +	spin_lock(&filp->table_lock);
> +	idr_for_each_entry(&filp->object_idr, tmp, i) {
> +		if (obj == tmp) {
> +			drm_gem_object_reference(obj);
> +			spin_unlock(&filp->table_lock);
> +			return i;
> +		}
> +	}

Please double check if that is still up to date.

I think we could as well try to use the DMA-buf handle tree for that.

Christian.

> +	spin_unlock(&filp->table_lock);
> +
> +	return 0;
> +}
> +
> +int amdgpu_gem_find_bo_by_cpu_mapping_ioctl(struct drm_device *dev, void *data,
> +					    struct drm_file *filp)
> +{
> +	union drm_amdgpu_gem_find_bo *args = data;
> +	struct drm_gem_object *gobj;
> +	struct amdgpu_bo *bo;
> +	struct ttm_buffer_object *tbo;
> +	struct vm_area_struct *vma;
> +	uint32_t handle;
> +	int r;
> +
> +	if (offset_in_page(args->in.addr | args->in.size))
> +		return -EINVAL;
> +
> +	down_read(&current->mm->mmap_sem);
> +	vma = find_vma(current->mm, args->in.addr);
> +	if (!vma || vma->vm_file != filp->filp ||
> +	    (args->in.size > (vma->vm_end - args->in.addr))) {
> +		args->out.handle = 0;
> +		up_read(&current->mm->mmap_sem);
> +		return -EINVAL;
> +	}
> +	args->out.offset = args->in.addr - vma->vm_start;
> +
> +	tbo = vma->vm_private_data;
> +	bo = container_of(tbo, struct amdgpu_bo, tbo);
> +	amdgpu_bo_ref(bo);
> +	gobj = &bo->gem_base;
> +
> +	handle = amdgpu_gem_get_handle_from_object(filp, gobj);
> +	if (!handle) {
> +		r = drm_gem_handle_create(filp, gobj, &handle);
> +		if (r) {
> +			DRM_ERROR("create gem handle failed\n");
> +			up_read(&current->mm->mmap_sem);
> +			return r;
> +		}
> +	}
> +	args->out.handle = handle;
> +	up_read(&current->mm->mmap_sem);
> +
> +	return 0;
> +}
> +
>   int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>   			     struct drm_file *filp)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 7956848..1bd2cc1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1100,7 +1100,8 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
>   	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_VA, amdgpu_gem_va_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
>   	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
>   	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> -	DRM_IOCTL_DEF_DRV(AMDGPU_FREESYNC, amdgpu_display_freesync_ioctl, DRM_MASTER)
> +	DRM_IOCTL_DEF_DRV(AMDGPU_FREESYNC, amdgpu_display_freesync_ioctl, DRM_MASTER),
> +	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_FIND_BO, amdgpu_gem_find_bo_by_cpu_mapping_ioctl, DRM_AUTH|DRM_RENDER_ALLOW)
>   };
>   const int amdgpu_max_kms_ioctl = ARRAY_SIZE(amdgpu_ioctls_kms);
>   
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 94444ee..000c415 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -54,6 +54,7 @@
>   #define DRM_AMDGPU_VM			0x13
>   #define DRM_AMDGPU_FENCE_TO_HANDLE	0x14
>   #define DRM_AMDGPU_SCHED		0x15
> +#define DRM_AMDGPU_GEM_FIND_BO		0x16
>   /* not upstream */
>   #define DRM_AMDGPU_FREESYNC	        0x5d
>   
> @@ -74,6 +75,7 @@
>   #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
>   #define DRM_IOCTL_AMDGPU_SCHED		DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
>   #define DRM_IOCTL_AMDGPU_FREESYNC	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FREESYNC, struct drm_amdgpu_freesync)
> +#define DRM_IOCTL_AMDGPU_GEM_FIND_BO	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_FIND_BO, union drm_amdgpu_gem_find_bo)
>   
>   /**
>    * DOC: memory domains
> @@ -392,6 +394,25 @@ struct drm_amdgpu_gem_wait_idle_out {
>   	struct drm_amdgpu_gem_wait_idle_out out;
>   };
>   
> +struct drm_amdgpu_gem_find_bo_in {
> +	/* CPU address */
> +	__u64			addr;
> +	/* memory size from CPU address */
> +	__u64			size;
> +};
> +
> +struct drm_amdgpu_gem_find_bo_out {
> +	/* offset in bo */
> +	__u64			offset;
> +	/* resulting GEM handle */
> +	__u32			handle;
> +};
> +
> +union drm_amdgpu_gem_find_bo {
> +	struct drm_amdgpu_gem_find_bo_in in;
> +	struct drm_amdgpu_gem_find_bo_out out;
> +};
> +
>   struct drm_amdgpu_wait_cs_in {
>   	/* Command submission handle
>            * handle equals 0 means none to wait for

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx