On 07/31/2018 09:49 AM, Zhou, David(ChunMing) wrote:
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.

Thanks for explanation.


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

Yes, I will.


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.

We may have to replace drm_gem_object_reference() with drm_gem_object_get().

On 2nd thought, do we really need to do reference every time?
if UMD find the same gem object for 3 times, it also need to explicitly 
free(put) that object for 3 times?



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

If the bo is not import/export, DMA-buf will be NULL always.

Regards,
Jerry


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