diff mbox series

[v2] drm/vkms: prime import support

Message ID 20190821135443.4826-1-oleg.vasilev@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/vkms: prime import support | expand

Commit Message

Oleg Vasilev Aug. 21, 2019, 1:54 p.m. UTC
Bring dmabuf sharing through implementing prime_import_sg_table callback.
This will help to validate userspace conformance in prime configurations
without using any actual hardware (e.g. in the cloud).

This enables kms_prime on vkms.

V2:
 - Rodrigo: styleguide + return code check

Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 12 +++++++++
 drivers/gpu/drm/vkms/vkms_drv.h |  9 +++++++
 drivers/gpu/drm/vkms/vkms_gem.c | 46 +++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

Comments

Rodrigo Siqueira Sept. 3, 2019, 12:55 p.m. UTC | #1
Hi Oleg,

First of all, thank you for keeping working on this feature. You did a
great job here. I just have a comments inline.

On 08/21, Oleg Vasilev wrote:
> Bring dmabuf sharing through implementing prime_import_sg_table callback.
> This will help to validate userspace conformance in prime configurations
> without using any actual hardware (e.g. in the cloud).
> 
> This enables kms_prime on vkms.
> 
> V2:
>  - Rodrigo: styleguide + return code check
> 
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Simon Ser <simon.ser@intel.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_drv.c | 12 +++++++++
>  drivers/gpu/drm/vkms/vkms_drv.h |  9 +++++++
>  drivers/gpu/drm/vkms/vkms_gem.c | 46 +++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 44ab9f8ef8be..09598f949b9b 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -11,7 +11,9 @@
>  
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
>  
> +#include <drm/drm_gem.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_drv.h>
> @@ -103,6 +105,8 @@ static struct drm_driver vkms_driver = {
>  	.gem_vm_ops		= &vkms_gem_vm_ops,
>  	.gem_free_object_unlocked = vkms_gem_free_object,
>  	.get_vblank_timestamp	= vkms_get_vblank_timestamp,
> +	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> +	.gem_prime_import_sg_table = vkms_prime_import_sg_table,
>  
>  	.name			= DRIVER_NAME,
>  	.desc			= DRIVER_DESC,
> @@ -157,6 +161,14 @@ static int __init vkms_init(void)
>  	if (ret)
>  		goto out_unregister;
>  
> +	ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
> +					   DMA_BIT_MASK(64));
> +
> +	if (ret) {
> +		DRM_ERROR("Could not initialize DMA support\n");
> +		goto out_fini;
> +	}
> +
>  	vkms_device->drm.irq_enabled = true;
>  
>  	ret = drm_vblank_init(&vkms_device->drm, 1);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 5a95100fa18b..6bf195c91097 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -126,6 +126,9 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>  				       u32 *handle,
>  				       u64 size);
>  
> +struct vkms_gem_object *vkms_gem_create_private(struct drm_device *dev,
> +						u64 size);
> +
>  vm_fault_t vkms_gem_fault(struct vm_fault *vmf);
>  
>  int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> @@ -137,6 +140,12 @@ int vkms_gem_vmap(struct drm_gem_object *obj);
>  
>  void vkms_gem_vunmap(struct drm_gem_object *obj);
>  
> +/* Prime */
> +struct drm_gem_object *
> +vkms_prime_import_sg_table(struct drm_device *dev,
> +			   struct dma_buf_attachment *attach,
> +			   struct sg_table *sg);
> +
>  /* CRC Support */
>  const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
>  					size_t *count);
> diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> index 6489bfe0a149..b7565f48e054 100644
> --- a/drivers/gpu/drm/vkms/vkms_gem.c
> +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> @@ -1,7 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  
> +#include <linux/dma-buf.h>
>  #include <linux/shmem_fs.h>
>  #include <linux/vmalloc.h>
> +#include <drm/drm_prime.h>
>  
>  #include "vkms_drv.h"
>  
> @@ -118,6 +120,25 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>  	return &obj->gem;
>  }
>  
> +struct vkms_gem_object *vkms_gem_create_private(struct drm_device *dev,
> +						u64 size)
> +{
> +	struct vkms_gem_object *obj;
> +
> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> +
> +	if (!obj)
> +		return ERR_PTR(-ENOMEM);
> +
> +	size = roundup(size, PAGE_SIZE);
> +
> +	drm_gem_private_object_init(dev, &obj->gem, size);
> +
> +	mutex_init(&obj->pages_lock);
> +
> +	return obj;
> +}
> +

I suppose that any other function or helper do not invoke this function,
am I right?

If so, I recommend you to remove it from this patch. IMHO, add a
function that is not used yet may make the code hard to understand and
maintain.

Thanks

>  int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
>  		     struct drm_mode_create_dumb *args)
>  {
> @@ -218,3 +239,28 @@ int vkms_gem_vmap(struct drm_gem_object *obj)
>  	mutex_unlock(&vkms_obj->pages_lock);
>  	return ret;
>  }
> +
> +struct drm_gem_object *
> +vkms_prime_import_sg_table(struct drm_device *dev,
> +			   struct dma_buf_attachment *attach,
> +			   struct sg_table *sg)
> +{
> +	struct vkms_gem_object *obj;
> +	int npages;
> +
> +	obj = __vkms_gem_create(dev, attach->dmabuf->size);
> +	if (IS_ERR(obj))
> +		return ERR_CAST(obj);
> +
> +	npages = PAGE_ALIGN(attach->dmabuf->size) / PAGE_SIZE;
> +	DRM_DEBUG_PRIME("Importing %d pages\n", npages);
> +
> +	obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> +	if (!obj->pages) {
> +		vkms_gem_free_object(&obj->gem);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	drm_prime_sg_to_page_addr_arrays(sg, obj->pages, NULL, npages);
> +	return &obj->gem;
> +}
> -- 
> 2.23.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 44ab9f8ef8be..09598f949b9b 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -11,7 +11,9 @@ 
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 
+#include <drm/drm_gem.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
@@ -103,6 +105,8 @@  static struct drm_driver vkms_driver = {
 	.gem_vm_ops		= &vkms_gem_vm_ops,
 	.gem_free_object_unlocked = vkms_gem_free_object,
 	.get_vblank_timestamp	= vkms_get_vblank_timestamp,
+	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
+	.gem_prime_import_sg_table = vkms_prime_import_sg_table,
 
 	.name			= DRIVER_NAME,
 	.desc			= DRIVER_DESC,
@@ -157,6 +161,14 @@  static int __init vkms_init(void)
 	if (ret)
 		goto out_unregister;
 
+	ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
+					   DMA_BIT_MASK(64));
+
+	if (ret) {
+		DRM_ERROR("Could not initialize DMA support\n");
+		goto out_fini;
+	}
+
 	vkms_device->drm.irq_enabled = true;
 
 	ret = drm_vblank_init(&vkms_device->drm, 1);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5a95100fa18b..6bf195c91097 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -126,6 +126,9 @@  struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
 				       u32 *handle,
 				       u64 size);
 
+struct vkms_gem_object *vkms_gem_create_private(struct drm_device *dev,
+						u64 size);
+
 vm_fault_t vkms_gem_fault(struct vm_fault *vmf);
 
 int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
@@ -137,6 +140,12 @@  int vkms_gem_vmap(struct drm_gem_object *obj);
 
 void vkms_gem_vunmap(struct drm_gem_object *obj);
 
+/* Prime */
+struct drm_gem_object *
+vkms_prime_import_sg_table(struct drm_device *dev,
+			   struct dma_buf_attachment *attach,
+			   struct sg_table *sg);
+
 /* CRC Support */
 const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
 					size_t *count);
diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
index 6489bfe0a149..b7565f48e054 100644
--- a/drivers/gpu/drm/vkms/vkms_gem.c
+++ b/drivers/gpu/drm/vkms/vkms_gem.c
@@ -1,7 +1,9 @@ 
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/dma-buf.h>
 #include <linux/shmem_fs.h>
 #include <linux/vmalloc.h>
+#include <drm/drm_prime.h>
 
 #include "vkms_drv.h"
 
@@ -118,6 +120,25 @@  struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
 	return &obj->gem;
 }
 
+struct vkms_gem_object *vkms_gem_create_private(struct drm_device *dev,
+						u64 size)
+{
+	struct vkms_gem_object *obj;
+
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	size = roundup(size, PAGE_SIZE);
+
+	drm_gem_private_object_init(dev, &obj->gem, size);
+
+	mutex_init(&obj->pages_lock);
+
+	return obj;
+}
+
 int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
 		     struct drm_mode_create_dumb *args)
 {
@@ -218,3 +239,28 @@  int vkms_gem_vmap(struct drm_gem_object *obj)
 	mutex_unlock(&vkms_obj->pages_lock);
 	return ret;
 }
+
+struct drm_gem_object *
+vkms_prime_import_sg_table(struct drm_device *dev,
+			   struct dma_buf_attachment *attach,
+			   struct sg_table *sg)
+{
+	struct vkms_gem_object *obj;
+	int npages;
+
+	obj = __vkms_gem_create(dev, attach->dmabuf->size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	npages = PAGE_ALIGN(attach->dmabuf->size) / PAGE_SIZE;
+	DRM_DEBUG_PRIME("Importing %d pages\n", npages);
+
+	obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
+	if (!obj->pages) {
+		vkms_gem_free_object(&obj->gem);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	drm_prime_sg_to_page_addr_arrays(sg, obj->pages, NULL, npages);
+	return &obj->gem;
+}