[RFC] dma-buf: Implement test module

[RFC] dma-buf: Implement test module

[Thierry Reding]

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
+
 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");
-- 
1.8.4.2

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 778 bytes --]

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

Thierry

[-- Attachment #1.2: dma-buf-test.c --]
[-- Type: text/x-c, Size: 8292 bytes --]

/*
 * Copyright (C) 2013 NVIDIA Corporation
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 as published by
 * the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
 * more details.
 *
 * You should have received a copy of the GNU General Public License along with
 * this program.  If not, see <http://www.gnu.org/licenses/>.
 */

#include <errno.h>
#include <fcntl.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

#include <sys/ioctl.h>
#include <sys/mman.h>

#include <linux/ioctl.h>

#include <xf86drm.h>
#include <xf86drmMode.h>
#include <drm_fourcc.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)

#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

static const char default_dmabuf_device[] = "/dev/dmabuf";
static const char default_dri_card[] = "/dev/dri/card0";

struct screen {
	int fd;

	unsigned int width;
	unsigned int height;
	unsigned int pitch;
	unsigned int depth;
	unsigned int bpp;

	drmModeModeInfo mode;
	uint32_t fb, old_fb;
	uint32_t connector;
	uint32_t format;
	uint32_t handle;
	uint32_t crtc;
};

static int probe_connector(struct screen *screen, drmModeConnectorPtr connector)
{
	drmModeEncoderPtr encoder;
	drmModeCrtcPtr crtc;
	drmModeFBPtr fb;

	encoder = drmModeGetEncoder(screen->fd, connector->encoder_id);
	if (!encoder)
		return -ENODEV;

	crtc = drmModeGetCrtc(screen->fd, encoder->crtc_id);
	if (!crtc) {
		drmModeFreeEncoder(encoder);
		return -ENODEV;
	}

	screen->old_fb = crtc->buffer_id;

	fb = drmModeGetFB(screen->fd, crtc->buffer_id);
	if (!fb) {
		/* TODO: create new framebuffer */
		drmModeFreeEncoder(encoder);
		drmModeFreeCrtc(crtc);
		return -ENOSYS;
	}

	screen->connector = connector->connector_id;
	screen->old_fb = crtc->buffer_id;
	screen->crtc = encoder->crtc_id;
	/* TODO: check crtc->mode_valid */
	screen->mode = crtc->mode;

	screen->width = fb->width;
	screen->height = fb->height;
	screen->pitch = fb->pitch;
	screen->depth = fb->depth;
	screen->bpp = fb->bpp;

	drmModeFreeEncoder(encoder);
	drmModeFreeCrtc(crtc);
	drmModeFreeFB(fb);

	return 0;
}

static int screen_open(struct screen **screenp, int fd)
{
	drmModeConnectorPtr connector;
	struct screen *screen;
	bool found = false;
	drmModeResPtr res;
	unsigned int i;
	int err;

	if (!screenp || fd < 0)
		return -EINVAL;

	screen = calloc(1, sizeof(*screen));
	if (!screen)
		return -ENOMEM;

	screen->format = DRM_FORMAT_XRGB8888;
	screen->fd = fd;

	res = drmModeGetResources(fd);
	if (!res) {
		free(screen);
		return -ENOMEM;
	}

	for (i = 0; i < res->count_connectors; i++) {
		connector = drmModeGetConnector(fd, res->connectors[i]);
		if (!connector)
			continue;

		if (connector->connection != DRM_MODE_CONNECTED) {
			drmModeFreeConnector(connector);
			continue;
		}

		err = probe_connector(screen, connector);
		if (err < 0) {
			drmModeFreeConnector(connector);
			continue;
		}

		found = true;
		break;
	}

	drmModeFreeResources(res);

	if (!found) {
		free(screen);
		return -ENODEV;
	}

	*screenp = screen;

	return 0;
}

int screen_close(struct screen *screen)
{
	struct drm_gem_close args;
	int err;

	err = drmModeSetCrtc(screen->fd, screen->crtc, screen->old_fb, 0, 0,
			     &screen->connector, 1, &screen->mode);
	if (err < 0) {
		fprintf(stderr, "drmModeSetCrtc() failed: %m\n");
		return -errno;
	}

	err = drmModeRmFB(screen->fd, screen->fb);
	if (err < 0) {
		fprintf(stderr, "drmModeRmFB() failed: %m\n");
		return -errno;
	}

	memset(&args, 0, sizeof(args));
	args.handle = screen->handle;

	err = ioctl(screen->fd, DRM_IOCTL_GEM_CLOSE, &args);
	if (err < 0) {
		fprintf(stderr, "ioctl(DRM_IOCTL_GEM_CLOSE) failed: %m\n");
		return -errno;
	}

	return 0;
}

int screen_import(struct screen *screen, int buf)
{
	uint32_t handles[4], pitches[4], offsets[4];
	int err;

	err = drmPrimeFDToHandle(screen->fd, buf, &screen->handle);
	if (err < 0) {
		fprintf(stderr, "drmPrimeFDToHandle() failed: %m\n");
		return -errno;
	}

	printf("DRM handle: %x\n", screen->handle);

	handles[0] = screen->handle;
	pitches[0] = screen->pitch;
	offsets[0] = 0;

	err = drmModeAddFB2(screen->fd, screen->width, screen->height,
			    screen->format, handles, pitches, offsets,
			    &screen->fb, 0);
	if (err < 0) {
		fprintf(stderr, "drmModeAddFB() failed: %m\n");
		return -errno;
	}

	err = drmModeSetCrtc(screen->fd, screen->crtc, screen->fb, 0, 0,
			     &screen->connector, 1, &screen->mode);
	if (err < 0) {
		fprintf(stderr, "drmModeSetCrtc() failed: %m\n");
		return -errno;
	}

	return 0;
}

int dmabuf_create(int fd, size_t size, int flags)
{
	struct dmabuf_create args;

	memset(&args, 0, sizeof(args));
	args.flags = flags;
	args.size = size;

	return ioctl(fd, DMABUF_IOCTL_CREATE, &args);
}

int dmabuf_delete(int fd)
{
	return ioctl(fd, DMABUF_IOCTL_DELETE, 0);
}

int dmabuf_export(int fd, int flags)
{
	return ioctl(fd, DMABUF_IOCTL_EXPORT, flags);
}

void *dmabuf_mmap(int buf, off_t offset, size_t size)
{
	void *ptr;

	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, buf, offset);
	if (ptr == MAP_FAILED)
		return NULL;

	return ptr;
}

void dmabuf_unmap(int buf, void *ptr, size_t size)
{
	munmap(ptr, size);
}

static void prepare_buffer(int buf, size_t size, struct screen *screen)
{
	void *ptr;

	/* TODO: handle other formats */
	if (screen->format != DRM_FORMAT_XRGB8888) {
		fprintf(stderr, "unsupported format: %x\n", screen->format);
		return;
	}

	ptr = dmabuf_mmap(buf, 0, size);
	if (ptr) {
		uint32_t colors[2] = { 0x00ff0000, 0x000000ff };
		unsigned int sx = 16;
		unsigned int sy = 16;
		uint32_t *fb = ptr;
		unsigned int i, j;

		for (j = 0; j < screen->height; j++) {
			unsigned int y = j / sy;

			for (i = 0; i < screen->width; i++) {
				unsigned int x = i / sx;

				*fb++ = colors[(x ^ y) & 1];
			}
		}

		dmabuf_unmap(buf, ptr, size);
	} else {
		fprintf(stderr, "dmabuf_mmap() failed: %m\n");
	}
}

int main(int argc, char *argv[])
{
	const char *dmabuf, *card;
	struct timeval timeout;
	struct screen *screen;
	int fd, err, buf, drm;
	size_t size;
	fd_set fds;

	if (argc < 2)
		dmabuf = default_dmabuf_device;
	else
		dmabuf = argv[1];

	if (argc < 3)
		card = default_dri_card;
	else
		card = argv[2];

	fd = open(dmabuf, O_RDWR);
	if (fd < 0) {
		fprintf(stderr, "open() failed: %m\n");
		return 1;
	}

	drm = open(card, O_RDWR);
	if (drm < 0) {
		fprintf(stderr, "open() failed: %m\n");
		return 1;
	}

	err = drmSetMaster(drm);
	if (err < 0) {
		fprintf(stderr, "drmSetMaster() failed: %m\n");
		return 1;
	}

	err = screen_open(&screen, drm);
	if (err < 0) {
		fprintf(stderr, "screen_open() failed: %s\n", strerror(-err));
		return 1;
	}

	size = screen->pitch * screen->height;

	err = dmabuf_create(fd, size, O_RDWR);
	if (err < 0) {
		fprintf(stderr, "dmabuf_create() failed: %m\n");
		return 1;
	}

	buf = dmabuf_export(fd, 0);
	if (buf < 0) {
		fprintf(stderr, "dmabuf_export() failed: %m\n");
		return 1;
	}

	printf("obtained DMA-BUF: %d\n", buf);

	prepare_buffer(buf, size, screen);

	err = screen_import(screen, buf);
	if (err < 0) {
		fprintf(stderr, "screen_import() failed: %s\n", strerror(-err));
		return 1;
	}

	FD_ZERO(&fds);
	FD_SET(STDIN_FILENO, &fds);

	memset(&timeout, 0, sizeof(timeout));
	timeout.tv_sec = 5;
	timeout.tv_usec = 0;

	err = select(STDIN_FILENO + 1, &fds, NULL, NULL, &timeout);
	if (err < 0) {
		fprintf(stderr, "select() failed: %m\n");
		return 1;
	}

	err = screen_close(screen);
	if (err < 0) {
		fprintf(stderr, "screen_close() failed: %s\n", strerror(-err));
		return 1;
	}

	err = dmabuf_delete(fd);
	if (err < 0) {
		fprintf(stderr, "dmabuf_delete() failed: %m\n");
		return 1;
	}

	close(buf);

	err = drmDropMaster(drm);
	if (err < 0) {
		fprintf(stderr, "drmDropMaster() failed: %m\n");
		return 1;
	}

	close(drm);
	close(fd);
	return 0;
}

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Daniel Vetter]

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
> +
>  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)

Shouldn't we put this into an uapi header somewhere? Also I think the
ioctl interface is a bit convoluted - I'd just make one single interface
which directly returns a new dma-buf fd. Removing it can be handled with
close, no other ioctls required imo. The explicit export step is kinda
only for subsystems that already have an existing buffer handle space, but
we don't have this here.

Otherwise very much wanted.
-Daniel

> +
> +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");
> -- 
> 1.8.4.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

On Thu, Dec 12, 2013 at 03:53:05PM +0100, Daniel Vetter wrote:
> On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote:
[...]
> > diff --git a/drivers/base/dma-buf-test.c b/drivers/base/dma-buf-test.c
[...]
> > +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)
> 
> Shouldn't we put this into an uapi header somewhere?

Yes, definitely. I just didn't want to go through all that trouble
before checking that anyone else actually thought this was a good
idea.

There are a few other things that probably need to be solved before this
can be merged, though. For instance, it currently doesn't compile on x86
because it uses dma_alloc_writecombine(). So there's ample room for
improvement.

> Also I think the ioctl interface is a bit convoluted - I'd just make
> one single interface which directly returns a new dma-buf fd. Removing
> it can be handled with close, no other ioctls required imo. The
> explicit export step is kinda only for subsystems that already have an
> existing buffer handle space, but we don't have this here.

I initially thought that this allowed more flexibility, but on the other
hand having to keep track of a number of buffers would add complexity to
the implementation and like you said it's probably not worth it. Also,
it's not like the code currently copes well with multiple buffers being
created. It just chickens out by returning -EBUSY.

The equivalent could be achieved by beefing up the DMABUF_IOCTL_CREATE
thusly:

	struct dmabuf_create {
		__u32 flags;
		__u32 size;
		__u32 fd;
		__u32 pad;
	};

Then use the size and flags parameters as before, but return the file
descriptor to the DMA-BUF in .fd. Then simply keep it open until the
descriptor is closed. That would also seem to be a more reliable
interface since closing the fd just decrements the reference count on
the DMA-BUF and therefore would keep it alive if somebody still kept
around a reference (the importer for instance).

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thomas Hellstrom]

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.

/Thomas

Re: [RFC] dma-buf: Implement test module

[Daniel Vetter]

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. Userptr won't
really make this worse, but imo we should fix this first before
expanding the use-cases too much. And getting dma-bufs to integrate
better into existing memory mangers like ttm will be a lot of pain.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] dma-buf: Implement test module

[Greg Kroah-Hartman]

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/ ?)

Also fix up the uapi stuff for the ioctls.

thanks,

greg k-h

Re: [RFC] dma-buf: Implement test module

[Greg Kroah-Hartman]

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.

> > +static struct miscdevice dmabuf_device = {
> +	.minor = 128,

Why did you pick this minor?  Why not just make it dynamic?

thanks,

greg k-h

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thomas Hellstrom]

On 12/14/2013 01:37 PM, Thierry Reding 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).

Actually, while the GEM prime implementation appears to pin an exported 
dma-buf on first attach, from the dma-buf documentation it seems 
sufficient to pin it on map or cpu access.

But what I assume Daniel is referring to is that there is no way for 
exporters to tell importers to force unmap() the dma-buf, so that it can be
unpinned?

Daniel, maybe you could elaborate a bit on this?

Thomas

>
> Thierry
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] dma-buf: Implement test module

[Rob Clark]

On Sat, Dec 14, 2013 at 7:47 AM, Thomas Hellstrom <thomas@shipmail.org> wrote:
> On 12/14/2013 01:37 PM, Thierry Reding 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).
>
>
> Actually, while the GEM prime implementation appears to pin an exported
> dma-buf on first attach, from the dma-buf documentation it seems sufficient
> to pin it on map or cpu access.
>
> But what I assume Daniel is referring to is that there is no way for
> exporters to tell importers to force unmap() the dma-buf, so that it can be
> unpinned?

yeah, or some way for importers to opportunistically keep around a
mapping rather than map/unmap on each use..

maybe we need something shrinker-ish for dmabuf?

BR,
-R

> Daniel, maybe you could elaborate a bit on this?
>
> Thomas
>
>>
>> Thierry
>>
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] dma-buf: Implement test module

[Thomas Hellstrom]

On 12/14/2013 02:02 PM, Rob Clark wrote:
> On Sat, Dec 14, 2013 at 7:47 AM, Thomas Hellstrom <thomas@shipmail.org> wrote:
>> On 12/14/2013 01:37 PM, Thierry Reding 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).
>>
>> Actually, while the GEM prime implementation appears to pin an exported
>> dma-buf on first attach, from the dma-buf documentation it seems sufficient
>> to pin it on map or cpu access.
>>
>> But what I assume Daniel is referring to is that there is no way for
>> exporters to tell importers to force unmap() the dma-buf, so that it can be
>> unpinned?
> yeah, or some way for importers to opportunistically keep around a
> mapping rather than map/unmap on each use..
>
> maybe we need something shrinker-ish for dmabuf?

Yes, I think that's needed for both the memory-shortage case where we 
want to unpin, and I guess it would desirable for
iommu space management as well.

/Thomas

>
> BR,
> -R
>
>

Re: [RFC] dma-buf: Implement test module

[Daniel Vetter]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] dma-buf: Implement test module

[Daniel Vetter]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] dma-buf: Implement test module

[Greg Kroah-Hartman]

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

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]

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/ ?)

Sorry for taking so long to get back on this. I've tried various ways to
make this work, but always ended up with something that didn't quite
work.

What I attempted was to put the test program into the samples/dma-buf
subdirectory and use something along these lines:

diff --git a/samples/dma-buf/Makefile b/samples/dma-buf/Makefile
new file mode 100644
index 000000000000..bcaa117474d7
--- /dev/null
+++ b/samples/dma-buf/Makefile
@@ -0,0 +1,8 @@
+obj- := dummy.o
+
+HOSTCFLAGS_dma-buf-test.o += $(shell pkg-config --cflags libdrm)
+HOSTLOADLIBES_dma-buf-test += $(shell pkg-config --libs libdrm)
+
+hostprogs-y := dma-buf-test
+
+always := $(hostprogs-y)

There are two things that don't work too well with this. First this
causes the build to break if the build machine doesn't have the new
public header (include/uapi/linux/dma-buf.h) installed yet. So the only
way to make this work would be by building the kernel once with SAMPLES
disabled, install the headers and then build again with SAMPLES enabled.
Which really isn't very nice.

One other option that I've tried is to modify the include path so that
the test program would get the in-tree copy of the public header file,
but that didn't build properly either because the header files aren't
properly sanitized and therefore the compiler complains about it
(include/uapi/linux/types.h).

One other disadvantage of carrying the sample program in the tree is
that there's only infrastructure to build programs natively on the build
machine. That's somewhat unfortunate because if you want to run the test
program on a different architecture you have to either compile the
kernel natively on that architecture (which isn't very practical on many
embedded devices) or cross-compile manually.

I think a much nicer solution would be to add infrastructure to cross-
compile these test programs, so that they end up being built for the
same architecture as the kernel image (i.e. using CROSS_COMPILE).

Adding Michal and the linux-kbuild mailing list, perhaps this has been
discussed before, or maybe somebody has a better idea on how to solve
this.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Sam Ravnborg]

> 
> There are two things that don't work too well with this. First this
> causes the build to break if the build machine doesn't have the new
> public header (include/uapi/linux/dma-buf.h) installed yet. So the only
> way to make this work would be by building the kernel once with SAMPLES
> disabled, install the headers and then build again with SAMPLES enabled.
> Which really isn't very nice.
> 
> One other option that I've tried is to modify the include path so that
> the test program would get the in-tree copy of the public header file,
> but that didn't build properly either because the header files aren't
> properly sanitized and therefore the compiler complains about it
> (include/uapi/linux/types.h).
> 
> One other disadvantage of carrying the sample program in the tree is
> that there's only infrastructure to build programs natively on the build
> machine. That's somewhat unfortunate because if you want to run the test
> program on a different architecture you have to either compile the
> kernel natively on that architecture (which isn't very practical on many
> embedded devices) or cross-compile manually.
> 
> I think a much nicer solution would be to add infrastructure to cross-
> compile these test programs, so that they end up being built for the
> same architecture as the kernel image (i.e. using CROSS_COMPILE).
> 
> Adding Michal and the linux-kbuild mailing list, perhaps this has been
> discussed before, or maybe somebody has a better idea on how to solve
> this.
I actually looked into this some time ago.
May try to dust off the patch.
IIRC the kernel provided headers were used for building - not the one installed on the machine.
And crosscompile were supported.

	Sam

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]

On Tue, Mar 25, 2014 at 07:01:10PM +0100, Sam Ravnborg wrote:
> > 
> > There are two things that don't work too well with this. First this
> > causes the build to break if the build machine doesn't have the new
> > public header (include/uapi/linux/dma-buf.h) installed yet. So the only
> > way to make this work would be by building the kernel once with SAMPLES
> > disabled, install the headers and then build again with SAMPLES enabled.
> > Which really isn't very nice.
> > 
> > One other option that I've tried is to modify the include path so that
> > the test program would get the in-tree copy of the public header file,
> > but that didn't build properly either because the header files aren't
> > properly sanitized and therefore the compiler complains about it
> > (include/uapi/linux/types.h).
> > 
> > One other disadvantage of carrying the sample program in the tree is
> > that there's only infrastructure to build programs natively on the build
> > machine. That's somewhat unfortunate because if you want to run the test
> > program on a different architecture you have to either compile the
> > kernel natively on that architecture (which isn't very practical on many
> > embedded devices) or cross-compile manually.
> > 
> > I think a much nicer solution would be to add infrastructure to cross-
> > compile these test programs, so that they end up being built for the
> > same architecture as the kernel image (i.e. using CROSS_COMPILE).
> > 
> > Adding Michal and the linux-kbuild mailing list, perhaps this has been
> > discussed before, or maybe somebody has a better idea on how to solve
> > this.
> I actually looked into this some time ago.
> May try to dust off the patch.
> IIRC the kernel provided headers were used for building - not the one installed on the machine.
> And crosscompile were supported.

That sounds exactly like what I'd want for this. If you need any help,
please let me know.

Thanks,
Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] dma-buf: Implement test module

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

On Wed, Mar 26, 2014 at 09:32:47AM +0100, Thierry Reding wrote:
> On Tue, Mar 25, 2014 at 07:01:10PM +0100, Sam Ravnborg wrote:
> > > 
> > > There are two things that don't work too well with this. First this
> > > causes the build to break if the build machine doesn't have the new
> > > public header (include/uapi/linux/dma-buf.h) installed yet. So the only
> > > way to make this work would be by building the kernel once with SAMPLES
> > > disabled, install the headers and then build again with SAMPLES enabled.
> > > Which really isn't very nice.
> > > 
> > > One other option that I've tried is to modify the include path so that
> > > the test program would get the in-tree copy of the public header file,
> > > but that didn't build properly either because the header files aren't
> > > properly sanitized and therefore the compiler complains about it
> > > (include/uapi/linux/types.h).
> > > 
> > > One other disadvantage of carrying the sample program in the tree is
> > > that there's only infrastructure to build programs natively on the build
> > > machine. That's somewhat unfortunate because if you want to run the test
> > > program on a different architecture you have to either compile the
> > > kernel natively on that architecture (which isn't very practical on many
> > > embedded devices) or cross-compile manually.
> > > 
> > > I think a much nicer solution would be to add infrastructure to cross-
> > > compile these test programs, so that they end up being built for the
> > > same architecture as the kernel image (i.e. using CROSS_COMPILE).
> > > 
> > > Adding Michal and the linux-kbuild mailing list, perhaps this has been
> > > discussed before, or maybe somebody has a better idea on how to solve
> > > this.
> > I actually looked into this some time ago.
> > May try to dust off the patch.
> > IIRC the kernel provided headers were used for building - not the one installed on the machine.
> > And crosscompile were supported.
> 
> That sounds exactly like what I'd want for this. If you need any help,
> please let me know.

Did you have any time to look into dusting off the patch? If not I'll
gladly take whatever you have and dust it off myself.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC] dma-buf: Implement test module

[Sam Ravnborg]

On Thu, Jul 10, 2014 at 11:55:26AM +0200, Thierry Reding wrote:
> On Wed, Mar 26, 2014 at 09:32:47AM +0100, Thierry Reding wrote:
> > On Tue, Mar 25, 2014 at 07:01:10PM +0100, Sam Ravnborg wrote:
> > > > 
> > > > There are two things that don't work too well with this. First this
> > > > causes the build to break if the build machine doesn't have the new
> > > > public header (include/uapi/linux/dma-buf.h) installed yet. So the only
> > > > way to make this work would be by building the kernel once with SAMPLES
> > > > disabled, install the headers and then build again with SAMPLES enabled.
> > > > Which really isn't very nice.
> > > > 
> > > > One other option that I've tried is to modify the include path so that
> > > > the test program would get the in-tree copy of the public header file,
> > > > but that didn't build properly either because the header files aren't
> > > > properly sanitized and therefore the compiler complains about it
> > > > (include/uapi/linux/types.h).
> > > > 
> > > > One other disadvantage of carrying the sample program in the tree is
> > > > that there's only infrastructure to build programs natively on the build
> > > > machine. That's somewhat unfortunate because if you want to run the test
> > > > program on a different architecture you have to either compile the
> > > > kernel natively on that architecture (which isn't very practical on many
> > > > embedded devices) or cross-compile manually.
> > > > 
> > > > I think a much nicer solution would be to add infrastructure to cross-
> > > > compile these test programs, so that they end up being built for the
> > > > same architecture as the kernel image (i.e. using CROSS_COMPILE).
> > > > 
> > > > Adding Michal and the linux-kbuild mailing list, perhaps this has been
> > > > discussed before, or maybe somebody has a better idea on how to solve
> > > > this.
> > > I actually looked into this some time ago.
> > > May try to dust off the patch.
> > > IIRC the kernel provided headers were used for building - not the one installed on the machine.
> > > And crosscompile were supported.
> > 
> > That sounds exactly like what I'd want for this. If you need any help,
> > please let me know.
> 
> Did you have any time to look into dusting off the patch? If not I'll
> gladly take whatever you have and dust it off myself.
Thanks for the reminder.
I got it almost working after simplifying it a lot.
I will be travelling for the next few days but will continue to work on this
after the weekend.

	Sam