[PATCH 0/3] Provide vmap/vunmap for VRAM helpers

[PATCH 0/3] Provide vmap/vunmap for VRAM helpers

[Thomas Zimmermann]

The vmap operation is pin+kmap, as already implemented for PRIME
support. The vunmap operation is the inverse. This patch set makes
both available for drivers that use VRAM helpers, and replaces the
respective code in ast and mgag200.

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    | 55 +++++++++++++++++++-----
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 +++-------
 include/drm/drm_gem_vram_helper.h        | 12 ++++++
 4 files changed, 71 insertions(+), 39 deletions(-)

--
2.22.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 pattern of temporarily pinning and kmap-ing the BO's memory is
common enough to justify helper functions that do and undo these
operations.

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.

The patch also adds a note about possible kmap counting. So far this
isn't required by drivers, but more complex use cases might make it
necessary.

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

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index e0fbfb6570cf..54758e4debca 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -340,6 +340,48 @@ 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
+ * @gem:	The GEM VRAM object to map
+ *
+ * The vmap function pins a GEM VRAM object to it's current location, either
+ * system or video memory, and maps it's buffer into kernel address space. As
+ * pinned object cannot be reloacted, you should not permanently pin objects.
+ *
+ * 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 = drm_gem_vram_pin(gbo, 0);
+	if (ret)
+		return ERR_PTR(ret);
+	base = drm_gem_vram_kmap(gbo, true, NULL);
+	if (IS_ERR(base)) {
+		drm_gem_vram_unpin(gbo);
+		return base;
+	}
+	return base;
+}
+EXPORT_SYMBOL(drm_gem_vram_vmap);
+
+/**
+ * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
+ * @gem:	The GEM VRAM object to unmap
+ * @vaddr:	The mapping's base address
+ */
+void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr)
+{
+	drm_gem_vram_kunmap(gbo);
+	drm_gem_vram_unpin(gbo);
+}
+EXPORT_SYMBOL(drm_gem_vram_vunmap);
+
 /**
  * drm_gem_vram_fill_create_dumb() - \
 	Helper for implementing &struct drm_driver.dumb_create
@@ -595,17 +637,11 @@ 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 = drm_gem_vram_pin(gbo, 0);
-	if (ret)
+	base = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(base))
 		return NULL;
-	base = drm_gem_vram_kmap(gbo, true, NULL);
-	if (IS_ERR(base)) {
-		drm_gem_vram_unpin(gbo);
-		return NULL;
-	}
 	return base;
 }
 
@@ -620,8 +656,7 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
 {
 	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
 
-	drm_gem_vram_kunmap(gbo);
-	drm_gem_vram_unpin(gbo);
+	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 b41d932eb53a..5192c169cec2 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -44,6 +44,16 @@ struct drm_gem_vram_object {
 	struct ttm_placement placement;
 	struct ttm_place placements[2];
 
+	/* TODO: Maybe implement a map counter.
+	 *
+	 * So far, drivers based on VRAM helpers don't have overlapping
+	 * mapping operations. A driver temporarily maps an object and
+	 * unmaps it ASAP. This works well for fbdev emulation or cursors.
+	 *
+	 * If we ever have a driver with buffer objects that are mapped
+	 * by multiple code fragments concurrently, we may need a map
+	 * counter to get the mapping right.
+	 */
 	int pin_count;
 };
 
@@ -84,6 +94,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.22.0

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

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

[Daniel Vetter]

On Wed, Jul 24, 2019 at 1:30 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The pattern of temporarily pinning and kmap-ing the BO's memory is
> common enough to justify helper functions that do and undo these
> operations.
>
> 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.
>
> The patch also adds a note about possible kmap counting. So far this
> isn't required by drivers, but more complex use cases might make it
> necessary.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 55 ++++++++++++++++++++++-----
>  include/drm/drm_gem_vram_helper.h     | 12 ++++++
>  2 files changed, 57 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index e0fbfb6570cf..54758e4debca 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -340,6 +340,48 @@ 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
> + * @gem:       The GEM VRAM object to map

variable names need to match, kernel-doc should be complaining here.

> + *
> + * The vmap function pins a GEM VRAM object to it's current location, either
> + * system or video memory, and maps it's buffer into kernel address space. As
> + * pinned object cannot be reloacted, you should not permanently pin objects.

Imo also good to point at the corresponding functions, i.e. reference
unmap here and in the unmap function reference the map function. And
please make sure the links work in the generated doc output.

> + *
> + * 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 = drm_gem_vram_pin(gbo, 0);
> +       if (ret)
> +               return ERR_PTR(ret);
> +       base = drm_gem_vram_kmap(gbo, true, NULL);
> +       if (IS_ERR(base)) {
> +               drm_gem_vram_unpin(gbo);
> +               return base;
> +       }
> +       return base;
> +}
> +EXPORT_SYMBOL(drm_gem_vram_vmap);
> +
> +/**
> + * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
> + * @gem:       The GEM VRAM object to unmap
> + * @vaddr:     The mapping's base address
> + */
> +void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr)
> +{
> +       drm_gem_vram_kunmap(gbo);
> +       drm_gem_vram_unpin(gbo);
> +}
> +EXPORT_SYMBOL(drm_gem_vram_vunmap);
> +
>  /**
>   * drm_gem_vram_fill_create_dumb() - \
>         Helper for implementing &struct drm_driver.dumb_create
> @@ -595,17 +637,11 @@ 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 = drm_gem_vram_pin(gbo, 0);
> -       if (ret)
> +       base = drm_gem_vram_vmap(gbo);
> +       if (IS_ERR(base))
>                 return NULL;
> -       base = drm_gem_vram_kmap(gbo, true, NULL);
> -       if (IS_ERR(base)) {
> -               drm_gem_vram_unpin(gbo);
> -               return NULL;
> -       }
>         return base;
>  }
>
> @@ -620,8 +656,7 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
>  {
>         struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
>
> -       drm_gem_vram_kunmap(gbo);
> -       drm_gem_vram_unpin(gbo);
> +       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 b41d932eb53a..5192c169cec2 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -44,6 +44,16 @@ struct drm_gem_vram_object {
>         struct ttm_placement placement;
>         struct ttm_place placements[2];
>
> +       /* TODO: Maybe implement a map counter.

Kerneldoc is missing here. You can use the inline style so that the
todo comment here is still included.
-Daniel

> +        *
> +        * So far, drivers based on VRAM helpers don't have overlapping
> +        * mapping operations. A driver temporarily maps an object and
> +        * unmaps it ASAP. This works well for fbdev emulation or cursors.
> +        *
> +        * If we ever have a driver with buffer objects that are mapped
> +        * by multiple code fragments concurrently, we may need a map
> +        * counter to get the mapping right.
> +        */
>         int pin_count;
>  };
>
> @@ -84,6 +94,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.22.0
>


-- 
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 1/3] drm/vram: Provide vmap and vunmap operations for GEM VRAM objects

[Thomas Zimmermann]

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

Hi

Am 24.07.19 um 14:00 schrieb Daniel Vetter:
> On Wed, Jul 24, 2019 at 1:30 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The pattern of temporarily pinning and kmap-ing the BO's memory is
>> common enough to justify helper functions that do and undo these
>> operations.
>>
>> 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.
>>
>> The patch also adds a note about possible kmap counting. So far this
>> isn't required by drivers, but more complex use cases might make it
>> necessary.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem_vram_helper.c | 55 ++++++++++++++++++++++-----
>>  include/drm/drm_gem_vram_helper.h     | 12 ++++++
>>  2 files changed, 57 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index e0fbfb6570cf..54758e4debca 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -340,6 +340,48 @@ 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
>> + * @gem:       The GEM VRAM object to map
> 
> variable names need to match, kernel-doc should be complaining here.
> 
>> + *
>> + * The vmap function pins a GEM VRAM object to it's current location, either
>> + * system or video memory, and maps it's buffer into kernel address space. As
>> + * pinned object cannot be reloacted, you should not permanently pin objects.
> 
> Imo also good to point at the corresponding functions, i.e. reference
> unmap here and in the unmap function reference the map function. And
> please make sure the links work in the generated doc output.
> 
>> + *
>> + * 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 = drm_gem_vram_pin(gbo, 0);
>> +       if (ret)
>> +               return ERR_PTR(ret);
>> +       base = drm_gem_vram_kmap(gbo, true, NULL);
>> +       if (IS_ERR(base)) {
>> +               drm_gem_vram_unpin(gbo);
>> +               return base;
>> +       }
>> +       return base;
>> +}
>> +EXPORT_SYMBOL(drm_gem_vram_vmap);
>> +
>> +/**
>> + * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
>> + * @gem:       The GEM VRAM object to unmap
>> + * @vaddr:     The mapping's base address
>> + */
>> +void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr)
>> +{
>> +       drm_gem_vram_kunmap(gbo);
>> +       drm_gem_vram_unpin(gbo);
>> +}
>> +EXPORT_SYMBOL(drm_gem_vram_vunmap);
>> +
>>  /**
>>   * drm_gem_vram_fill_create_dumb() - \
>>         Helper for implementing &struct drm_driver.dumb_create
>> @@ -595,17 +637,11 @@ 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 = drm_gem_vram_pin(gbo, 0);
>> -       if (ret)
>> +       base = drm_gem_vram_vmap(gbo);
>> +       if (IS_ERR(base))
>>                 return NULL;
>> -       base = drm_gem_vram_kmap(gbo, true, NULL);
>> -       if (IS_ERR(base)) {
>> -               drm_gem_vram_unpin(gbo);
>> -               return NULL;
>> -       }
>>         return base;
>>  }
>>
>> @@ -620,8 +656,7 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
>>  {
>>         struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
>>
>> -       drm_gem_vram_kunmap(gbo);
>> -       drm_gem_vram_unpin(gbo);
>> +       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 b41d932eb53a..5192c169cec2 100644
>> --- a/include/drm/drm_gem_vram_helper.h
>> +++ b/include/drm/drm_gem_vram_helper.h
>> @@ -44,6 +44,16 @@ struct drm_gem_vram_object {
>>         struct ttm_placement placement;
>>         struct ttm_place placements[2];
>>
>> +       /* TODO: Maybe implement a map counter.
> 
> Kerneldoc is missing here. You can use the inline style so that the
> todo comment here is still included.
> -Daniel

Actually, this comment wasn't intended to be in the kernel docs. I guess
I integrate it into the structure's overall documentation then.

Best regards
Thomas

>> +        *
>> +        * So far, drivers based on VRAM helpers don't have overlapping
>> +        * mapping operations. A driver temporarily maps an object and
>> +        * unmaps it ASAP. This works well for fbdev emulation or cursors.
>> +        *
>> +        * If we ever have a driver with buffer objects that are mapped
>> +        * by multiple code fragments concurrently, we may need a map
>> +        * counter to get the mapping right.
>> +        */
>>         int pin_count;
>>  };
>>
>> @@ -84,6 +94,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.22.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

[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 c792362024a5..6e9434175c46 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1176,26 +1176,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;
@@ -1230,16 +1226,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.22.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.22.0

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

[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