[PATCH V2 0/5] drm/vkms: Updates to meet basic kms_flip requirements

[PATCH V2 0/5] drm/vkms: Updates to meet basic kms_flip requirements

[Rodrigo Siqueira]

Currently, we are trying to make VKMS pass in the kms_flip test (IGT).
As a result, we made a series of small changes in the module with the
goal to meet some of the necessary steps required by kms_flip. This
patchset comprises all the simple modifications used until now to make
the kms_flip partially works. It is important to highlight, that VKMS
still not pass in the kms_flip, but I send these modifications with the
intention to avoid rework.

Changes in V2:
 - Add dumb buffer support

Rodrigo Siqueira (5):
  drm/vkms: Add dumb operations
  drm/vkms: Add helper for framebuffer create
  drm/vkms: Add atomic helpers functions
  drm/vkms: Add connectors helpers
  drm/vkms: Add plane helper struct

 drivers/gpu/drm/vkms/Makefile      |   2 +-
 drivers/gpu/drm/vkms/vkms_crtc.c   |  18 ++++
 drivers/gpu/drm/vkms/vkms_drv.c    |  18 ++--
 drivers/gpu/drm/vkms/vkms_drv.h    |  30 ++++++
 drivers/gpu/drm/vkms/vkms_gem.c    | 168 +++++++++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_output.c |  27 +++++
 drivers/gpu/drm/vkms/vkms_plane.c  |  18 ++++
 7 files changed, 274 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V2 1/5] drm/vkms: Add dumb operations

[Rodrigo Siqueira]

VKMS currently does not handle dumb data, and as a consequence, it does
not provide mechanisms for handling gem. This commit adds the necessary
support for gem object/handler and the dumb functions.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/Makefile   |   2 +-
 drivers/gpu/drm/vkms/vkms_drv.c |   9 ++
 drivers/gpu/drm/vkms/vkms_drv.h |  21 ++++
 drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
 4 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c

diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 3f774a6a9c58..986297da51bf 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -1,3 +1,3 @@
-vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
+vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
 
 obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 740a4cbfed91..638bab9083b5 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
 	.release	= drm_release,
 };
 
+static const struct vm_operations_struct vkms_gem_vm_ops = {
+	.fault = vkms_gem_fault,
+	.open = drm_gem_vm_open,
+	.close = drm_gem_vm_close,
+};
+
 static void vkms_release(struct drm_device *dev)
 {
 	struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
@@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
 	.release		= vkms_release,
 	.fops			= &vkms_driver_fops,
+	.dumb_create		= vkms_dumb_create,
+	.dumb_map_offset	= vkms_dumb_map,
+	.gem_vm_ops		= &vkms_gem_vm_ops,
 
 	.name			= DRIVER_NAME,
 	.desc			= DRIVER_DESC,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index b0f9d2e61a42..54bb3dd2b2c1 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -3,6 +3,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm.h>
+#include <drm/drm_gem.h>
 #include <drm/drm_encoder.h>
 
 static const u32 vkms_formats[] = {
@@ -21,6 +22,12 @@ struct vkms_device {
 	struct vkms_output output;
 };
 
+struct vkms_gem_object {
+	struct drm_gem_object gem;
+	struct mutex pages_lock; /* Page lock used in page fault handler */
+	struct page **pages;
+};
+
 int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		   struct drm_plane *primary, struct drm_plane *cursor);
 
@@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
 
 struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
 
+/* Gem stuff */
+struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
+				       struct drm_file *file,
+				       u32 *handle,
+				       u64 size);
+
+int vkms_gem_fault(struct vm_fault *vmf);
+
+int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
+		     struct drm_mode_create_dumb *args);
+
+int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
+		  u32 handle, u64 *offset);
+
 #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
new file mode 100644
index 000000000000..9f820f56b9e0
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_gem.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/shmem_fs.h>
+
+#include "vkms_drv.h"
+
+static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
+						 u64 size)
+{
+	struct vkms_gem_object *obj;
+	int ret;
+
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	size = roundup(size, PAGE_SIZE);
+	ret = drm_gem_object_init(dev, &obj->gem, size);
+	if (ret) {
+		kfree(obj);
+		return ERR_PTR(ret);
+	}
+
+	mutex_init(&obj->pages_lock);
+
+	return obj;
+}
+
+int vkms_gem_fault(struct vm_fault *vmf)
+{
+	struct vm_area_struct *vma = vmf->vma;
+	struct vkms_gem_object *obj = vma->vm_private_data;
+	unsigned long vaddr = vmf->address;
+	pgoff_t page_offset;
+	loff_t num_pages;
+	int ret;
+
+	page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
+	num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
+
+	if (page_offset > num_pages)
+		return VM_FAULT_SIGBUS;
+
+	ret = -ENOENT;
+	mutex_lock(&obj->pages_lock);
+	if (obj->pages) {
+		get_page(obj->pages[page_offset]);
+		vmf->page = obj->pages[page_offset];
+		ret = 0;
+	}
+	mutex_unlock(&obj->pages_lock);
+	if (ret) {
+		struct page *page;
+		struct address_space *mapping;
+
+		mapping = file_inode(obj->gem.filp)->i_mapping;
+		page = shmem_read_mapping_page(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;
+}
+
+struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
+				       struct drm_file *file,
+				       u32 *handle,
+				       u64 size)
+{
+	struct vkms_gem_object *obj;
+	int ret;
+
+	if (!file || !dev || !handle)
+		return ERR_PTR(-EINVAL);
+
+	obj = __vkms_gem_create(dev, size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	ret = drm_gem_handle_create(file, &obj->gem, handle);
+	drm_gem_object_put_unlocked(&obj->gem);
+	if (ret) {
+		drm_gem_object_release(&obj->gem);
+		kfree(obj);
+		return ERR_PTR(ret);
+	}
+
+	return &obj->gem;
+}
+
+int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
+		     struct drm_mode_create_dumb *args)
+{
+	struct drm_gem_object *gem_obj;
+	u64 pitch, size;
+
+	if (!args || !dev || !file)
+		return -EINVAL;
+
+	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
+	size = pitch * args->height;
+
+	if (!size)
+		return -EINVAL;
+
+	gem_obj = vkms_gem_create(dev, file, &args->handle, size);
+	if (IS_ERR(gem_obj))
+		return PTR_ERR(gem_obj);
+
+	args->size = gem_obj->size;
+	args->pitch = pitch;
+
+	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
+
+	return 0;
+}
+
+int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
+		  u32 handle, u64 *offset)
+{
+	struct drm_gem_object *obj;
+	int ret;
+
+	obj = drm_gem_object_lookup(file, handle);
+	if (!obj)
+		return -ENOENT;
+
+	if (!obj->filp) {
+		ret = -EINVAL;
+		goto unref;
+	}
+
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto unref;
+
+	*offset = drm_vma_node_offset_addr(&obj->vma_node);
+unref:
+	drm_gem_object_put_unlocked(obj);
+
+	return ret;
+}
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Haneen Mohammed]

On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:
> VKMS currently does not handle dumb data, and as a consequence, it does
> not provide mechanisms for handling gem. This commit adds the necessary
> support for gem object/handler and the dumb functions.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---

This looks good to me except with missing gem_free_object_unlocked callback,
which causes warning: Memory manager not clean during takedown. 

Maybe it will be easier if we add this in another patch instead of creating v3 of
this patchset?

>  drivers/gpu/drm/vkms/Makefile   |   2 +-
>  drivers/gpu/drm/vkms/vkms_drv.c |   9 ++
>  drivers/gpu/drm/vkms/vkms_drv.h |  21 ++++
>  drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
>  4 files changed, 199 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c
> 
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 3f774a6a9c58..986297da51bf 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -1,3 +1,3 @@
> -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
>  
>  obj-$(CONFIG_DRM_VKMS) += vkms.o
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 740a4cbfed91..638bab9083b5 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
>  	.release	= drm_release,
>  };
>  
> +static const struct vm_operations_struct vkms_gem_vm_ops = {
> +	.fault = vkms_gem_fault,
> +	.open = drm_gem_vm_open,
> +	.close = drm_gem_vm_close,
> +};
> +
>  static void vkms_release(struct drm_device *dev)
>  {
>  	struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
> @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
>  	.release		= vkms_release,
>  	.fops			= &vkms_driver_fops,
> +	.dumb_create		= vkms_dumb_create,
> +	.dumb_map_offset	= vkms_dumb_map,
> +	.gem_vm_ops		= &vkms_gem_vm_ops,
>  
>  	.name			= DRIVER_NAME,
>  	.desc			= DRIVER_DESC,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index b0f9d2e61a42..54bb3dd2b2c1 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -3,6 +3,7 @@
>  
>  #include <drm/drmP.h>
>  #include <drm/drm.h>
> +#include <drm/drm_gem.h>
>  #include <drm/drm_encoder.h>
>  
>  static const u32 vkms_formats[] = {
> @@ -21,6 +22,12 @@ struct vkms_device {
>  	struct vkms_output output;
>  };
>  
> +struct vkms_gem_object {
> +	struct drm_gem_object gem;
> +	struct mutex pages_lock; /* Page lock used in page fault handler */
> +	struct page **pages;
> +};
> +
>  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		   struct drm_plane *primary, struct drm_plane *cursor);
>  
> @@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
>  
>  struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
>  
> +/* Gem stuff */
> +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> +				       struct drm_file *file,
> +				       u32 *handle,
> +				       u64 size);
> +
> +int vkms_gem_fault(struct vm_fault *vmf);
> +
> +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> +		     struct drm_mode_create_dumb *args);
> +
> +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> +		  u32 handle, u64 *offset);
> +
>  #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> new file mode 100644
> index 000000000000..9f820f56b9e0
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/shmem_fs.h>
> +
> +#include "vkms_drv.h"
> +
> +static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
> +						 u64 size)
> +{
> +	struct vkms_gem_object *obj;
> +	int ret;
> +
> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> +	if (!obj)
> +		return ERR_PTR(-ENOMEM);
> +
> +	size = roundup(size, PAGE_SIZE);
> +	ret = drm_gem_object_init(dev, &obj->gem, size);
> +	if (ret) {
> +		kfree(obj);
> +		return ERR_PTR(ret);
> +	}
> +
> +	mutex_init(&obj->pages_lock);
> +
> +	return obj;
> +}
> +
> +int vkms_gem_fault(struct vm_fault *vmf)
> +{
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct vkms_gem_object *obj = vma->vm_private_data;
> +	unsigned long vaddr = vmf->address;
> +	pgoff_t page_offset;
> +	loff_t num_pages;
> +	int ret;
> +
> +	page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
> +	num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
> +
> +	if (page_offset > num_pages)
> +		return VM_FAULT_SIGBUS;
> +
> +	ret = -ENOENT;
> +	mutex_lock(&obj->pages_lock);
> +	if (obj->pages) {
> +		get_page(obj->pages[page_offset]);
> +		vmf->page = obj->pages[page_offset];
> +		ret = 0;
> +	}
> +	mutex_unlock(&obj->pages_lock);
> +	if (ret) {
> +		struct page *page;
> +		struct address_space *mapping;
> +
> +		mapping = file_inode(obj->gem.filp)->i_mapping;
> +		page = shmem_read_mapping_page(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;
> +}
> +
> +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> +				       struct drm_file *file,
> +				       u32 *handle,
> +				       u64 size)
> +{
> +	struct vkms_gem_object *obj;
> +	int ret;
> +
> +	if (!file || !dev || !handle)
> +		return ERR_PTR(-EINVAL);
> +
> +	obj = __vkms_gem_create(dev, size);
> +	if (IS_ERR(obj))
> +		return ERR_CAST(obj);
> +
> +	ret = drm_gem_handle_create(file, &obj->gem, handle);
> +	drm_gem_object_put_unlocked(&obj->gem);
> +	if (ret) {
> +		drm_gem_object_release(&obj->gem);
> +		kfree(obj);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return &obj->gem;
> +}
> +
> +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> +		     struct drm_mode_create_dumb *args)
> +{
> +	struct drm_gem_object *gem_obj;
> +	u64 pitch, size;
> +
> +	if (!args || !dev || !file)
> +		return -EINVAL;
> +
> +	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> +	size = pitch * args->height;
> +
> +	if (!size)
> +		return -EINVAL;
> +
> +	gem_obj = vkms_gem_create(dev, file, &args->handle, size);
> +	if (IS_ERR(gem_obj))
> +		return PTR_ERR(gem_obj);
> +
> +	args->size = gem_obj->size;
> +	args->pitch = pitch;
> +
> +	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
> +
> +	return 0;
> +}
> +
> +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> +		  u32 handle, u64 *offset)
> +{
> +	struct drm_gem_object *obj;
> +	int ret;
> +
> +	obj = drm_gem_object_lookup(file, handle);
> +	if (!obj)
> +		return -ENOENT;
> +
> +	if (!obj->filp) {
> +		ret = -EINVAL;
> +		goto unref;
> +	}
> +
> +	ret = drm_gem_create_mmap_offset(obj);
> +	if (ret)
> +		goto unref;
> +
> +	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> +unref:
> +	drm_gem_object_put_unlocked(obj);
> +
> +	return ret;
> +}
> -- 
> 2.17.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Daniel Vetter]

On Thu, Jul 05, 2018 at 11:21:19PM +0300, Haneen Mohammed wrote:
> On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:
> > VKMS currently does not handle dumb data, and as a consequence, it does
> > not provide mechanisms for handling gem. This commit adds the necessary
> > support for gem object/handler and the dumb functions.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> 
> This looks good to me except with missing gem_free_object_unlocked callback,
> which causes warning: Memory manager not clean during takedown. 
> 
> Maybe it will be easier if we add this in another patch instead of creating v3 of
> this patchset?

Ah this is the patch series that didn't land yet ...

Rodrigo, can you pls respin with the issue fixed that Haneen spotted?

Haneen, can you pls review the other patches in this series too?

Also, do we have any other patch series that's not yet applied? With two
interns working on the same thing it's a bit harder to keep track of stuff
...

Thanks, Daniel

> 
> >  drivers/gpu/drm/vkms/Makefile   |   2 +-
> >  drivers/gpu/drm/vkms/vkms_drv.c |   9 ++
> >  drivers/gpu/drm/vkms/vkms_drv.h |  21 ++++
> >  drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
> >  4 files changed, 199 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c
> > 
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 3f774a6a9c58..986297da51bf 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,3 +1,3 @@
> > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> >  
> >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 740a4cbfed91..638bab9083b5 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
> >  	.release	= drm_release,
> >  };
> >  
> > +static const struct vm_operations_struct vkms_gem_vm_ops = {
> > +	.fault = vkms_gem_fault,
> > +	.open = drm_gem_vm_open,
> > +	.close = drm_gem_vm_close,
> > +};
> > +
> >  static void vkms_release(struct drm_device *dev)
> >  {
> >  	struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
> > @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
> >  	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> >  	.release		= vkms_release,
> >  	.fops			= &vkms_driver_fops,
> > +	.dumb_create		= vkms_dumb_create,
> > +	.dumb_map_offset	= vkms_dumb_map,
> > +	.gem_vm_ops		= &vkms_gem_vm_ops,
> >  
> >  	.name			= DRIVER_NAME,
> >  	.desc			= DRIVER_DESC,
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index b0f9d2e61a42..54bb3dd2b2c1 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -3,6 +3,7 @@
> >  
> >  #include <drm/drmP.h>
> >  #include <drm/drm.h>
> > +#include <drm/drm_gem.h>
> >  #include <drm/drm_encoder.h>
> >  
> >  static const u32 vkms_formats[] = {
> > @@ -21,6 +22,12 @@ struct vkms_device {
> >  	struct vkms_output output;
> >  };
> >  
> > +struct vkms_gem_object {
> > +	struct drm_gem_object gem;
> > +	struct mutex pages_lock; /* Page lock used in page fault handler */
> > +	struct page **pages;
> > +};
> > +
> >  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> >  		   struct drm_plane *primary, struct drm_plane *cursor);
> >  
> > @@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
> >  
> >  struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> >  
> > +/* Gem stuff */
> > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > +				       struct drm_file *file,
> > +				       u32 *handle,
> > +				       u64 size);
> > +
> > +int vkms_gem_fault(struct vm_fault *vmf);
> > +
> > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> > +		     struct drm_mode_create_dumb *args);
> > +
> > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> > +		  u32 handle, u64 *offset);
> > +
> >  #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > new file mode 100644
> > index 000000000000..9f820f56b9e0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > @@ -0,0 +1,168 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/shmem_fs.h>
> > +
> > +#include "vkms_drv.h"
> > +
> > +static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
> > +						 u64 size)
> > +{
> > +	struct vkms_gem_object *obj;
> > +	int ret;
> > +
> > +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> > +	if (!obj)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	size = roundup(size, PAGE_SIZE);
> > +	ret = drm_gem_object_init(dev, &obj->gem, size);
> > +	if (ret) {
> > +		kfree(obj);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	mutex_init(&obj->pages_lock);
> > +
> > +	return obj;
> > +}
> > +
> > +int vkms_gem_fault(struct vm_fault *vmf)
> > +{
> > +	struct vm_area_struct *vma = vmf->vma;
> > +	struct vkms_gem_object *obj = vma->vm_private_data;
> > +	unsigned long vaddr = vmf->address;
> > +	pgoff_t page_offset;
> > +	loff_t num_pages;
> > +	int ret;
> > +
> > +	page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
> > +	num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
> > +
> > +	if (page_offset > num_pages)
> > +		return VM_FAULT_SIGBUS;
> > +
> > +	ret = -ENOENT;
> > +	mutex_lock(&obj->pages_lock);
> > +	if (obj->pages) {
> > +		get_page(obj->pages[page_offset]);
> > +		vmf->page = obj->pages[page_offset];
> > +		ret = 0;
> > +	}
> > +	mutex_unlock(&obj->pages_lock);
> > +	if (ret) {
> > +		struct page *page;
> > +		struct address_space *mapping;
> > +
> > +		mapping = file_inode(obj->gem.filp)->i_mapping;
> > +		page = shmem_read_mapping_page(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;
> > +}
> > +
> > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > +				       struct drm_file *file,
> > +				       u32 *handle,
> > +				       u64 size)
> > +{
> > +	struct vkms_gem_object *obj;
> > +	int ret;
> > +
> > +	if (!file || !dev || !handle)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	obj = __vkms_gem_create(dev, size);
> > +	if (IS_ERR(obj))
> > +		return ERR_CAST(obj);
> > +
> > +	ret = drm_gem_handle_create(file, &obj->gem, handle);
> > +	drm_gem_object_put_unlocked(&obj->gem);
> > +	if (ret) {
> > +		drm_gem_object_release(&obj->gem);
> > +		kfree(obj);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	return &obj->gem;
> > +}
> > +
> > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> > +		     struct drm_mode_create_dumb *args)
> > +{
> > +	struct drm_gem_object *gem_obj;
> > +	u64 pitch, size;
> > +
> > +	if (!args || !dev || !file)
> > +		return -EINVAL;
> > +
> > +	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> > +	size = pitch * args->height;
> > +
> > +	if (!size)
> > +		return -EINVAL;
> > +
> > +	gem_obj = vkms_gem_create(dev, file, &args->handle, size);
> > +	if (IS_ERR(gem_obj))
> > +		return PTR_ERR(gem_obj);
> > +
> > +	args->size = gem_obj->size;
> > +	args->pitch = pitch;
> > +
> > +	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
> > +
> > +	return 0;
> > +}
> > +
> > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> > +		  u32 handle, u64 *offset)
> > +{
> > +	struct drm_gem_object *obj;
> > +	int ret;
> > +
> > +	obj = drm_gem_object_lookup(file, handle);
> > +	if (!obj)
> > +		return -ENOENT;
> > +
> > +	if (!obj->filp) {
> > +		ret = -EINVAL;
> > +		goto unref;
> > +	}
> > +
> > +	ret = drm_gem_create_mmap_offset(obj);
> > +	if (ret)
> > +		goto unref;
> > +
> > +	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> > +unref:
> > +	drm_gem_object_put_unlocked(obj);
> > +
> > +	return ret;
> > +}
> > -- 
> > 2.17.1
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Rodrigo Siqueira]

On 07/11, Daniel Vetter wrote:
> On Thu, Jul 05, 2018 at 11:21:19PM +0300, Haneen Mohammed wrote:
> > On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:
> > > VKMS currently does not handle dumb data, and as a consequence, it does
> > > not provide mechanisms for handling gem. This commit adds the necessary
> > > support for gem object/handler and the dumb functions.
> > > 
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > ---
> > 
> > This looks good to me except with missing gem_free_object_unlocked callback,
> > which causes warning: Memory manager not clean during takedown. 
> > 
> > Maybe it will be easier if we add this in another patch instead of creating v3 of
> > this patchset?
> 
> Ah this is the patch series that didn't land yet ...

Hi, thanks for all the feedbacks.
 
> Rodrigo, can you pls respin with the issue fixed that Haneen spotted?

Yes,

I will prepare a new version of this patchset based on all the comments.
Just one question, should I create a new version of the patchset that
includes the patch related with the hrtimer simulation or keep it
separate?
 
> Haneen, can you pls review the other patches in this series too?
> 
> Also, do we have any other patch series that's not yet applied? With two
> interns working on the same thing it's a bit harder to keep track of stuff
> ...
> 
> Thanks, Daniel
> 
> > 
> > >  drivers/gpu/drm/vkms/Makefile   |   2 +-
> > >  drivers/gpu/drm/vkms/vkms_drv.c |   9 ++
> > >  drivers/gpu/drm/vkms/vkms_drv.h |  21 ++++
> > >  drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
> > >  4 files changed, 199 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > > index 3f774a6a9c58..986297da51bf 100644
> > > --- a/drivers/gpu/drm/vkms/Makefile
> > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > @@ -1,3 +1,3 @@
> > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > >  
> > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > > index 740a4cbfed91..638bab9083b5 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
> > >  	.release	= drm_release,
> > >  };
> > >  
> > > +static const struct vm_operations_struct vkms_gem_vm_ops = {
> > > +	.fault = vkms_gem_fault,
> > > +	.open = drm_gem_vm_open,
> > > +	.close = drm_gem_vm_close,
> > > +};
> > > +
> > >  static void vkms_release(struct drm_device *dev)
> > >  {
> > >  	struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
> > > @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
> > >  	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> > >  	.release		= vkms_release,
> > >  	.fops			= &vkms_driver_fops,
> > > +	.dumb_create		= vkms_dumb_create,
> > > +	.dumb_map_offset	= vkms_dumb_map,
> > > +	.gem_vm_ops		= &vkms_gem_vm_ops,
> > >  
> > >  	.name			= DRIVER_NAME,
> > >  	.desc			= DRIVER_DESC,
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index b0f9d2e61a42..54bb3dd2b2c1 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -3,6 +3,7 @@
> > >  
> > >  #include <drm/drmP.h>
> > >  #include <drm/drm.h>
> > > +#include <drm/drm_gem.h>
> > >  #include <drm/drm_encoder.h>
> > >  
> > >  static const u32 vkms_formats[] = {
> > > @@ -21,6 +22,12 @@ struct vkms_device {
> > >  	struct vkms_output output;
> > >  };
> > >  
> > > +struct vkms_gem_object {
> > > +	struct drm_gem_object gem;
> > > +	struct mutex pages_lock; /* Page lock used in page fault handler */
> > > +	struct page **pages;
> > > +};
> > > +
> > >  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> > >  		   struct drm_plane *primary, struct drm_plane *cursor);
> > >  
> > > @@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
> > >  
> > >  struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
> > >  
> > > +/* Gem stuff */
> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > +				       struct drm_file *file,
> > > +				       u32 *handle,
> > > +				       u64 size);
> > > +
> > > +int vkms_gem_fault(struct vm_fault *vmf);
> > > +
> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> > > +		     struct drm_mode_create_dumb *args);
> > > +
> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> > > +		  u32 handle, u64 *offset);
> > > +
> > >  #endif /* _VKMS_DRV_H_ */
> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > new file mode 100644
> > > index 000000000000..9f820f56b9e0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > @@ -0,0 +1,168 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include <linux/shmem_fs.h>
> > > +
> > > +#include "vkms_drv.h"
> > > +
> > > +static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
> > > +						 u64 size)
> > > +{
> > > +	struct vkms_gem_object *obj;
> > > +	int ret;
> > > +
> > > +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> > > +	if (!obj)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	size = roundup(size, PAGE_SIZE);
> > > +	ret = drm_gem_object_init(dev, &obj->gem, size);
> > > +	if (ret) {
> > > +		kfree(obj);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > > +	mutex_init(&obj->pages_lock);
> > > +
> > > +	return obj;
> > > +}
> > > +
> > > +int vkms_gem_fault(struct vm_fault *vmf)
> > > +{
> > > +	struct vm_area_struct *vma = vmf->vma;
> > > +	struct vkms_gem_object *obj = vma->vm_private_data;
> > > +	unsigned long vaddr = vmf->address;
> > > +	pgoff_t page_offset;
> > > +	loff_t num_pages;
> > > +	int ret;
> > > +
> > > +	page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
> > > +	num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
> > > +
> > > +	if (page_offset > num_pages)
> > > +		return VM_FAULT_SIGBUS;
> > > +
> > > +	ret = -ENOENT;
> > > +	mutex_lock(&obj->pages_lock);
> > > +	if (obj->pages) {
> > > +		get_page(obj->pages[page_offset]);
> > > +		vmf->page = obj->pages[page_offset];
> > > +		ret = 0;
> > > +	}
> > > +	mutex_unlock(&obj->pages_lock);
> > > +	if (ret) {
> > > +		struct page *page;
> > > +		struct address_space *mapping;
> > > +
> > > +		mapping = file_inode(obj->gem.filp)->i_mapping;
> > > +		page = shmem_read_mapping_page(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;
> > > +}
> > > +
> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > +				       struct drm_file *file,
> > > +				       u32 *handle,
> > > +				       u64 size)
> > > +{
> > > +	struct vkms_gem_object *obj;
> > > +	int ret;
> > > +
> > > +	if (!file || !dev || !handle)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	obj = __vkms_gem_create(dev, size);
> > > +	if (IS_ERR(obj))
> > > +		return ERR_CAST(obj);
> > > +
> > > +	ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > +	drm_gem_object_put_unlocked(&obj->gem);
> > > +	if (ret) {
> > > +		drm_gem_object_release(&obj->gem);
> > > +		kfree(obj);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > > +	return &obj->gem;
> > > +}
> > > +
> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
> > > +		     struct drm_mode_create_dumb *args)
> > > +{
> > > +	struct drm_gem_object *gem_obj;
> > > +	u64 pitch, size;
> > > +
> > > +	if (!args || !dev || !file)
> > > +		return -EINVAL;
> > > +
> > > +	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> > > +	size = pitch * args->height;
> > > +
> > > +	if (!size)
> > > +		return -EINVAL;
> > > +
> > > +	gem_obj = vkms_gem_create(dev, file, &args->handle, size);
> > > +	if (IS_ERR(gem_obj))
> > > +		return PTR_ERR(gem_obj);
> > > +
> > > +	args->size = gem_obj->size;
> > > +	args->pitch = pitch;
> > > +
> > > +	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
> > > +		  u32 handle, u64 *offset)
> > > +{
> > > +	struct drm_gem_object *obj;
> > > +	int ret;
> > > +
> > > +	obj = drm_gem_object_lookup(file, handle);
> > > +	if (!obj)
> > > +		return -ENOENT;
> > > +
> > > +	if (!obj->filp) {
> > > +		ret = -EINVAL;
> > > +		goto unref;
> > > +	}
> > > +
> > > +	ret = drm_gem_create_mmap_offset(obj);
> > > +	if (ret)
> > > +		goto unref;
> > > +
> > > +	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> > > +unref:
> > > +	drm_gem_object_put_unlocked(obj);
> > > +
> > > +	return ret;
> > > +}
> > > -- 
> > > 2.17.1
> > > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Daniel Vetter]

On Wed, Jul 11, 2018 at 4:21 PM, Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
> On 07/11, Daniel Vetter wrote:
>> On Thu, Jul 05, 2018 at 11:21:19PM +0300, Haneen Mohammed wrote:
>> > On Thu, Jun 21, 2018 at 09:16:13AM -0300, Rodrigo Siqueira wrote:
>> > > VKMS currently does not handle dumb data, and as a consequence, it does
>> > > not provide mechanisms for handling gem. This commit adds the necessary
>> > > support for gem object/handler and the dumb functions.
>> > >
>> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
>> > > ---
>> >
>> > This looks good to me except with missing gem_free_object_unlocked callback,
>> > which causes warning: Memory manager not clean during takedown.
>> >
>> > Maybe it will be easier if we add this in another patch instead of creating v3 of
>> > this patchset?
>>
>> Ah this is the patch series that didn't land yet ...
>
> Hi, thanks for all the feedbacks.
>
>> Rodrigo, can you pls respin with the issue fixed that Haneen spotted?
>
> Yes,
>
> I will prepare a new version of this patchset based on all the comments.
> Just one question, should I create a new version of the patchset that
> includes the patch related with the hrtimer simulation or keep it
> separate?

Since the hrtimer stuff doesn't apply without this prep work I'd say
all together. Separate patch series only makes sense if it's truly
separate stuff, without any dependencies between the 2 patch series.
-Daniel

>
>> Haneen, can you pls review the other patches in this series too?
>>
>> Also, do we have any other patch series that's not yet applied? With two
>> interns working on the same thing it's a bit harder to keep track of stuff
>> ...
>>
>> Thanks, Daniel
>>
>> >
>> > >  drivers/gpu/drm/vkms/Makefile   |   2 +-
>> > >  drivers/gpu/drm/vkms/vkms_drv.c |   9 ++
>> > >  drivers/gpu/drm/vkms/vkms_drv.h |  21 ++++
>> > >  drivers/gpu/drm/vkms/vkms_gem.c | 168 ++++++++++++++++++++++++++++++++
>> > >  4 files changed, 199 insertions(+), 1 deletion(-)
>> > >  create mode 100644 drivers/gpu/drm/vkms/vkms_gem.c
>> > >
>> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
>> > > index 3f774a6a9c58..986297da51bf 100644
>> > > --- a/drivers/gpu/drm/vkms/Makefile
>> > > +++ b/drivers/gpu/drm/vkms/Makefile
>> > > @@ -1,3 +1,3 @@
>> > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o
>> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
>> > >
>> > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
>> > > index 740a4cbfed91..638bab9083b5 100644
>> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
>> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>> > > @@ -37,6 +37,12 @@ static const struct file_operations vkms_driver_fops = {
>> > >   .release        = drm_release,
>> > >  };
>> > >
>> > > +static const struct vm_operations_struct vkms_gem_vm_ops = {
>> > > + .fault = vkms_gem_fault,
>> > > + .open = drm_gem_vm_open,
>> > > + .close = drm_gem_vm_close,
>> > > +};
>> > > +
>> > >  static void vkms_release(struct drm_device *dev)
>> > >  {
>> > >   struct vkms_device *vkms = container_of(dev, struct vkms_device, drm);
>> > > @@ -50,6 +56,9 @@ static struct drm_driver vkms_driver = {
>> > >   .driver_features        = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
>> > >   .release                = vkms_release,
>> > >   .fops                   = &vkms_driver_fops,
>> > > + .dumb_create            = vkms_dumb_create,
>> > > + .dumb_map_offset        = vkms_dumb_map,
>> > > + .gem_vm_ops             = &vkms_gem_vm_ops,
>> > >
>> > >   .name                   = DRIVER_NAME,
>> > >   .desc                   = DRIVER_DESC,
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
>> > > index b0f9d2e61a42..54bb3dd2b2c1 100644
>> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> > > @@ -3,6 +3,7 @@
>> > >
>> > >  #include <drm/drmP.h>
>> > >  #include <drm/drm.h>
>> > > +#include <drm/drm_gem.h>
>> > >  #include <drm/drm_encoder.h>
>> > >
>> > >  static const u32 vkms_formats[] = {
>> > > @@ -21,6 +22,12 @@ struct vkms_device {
>> > >   struct vkms_output output;
>> > >  };
>> > >
>> > > +struct vkms_gem_object {
>> > > + struct drm_gem_object gem;
>> > > + struct mutex pages_lock; /* Page lock used in page fault handler */
>> > > + struct page **pages;
>> > > +};
>> > > +
>> > >  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>> > >              struct drm_plane *primary, struct drm_plane *cursor);
>> > >
>> > > @@ -28,4 +35,18 @@ int vkms_output_init(struct vkms_device *vkmsdev);
>> > >
>> > >  struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev);
>> > >
>> > > +/* Gem stuff */
>> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>> > > +                                struct drm_file *file,
>> > > +                                u32 *handle,
>> > > +                                u64 size);
>> > > +
>> > > +int vkms_gem_fault(struct vm_fault *vmf);
>> > > +
>> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
>> > > +              struct drm_mode_create_dumb *args);
>> > > +
>> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
>> > > +           u32 handle, u64 *offset);
>> > > +
>> > >  #endif /* _VKMS_DRV_H_ */
>> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
>> > > new file mode 100644
>> > > index 000000000000..9f820f56b9e0
>> > > --- /dev/null
>> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
>> > > @@ -0,0 +1,168 @@
>> > > +// SPDX-License-Identifier: GPL-2.0
>> > > +/*
>> > > + * This program is free software; you can redistribute it and/or modify
>> > > + * it under the terms of the GNU General Public License as published by
>> > > + * the Free Software Foundation; either version 2 of the License, or
>> > > + * (at your option) any later version.
>> > > + */
>> > > +
>> > > +#include <linux/shmem_fs.h>
>> > > +
>> > > +#include "vkms_drv.h"
>> > > +
>> > > +static struct vkms_gem_object *__vkms_gem_create(struct drm_device *dev,
>> > > +                                          u64 size)
>> > > +{
>> > > + struct vkms_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>> > > + if (!obj)
>> > > +         return ERR_PTR(-ENOMEM);
>> > > +
>> > > + size = roundup(size, PAGE_SIZE);
>> > > + ret = drm_gem_object_init(dev, &obj->gem, size);
>> > > + if (ret) {
>> > > +         kfree(obj);
>> > > +         return ERR_PTR(ret);
>> > > + }
>> > > +
>> > > + mutex_init(&obj->pages_lock);
>> > > +
>> > > + return obj;
>> > > +}
>> > > +
>> > > +int vkms_gem_fault(struct vm_fault *vmf)
>> > > +{
>> > > + struct vm_area_struct *vma = vmf->vma;
>> > > + struct vkms_gem_object *obj = vma->vm_private_data;
>> > > + unsigned long vaddr = vmf->address;
>> > > + pgoff_t page_offset;
>> > > + loff_t num_pages;
>> > > + int ret;
>> > > +
>> > > + page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
>> > > + num_pages = DIV_ROUND_UP(obj->gem.size, PAGE_SIZE);
>> > > +
>> > > + if (page_offset > num_pages)
>> > > +         return VM_FAULT_SIGBUS;
>> > > +
>> > > + ret = -ENOENT;
>> > > + mutex_lock(&obj->pages_lock);
>> > > + if (obj->pages) {
>> > > +         get_page(obj->pages[page_offset]);
>> > > +         vmf->page = obj->pages[page_offset];
>> > > +         ret = 0;
>> > > + }
>> > > + mutex_unlock(&obj->pages_lock);
>> > > + if (ret) {
>> > > +         struct page *page;
>> > > +         struct address_space *mapping;
>> > > +
>> > > +         mapping = file_inode(obj->gem.filp)->i_mapping;
>> > > +         page = shmem_read_mapping_page(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;
>> > > +}
>> > > +
>> > > +struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>> > > +                                struct drm_file *file,
>> > > +                                u32 *handle,
>> > > +                                u64 size)
>> > > +{
>> > > + struct vkms_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + if (!file || !dev || !handle)
>> > > +         return ERR_PTR(-EINVAL);
>> > > +
>> > > + obj = __vkms_gem_create(dev, size);
>> > > + if (IS_ERR(obj))
>> > > +         return ERR_CAST(obj);
>> > > +
>> > > + ret = drm_gem_handle_create(file, &obj->gem, handle);
>> > > + drm_gem_object_put_unlocked(&obj->gem);
>> > > + if (ret) {
>> > > +         drm_gem_object_release(&obj->gem);
>> > > +         kfree(obj);
>> > > +         return ERR_PTR(ret);
>> > > + }
>> > > +
>> > > + return &obj->gem;
>> > > +}
>> > > +
>> > > +int vkms_dumb_create(struct drm_file *file, struct drm_device *dev,
>> > > +              struct drm_mode_create_dumb *args)
>> > > +{
>> > > + struct drm_gem_object *gem_obj;
>> > > + u64 pitch, size;
>> > > +
>> > > + if (!args || !dev || !file)
>> > > +         return -EINVAL;
>> > > +
>> > > + pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
>> > > + size = pitch * args->height;
>> > > +
>> > > + if (!size)
>> > > +         return -EINVAL;
>> > > +
>> > > + gem_obj = vkms_gem_create(dev, file, &args->handle, size);
>> > > + if (IS_ERR(gem_obj))
>> > > +         return PTR_ERR(gem_obj);
>> > > +
>> > > + args->size = gem_obj->size;
>> > > + args->pitch = pitch;
>> > > +
>> > > + DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
>> > > +
>> > > + return 0;
>> > > +}
>> > > +
>> > > +int vkms_dumb_map(struct drm_file *file, struct drm_device *dev,
>> > > +           u32 handle, u64 *offset)
>> > > +{
>> > > + struct drm_gem_object *obj;
>> > > + int ret;
>> > > +
>> > > + obj = drm_gem_object_lookup(file, handle);
>> > > + if (!obj)
>> > > +         return -ENOENT;
>> > > +
>> > > + if (!obj->filp) {
>> > > +         ret = -EINVAL;
>> > > +         goto unref;
>> > > + }
>> > > +
>> > > + ret = drm_gem_create_mmap_offset(obj);
>> > > + if (ret)
>> > > +         goto unref;
>> > > +
>> > > + *offset = drm_vma_node_offset_addr(&obj->vma_node);
>> > > +unref:
>> > > + drm_gem_object_put_unlocked(obj);
>> > > +
>> > > + return ret;
>> > > +}
>> > > --
>> > > 2.17.1
>> > >
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Chris Wilson]

Quoting Rodrigo Siqueira (2018-06-21 13:16:13)
> VKMS currently does not handle dumb data, and as a consequence, it does
> not provide mechanisms for handling gem. This commit adds the necessary
> support for gem object/handler and the dumb functions.

I may have been naive, but I didn't think vkms would have need for even the
dumb GEM object API as it would be a paired with vgem (or any other GEM
driver) and use dmabuf to import backing storage.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/5] drm/vkms: Add dumb operations

[Daniel Vetter]

On Fri, Jul 6, 2018 at 9:27 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Rodrigo Siqueira (2018-06-21 13:16:13)
>> VKMS currently does not handle dumb data, and as a consequence, it does
>> not provide mechanisms for handling gem. This commit adds the necessary
>> support for gem object/handler and the dumb functions.
>
> I may have been naive, but I didn't think vkms would have need for even the
> dumb GEM object API as it would be a paired with vgem (or any other GEM
> driver) and use dmabuf to import backing storage.

dumb buffers is a part of the kms api. Userspace expects it to be
there, like for all other kms drivers.

But yes eventually I think we should also have some nice prime tests
using vgem. Using crcs it should be possible to check that the fences
are used correctly for flips using only generic kms interfaces, and so
write a purely generic kms + vgem prime testcase.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V2 2/5] drm/vkms: Add helper for framebuffer create

[Rodrigo Siqueira]

This patch adds the basic hook required to create framebuffer which is
necessary for providing some of the vkms features.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 638bab9083b5..cc046fff985c 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -9,6 +9,8 @@
 #include <drm/drm_gem.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_fb_helper.h>
 #include "vkms_drv.h"
 
 #define DRIVER_NAME	"vkms"
@@ -68,6 +70,7 @@ static struct drm_driver vkms_driver = {
 };
 
 static const struct drm_mode_config_funcs vkms_mode_funcs = {
+	.fb_create = drm_gem_fb_create,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V2 3/5] drm/vkms: Add atomic helpers functions

[Rodrigo Siqueira]

This patch adds the struct drm_crtc_helper_funcs with simple
atomic_check and atomic_enable functions.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bf76cd39ece7..84cc05506b09 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -19,6 +19,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
 };
 
+static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
+				  struct drm_crtc_state *state)
+{
+	return 0;
+}
+
+static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
+				    struct drm_crtc_state *old_state)
+{
+}
+
+static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+	.atomic_check  = vkms_crtc_atomic_check,
+	.atomic_enable = vkms_crtc_atomic_enable,
+};
+
 int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		   struct drm_plane *primary, struct drm_plane *cursor)
 {
@@ -31,5 +47,7 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		return ret;
 	}
 
+	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
+
 	return ret;
 }
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 3/5] drm/vkms: Add atomic helpers functions

[Daniel Vetter]

On Thu, Jun 21, 2018 at 09:16:41AM -0300, Rodrigo Siqueira wrote:
> This patch adds the struct drm_crtc_helper_funcs with simple
> atomic_check and atomic_enable functions.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index bf76cd39ece7..84cc05506b09 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -19,6 +19,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>  	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
>  };
>  
> +static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> +				  struct drm_crtc_state *state)
> +{
> +	return 0;
> +}
> +
> +static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *old_state)
> +{
> +}

Please don't add empty functions when not requird (these callbacks should
all be optional). Also I'd squash this patch in with the patch adding the
vblank hrtimer, splitting this out doesn't really make sense.
-Daniel
> +
> +static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
> +	.atomic_check  = vkms_crtc_atomic_check,
> +	.atomic_enable = vkms_crtc_atomic_enable,
> +};
> +
>  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		   struct drm_plane *primary, struct drm_plane *cursor)
>  {
> @@ -31,5 +47,7 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		return ret;
>  	}
>  
> +	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> +
>  	return ret;
>  }
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V2 4/5] drm/vkms: Add connectors helpers

[Rodrigo Siqueira]

This patch adds the struct drm_connector_helper_funcs with some
necessary hooks. Additionally, it also adds some missing hooks at
drm_connector_funcs.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c    |  6 ------
 drivers/gpu/drm/vkms/vkms_drv.h    |  9 +++++++++
 drivers/gpu/drm/vkms/vkms_output.c | 27 +++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index cc046fff985c..fe93f8c17997 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -19,12 +19,6 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
-#define XRES_MIN    32
-#define YRES_MIN    32
-
-#define XRES_MAX  8192
-#define YRES_MAX  8192
-
 static struct vkms_device *vkms_device;
 
 static const struct file_operations vkms_driver_fops = {
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 54bb3dd2b2c1..76f1720f81a5 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -6,6 +6,15 @@
 #include <drm/drm_gem.h>
 #include <drm/drm_encoder.h>
 
+#define XRES_MIN    32
+#define YRES_MIN    32
+
+#define XRES_DEF  1024
+#define YRES_DEF   768
+
+#define XRES_MAX  8192
+#define YRES_MAX  8192
+
 static const u32 vkms_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 48143eac3c12..fef3b1c1b054 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -8,6 +8,7 @@
 
 #include "vkms_drv.h"
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 static void vkms_connector_destroy(struct drm_connector *connector)
 {
@@ -18,12 +19,36 @@ static void vkms_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_funcs vkms_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = vkms_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static const struct drm_encoder_funcs vkms_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
+static int vkms_conn_get_modes(struct drm_connector *connector)
+{
+	int count;
+
+	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
+	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
+
+	return count;
+}
+
+static int vkms_conn_mode_valid(struct drm_connector *connector,
+				struct drm_display_mode *mode)
+{
+	return MODE_OK;
+}
+
+static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
+	.get_modes    = vkms_conn_get_modes,
+	.mode_valid   = vkms_conn_mode_valid,
+};
+
 int vkms_output_init(struct vkms_device *vkmsdev)
 {
 	struct vkms_output *output = &vkmsdev->output;
@@ -49,6 +74,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
 		goto err_connector;
 	}
 
+	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
+
 	ret = drm_connector_register(connector);
 	if (ret) {
 		DRM_ERROR("Failed to register connector\n");
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 4/5] drm/vkms: Add connectors helpers

[Daniel Vetter]

On Thu, Jun 21, 2018 at 09:17:09AM -0300, Rodrigo Siqueira wrote:
> This patch adds the struct drm_connector_helper_funcs with some
> necessary hooks. Additionally, it also adds some missing hooks at
> drm_connector_funcs.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_drv.c    |  6 ------
>  drivers/gpu/drm/vkms/vkms_drv.h    |  9 +++++++++
>  drivers/gpu/drm/vkms/vkms_output.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index cc046fff985c..fe93f8c17997 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -19,12 +19,6 @@
>  #define DRIVER_MAJOR	1
>  #define DRIVER_MINOR	0
>  
> -#define XRES_MIN    32
> -#define YRES_MIN    32
> -
> -#define XRES_MAX  8192
> -#define YRES_MAX  8192
> -
>  static struct vkms_device *vkms_device;
>  
>  static const struct file_operations vkms_driver_fops = {
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 54bb3dd2b2c1..76f1720f81a5 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -6,6 +6,15 @@
>  #include <drm/drm_gem.h>
>  #include <drm/drm_encoder.h>
>  
> +#define XRES_MIN    32
> +#define YRES_MIN    32
> +
> +#define XRES_DEF  1024
> +#define YRES_DEF   768
> +
> +#define XRES_MAX  8192
> +#define YRES_MAX  8192
> +
>  static const u32 vkms_formats[] = {
>  	DRM_FORMAT_XRGB8888,
>  };
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 48143eac3c12..fef3b1c1b054 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -8,6 +8,7 @@
>  
>  #include "vkms_drv.h"
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_atomic_helper.h>
>  
>  static void vkms_connector_destroy(struct drm_connector *connector)
>  {
> @@ -18,12 +19,36 @@ static void vkms_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_funcs vkms_connector_funcs = {
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.destroy = vkms_connector_destroy,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
>  static const struct drm_encoder_funcs vkms_encoder_funcs = {
>  	.destroy = drm_encoder_cleanup,
>  };
>  
> +static int vkms_conn_get_modes(struct drm_connector *connector)
> +{
> +	int count;
> +
> +	count = drm_add_modes_noedid(connector, XRES_MAX, YRES_MAX);
> +	drm_set_preferred_mode(connector, XRES_DEF, YRES_DEF);
> +
> +	return count;
> +}
> +
> +static int vkms_conn_mode_valid(struct drm_connector *connector,
> +				struct drm_display_mode *mode)
> +{
> +	return MODE_OK;
> +}

No need to implement this dummy function, it's the default behaviour.
Please remove. Otherwise lgtm.

Thanks, Daniel

> +
> +static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
> +	.get_modes    = vkms_conn_get_modes,
> +	.mode_valid   = vkms_conn_mode_valid,
> +};
> +
>  int vkms_output_init(struct vkms_device *vkmsdev)
>  {
>  	struct vkms_output *output = &vkmsdev->output;
> @@ -49,6 +74,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>  		goto err_connector;
>  	}
>  
> +	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> +
>  	ret = drm_connector_register(connector);
>  	if (ret) {
>  		DRM_ERROR("Failed to register connector\n");
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V2 5/5] drm/vkms: Add plane helper struct

[Rodrigo Siqueira]

This patch adds the struct drm_plane_helper_funcs and the required atomic
hooks.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_plane.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 2c25b1d6ab5b..f7f63143f6d0 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -19,6 +19,22 @@ static const struct drm_plane_funcs vkms_plane_funcs = {
 	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
 };
 
+static int vkms_plane_atomic_check(struct drm_plane *plane,
+				   struct drm_plane_state *state)
+{
+	return 0;
+}
+
+static void vkms_primary_plane_update(struct drm_plane *plane,
+				      struct drm_plane_state *old_state)
+{
+}
+
+static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
+	.atomic_check		= vkms_plane_atomic_check,
+	.atomic_update		= vkms_primary_plane_update,
+};
+
 struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
 {
 	struct drm_device *dev = &vkmsdev->drm;
@@ -42,5 +58,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
 		return ERR_PTR(ret);
 	}
 
+	drm_plane_helper_add(plane, &vkms_primary_helper_funcs);
+
 	return plane;
 }
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 5/5] drm/vkms: Add plane helper struct

[Daniel Vetter]

On Thu, Jun 21, 2018 at 09:17:25AM -0300, Rodrigo Siqueira wrote:
> This patch adds the struct drm_plane_helper_funcs and the required atomic
> hooks.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_plane.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 2c25b1d6ab5b..f7f63143f6d0 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -19,6 +19,22 @@ static const struct drm_plane_funcs vkms_plane_funcs = {
>  	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
>  };
>  
> +static int vkms_plane_atomic_check(struct drm_plane *plane,
> +				   struct drm_plane_state *state)
> +{
> +	return 0;
> +}
> +
> +static void vkms_primary_plane_update(struct drm_plane *plane,
> +				      struct drm_plane_state *old_state)
> +{
> +}

Again no dummy functions pls, and I'd squash this in with whatever patch
actually needs it. Not sure we do need it already (I think Haneen does,
but she can pick this all up into her series).
-Daniel

> +
> +static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
> +	.atomic_check		= vkms_plane_atomic_check,
> +	.atomic_update		= vkms_primary_plane_update,
> +};
> +
>  struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
>  {
>  	struct drm_device *dev = &vkmsdev->drm;
> @@ -42,5 +58,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev)
>  		return ERR_PTR(ret);
>  	}
>  
> +	drm_plane_helper_add(plane, &vkms_primary_helper_funcs);
> +
>  	return plane;
>  }
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel