[PATCH 0/3] drm/vram: Provide GEM VRAM vmap()/vunmap/()

[PATCH 0/3] drm/vram: Provide GEM VRAM vmap()/vunmap/()

[Thomas Zimmermann]

The ast and mgag200 drivers pin() and kmap() cursor buffers; essentially
reimplementing vmap(). We can share some code by using the respective
functionality from GEM VRAM buffer objects.

Thomas Zimmermann (3):
  drm/vram: Provide vmap and vunmap operations for GEM VRAM objects
  drm/ast: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
  drm/mgag200: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO

 drivers/gpu/drm/ast/ast_mode.c           |  21 ++---
 drivers/gpu/drm/drm_gem_vram_helper.c    | 106 ++++++++++++++++-------
 drivers/gpu/drm/mgag200/mgag200_cursor.c |  22 ++---
 include/drm/drm_gem_vram_helper.h        |   5 +-
 4 files changed, 93 insertions(+), 61 deletions(-)

--
2.23.0

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

[PATCH 1/3] drm/vram: Provide vmap and vunmap operations for GEM VRAM objects

[Thomas Zimmermann]

The implementation of vmap and vunmap for GEM VRAM helpers is
already in PRIME helpers. The patch moves the operations to separate
functions and exports them for general use.

v3:
	* remove v2's obsolete note on ref-counting
v2:
	* fix documentation
	* add cross references to function documentation
	* document (the lack of) ref-counting for GEM VRAM BO mappings

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 106 ++++++++++++++++++--------
 include/drm/drm_gem_vram_helper.h     |   5 +-
 2 files changed, 79 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index ab9f8523d887..42423d15dfcc 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -392,6 +392,77 @@ void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_kunmap);
 
+/**
+ * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
+ *                       space
+ * @gbo:	The GEM VRAM object to map
+ *
+ * The vmap function pins a GEM VRAM object to its current location, either
+ * system or video memory, and maps its buffer into kernel address space.
+ * As pinned object cannot be relocated, you should avoid pinning objects
+ * permanently. Call drm_gem_vram_vunmap() with the returned address to
+ * unmap and unpin the GEM VRAM object.
+ *
+ * If you have special requirements for the pinning or mapping operations,
+ * call drm_gem_vram_pin() and drm_gem_vram_kmap() directly.
+ *
+ * Returns:
+ * The buffer's virtual address on success, or
+ * an ERR_PTR()-encoded error code otherwise.
+ */
+void *drm_gem_vram_vmap(struct drm_gem_vram_object *gbo)
+{
+	int ret;
+	void *base;
+
+	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = drm_gem_vram_pin_locked(gbo, 0);
+	if (ret)
+		goto err_ttm_bo_unreserve;
+	base = drm_gem_vram_kmap_locked(gbo, true, NULL);
+	if (IS_ERR(base)) {
+		ret = PTR_ERR(base);
+		goto err_drm_gem_vram_unpin_locked;
+	}
+
+	ttm_bo_unreserve(&gbo->bo);
+
+	return base;
+
+err_drm_gem_vram_unpin_locked:
+	drm_gem_vram_unpin_locked(gbo);
+err_ttm_bo_unreserve:
+	ttm_bo_unreserve(&gbo->bo);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(drm_gem_vram_vmap);
+
+/**
+ * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
+ * @gbo:	The GEM VRAM object to unmap
+ * @vaddr:	The mapping's base address as returned by drm_gem_vram_vmap()
+ *
+ * A call to drm_gem_vram_vunmap() unmaps and unpins a GEM VRAM buffer. See
+ * the documentation for drm_gem_vram_vmap() for more information.
+ */
+void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr)
+{
+	int ret;
+
+	ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
+	if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
+		return;
+
+	drm_gem_vram_kunmap_locked(gbo);
+	drm_gem_vram_unpin_locked(gbo);
+
+	ttm_bo_unreserve(&gbo->bo);
+}
+EXPORT_SYMBOL(drm_gem_vram_vunmap);
+
 /**
  * drm_gem_vram_fill_create_dumb() - \
 	Helper for implementing &struct drm_driver.dumb_create
@@ -622,31 +693,12 @@ static void drm_gem_vram_object_unpin(struct drm_gem_object *gem)
 static void *drm_gem_vram_object_vmap(struct drm_gem_object *gem)
 {
 	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
-	int ret;
 	void *base;
 
-	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
-	if (ret)
-		return ERR_PTR(ret);
-
-	ret = drm_gem_vram_pin_locked(gbo, 0);
-	if (ret)
-		goto err_ttm_bo_unreserve;
-	base = drm_gem_vram_kmap_locked(gbo, true, NULL);
-	if (IS_ERR(base)) {
-		ret = PTR_ERR(base);
-		goto err_drm_gem_vram_unpin_locked;
-	}
-
-	ttm_bo_unreserve(&gbo->bo);
-
+	base = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(base))
+		return NULL;
 	return base;
-
-err_drm_gem_vram_unpin_locked:
-	drm_gem_vram_unpin_locked(gbo);
-err_ttm_bo_unreserve:
-	ttm_bo_unreserve(&gbo->bo);
-	return ERR_PTR(ret);
 }
 
 /**
@@ -659,16 +711,8 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
 				       void *vaddr)
 {
 	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
-	int ret;
-
-	ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
-	if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
-		return;
 
-	drm_gem_vram_kunmap_locked(gbo);
-	drm_gem_vram_unpin_locked(gbo);
-
-	ttm_bo_unreserve(&gbo->bo);
+	drm_gem_vram_vunmap(gbo, vaddr);
 }
 
 /*
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 9aaef4f8c327..418eb1122861 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -43,7 +43,8 @@ struct vm_area_struct;
  * operations. So a buffer object that has been pinned N times with
  * drm_gem_vram_pin() must be unpinned N times with
  * drm_gem_vram_unpin(). The same applies to pairs of
- * drm_gem_vram_kmap() and drm_gem_vram_kunmap().
+ * drm_gem_vram_kmap() and drm_gem_vram_kunmap(), as well as pairs of
+ * drm_gem_vram_vmap() and drm_gem_vram_vunmap().
  */
 struct drm_gem_vram_object {
 	struct ttm_buffer_object bo;
@@ -101,6 +102,8 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
 void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
 			bool *is_iomem);
 void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo);
+void *drm_gem_vram_vmap(struct drm_gem_vram_object *gbo);
+void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr);
 
 int drm_gem_vram_fill_create_dumb(struct drm_file *file,
 				  struct drm_device *dev,
-- 
2.23.0

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

[PATCH 2/3] drm/ast: Use drm_gem_vram_{vmap, vunmap}() to map cursor source BO

[Thomas Zimmermann]

The VRAM helper's vmap interfaces provide pinning and mapping of BO
memory. This patch replaces the respective code in ast cursor handling.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d349c721501c..5f4de03ff9de 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1179,26 +1179,22 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 		return -ENOENT;
 	}
 	gbo = drm_gem_vram_of_gem(obj);
-
-	ret = drm_gem_vram_pin(gbo, 0);
-	if (ret)
-		goto err_drm_gem_object_put_unlocked;
-	src = drm_gem_vram_kmap(gbo, true, NULL);
+	src = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(src)) {
 		ret = PTR_ERR(src);
-		goto err_drm_gem_vram_unpin;
+		goto err_drm_gem_object_put_unlocked;
 	}
 
 	dst = drm_gem_vram_kmap(drm_gem_vram_of_gem(ast->cursor_cache),
 				false, NULL);
 	if (IS_ERR(dst)) {
 		ret = PTR_ERR(dst);
-		goto err_drm_gem_vram_kunmap;
+		goto err_drm_gem_vram_vunmap;
 	}
 	dst_gpu = drm_gem_vram_offset(drm_gem_vram_of_gem(ast->cursor_cache));
 	if (dst_gpu < 0) {
 		ret = (int)dst_gpu;
-		goto err_drm_gem_vram_kunmap;
+		goto err_drm_gem_vram_vunmap;
 	}
 
 	dst += (AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE)*ast->next_cursor;
@@ -1233,16 +1229,13 @@ static int ast_cursor_set(struct drm_crtc *crtc,
 
 	ast_show_cursor(crtc);
 
-	drm_gem_vram_kunmap(gbo);
-	drm_gem_vram_unpin(gbo);
+	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_object_put_unlocked(obj);
 
 	return 0;
 
-err_drm_gem_vram_kunmap:
-	drm_gem_vram_kunmap(gbo);
-err_drm_gem_vram_unpin:
-	drm_gem_vram_unpin(gbo);
+err_drm_gem_vram_vunmap:
+	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_object_put_unlocked:
 	drm_gem_object_put_unlocked(obj);
 	return ret;
-- 
2.23.0

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

[PATCH 3/3] drm/mgag200: Use drm_gem_vram_{vmap, vunmap}() to map cursor source BO

[Thomas Zimmermann]

The VRAM helper's vmap interfaces provide pinning and mapping of BO
memory. This patch replaces the respective code in mgag200 cursor handling.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 289ce3e29032..89f61573a497 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -85,17 +85,12 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	if (!obj)
 		return -ENOENT;
 	gbo = drm_gem_vram_of_gem(obj);
-	ret = drm_gem_vram_pin(gbo, 0);
-	if (ret) {
-		dev_err(&dev->pdev->dev, "failed to lock user bo\n");
-		goto err_drm_gem_object_put_unlocked;
-	}
-	src = drm_gem_vram_kmap(gbo, true, NULL);
+	src = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(src)) {
 		ret = PTR_ERR(src);
 		dev_err(&dev->pdev->dev,
-			"failed to kmap user buffer updates\n");
-		goto err_drm_gem_vram_unpin_src;
+			"failed to map user buffer updates\n");
+		goto err_drm_gem_object_put_unlocked;
 	}
 
 	/* Pin and map up-coming buffer to write colour indices */
@@ -103,7 +98,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	if (ret) {
 		dev_err(&dev->pdev->dev,
 			"failed to pin cursor buffer: %d\n", ret);
-		goto err_drm_gem_vram_kunmap_src;
+		goto err_drm_gem_vram_vunmap;
 	}
 	dst = drm_gem_vram_kmap(pixels_next, true, NULL);
 	if (IS_ERR(dst)) {
@@ -213,8 +208,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	mdev->cursor.pixels_current = pixels_next;
 
 	drm_gem_vram_kunmap(pixels_next);
-	drm_gem_vram_kunmap(gbo);
-	drm_gem_vram_unpin(gbo);
+	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_object_put_unlocked(obj);
 
 	return 0;
@@ -223,10 +217,8 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	drm_gem_vram_kunmap(pixels_next);
 err_drm_gem_vram_unpin_dst:
 	drm_gem_vram_unpin(pixels_next);
-err_drm_gem_vram_kunmap_src:
-	drm_gem_vram_kunmap(gbo);
-err_drm_gem_vram_unpin_src:
-	drm_gem_vram_unpin(gbo);
+err_drm_gem_vram_vunmap:
+	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_object_put_unlocked:
 	drm_gem_object_put_unlocked(obj);
 	return ret;
-- 
2.23.0

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

Re: [PATCH 0/3] drm/vram: Provide GEM VRAM vmap()/vunmap/()

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1238 bytes --]

FYI this is actually version 3 of the patch set posted at
[1] and [2]

[1] https://lists.freedesktop.org/archives/dri-devel/2019-July/227823.html
[2] https://lists.freedesktop.org/archives/dri-devel/2019-July/228074.html

Am 11.09.19 um 14:03 schrieb Thomas Zimmermann:
> The ast and mgag200 drivers pin() and kmap() cursor buffers; essentially
> reimplementing vmap(). We can share some code by using the respective
> functionality from GEM VRAM buffer objects.
> 
> Thomas Zimmermann (3):
>   drm/vram: Provide vmap and vunmap operations for GEM VRAM objects
>   drm/ast: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
>   drm/mgag200: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
> 
>  drivers/gpu/drm/ast/ast_mode.c           |  21 ++---
>  drivers/gpu/drm/drm_gem_vram_helper.c    | 106 ++++++++++++++++-------
>  drivers/gpu/drm/mgag200/mgag200_cursor.c |  22 ++---
>  include/drm/drm_gem_vram_helper.h        |   5 +-
>  4 files changed, 93 insertions(+), 61 deletions(-)
> 
> --
> 2.23.0
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 0/3] drm/vram: Provide GEM VRAM vmap()/vunmap/()

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1031 bytes --]

ping for a review

Am 11.09.19 um 14:03 schrieb Thomas Zimmermann:
> The ast and mgag200 drivers pin() and kmap() cursor buffers; essentially
> reimplementing vmap(). We can share some code by using the respective
> functionality from GEM VRAM buffer objects.
> 
> Thomas Zimmermann (3):
>   drm/vram: Provide vmap and vunmap operations for GEM VRAM objects
>   drm/ast: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
>   drm/mgag200: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
> 
>  drivers/gpu/drm/ast/ast_mode.c           |  21 ++---
>  drivers/gpu/drm/drm_gem_vram_helper.c    | 106 ++++++++++++++++-------
>  drivers/gpu/drm/mgag200/mgag200_cursor.c |  22 ++---
>  include/drm/drm_gem_vram_helper.h        |   5 +-
>  4 files changed, 93 insertions(+), 61 deletions(-)
> 
> --
> 2.23.0
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 0/3] drm/vram: Provide GEM VRAM vmap()/vunmap/()

[Gerd Hoffmann]

On Wed, Sep 11, 2019 at 02:03:49PM +0200, Thomas Zimmermann wrote:
> The ast and mgag200 drivers pin() and kmap() cursor buffers; essentially
> reimplementing vmap(). We can share some code by using the respective
> functionality from GEM VRAM buffer objects.
> 
> Thomas Zimmermann (3):
>   drm/vram: Provide vmap and vunmap operations for GEM VRAM objects
>   drm/ast: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
>   drm/mgag200: Use drm_gem_vram_{vmap,vunmap}() to map cursor source BO
> 
>  drivers/gpu/drm/ast/ast_mode.c           |  21 ++---
>  drivers/gpu/drm/drm_gem_vram_helper.c    | 106 ++++++++++++++++-------
>  drivers/gpu/drm/mgag200/mgag200_cursor.c |  22 ++---
>  include/drm/drm_gem_vram_helper.h        |   5 +-
>  4 files changed, 93 insertions(+), 61 deletions(-)

Series is
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd

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

[PATCH 3/3] drm/mgag200: Use drm_gem_vram_{vmap, vunmap}() to map cursor source BO

[Thomas Zimmermann]

The VRAM helper's vmap interfaces provide pinning and mapping of BO
memory. This patch replaces the respective code in mgag200 cursor handling.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 289ce3e29032..89f61573a497 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -85,17 +85,12 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	if (!obj)
 		return -ENOENT;
 	gbo = drm_gem_vram_of_gem(obj);
-	ret = drm_gem_vram_pin(gbo, 0);
-	if (ret) {
-		dev_err(&dev->pdev->dev, "failed to lock user bo\n");
-		goto err_drm_gem_object_put_unlocked;
-	}
-	src = drm_gem_vram_kmap(gbo, true, NULL);
+	src = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(src)) {
 		ret = PTR_ERR(src);
 		dev_err(&dev->pdev->dev,
-			"failed to kmap user buffer updates\n");
-		goto err_drm_gem_vram_unpin_src;
+			"failed to map user buffer updates\n");
+		goto err_drm_gem_object_put_unlocked;
 	}
 
 	/* Pin and map up-coming buffer to write colour indices */
@@ -103,7 +98,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	if (ret) {
 		dev_err(&dev->pdev->dev,
 			"failed to pin cursor buffer: %d\n", ret);
-		goto err_drm_gem_vram_kunmap_src;
+		goto err_drm_gem_vram_vunmap;
 	}
 	dst = drm_gem_vram_kmap(pixels_next, true, NULL);
 	if (IS_ERR(dst)) {
@@ -213,8 +208,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	mdev->cursor.pixels_current = pixels_next;
 
 	drm_gem_vram_kunmap(pixels_next);
-	drm_gem_vram_kunmap(gbo);
-	drm_gem_vram_unpin(gbo);
+	drm_gem_vram_vunmap(gbo, src);
 	drm_gem_object_put_unlocked(obj);
 
 	return 0;
@@ -223,10 +217,8 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	drm_gem_vram_kunmap(pixels_next);
 err_drm_gem_vram_unpin_dst:
 	drm_gem_vram_unpin(pixels_next);
-err_drm_gem_vram_kunmap_src:
-	drm_gem_vram_kunmap(gbo);
-err_drm_gem_vram_unpin_src:
-	drm_gem_vram_unpin(gbo);
+err_drm_gem_vram_vunmap:
+	drm_gem_vram_vunmap(gbo, src);
 err_drm_gem_object_put_unlocked:
 	drm_gem_object_put_unlocked(obj);
 	return ret;
-- 
2.22.0

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