| Message ID | 1386858989-1487-1-git-send-email-treding@nvidia.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 12, 2013 at 06:04:13PM -0800, Greg Kroah-Hartman wrote: > On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote: > > This is a simple test module that can be used to allocate, export and > > delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > > systems that lack a real second driver. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/base/Kconfig | 4 + > > drivers/base/Makefile | 1 + > > drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 313 insertions(+) > > create mode 100644 drivers/base/dma-buf-test.c > > > > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > > index e373671652b0..bed2abb9491b 100644 > > --- a/drivers/base/Kconfig > > +++ b/drivers/base/Kconfig > > @@ -200,6 +200,10 @@ config DMA_SHARED_BUFFER > > APIs extension; the file's descriptor can then be passed on to other > > driver. > > > > +config DMA_BUF_TEST > > + tristate "DMA-BUF test module" > > + depends on DMA_SHARED_BUFFER > > We need some good documentation here. I agree. The description should go into more details about what exactly this is meant to address. > > > +static struct miscdevice dmabuf_device = { > > + .minor = 128, > > Why did you pick this minor? Why not just make it dynamic? It seemed like minors are statically allocated for miscdevice and 128 seemed to be as good as any. The tentative plan was to go through the official way of having one allocated as explained in the comment in include/linux/miscdevice.h. Reading that comment again, there's MISC_DYNAMIC_MINOR, which I guess would be appropriate here. Chances are that if you want to test DMA-BUF you'll have a reasonably modern userspace that will create the device dynamically. Alternatively I guess I could instead turn this into a more full-fledged cdev and do the whole alloc_chrdev_region() dance. Although that will only allocate the major dynamically. I'm not aware of any function that just allocates a single major/minor pair completely dynamically. Is there one that you could point me to? Thierry
On Thu, Dec 12, 2013 at 06:02:58PM -0800, Greg Kroah-Hartman wrote: > On Thu, Dec 12, 2013 at 03:42:12PM +0100, Thierry Reding wrote: > > On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote: > > > This is a simple test module that can be used to allocate, export and > > > delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > > > systems that lack a real second driver. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/base/Kconfig | 4 + > > > drivers/base/Makefile | 1 + > > > drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 313 insertions(+) > > > create mode 100644 drivers/base/dma-buf-test.c > > > > And attached is a small test program that I've been using to test this > > new module. It can be built with: > > > > $ gcc -O2 -g -Wall -Werror -I/usr/include/libdrm -o dma-buf-test dma-buf-test.c -ldrm > > Please put this in the patch as well (scripts/tests/ ?) There is also samples/ which already contains various test programs of a similar nature. > Also fix up the uapi stuff for the ioctls. Will do. Thanks, Thierry
On Thu, Dec 12, 2013 at 11:30:23PM +0100, Daniel Vetter wrote: > On Thu, Dec 12, 2013 at 8:34 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote: > > On 12/12/2013 03:36 PM, Thierry Reding wrote: > >> > >> This is a simple test module that can be used to allocate, export and > >> delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > >> systems that lack a real second driver. > >> > >> > > > > Looks nice. I wonder whether this could be extended to create a "streaming" > > dma-buf from a user space mapping. That could be used as a generic way to > > implement streaming (user) buffer objects, rather than to add explicit > > support for those in, for example, TTM. > > Atm there's no way to get gpus to unbind their dma-buf mappings, so > their essentially pinned forever from first use on. Shouldn't this work by simply calling the GEM_CLOSE IOCTL on the handle returned by drmPrimeFDToHandle()? I mean that should drop the last reference on the GEM object and cause it to be cleaned up (which should include detaching the DMA-BUF). Thierry
On Thu, Dec 12, 2013 at 08:34:28PM +0100, Thomas Hellstrom wrote: > On 12/12/2013 03:36 PM, Thierry Reding wrote: > >This is a simple test module that can be used to allocate, export and > >delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > >systems that lack a real second driver. > > > > > > Looks nice. I wonder whether this could be extended to create a "streaming" > dma-buf from a user space mapping. That could be used as a generic way to > implement streaming (user) buffer objects, rather than to add explicit > support for those in, for example, TTM. I'm somewhat reluctant to beef this up needlessly to prevent it from being used for purposes other than testing. Thierry
On Sat, Dec 14, 2013 at 1:37 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, Dec 12, 2013 at 11:30:23PM +0100, Daniel Vetter wrote: >> On Thu, Dec 12, 2013 at 8:34 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote: >> > On 12/12/2013 03:36 PM, Thierry Reding wrote: >> >> >> >> This is a simple test module that can be used to allocate, export and >> >> delete DMA-BUF objects. It can be used to test DMA-BUF sharing in >> >> systems that lack a real second driver. >> >> >> >> >> > >> > Looks nice. I wonder whether this could be extended to create a "streaming" >> > dma-buf from a user space mapping. That could be used as a generic way to >> > implement streaming (user) buffer objects, rather than to add explicit >> > support for those in, for example, TTM. >> >> Atm there's no way to get gpus to unbind their dma-buf mappings, so >> their essentially pinned forever from first use on. > > Shouldn't this work by simply calling the GEM_CLOSE IOCTL on the handle > returned by drmPrimeFDToHandle()? I mean that should drop the last > reference on the GEM object and cause it to be cleaned up (which should > include detaching the DMA-BUF). Yeah, but that has the side-effect of also dropping the reference. And requires an explicit action from userspace. The entire point of kernel buffer managers is that this is all done transparently in the kernel and when low on memory some objects get swapped out behind userspaces back (and then pulled in again if needed). -Daniel
On Sat, Dec 14, 2013 at 01:16:21PM +0100, Thierry Reding wrote: > On Thu, Dec 12, 2013 at 06:04:13PM -0800, Greg Kroah-Hartman wrote: > > On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote: > > > This is a simple test module that can be used to allocate, export and > > > delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > > > systems that lack a real second driver. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/base/Kconfig | 4 + > > > drivers/base/Makefile | 1 + > > > drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 313 insertions(+) > > > create mode 100644 drivers/base/dma-buf-test.c > > > > > > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > > > index e373671652b0..bed2abb9491b 100644 > > > --- a/drivers/base/Kconfig > > > +++ b/drivers/base/Kconfig > > > @@ -200,6 +200,10 @@ config DMA_SHARED_BUFFER > > > APIs extension; the file's descriptor can then be passed on to other > > > driver. > > > > > > +config DMA_BUF_TEST > > > + tristate "DMA-BUF test module" > > > + depends on DMA_SHARED_BUFFER > > > > We need some good documentation here. > > I agree. The description should go into more details about what exactly > this is meant to address. > > > > > +static struct miscdevice dmabuf_device = { > > > + .minor = 128, > > > > Why did you pick this minor? Why not just make it dynamic? > > It seemed like minors are statically allocated for miscdevice and 128 > seemed to be as good as any. The tentative plan was to go through the > official way of having one allocated as explained in the comment in > include/linux/miscdevice.h. > > Reading that comment again, there's MISC_DYNAMIC_MINOR, which I guess > would be appropriate here. Chances are that if you want to test DMA-BUF > you'll have a reasonably modern userspace that will create the device > dynamically. > > Alternatively I guess I could instead turn this into a more full-fledged > cdev and do the whole alloc_chrdev_region() dance. Although that will > only allocate the major dynamically. > > I'm not aware of any function that just allocates a single major/minor > pair completely dynamically. Is there one that you could point me to? miscdev seems appropriate for me, I don't think we'll ever need a 2nd dma-buf node. After all the entire point of sharing is to have system-wide access, so having a 2nd dma-buf domain/thing doesn't look useful at all. -Daniel
On Sat, Dec 14, 2013 at 01:16:21PM +0100, Thierry Reding wrote: > On Thu, Dec 12, 2013 at 06:04:13PM -0800, Greg Kroah-Hartman wrote: > > On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote: > > > This is a simple test module that can be used to allocate, export and > > > delete DMA-BUF objects. It can be used to test DMA-BUF sharing in > > > systems that lack a real second driver. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/base/Kconfig | 4 + > > > drivers/base/Makefile | 1 + > > > drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 313 insertions(+) > > > create mode 100644 drivers/base/dma-buf-test.c > > > > > > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > > > index e373671652b0..bed2abb9491b 100644 > > > --- a/drivers/base/Kconfig > > > +++ b/drivers/base/Kconfig > > > @@ -200,6 +200,10 @@ config DMA_SHARED_BUFFER > > > APIs extension; the file's descriptor can then be passed on to other > > > driver. > > > > > > +config DMA_BUF_TEST > > > + tristate "DMA-BUF test module" > > > + depends on DMA_SHARED_BUFFER > > > > We need some good documentation here. > > I agree. The description should go into more details about what exactly > this is meant to address. > > > > > +static struct miscdevice dmabuf_device = { > > > + .minor = 128, > > > > Why did you pick this minor? Why not just make it dynamic? > > It seemed like minors are statically allocated for miscdevice and 128 > seemed to be as good as any. The tentative plan was to go through the > official way of having one allocated as explained in the comment in > include/linux/miscdevice.h. > > Reading that comment again, there's MISC_DYNAMIC_MINOR, which I guess > would be appropriate here. Chances are that if you want to test DMA-BUF > you'll have a reasonably modern userspace that will create the device > dynamically. > > Alternatively I guess I could instead turn this into a more full-fledged > cdev and do the whole alloc_chrdev_region() dance. Although that will > only allocate the major dynamically. No, just use a dynamic misc device and all will be fine. Bonus is that you can create multiple misc devices if you ever really need to in the future, with no need to change much code at all (i.e. no chrdev crud.) > I'm not aware of any function that just allocates a single major/minor > pair completely dynamically. Is there one that you could point me to? Nope, that's what misc devices are for :) greg k-h
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index e373671652b0..bed2abb9491b 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -200,6 +200,10 @@ config DMA_SHARED_BUFFER APIs extension; the file's descriptor can then be passed on to other driver. +config DMA_BUF_TEST + tristate "DMA-BUF test module" + depends on DMA_SHARED_BUFFER + config DMA_CMA bool "DMA Contiguous Memory Allocator" depends on HAVE_DMA_CONTIGUOUS && CMA diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 94e8a80e87f8..cad983b6626f 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -25,3 +25,4 @@ obj-$(CONFIG_PINCTRL) += pinctrl.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG +obj-$(CONFIG_DMA_BUF_TEST) += dma-buf-test.o diff --git a/drivers/base/dma-buf-test.c b/drivers/base/dma-buf-test.c new file mode 100644 index 000000000000..f5498b74a09b --- /dev/null +++ b/drivers/base/dma-buf-test.c @@ -0,0 +1,308 @@ +#include <linux/dma-buf.h> +#include <linux/err.h> +#include <linux/fs.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +struct dmabuf_create { + __u32 flags; + __u32 size; +}; + +#define DMABUF_IOCTL_BASE 'D' +#define DMABUF_IOCTL_CREATE _IOWR(DMABUF_IOCTL_BASE, 0, struct dmabuf_create) +#define DMABUF_IOCTL_DELETE _IOWR(DMABUF_IOCTL_BASE, 1, int) +#define DMABUF_IOCTL_EXPORT _IOWR(DMABUF_IOCTL_BASE, 2, int) + +struct dmabuf_file { + struct dma_buf *buf; + dma_addr_t phys; + size_t size; + void *virt; +}; + +static int dmabuf_attach(struct dma_buf *buf, struct device *dev, + struct dma_buf_attachment *attach) +{ + return 0; +} + +static void dmabuf_detach(struct dma_buf *buf, + struct dma_buf_attachment *attach) +{ +} + +static struct sg_table *dmabuf_map_dma_buf(struct dma_buf_attachment *attach, + enum dma_data_direction dir) +{ + struct dmabuf_file *priv = attach->dmabuf->priv; + struct sg_table *sgt; + + sgt = kmalloc(sizeof(*sgt), GFP_KERNEL); + if (!sgt) + return NULL; + + if (sg_alloc_table(sgt, 1, GFP_KERNEL)) { + kfree(sgt); + return NULL; + } + + sg_dma_address(sgt->sgl) = priv->phys; + sg_dma_len(sgt->sgl) = priv->size; + + return sgt; +} + +static void dmabuf_unmap_dma_buf(struct dma_buf_attachment *attach, + struct sg_table *sgt, + enum dma_data_direction dir) +{ + sg_free_table(sgt); + kfree(sgt); +} + +static void dmabuf_release(struct dma_buf *buf) +{ +} + +static int dmabuf_begin_cpu_access(struct dma_buf *buf, size_t size, + size_t length, + enum dma_data_direction direction) +{ + return 0; +} + +static void dmabuf_end_cpu_access(struct dma_buf *buf, size_t size, + size_t length, + enum dma_data_direction direction) +{ +} + +static void *dmabuf_kmap_atomic(struct dma_buf *buf, unsigned long page) +{ + return NULL; +} + +static void dmabuf_kunmap_atomic(struct dma_buf *buf, unsigned long page, + void *vaddr) +{ +} + +static void *dmabuf_kmap(struct dma_buf *buf, unsigned long page) +{ + return NULL; +} + +static void dmabuf_kunmap(struct dma_buf *buf, unsigned long page, void *vaddr) +{ +} + +static void dmabuf_vm_open(struct vm_area_struct *vma) +{ +} + +static void dmabuf_vm_close(struct vm_area_struct *vma) +{ +} + +static int dmabuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) +{ + return 0; +} + +static const struct vm_operations_struct dmabuf_vm_ops = { + .open = dmabuf_vm_open, + .close = dmabuf_vm_close, + .fault = dmabuf_vm_fault, +}; + +static int dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma) +{ + pgprot_t prot = vm_get_page_prot(vma->vm_flags); + struct dmabuf_file *priv = buf->priv; + + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP; + vma->vm_ops = &dmabuf_vm_ops; + vma->vm_private_data = priv; + vma->vm_page_prot = pgprot_writecombine(prot); + + return remap_pfn_range(vma, vma->vm_start, priv->phys >> PAGE_SHIFT, + vma->vm_end - vma->vm_start, vma->vm_page_prot); +} + +static void *dmabuf_vmap(struct dma_buf *buf) +{ + return NULL; +} + +static void dmabuf_vunmap(struct dma_buf *buf, void *vaddr) +{ +} + +static const struct dma_buf_ops dmabuf_ops = { + .attach = dmabuf_attach, + .detach = dmabuf_detach, + .map_dma_buf = dmabuf_map_dma_buf, + .unmap_dma_buf = dmabuf_unmap_dma_buf, + .release = dmabuf_release, + .begin_cpu_access = dmabuf_begin_cpu_access, + .end_cpu_access = dmabuf_end_cpu_access, + .kmap_atomic = dmabuf_kmap_atomic, + .kunmap_atomic = dmabuf_kunmap_atomic, + .kmap = dmabuf_kmap, + .kunmap = dmabuf_kunmap, + .mmap = dmabuf_mmap, + .vmap = dmabuf_vmap, + .vunmap = dmabuf_vunmap, +}; + +static int dmabuf_file_open(struct inode *inode, struct file *file) +{ + struct dmabuf_file *priv; + int ret = 0; + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + file->private_data = priv; + + return ret; +} + +static int dmabuf_file_release(struct inode *inode, struct file *file) +{ + struct dmabuf_file *priv = file->private_data; + int ret = 0; + + if (priv->virt) + dma_free_writecombine(NULL, priv->size, priv->virt, priv->phys); + + if (priv->buf) + dma_buf_put(priv->buf); + + kfree(priv); + + return ret; +} + +static int dmabuf_ioctl_create(struct dmabuf_file *priv, const void __user *data) +{ + struct dmabuf_create args; + int ret = 0; + + if (priv->buf || priv->virt) + return -EBUSY; + + if (copy_from_user(&args, data, sizeof(args))) + return -EFAULT; + + priv->virt = dma_alloc_writecombine(NULL, args.size, &priv->phys, + GFP_KERNEL | __GFP_NOWARN); + if (!priv->virt) + return -ENOMEM; + + priv->buf = dma_buf_export(priv, &dmabuf_ops, args.size, args.flags); + if (!priv->buf) { + ret = -ENOMEM; + goto free; + } + + if (IS_ERR(priv->buf)) { + ret = PTR_ERR(priv->buf); + goto free; + } + + priv->size = args.size; + + return 0; + +free: + dma_free_writecombine(NULL, priv->size, priv->virt, priv->phys); + priv->virt = NULL; + return ret; +} + +static int dmabuf_ioctl_delete(struct dmabuf_file *priv, unsigned long flags) +{ + dma_free_writecombine(NULL, priv->size, priv->virt, priv->phys); + priv->virt = NULL; + priv->phys = 0; + priv->size = 0; + + dma_buf_put(priv->buf); + priv->buf = NULL; + + return 0; +} + +static int dmabuf_ioctl_export(struct dmabuf_file *priv, unsigned long flags) +{ + int err; + + get_dma_buf(priv->buf); + + err = dma_buf_fd(priv->buf, flags); + if (err < 0) + dma_buf_put(priv->buf); + + return err; +} + +static long dmabuf_file_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct dmabuf_file *priv = file->private_data; + long ret = 0; + + switch (cmd) { + case DMABUF_IOCTL_CREATE: + ret = dmabuf_ioctl_create(priv, (const void __user *)arg); + break; + + case DMABUF_IOCTL_DELETE: + ret = dmabuf_ioctl_delete(priv, arg); + break; + + case DMABUF_IOCTL_EXPORT: + ret = dmabuf_ioctl_export(priv, arg); + break; + + default: + ret = -ENOTTY; + break; + } + + return ret; +} + +static const struct file_operations dmabuf_fops = { + .owner = THIS_MODULE, + .open = dmabuf_file_open, + .release = dmabuf_file_release, + .unlocked_ioctl = dmabuf_file_ioctl, +}; + +static struct miscdevice dmabuf_device = { + .minor = 128, + .name = "dmabuf", + .fops = &dmabuf_fops, +}; + +static int __init dmabuf_init(void) +{ + return misc_register(&dmabuf_device); +} +module_init(dmabuf_init); + +static void __exit dmabuf_exit(void) +{ + misc_deregister(&dmabuf_device); +} +module_exit(dmabuf_exit); + +MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); +MODULE_DESCRIPTION("DMA-BUF test driver"); +MODULE_LICENSE("GPL v2");
This is a simple test module that can be used to allocate, export and delete DMA-BUF objects. It can be used to test DMA-BUF sharing in systems that lack a real second driver. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/base/Kconfig | 4 + drivers/base/Makefile | 1 + drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 313 insertions(+) create mode 100644 drivers/base/dma-buf-test.c