Typo, excepted -> expected

-----Original Message-----
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of Zhou, 
David(ChunMing)
Sent: Tuesday, July 31, 2018 9:41 AM
To: Koenig, Christian <christian.koe...@amd.com>; Zhang, Jerry 
<jerry.zh...@amd.com>; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of 
bo (v3)

Thanks for Jerry  still  remembers  this series.

Hi Christian,

For upstream of this feature, seems we already had agreement long time ago. Two 
reasons for upstreaming:
1. this bug was found by an opengl game, so this bug also is in mesa driver in 
theory.
2. after upstream these patches, we can reduce pro specific patches, and close 
to open source.

Btw, an unit test is excepted when upstreaming, I remember Alex mentioned.

Thanks,
David Zhou
-----Original Message-----
From: Christian König <ckoenig.leichtzumer...@gmail.com>
Sent: Monday, July 30, 2018 6:48 PM
To: Zhang, Jerry <jerry.zh...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Zhou, David(ChunMing) <david1.z...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of 
bo (v3)

Am 30.07.2018 um 12:02 schrieb Junwei Zhang:
> From: Chunming Zhou <david1.z...@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.z...@amd.com>
> Signed-off-by: Junwei Zhang <jerry.zh...@amd.com> (v3)
> Reviewed-by: Christian König <christian.koe...@amd.com>
> Reviewed-by: Jammy Zhou <jammy.z...@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to