[PATCH v2 0/2] drm/udl: Map pages with SHMEM helpers

[PATCH v2 0/2] drm/udl: Map pages with SHMEM helpers

[Thomas Zimmermann]

(was: Default to cachable mappings for GEM SHMEM)

Udl implments cached mappings. Convert it to SHMEM's default helpers.

v2:
	* keep writecombine mappings as the default
	* add SHMEM helper for creating BOs with cached mappings
	* update udl on the new helper

Thomas Zimmermann (2):
  drm/shmem-helper: Add .gem_create_object helper that sets map_cached
    flag
  drm/udl: Use GEM vmap/mmap function from SHMEM helpers

 drivers/gpu/drm/drm_gem_shmem_helper.c |  27 +++++++
 drivers/gpu/drm/udl/Makefile           |   2 +-
 drivers/gpu/drm/udl/udl_drv.c          |   4 +-
 drivers/gpu/drm/udl/udl_drv.h          |   3 -
 drivers/gpu/drm/udl/udl_gem.c          | 106 -------------------------
 include/drm/drm_gem_shmem_helper.h     |   4 +
 6 files changed, 34 insertions(+), 112 deletions(-)
 delete mode 100644 drivers/gpu/drm/udl/udl_gem.c

--
2.26.2

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

[PATCH v2 1/2] drm/shmem-helper: Add .gem_create_object helper that sets map_cached flag

[Thomas Zimmermann]

The helper drm_gem_shmem_create_object_cached() allocates an GEM SHMEM
object and sets the map_cached flag. Useful for drivers that want cached
mappings.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 27 ++++++++++++++++++++++++++
 include/drm/drm_gem_shmem_helper.h     |  4 ++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index df31e5782eed1..ab28b7ff864cb 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -433,6 +433,33 @@ bool drm_gem_shmem_purge(struct drm_gem_object *obj)
 }
 EXPORT_SYMBOL(drm_gem_shmem_purge);
 
+/**
+ * drm_gem_shmem_create_object_cached - Create a shmem buffer object with
+ *                                      cached mappings
+ * @dev: DRM device
+ * @size: Size of the object to allocate
+ *
+ * By default, shmem buffer objects use writecombine mappings. This
+ * function implements struct drm_driver.gem_create_object for shmem
+ * buffer objects with cached mappings.
+ *
+ * Returns:
+ * A struct drm_gem_shmem_object * on success or NULL negative on failure.
+ */
+struct drm_gem_object *
+drm_gem_shmem_create_object_cached(struct drm_device *dev, size_t size)
+{
+       struct drm_gem_shmem_object *shmem;
+
+       shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+       if (!shmem)
+               return NULL;
+       shmem->map_cached = true;
+
+       return &shmem->base;
+}
+EXPORT_SYMBOL(drm_gem_shmem_create_object_cached);
+
 /**
  * drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
  * @file: DRM file structure to create the dumb buffer for
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index 294b2931c4cc0..5381f0c8cf6f6 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -132,6 +132,10 @@ struct drm_gem_shmem_object *
 drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
 				 struct drm_device *dev, size_t size,
 				 uint32_t *handle);
+
+struct drm_gem_object *
+drm_gem_shmem_create_object_cached(struct drm_device *dev, size_t size);
+
 int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
 			      struct drm_mode_create_dumb *args);
 
-- 
2.26.2

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

[PATCH v2 2/2] drm/udl: Use GEM vmap/mmap function from SHMEM helpers

[Thomas Zimmermann]

The udl driver contains an implementation of GEM vmap and mmap
operations that is identical to the common SHMEM helper; except
that udl's code uses cached pages by default.

Convert udl to regular SHMEM helper functions. There's no reason
to have udl behave differently from all other SHMEM drivers. The
udl driver uses the SHMEM helper to enable caching.

v2:
	* implement .gem_create_object with SHMEM helper

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/udl/Makefile  |   2 +-
 drivers/gpu/drm/udl/udl_drv.c |   4 +-
 drivers/gpu/drm/udl/udl_drv.h |   3 -
 drivers/gpu/drm/udl/udl_gem.c | 106 ----------------------------------
 4 files changed, 3 insertions(+), 112 deletions(-)
 delete mode 100644 drivers/gpu/drm/udl/udl_gem.c

diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
index b50179bb4de06..24d61f61d7db2 100644
--- a/drivers/gpu/drm/udl/Makefile
+++ b/drivers/gpu/drm/udl/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
-udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o udl_gem.o
+udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_main.o udl_transfer.o
 
 obj-$(CONFIG_DRM_UDL) := udl.o
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d1aa50fd6d65a..96d4317a2c1bd 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -37,8 +37,8 @@ DEFINE_DRM_GEM_FOPS(udl_driver_fops);
 static struct drm_driver driver = {
 	.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
 
-	/* gem hooks */
-	.gem_create_object = udl_driver_gem_create_object,
+	/* GEM hooks */
+	.gem_create_object = drm_gem_shmem_create_object_cached,
 
 	.fops = &udl_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 2642f94a63fc8..b1461f30780bc 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -81,9 +81,6 @@ int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr,
 		     const char *front, char **urb_buf_ptr,
 		     u32 byte_offset, u32 device_byte_offset, u32 byte_width);
 
-struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
-						    size_t size);
-
 int udl_drop_usb(struct drm_device *dev);
 
 #define CMD_WRITE_RAW8   "\xAF\x60" /**< 8 bit raw write command. */
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
deleted file mode 100644
index b6e26f98aa0af..0000000000000
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ /dev/null
@@ -1,106 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2012 Red Hat
- */
-
-#include <linux/dma-buf.h>
-#include <linux/vmalloc.h>
-
-#include <drm/drm_drv.h>
-#include <drm/drm_gem_shmem_helper.h>
-#include <drm/drm_mode.h>
-#include <drm/drm_prime.h>
-
-#include "udl_drv.h"
-
-/*
- * GEM object funcs
- */
-
-static int udl_gem_object_mmap(struct drm_gem_object *obj,
-			       struct vm_area_struct *vma)
-{
-	int ret;
-
-	ret = drm_gem_shmem_mmap(obj, vma);
-	if (ret)
-		return ret;
-
-	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
-	if (obj->import_attach)
-		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
-	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-
-	return 0;
-}
-
-static void *udl_gem_object_vmap(struct drm_gem_object *obj)
-{
-	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
-	int ret;
-
-	ret = mutex_lock_interruptible(&shmem->vmap_lock);
-	if (ret)
-		return ERR_PTR(ret);
-
-	if (shmem->vmap_use_count++ > 0)
-		goto out;
-
-	ret = drm_gem_shmem_get_pages(shmem);
-	if (ret)
-		goto err_zero_use;
-
-	if (obj->import_attach)
-		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
-	else
-		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
-				    VM_MAP, PAGE_KERNEL);
-
-	if (!shmem->vaddr) {
-		DRM_DEBUG_KMS("Failed to vmap pages\n");
-		ret = -ENOMEM;
-		goto err_put_pages;
-	}
-
-out:
-	mutex_unlock(&shmem->vmap_lock);
-	return shmem->vaddr;
-
-err_put_pages:
-	drm_gem_shmem_put_pages(shmem);
-err_zero_use:
-	shmem->vmap_use_count = 0;
-	mutex_unlock(&shmem->vmap_lock);
-	return ERR_PTR(ret);
-}
-
-static const struct drm_gem_object_funcs udl_gem_object_funcs = {
-	.free = drm_gem_shmem_free_object,
-	.print_info = drm_gem_shmem_print_info,
-	.pin = drm_gem_shmem_pin,
-	.unpin = drm_gem_shmem_unpin,
-	.get_sg_table = drm_gem_shmem_get_sg_table,
-	.vmap = udl_gem_object_vmap,
-	.vunmap = drm_gem_shmem_vunmap,
-	.mmap = udl_gem_object_mmap,
-};
-
-/*
- * Helpers for struct drm_driver
- */
-
-struct drm_gem_object *udl_driver_gem_create_object(struct drm_device *dev,
-						    size_t size)
-{
-	struct drm_gem_shmem_object *shmem;
-	struct drm_gem_object *obj;
-
-	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
-	if (!shmem)
-		return NULL;
-
-	obj = &shmem->base;
-	obj->funcs = &udl_gem_object_funcs;
-
-	return obj;
-}
-- 
2.26.2

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

Re: [PATCH v2 0/2] drm/udl: Map pages with SHMEM helpers

[Emil Velikov]

On Tue, 19 May 2020 at 09:04, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> (was: Default to cachable mappings for GEM SHMEM)
>
> Udl implments cached mappings. Convert it to SHMEM's default helpers.
>
> v2:
>         * keep writecombine mappings as the default
>         * add SHMEM helper for creating BOs with cached mappings
>         * update udl on the new helper
>
> Thomas Zimmermann (2):
>   drm/shmem-helper: Add .gem_create_object helper that sets map_cached
>     flag
>   drm/udl: Use GEM vmap/mmap function from SHMEM helpers
>
For the series:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

>  drivers/gpu/drm/drm_gem_shmem_helper.c |  27 +++++++
>  drivers/gpu/drm/udl/Makefile           |   2 +-
>  drivers/gpu/drm/udl/udl_drv.c          |   4 +-
>  drivers/gpu/drm/udl/udl_drv.h          |   3 -
>  drivers/gpu/drm/udl/udl_gem.c          | 106 -------------------------
>  include/drm/drm_gem_shmem_helper.h     |   4 +
>  6 files changed, 34 insertions(+), 112 deletions(-)
>  delete mode 100644 drivers/gpu/drm/udl/udl_gem.c
>
Please keep going with these nice stats \o/

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

Re: [PATCH v2 2/2] drm/udl: Use GEM vmap/mmap function from SHMEM helpers

[Noralf Trønnes]

Den 19.05.2020 10.04, skrev Thomas Zimmermann:
> The udl driver contains an implementation of GEM vmap and mmap
> operations that is identical to the common SHMEM helper; except
> that udl's code uses cached pages by default.
> 
> Convert udl to regular SHMEM helper functions. There's no reason
> to have udl behave differently from all other SHMEM drivers. The
> udl driver uses the SHMEM helper to enable caching.
> 
> v2:
> 	* implement .gem_create_object with SHMEM helper
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index d1aa50fd6d65a..96d4317a2c1bd 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -37,8 +37,8 @@ DEFINE_DRM_GEM_FOPS(udl_driver_fops);
>  static struct drm_driver driver = {
>  	.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
>  
> -	/* gem hooks */
> -	.gem_create_object = udl_driver_gem_create_object,
> +	/* GEM hooks */
> +	.gem_create_object = drm_gem_shmem_create_object_cached,
>  
>  	.fops = &udl_driver_fops,
>  	DRM_GEM_SHMEM_DRIVER_OPS,

You could add a DRM_GEM_SHMEM_CACHED_DRIVER_OPS macro and use that instead.

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