[PATCHv4 0/3] dma_buf import support for vgem

[PATCHv4 0/3] dma_buf import support for vgem

[Laura Abbott]

Hi,

This v4 of the series to add dma_buf import functions for vgem. This version
primarily focuses on adding a new approach for an alternate dma_buf attach
after platformdev was removed.

Thanks,
Laura

Laura Abbott (3):
  drm/vgem: Add a dummy platform device
  drm/prime: Introduce drm_gem_prime_import_dev
  drm/vgem: Enable dmabuf import interfaces

 drivers/gpu/drm/drm_prime.c     |  30 ++++++--
 drivers/gpu/drm/vgem/vgem_drv.c | 155 +++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/vgem/vgem_drv.h |   2 +
 include/drm/drm_prime.h         |   5 ++
 4 files changed, 154 insertions(+), 38 deletions(-)

-- 
2.7.4

[PATCHv4 1/3] drm/vgem: Add a dummy platform device

[Laura Abbott]

The vgem driver is currently registered independent of any actual
device. Some usage of the dmabuf APIs require an actual device structure
to do anything. Register a dummy platform device for use with dmabuf.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: Switch from the now removed platformdev to a static platform device.
---
 drivers/gpu/drm/vgem/vgem_drv.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 9fee38a..d1d98af 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -42,6 +42,8 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
+static struct platform_device *vgem_platform;
+
 static void vgem_gem_free_object(struct drm_gem_object *obj)
 {
 	struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
@@ -335,11 +337,20 @@ static int __init vgem_init(void)
 	int ret;
 
 	vgem_device = drm_dev_alloc(&vgem_driver, NULL);
-	if (IS_ERR(vgem_device)) {
-		ret = PTR_ERR(vgem_device);
+	if (IS_ERR(vgem_device))
+		return PTR_ERR(vgem_device);
+
+	vgem_platform = platform_device_register_simple("vgem",
+					-1, NULL, 0);
+
+	if (!vgem_platform) {
+		ret = -ENODEV;
 		goto out;
 	}
 
+	dma_coerce_mask_and_coherent(&vgem_platform->dev,
+					DMA_BIT_MASK(64));
+
 	ret  = drm_dev_register(vgem_device, 0);
 	if (ret)
 		goto out_unref;
@@ -347,13 +358,15 @@ static int __init vgem_init(void)
 	return 0;
 
 out_unref:
-	drm_dev_unref(vgem_device);
+	platform_device_unregister(vgem_platform);
 out:
+	drm_dev_unref(vgem_device);
 	return ret;
 }
 
 static void __exit vgem_exit(void)
 {
+	platform_device_unregister(vgem_platform);
 	drm_dev_unregister(vgem_device);
 	drm_dev_unref(vgem_device);
 }
-- 
2.7.4

[PATCHv4 2/3] drm/prime: Introduce drm_gem_prime_import_dev

[Laura Abbott]

The existing drm_gem_prime_import function uses the underlying
struct device of a drm_device for attaching to a dma_buf. Some drivers
(notably vgem) may not have an underlying device structure. Offer
an alternate function to attach using any available device structure.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: Alternate implemntation to take an arbitrary struct dev instead of just
a platform device.

This was different enough that I dropped the previous Reviewed-by
---
 drivers/gpu/drm/drm_prime.c | 30 ++++++++++++++++++++++++------
 include/drm/drm_prime.h     |  5 +++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 9fb65b7..5ad9a26 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -595,15 +595,18 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
 /**
- * drm_gem_prime_import - helper library implementation of the import callback
+ * drm_gem_prime_import_dev - core implementation of the import callback
  * @dev: drm_device to import into
  * @dma_buf: dma-buf object to import
+ * @attach_dev: struct device to dma_buf attach
  *
- * This is the implementation of the gem_prime_import functions for GEM drivers
- * using the PRIME helpers.
+ * This is the core of drm_gem_prime_import. It's designed to be called by
+ * drivers who want to use a different device structure than dev->dev for
+ * attaching via dma_buf.
  */
-struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
-					    struct dma_buf *dma_buf)
+struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
+					    struct dma_buf *dma_buf,
+					    struct device *attach_dev)
 {
 	struct dma_buf_attachment *attach;
 	struct sg_table *sgt;
@@ -625,7 +628,7 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
 	if (!dev->driver->gem_prime_import_sg_table)
 		return ERR_PTR(-EINVAL);
 
-	attach = dma_buf_attach(dma_buf, dev->dev);
+	attach = dma_buf_attach(dma_buf, attach_dev);
 	if (IS_ERR(attach))
 		return ERR_CAST(attach);
 
@@ -655,6 +658,21 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
 
 	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL(drm_gem_prime_import_dev);
+
+/**
+ * drm_gem_prime_import - helper library implementation of the import callback
+ * @dev: drm_device to import into
+ * @dma_buf: dma-buf object to import
+ *
+ * This is the implementation of the gem_prime_import functions for GEM drivers
+ * using the PRIME helpers.
+ */
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+					    struct dma_buf *dma_buf)
+{
+	return drm_gem_prime_import_dev(dev, dma_buf, dev->dev);
+}
 EXPORT_SYMBOL(drm_gem_prime_import);
 
 /**
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 0b2a235..46fd1fb 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -65,6 +65,11 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 			       int *prime_fd);
 struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
 					    struct dma_buf *dma_buf);
+
+struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
+						struct dma_buf *dma_buf,
+						struct device *attach_dev);
+
 int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
-- 
2.7.4

[PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Laura Abbott]

Enable the GEM dma-buf import interfaces in addition to the export
interfaces. This lets vgem be used as a test source for other allocators
(e.g. Ion).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: Use new drm_gem_prime_import_dev function
---
 drivers/gpu/drm/vgem/vgem_drv.c | 136 +++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/vgem/vgem_drv.h |   2 +
 2 files changed, 109 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index d1d98af..c9381d45 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -48,6 +48,11 @@ static void vgem_gem_free_object(struct drm_gem_object *obj)
 {
 	struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
 
+	drm_free_large(vgem_obj->pages);
+
+	if (obj->import_attach)
+		drm_prime_gem_destroy(obj, vgem_obj->table);
+
 	drm_gem_object_release(obj);
 	kfree(vgem_obj);
 }
@@ -58,26 +63,49 @@ static int vgem_gem_fault(struct vm_fault *vmf)
 	struct drm_vgem_gem_object *obj = vma->vm_private_data;
 	/* We don't use vmf->pgoff since that has the fake offset */
 	unsigned long vaddr = vmf->address;
-	struct page *page;
-
-	page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping,
-				       (vaddr - vma->vm_start) >> PAGE_SHIFT);
-	if (!IS_ERR(page)) {
-		vmf->page = page;
-		return 0;
-	} else switch (PTR_ERR(page)) {
-		case -ENOSPC:
-		case -ENOMEM:
-			return VM_FAULT_OOM;
-		case -EBUSY:
-			return VM_FAULT_RETRY;
-		case -EFAULT:
-		case -EINVAL:
-			return VM_FAULT_SIGBUS;
-		default:
-			WARN_ON_ONCE(PTR_ERR(page));
-			return VM_FAULT_SIGBUS;
+	int ret;
+	loff_t num_pages;
+	pgoff_t page_offset;
+	page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
+
+	num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
+
+	if (page_offset > num_pages)
+		return VM_FAULT_SIGBUS;
+
+	if (obj->pages) {
+		get_page(obj->pages[page_offset]);
+		vmf->page = obj->pages[page_offset];
+		ret = 0;
+	} else {
+		struct page *page;
+
+		page = shmem_read_mapping_page(
+					file_inode(obj->base.filp)->i_mapping,
+					page_offset);
+		if (!IS_ERR(page)) {
+			vmf->page = page;
+			ret = 0;
+		} else switch (PTR_ERR(page)) {
+			case -ENOSPC:
+			case -ENOMEM:
+				ret = VM_FAULT_OOM;
+				break;
+			case -EBUSY:
+				ret = VM_FAULT_RETRY;
+				break;
+			case -EFAULT:
+			case -EINVAL:
+				ret = VM_FAULT_SIGBUS;
+				break;
+			default:
+				WARN_ON(PTR_ERR(page));
+				ret = VM_FAULT_SIGBUS;
+				break;
+		}
+
 	}
+	return ret;
 }
 
 static const struct vm_operations_struct vgem_gem_vm_ops = {
@@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
 	kfree(vfile);
 }
 
-/* ioctls */
-
-static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
-					      struct drm_file *file,
-					      unsigned int *handle,
-					      unsigned long size)
+static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
+						unsigned long size)
 {
 	struct drm_vgem_gem_object *obj;
 	int ret;
@@ -129,8 +153,31 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	ret = drm_gem_object_init(dev, &obj->base, roundup(size, PAGE_SIZE));
-	if (ret)
-		goto err_free;
+	if (ret) {
+		kfree(obj);
+		return ERR_PTR(ret);
+	}
+
+	return obj;
+}
+
+static void __vgem_gem_destroy(struct drm_vgem_gem_object *obj)
+{
+	drm_gem_object_release(&obj->base);
+	kfree(obj);
+}
+
+static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
+					      struct drm_file *file,
+					      unsigned int *handle,
+					      unsigned long size)
+{
+	struct drm_vgem_gem_object *obj;
+	int ret;
+
+	obj = __vgem_gem_create(dev, size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, handle);
 	drm_gem_object_unreference_unlocked(&obj->base);
@@ -139,9 +186,8 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
 
 	return &obj->base;
 
-err_free:
-	kfree(obj);
 err:
+	__vgem_gem_destroy(obj);
 	return ERR_PTR(ret);
 }
 
@@ -258,6 +304,35 @@ static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj)
 	return st;
 }
 
+static struct drm_gem_object* vgem_prime_import(struct drm_device *dev,
+						struct dma_buf *dma_buf)
+{
+	return drm_gem_prime_import_dev(dev, dma_buf, &vgem_platform->dev);
+}
+
+static struct drm_gem_object *vgem_prime_import_sg_table(struct drm_device *dev,
+			struct dma_buf_attachment *attach, struct sg_table *sg)
+{
+	struct drm_vgem_gem_object *obj;
+	int npages;
+
+	obj = __vgem_gem_create(dev, attach->dmabuf->size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	npages = PAGE_ALIGN(attach->dmabuf->size) / PAGE_SIZE;
+
+	obj->table = sg;
+	obj->pages = drm_malloc_ab(npages, sizeof(struct page *));
+	if (!obj->pages) {
+		__vgem_gem_destroy(obj);
+		return ERR_PTR(-ENOMEM);
+	}
+	drm_prime_sg_to_page_addr_arrays(obj->table, obj->pages, NULL,
+					npages);
+	return &obj->base;
+}
+
 static void *vgem_prime_vmap(struct drm_gem_object *obj)
 {
 	long n_pages = obj->size >> PAGE_SHIFT;
@@ -316,8 +391,11 @@ static struct drm_driver vgem_driver = {
 	.dumb_map_offset		= vgem_gem_dumb_map,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_pin = vgem_prime_pin,
+	.gem_prime_import = vgem_prime_import,
 	.gem_prime_export = drm_gem_prime_export,
+	.gem_prime_import_sg_table = vgem_prime_import_sg_table,
 	.gem_prime_get_sg_table = vgem_prime_get_sg_table,
 	.gem_prime_vmap = vgem_prime_vmap,
 	.gem_prime_vunmap = vgem_prime_vunmap,
diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
index cb59c7a..1aae014 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.h
+++ b/drivers/gpu/drm/vgem/vgem_drv.h
@@ -43,6 +43,8 @@ struct vgem_file {
 #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
 struct drm_vgem_gem_object {
 	struct drm_gem_object base;
+	struct page **pages;
+	struct sg_table *table;
 };
 
 int vgem_fence_open(struct vgem_file *file);
-- 
2.7.4

Re: [PATCHv4 2/3] drm/prime: Introduce drm_gem_prime_import_dev

[Chris Wilson]

On Thu, May 04, 2017 at 11:45:47AM -0700, Laura Abbott wrote:
> 
> The existing drm_gem_prime_import function uses the underlying
> struct device of a drm_device for attaching to a dma_buf. Some drivers
> (notably vgem) may not have an underlying device structure. Offer
> an alternate function to attach using any available device structure.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v4: Alternate implemntation to take an arbitrary struct dev instead of just
> a platform device.
> 
> This was different enough that I dropped the previous Reviewed-by
> ---
>  drivers/gpu/drm/drm_prime.c | 30 ++++++++++++++++++++++++------
>  include/drm/drm_prime.h     |  5 +++++
>  2 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 9fb65b7..5ad9a26 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -595,15 +595,18 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
>  
>  /**
> - * drm_gem_prime_import - helper library implementation of the import callback
> + * drm_gem_prime_import_dev - core implementation of the import callback
>   * @dev: drm_device to import into
>   * @dma_buf: dma-buf object to import
> + * @attach_dev: struct device to dma_buf attach
>   *
> - * This is the implementation of the gem_prime_import functions for GEM drivers
> - * using the PRIME helpers.
> + * This is the core of drm_gem_prime_import. It's designed to be called by
> + * drivers who want to use a different device structure than dev->dev for
> + * attaching via dma_buf.
>   */
> -struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> -					    struct dma_buf *dma_buf)
> +struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
> +					    struct dma_buf *dma_buf,
> +					    struct device *attach_dev)

My critique would be that this should be called
drm_gem_prime_import_for_device()

Either way (though naturally I like my suggestion ;),
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
>   

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCHv4 1/3] drm/vgem: Add a dummy platform device

[Chris Wilson]

On Thu, May 04, 2017 at 11:45:46AM -0700, Laura Abbott wrote:
> 
> The vgem driver is currently registered independent of any actual
> device. Some usage of the dmabuf APIs require an actual device structure
> to do anything. Register a dummy platform device for use with dmabuf.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v4: Switch from the now removed platformdev to a static platform device.

I was thinking of avoiding the static, i.e.

static struct vgem_device {
	struct drm_device drm;

	struct device *platform;
} *vgem_device;

vgem_init():

vgem_device = kzalloc(sizeof(*vgem_device), GFP_KERNEEL);

ret = drm_dev_init(&vgem_device->drm, &vgem_drv, NULL);

vgem_device->platform = platform_device_register_simple("vgem");

And then platform_device_unregister() should be done in a new
vgem_drv.release callback.

I'm not going to insist upon it as I can send a patch to move over to
the "modern" drm_device subclassing later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Chris Wilson]

On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote:
> 
> Enable the GEM dma-buf import interfaces in addition to the export
> interfaces. This lets vgem be used as a test source for other allocators
> (e.g. Ion).
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v4: Use new drm_gem_prime_import_dev function
> ---
>  static const struct vm_operations_struct vgem_gem_vm_ops = {
> @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
>  	kfree(vfile);
>  }
>  
> -/* ioctls */
> -
> -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> -					      struct drm_file *file,
> -					      unsigned int *handle,
> -					      unsigned long size)
> +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
> +						unsigned long size)

I'm going to guess that doesn't line up anymore. If checkpatch isn't
complaining, then sorry for the noise.

> +static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> +					      struct drm_file *file,
> +					      unsigned int *handle,
> +					      unsigned long size)

Ditto.

Lgtm, so r-b still good.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Joe Perches]

On Thu, 2017-05-04 at 21:25 +0100, Chris Wilson wrote:
> On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote:
> > 
> > Enable the GEM dma-buf import interfaces in addition to the export
> > interfaces. This lets vgem be used as a test source for other allocators
> > (e.g. Ion).
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Laura Abbott <labbott@redhat.com>
> > ---
> > v4: Use new drm_gem_prime_import_dev function
> > ---
> >  static const struct vm_operations_struct vgem_gem_vm_ops = {
> > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
> >  	kfree(vfile);
> >  }
> >  
> > -/* ioctls */
> > -
> > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > -					      struct drm_file *file,
> > -					      unsigned int *handle,
> > -					      unsigned long size)
> > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
> > +						unsigned long size)
> 
> I'm going to guess that doesn't line up anymore. If checkpatch isn't
> complaining, then sorry for the noise.

Because of the very long identifiers, perhaps a
nicer way to write this is like:

static struct drm_vgem_gem_object *
__vgen_gem_create(struct drm_device *dev, unsigned long size);

> > +static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > +					      struct drm_file *file,
> > +					      unsigned int *handle,
> > +					      unsigned long size)
> 
> Ditto.

etc...

Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Daniel Vetter]

On Thu, May 04, 2017 at 09:25:03PM +0100, Chris Wilson wrote:
> On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote:
> > 
> > Enable the GEM dma-buf import interfaces in addition to the export
> > interfaces. This lets vgem be used as a test source for other allocators
> > (e.g. Ion).
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Laura Abbott <labbott@redhat.com>
> > ---
> > v4: Use new drm_gem_prime_import_dev function
> > ---
> >  static const struct vm_operations_struct vgem_gem_vm_ops = {
> > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
> >  	kfree(vfile);
> >  }
> >  
> > -/* ioctls */
> > -
> > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > -					      struct drm_file *file,
> > -					      unsigned int *handle,
> > -					      unsigned long size)
> > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
> > +						unsigned long size)
> 
> I'm going to guess that doesn't line up anymore. If checkpatch isn't
> complaining, then sorry for the noise.
> 
> > +static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > +					      struct drm_file *file,
> > +					      unsigned int *handle,
> > +					      unsigned long size)
> 
> Ditto.
> 
> Lgtm, so r-b still good.

I applied all three as-is, I think we can polish more as follow-ups.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces

[Chris Wilson]

On Thu, May 04, 2017 at 09:09:54PM -0700, Joe Perches wrote:
> On Thu, 2017-05-04 at 21:25 +0100, Chris Wilson wrote:
> > On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote:
> > > 
> > > Enable the GEM dma-buf import interfaces in addition to the export
> > > interfaces. This lets vgem be used as a test source for other allocators
> > > (e.g. Ion).
> > > 
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Laura Abbott <labbott@redhat.com>
> > > ---
> > > v4: Use new drm_gem_prime_import_dev function
> > > ---
> > >  static const struct vm_operations_struct vgem_gem_vm_ops = {
> > > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
> > >  	kfree(vfile);
> > >  }
> > >  
> > > -/* ioctls */
> > > -
> > > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > > -					      struct drm_file *file,
> > > -					      unsigned int *handle,
> > > -					      unsigned long size)
> > > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
> > > +						unsigned long size)
> > 
> > I'm going to guess that doesn't line up anymore. If checkpatch isn't
> > complaining, then sorry for the noise.
> 
> Because of the very long identifiers, perhaps a
> nicer way to write this is like:
> 
> static struct drm_vgem_gem_object *
> __vgen_gem_create(struct drm_device *dev, unsigned long size);

Yes, we frequently use that pattern for very_long_function_names.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCHv4 2/3] drm/prime: Introduce drm_gem_prime_import_dev

[kbuild test robot]

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

Hi Laura,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20170505]
[cannot apply to v4.11]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/dma_buf-import-support-for-vgem/20170505-173856
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   In file included from include/drm/drm_file.h:38:0,
                    from drivers/gpu/drm/drm_file.c:38:
>> include/drm/drm_prime.h:71:14: warning: 'struct device' declared inside parameter list will not be visible outside of this definition or declaration
          struct device *attach_dev);
                 ^~~~~~

vim +71 include/drm/drm_prime.h

    55	
    56	struct drm_device;
    57	struct drm_gem_object;
    58	struct drm_file;
    59	
    60	struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
    61					     struct drm_gem_object *obj,
    62					     int flags);
    63	int drm_gem_prime_handle_to_fd(struct drm_device *dev,
    64				       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
    65				       int *prime_fd);
    66	struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
    67						    struct dma_buf *dma_buf);
    68	
    69	struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
    70							struct dma_buf *dma_buf,
  > 71							struct device *attach_dev);
    72	
    73	int drm_gem_prime_fd_to_handle(struct drm_device *dev,
    74				       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
    75	struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
    76					      struct dma_buf_export_info *exp_info);
    77	void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
    78	
    79	int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22486 bytes --]