Re: [PATCH 3/4] drm/exynos: added userptr feature.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 23, 2012 at 7:43 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
> this feature could be used to use memory region allocated by malloc() in user
> mode and mmaped memory region allocated by other memory allocators. userptr
> interface can identify memory type through vm_flags value and would get
> pages or page frame numbers to user space appropriately.

I apologize for being a little late to jump in on this thread, but...

I must confess to not being a huge fan of userptr.  It really is
opening a can of worms, and seems best avoided if at all possible.
I'm not entirely sure the use-case for which you require this, but I
wonder if there isn't an alternative way?   I mean, the main case I
could think of for mapping userspace memory would be something like
texture upload.  But could that be handled in an alternative way,
something like a pwrite or texture_upload API, which could temporarily
pin the userspace memory, kick off a dma from that user buffer to a
proper GEM buffer, and then unpin the user buffer when the DMA
completes?

BR,
-R

> Signed-off-by: Inki Dae <inki.dae@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |    2 +
>  drivers/gpu/drm/exynos/exynos_drm_gem.c |  258 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_gem.h |   13 ++-
>  include/drm/exynos_drm.h                |   25 +++-
>  4 files changed, 296 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index f58a487..5bb0361 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -211,6 +211,8 @@ static struct drm_ioctl_desc exynos_ioctls[] = {
>                        DRM_AUTH),
>        DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MMAP,
>                        exynos_drm_gem_mmap_ioctl, DRM_UNLOCKED | DRM_AUTH),
> +       DRM_IOCTL_DEF_DRV(EXYNOS_GEM_USERPTR,
> +                       exynos_drm_gem_userptr_ioctl, DRM_UNLOCKED),
>        DRM_IOCTL_DEF_DRV(EXYNOS_PLANE_SET_ZPOS, exynos_plane_set_zpos_ioctl,
>                        DRM_UNLOCKED | DRM_AUTH),
>        DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index afd0cd4..b68d4ea 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -66,6 +66,43 @@ static int check_gem_flags(unsigned int flags)
>        return 0;
>  }
>
> +static struct vm_area_struct *get_vma(struct vm_area_struct *vma)
> +{
> +       struct vm_area_struct *vma_copy;
> +
> +       vma_copy = kmalloc(sizeof(*vma_copy), GFP_KERNEL);
> +       if (!vma_copy)
> +               return NULL;
> +
> +       if (vma->vm_ops && vma->vm_ops->open)
> +               vma->vm_ops->open(vma);
> +
> +       if (vma->vm_file)
> +               get_file(vma->vm_file);
> +
> +       memcpy(vma_copy, vma, sizeof(*vma));
> +
> +       vma_copy->vm_mm = NULL;
> +       vma_copy->vm_next = NULL;
> +       vma_copy->vm_prev = NULL;
> +
> +       return vma_copy;
> +}
> +
> +static void put_vma(struct vm_area_struct *vma)
> +{
> +       if (!vma)
> +               return;
> +
> +       if (vma->vm_ops && vma->vm_ops->close)
> +               vma->vm_ops->close(vma);
> +
> +       if (vma->vm_file)
> +               fput(vma->vm_file);
> +
> +       kfree(vma);
> +}
> +
>  static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
>                                        struct vm_area_struct *vma)
>  {
> @@ -254,6 +291,41 @@ static void exynos_drm_gem_put_pages(struct drm_gem_object *obj)
>        /* add some codes for UNCACHED type here. TODO */
>  }
>
> +static void exynos_drm_put_userptr(struct drm_gem_object *obj)
> +{
> +       struct exynos_drm_gem_obj *exynos_gem_obj;
> +       struct exynos_drm_gem_buf *buf;
> +       struct vm_area_struct *vma;
> +       int npages;
> +
> +       exynos_gem_obj = to_exynos_gem_obj(obj);
> +       buf = exynos_gem_obj->buffer;
> +       vma = exynos_gem_obj->vma;
> +
> +       if (vma && (vma->vm_flags & VM_PFNMAP) && (vma->vm_pgoff)) {
> +               put_vma(exynos_gem_obj->vma);
> +               goto out;
> +       }
> +
> +       npages = buf->size >> PAGE_SHIFT;
> +
> +       npages--;
> +       while (npages >= 0) {
> +               if (buf->write)
> +                       set_page_dirty_lock(buf->pages[npages]);
> +
> +               put_page(buf->pages[npages]);
> +               npages--;
> +       }
> +
> +out:
> +       kfree(buf->pages);
> +       buf->pages = NULL;
> +
> +       kfree(buf->sgt);
> +       buf->sgt = NULL;
> +}
> +
>  static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
>                                        struct drm_file *file_priv,
>                                        unsigned int *handle)
> @@ -293,6 +365,8 @@ void exynos_drm_gem_destroy(struct exynos_drm_gem_obj *exynos_gem_obj)
>
>        if (exynos_gem_obj->flags & EXYNOS_BO_NONCONTIG)
>                exynos_drm_gem_put_pages(obj);
> +       else if (exynos_gem_obj->flags & EXYNOS_BO_USERPTR)
> +               exynos_drm_put_userptr(obj);
>        else
>                exynos_drm_free_buf(obj->dev, exynos_gem_obj->flags, buf);
>
> @@ -606,6 +680,190 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
>        return 0;
>  }
>
> +static int exynos_drm_get_userptr(struct drm_device *dev,
> +                               struct exynos_drm_gem_obj *obj,
> +                               unsigned long userptr,
> +                               unsigned int write)
> +{
> +       unsigned int get_npages;
> +       unsigned long npages = 0;
> +       struct vm_area_struct *vma;
> +       struct exynos_drm_gem_buf *buf = obj->buffer;
> +
> +       vma = find_vma(current->mm, userptr);
> +
> +       /* the memory region mmaped with VM_PFNMAP. */
> +       if (vma && (vma->vm_flags & VM_PFNMAP) && (vma->vm_pgoff)) {
> +               unsigned long this_pfn, prev_pfn, pa;
> +               unsigned long start, end, offset;
> +               struct scatterlist *sgl;
> +
> +               start = userptr;
> +               offset = userptr & ~PAGE_MASK;
> +               end = start + buf->size;
> +               sgl = buf->sgt->sgl;
> +
> +               for (prev_pfn = 0; start < end; start += PAGE_SIZE) {
> +                       int ret = follow_pfn(vma, start, &this_pfn);
> +                       if (ret)
> +                               return ret;
> +
> +                       if (prev_pfn == 0)
> +                               pa = this_pfn << PAGE_SHIFT;
> +                       else if (this_pfn != prev_pfn + 1)
> +                               return -EFAULT;
> +
> +                       sg_dma_address(sgl) = (pa + offset);
> +                       sg_dma_len(sgl) = PAGE_SIZE;
> +                       prev_pfn = this_pfn;
> +                       pa += PAGE_SIZE;
> +                       npages++;
> +                       sgl = sg_next(sgl);
> +               }
> +
> +               buf->dma_addr = pa + offset;
> +
> +               obj->vma = get_vma(vma);
> +               if (!obj->vma)
> +                       return -ENOMEM;
> +
> +               buf->pfnmap = true;
> +
> +               return npages;
> +       }
> +
> +       buf->write = write;
> +       npages = buf->size >> PAGE_SHIFT;
> +
> +       down_read(&current->mm->mmap_sem);
> +       get_npages = get_user_pages(current, current->mm, userptr,
> +                                       npages, write, 1, buf->pages, NULL);
> +       up_read(&current->mm->mmap_sem);
> +       if (get_npages != npages)
> +               DRM_ERROR("failed to get user_pages.\n");
> +
> +       buf->pfnmap = false;
> +
> +       return get_npages;
> +}
> +
> +int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
> +                                     struct drm_file *file_priv)
> +{
> +       struct exynos_drm_gem_obj *exynos_gem_obj;
> +       struct drm_exynos_gem_userptr *args = data;
> +       struct exynos_drm_gem_buf *buf;
> +       struct scatterlist *sgl;
> +       unsigned long size, userptr;
> +       unsigned int npages;
> +       int ret, get_npages;
> +
> +       DRM_DEBUG_KMS("%s\n", __FILE__);
> +
> +       if (!args->size) {
> +               DRM_ERROR("invalid size.\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = check_gem_flags(args->flags);
> +       if (ret)
> +               return ret;
> +
> +       size = roundup_gem_size(args->size, EXYNOS_BO_USERPTR);
> +       userptr = args->userptr;
> +
> +       buf = exynos_drm_init_buf(dev, size);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       exynos_gem_obj = exynos_drm_gem_init(dev, size);
> +       if (!exynos_gem_obj) {
> +               ret = -ENOMEM;
> +               goto err_free_buffer;
> +       }
> +
> +       buf->sgt = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
> +       if (!buf->sgt) {
> +               DRM_ERROR("failed to allocate buf->sgt.\n");
> +               ret = -ENOMEM;
> +               goto err_release_gem;
> +       }
> +
> +       npages = size >> PAGE_SHIFT;
> +
> +       ret = sg_alloc_table(buf->sgt, npages, GFP_KERNEL);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to initailize sg table.\n");
> +               goto err_free_sgt;
> +       }
> +
> +       buf->pages = kzalloc(npages * sizeof(struct page *), GFP_KERNEL);
> +       if (!buf->pages) {
> +               DRM_ERROR("failed to allocate buf->pages\n");
> +               ret = -ENOMEM;
> +               goto err_free_table;
> +       }
> +
> +       exynos_gem_obj->buffer = buf;
> +
> +       get_npages = exynos_drm_get_userptr(dev, exynos_gem_obj, userptr, 1);
> +       if (get_npages != npages) {
> +               DRM_ERROR("failed to get user_pages.\n");
> +               ret = get_npages;
> +               goto err_release_userptr;
> +       }
> +
> +       ret = exynos_drm_gem_handle_create(&exynos_gem_obj->base, file_priv,
> +                                               &args->handle);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to create gem handle.\n");
> +               goto err_release_userptr;
> +       }
> +
> +       sgl = buf->sgt->sgl;
> +
> +       /*
> +        * if buf->pfnmap is true then update sgl of sgt with pages but
> +        * if buf->pfnmap is false then it means the sgl was updated already
> +        * so it doesn't need to update the sgl.
> +        */
> +       if (!buf->pfnmap) {
> +               unsigned int i = 0;
> +
> +               /* set all pages to sg list. */
> +               while (i < npages) {
> +                       sg_set_page(sgl, buf->pages[i], PAGE_SIZE, 0);
> +                       sg_dma_address(sgl) = page_to_phys(buf->pages[i]);
> +                       i++;
> +                       sgl = sg_next(sgl);
> +               }
> +       }
> +
> +       /* always use EXYNOS_BO_USERPTR as memory type for userptr. */
> +       exynos_gem_obj->flags |= EXYNOS_BO_USERPTR;
> +
> +       return 0;
> +
> +err_release_userptr:
> +       get_npages--;
> +       while (get_npages >= 0)
> +               put_page(buf->pages[get_npages--]);
> +       kfree(buf->pages);
> +       buf->pages = NULL;
> +err_free_table:
> +       sg_free_table(buf->sgt);
> +err_free_sgt:
> +       kfree(buf->sgt);
> +       buf->sgt = NULL;
> +err_release_gem:
> +       drm_gem_object_release(&exynos_gem_obj->base);
> +       kfree(exynos_gem_obj);
> +       exynos_gem_obj = NULL;
> +err_free_buffer:
> +       exynos_drm_free_buf(dev, 0, buf);
> +       return ret;
> +}
> +
>  int exynos_drm_gem_init_object(struct drm_gem_object *obj)
>  {
>        DRM_DEBUG_KMS("%s\n", __FILE__);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index efc8252..1de2241 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -29,7 +29,8 @@
>  #define to_exynos_gem_obj(x)   container_of(x,\
>                        struct exynos_drm_gem_obj, base)
>
> -#define IS_NONCONTIG_BUFFER(f)         (f & EXYNOS_BO_NONCONTIG)
> +#define IS_NONCONTIG_BUFFER(f)         ((f & EXYNOS_BO_NONCONTIG) ||\
> +                                       (f & EXYNOS_BO_USERPTR))
>
>  /*
>  * exynos drm gem buffer structure.
> @@ -38,18 +39,23 @@
>  * @dma_addr: bus address(accessed by dma) to allocated memory region.
>  *     - this address could be physical address without IOMMU and
>  *     device address with IOMMU.
> + * @write: whether pages will be written to by the caller.
>  * @sgt: sg table to transfer page data.
>  * @pages: contain all pages to allocated memory region.
>  * @page_size: could be 4K, 64K or 1MB.
>  * @size: size of allocated memory region.
> + * @pfnmap: indicate whether memory region from userptr is mmaped with
> + *     VM_PFNMAP or not.
>  */
>  struct exynos_drm_gem_buf {
>        void __iomem            *kvaddr;
>        dma_addr_t              dma_addr;
> +       unsigned int            write;
>        struct sg_table         *sgt;
>        struct page             **pages;
>        unsigned long           page_size;
>        unsigned long           size;
> +       bool                    pfnmap;
>  };
>
>  /*
> @@ -73,6 +79,7 @@ struct exynos_drm_gem_obj {
>        struct drm_gem_object           base;
>        struct exynos_drm_gem_buf       *buffer;
>        unsigned long                   size;
> +       struct vm_area_struct           *vma;
>        unsigned int                    flags;
>  };
>
> @@ -127,6 +134,10 @@ int exynos_drm_gem_map_offset_ioctl(struct drm_device *dev, void *data,
>  int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data,
>                              struct drm_file *file_priv);
>
> +/* map user space allocated by malloc to pages. */
> +int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
> +                                     struct drm_file *file_priv);
> +
>  /* initialize gem object. */
>  int exynos_drm_gem_init_object(struct drm_gem_object *obj);
>
> diff --git a/include/drm/exynos_drm.h b/include/drm/exynos_drm.h
> index 2d6eb06..48eda6e 100644
> --- a/include/drm/exynos_drm.h
> +++ b/include/drm/exynos_drm.h
> @@ -75,6 +75,23 @@ struct drm_exynos_gem_mmap {
>  };
>
>  /**
> + * User-requested user space importing structure
> + *
> + * @userptr: user space address allocated by malloc.
> + * @size: size to the buffer allocated by malloc.
> + * @flags: indicate user-desired cache attribute to map the allocated buffer
> + *     to kernel space.
> + * @handle: a returned handle to created gem object.
> + *     - this handle will be set by gem module of kernel side.
> + */
> +struct drm_exynos_gem_userptr {
> +       uint64_t userptr;
> +       uint64_t size;
> +       unsigned int flags;
> +       unsigned int handle;
> +};
> +
> +/**
>  * A structure for user connection request of virtual display.
>  *
>  * @connection: indicate whether doing connetion or not by user.
> @@ -105,13 +122,16 @@ enum e_drm_exynos_gem_mem_type {
>        EXYNOS_BO_CACHABLE      = 1 << 1,
>        /* write-combine mapping. */
>        EXYNOS_BO_WC            = 1 << 2,
> +       /* user space memory allocated by malloc. */
> +       EXYNOS_BO_USERPTR       = 1 << 3,
>        EXYNOS_BO_MASK          = EXYNOS_BO_NONCONTIG | EXYNOS_BO_CACHABLE |
> -                                       EXYNOS_BO_WC
> +                                       EXYNOS_BO_WC | EXYNOS_BO_USERPTR
>  };
>
>  #define DRM_EXYNOS_GEM_CREATE          0x00
>  #define DRM_EXYNOS_GEM_MAP_OFFSET      0x01
>  #define DRM_EXYNOS_GEM_MMAP            0x02
> +#define DRM_EXYNOS_GEM_USERPTR         0x03
>  /* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
>  #define DRM_EXYNOS_PLANE_SET_ZPOS      0x06
>  #define DRM_EXYNOS_VIDI_CONNECTION     0x07
> @@ -125,6 +145,9 @@ enum e_drm_exynos_gem_mem_type {
>  #define DRM_IOCTL_EXYNOS_GEM_MMAP      DRM_IOWR(DRM_COMMAND_BASE + \
>                DRM_EXYNOS_GEM_MMAP, struct drm_exynos_gem_mmap)
>
> +#define DRM_IOCTL_EXYNOS_GEM_USERPTR   DRM_IOWR(DRM_COMMAND_BASE + \
> +               DRM_EXYNOS_GEM_USERPTR, struct drm_exynos_gem_userptr)
> +
>  #define DRM_IOCTL_EXYNOS_PLANE_SET_ZPOS        DRM_IOWR(DRM_COMMAND_BASE + \
>                DRM_EXYNOS_PLANE_SET_ZPOS, struct drm_exynos_plane_set_zpos)
>
> --
> 1.7.4.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux