[PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[Thomas Zimmermann]

VRAM helpers support ref counting for pin and vmap operations, no need
to avoid these operations, by employing the internal kmap interface. Just
use drm_gem_vram_vmap() and let it handle the details.

Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the
last user of these internal functions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 56 +--------------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++--
 include/drm/drm_gem_vram_helper.h     |  3 --
 3 files changed, 8 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 07447abb4134..0e3cdc40379c 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
  * hardware's draing engine.
  *
  * To access a buffer object's memory from the DRM driver, call
- * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel address
- * space and returns the memory address. Use drm_gem_vram_kunmap() to
+ * drm_gem_vram_vmap(). It maps the buffer into kernel address
+ * space and returns the memory address. Use drm_gem_vram_vunmap() to
  * release the mapping.
  */
 
@@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
 	return kmap->virtual;
 }
 
-/**
- * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address space
- * @gbo:	the GEM VRAM object
- * @map:	establish a mapping if necessary
- * @is_iomem:	returns true if the mapped memory is I/O memory, or false \
-	otherwise; can be NULL
- *
- * This function maps the buffer object into the kernel's address space
- * or returns the current mapping. If the parameter map is false, the
- * function only queries the current mapping, but does not establish a
- * new one.
- *
- * Returns:
- * The buffers virtual address if mapped, or
- * NULL if not mapped, or
- * an ERR_PTR()-encoded error code otherwise.
- */
-void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
-			bool *is_iomem)
-{
-	int ret;
-	void *virtual;
-
-	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
-	if (ret)
-		return ERR_PTR(ret);
-	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
-	ttm_bo_unreserve(&gbo->bo);
-
-	return virtual;
-}
-EXPORT_SYMBOL(drm_gem_vram_kmap);
-
 static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
 {
 	if (WARN_ON_ONCE(!gbo->kmap_use_count))
@@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
 	 */
 }
 
-/**
- * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
- * @gbo:	the GEM VRAM object
- */
-void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
-{
-	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);
-	ttm_bo_unreserve(&gbo->bo);
-}
-EXPORT_SYMBOL(drm_gem_vram_kunmap);
-
 /**
  * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
  *                       space
@@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap);
  * 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.
diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index d9a5af62af89..4fcc0a542b8a 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
 
 	vbox_crtc->cursor_enabled = true;
 
-	/* pinning is done in prepare/cleanup framebuffer */
-	src = drm_gem_vram_kmap(gbo, true, NULL);
+	src = drm_gem_vram_vmap(gbo);
 	if (IS_ERR(src)) {
+		/*
+		 * BUG: we should have pinned the BO in prepare_fb().
+		 */
 		mutex_unlock(&vbox->hw_mutex);
-		DRM_WARN("Could not kmap cursor bo, skipping update\n");
+		DRM_WARN("Could not map cursor bo, skipping update\n");
 		return;
 	}
 
@@ -414,7 +416,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
 	data_size = width * height * 4 + mask_size;
 
 	copy_cursor_image(src, vbox->cursor_data, width, height, mask_size);
-	drm_gem_vram_kunmap(gbo);
+	drm_gem_vram_vunmap(gbo, src);
 
 	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
 		VBOX_MOUSE_POINTER_ALPHA;
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 035332f3723f..b34f1da89cc7 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -101,9 +101,6 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
 s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
 int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
 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);
 
-- 
2.28.0

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

Re: [PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[Daniel Vetter]

On Fri, Sep 11, 2020 at 09:59:22AM +0200, Thomas Zimmermann wrote:
> VRAM helpers support ref counting for pin and vmap operations, no need
> to avoid these operations, by employing the internal kmap interface. Just
> use drm_gem_vram_vmap() and let it handle the details.
> 
> Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the
> last user of these internal functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Oh I got confused for a bit about ttm's usage of kmap. Usually that means
the per-page mapping done by the kmap() functions, might be a good idea to
rename that to vmap for consistency with dma-buf and everything outside of
ttm.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 56 +--------------------------
>  drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++--
>  include/drm/drm_gem_vram_helper.h     |  3 --
>  3 files changed, 8 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 07447abb4134..0e3cdc40379c 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
>   * hardware's draing engine.
>   *
>   * To access a buffer object's memory from the DRM driver, call
> - * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel address
> - * space and returns the memory address. Use drm_gem_vram_kunmap() to
> + * drm_gem_vram_vmap(). It maps the buffer into kernel address
> + * space and returns the memory address. Use drm_gem_vram_vunmap() to
>   * release the mapping.
>   */
>  
> @@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
>  	return kmap->virtual;
>  }
>  
> -/**
> - * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address space
> - * @gbo:	the GEM VRAM object
> - * @map:	establish a mapping if necessary
> - * @is_iomem:	returns true if the mapped memory is I/O memory, or false \
> -	otherwise; can be NULL
> - *
> - * This function maps the buffer object into the kernel's address space
> - * or returns the current mapping. If the parameter map is false, the
> - * function only queries the current mapping, but does not establish a
> - * new one.
> - *
> - * Returns:
> - * The buffers virtual address if mapped, or
> - * NULL if not mapped, or
> - * an ERR_PTR()-encoded error code otherwise.
> - */
> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
> -			bool *is_iomem)
> -{
> -	int ret;
> -	void *virtual;
> -
> -	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
> -	if (ret)
> -		return ERR_PTR(ret);
> -	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
> -	ttm_bo_unreserve(&gbo->bo);
> -
> -	return virtual;
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kmap);
> -
>  static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>  {
>  	if (WARN_ON_ONCE(!gbo->kmap_use_count))
> @@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>  	 */
>  }
>  
> -/**
> - * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
> - * @gbo:	the GEM VRAM object
> - */
> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
> -{
> -	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);
> -	ttm_bo_unreserve(&gbo->bo);
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kunmap);
> -
>  /**
>   * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
>   *                       space
> @@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap);
>   * 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.
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index d9a5af62af89..4fcc0a542b8a 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>  
>  	vbox_crtc->cursor_enabled = true;
>  
> -	/* pinning is done in prepare/cleanup framebuffer */
> -	src = drm_gem_vram_kmap(gbo, true, NULL);
> +	src = drm_gem_vram_vmap(gbo);
>  	if (IS_ERR(src)) {
> +		/*
> +		 * BUG: we should have pinned the BO in prepare_fb().
> +		 */
>  		mutex_unlock(&vbox->hw_mutex);
> -		DRM_WARN("Could not kmap cursor bo, skipping update\n");
> +		DRM_WARN("Could not map cursor bo, skipping update\n");
>  		return;
>  	}
>  
> @@ -414,7 +416,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>  	data_size = width * height * 4 + mask_size;
>  
>  	copy_cursor_image(src, vbox->cursor_data, width, height, mask_size);
> -	drm_gem_vram_kunmap(gbo);
> +	drm_gem_vram_vunmap(gbo, src);
>  
>  	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
>  		VBOX_MOUSE_POINTER_ALPHA;
> diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
> index 035332f3723f..b34f1da89cc7 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -101,9 +101,6 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
>  s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
>  int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
>  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);
>  
> -- 
> 2.28.0
> 

-- 
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] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[Hans de Goede]

Hi,

On 9/11/20 9:59 AM, Thomas Zimmermann wrote:
> VRAM helpers support ref counting for pin and vmap operations, no need
> to avoid these operations, by employing the internal kmap interface. Just
> use drm_gem_vram_vmap() and let it handle the details.
> 
> Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the
> last user of these internal functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Nice cleanup.

I've given this a test-run in an actual VirtualBox vm, focussing on
cursor sprite changes and I don't see any regressions, so:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Thomas, I assume that you will push this upstream yourself?

Regards,

Hans


> ---
>   drivers/gpu/drm/drm_gem_vram_helper.c | 56 +--------------------------
>   drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++--
>   include/drm/drm_gem_vram_helper.h     |  3 --
>   3 files changed, 8 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 07447abb4134..0e3cdc40379c 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
>    * hardware's draing engine.
>    *
>    * To access a buffer object's memory from the DRM driver, call
> - * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel address
> - * space and returns the memory address. Use drm_gem_vram_kunmap() to
> + * drm_gem_vram_vmap(). It maps the buffer into kernel address
> + * space and returns the memory address. Use drm_gem_vram_vunmap() to
>    * release the mapping.
>    */
>   
> @@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
>   	return kmap->virtual;
>   }
>   
> -/**
> - * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address space
> - * @gbo:	the GEM VRAM object
> - * @map:	establish a mapping if necessary
> - * @is_iomem:	returns true if the mapped memory is I/O memory, or false \
> -	otherwise; can be NULL
> - *
> - * This function maps the buffer object into the kernel's address space
> - * or returns the current mapping. If the parameter map is false, the
> - * function only queries the current mapping, but does not establish a
> - * new one.
> - *
> - * Returns:
> - * The buffers virtual address if mapped, or
> - * NULL if not mapped, or
> - * an ERR_PTR()-encoded error code otherwise.
> - */
> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
> -			bool *is_iomem)
> -{
> -	int ret;
> -	void *virtual;
> -
> -	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
> -	if (ret)
> -		return ERR_PTR(ret);
> -	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
> -	ttm_bo_unreserve(&gbo->bo);
> -
> -	return virtual;
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kmap);
> -
>   static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>   {
>   	if (WARN_ON_ONCE(!gbo->kmap_use_count))
> @@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>   	 */
>   }
>   
> -/**
> - * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
> - * @gbo:	the GEM VRAM object
> - */
> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
> -{
> -	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);
> -	ttm_bo_unreserve(&gbo->bo);
> -}
> -EXPORT_SYMBOL(drm_gem_vram_kunmap);
> -
>   /**
>    * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
>    *                       space
> @@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap);
>    * 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.
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index d9a5af62af89..4fcc0a542b8a 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>   
>   	vbox_crtc->cursor_enabled = true;
>   
> -	/* pinning is done in prepare/cleanup framebuffer */
> -	src = drm_gem_vram_kmap(gbo, true, NULL);
> +	src = drm_gem_vram_vmap(gbo);
>   	if (IS_ERR(src)) {
> +		/*
> +		 * BUG: we should have pinned the BO in prepare_fb().
> +		 */
>   		mutex_unlock(&vbox->hw_mutex);
> -		DRM_WARN("Could not kmap cursor bo, skipping update\n");
> +		DRM_WARN("Could not map cursor bo, skipping update\n");
>   		return;
>   	}
>   
> @@ -414,7 +416,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
>   	data_size = width * height * 4 + mask_size;
>   
>   	copy_cursor_image(src, vbox->cursor_data, width, height, mask_size);
> -	drm_gem_vram_kunmap(gbo);
> +	drm_gem_vram_vunmap(gbo, src);
>   
>   	flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
>   		VBOX_MOUSE_POINTER_ALPHA;
> diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
> index 035332f3723f..b34f1da89cc7 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -101,9 +101,6 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
>   s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
>   int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
>   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);
>   
> 

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

Re: [PATCH] drm/vboxvideo: Use drm_gem_vram_vmap() interfaces

[Thomas Zimmermann]

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

Hi

Am 11.09.20 um 16:07 schrieb Hans de Goede:
> Hi,
> 
> On 9/11/20 9:59 AM, Thomas Zimmermann wrote:
>> VRAM helpers support ref counting for pin and vmap operations, no need
>> to avoid these operations, by employing the internal kmap interface. Just
>> use drm_gem_vram_vmap() and let it handle the details.
>>
>> Also unexport the kmap interfaces from VRAM helpers. Vboxvideo was the
>> last user of these internal functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Nice cleanup.
> 
> I've given this a test-run in an actual VirtualBox vm, focussing on
> cursor sprite changes and I don't see any regressions, so:
> 
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> 
> Thomas, I assume that you will push this upstream yourself?

It's merged now. Thanks for testing the change.

Best regards
Thomas

> 
> Regards,
> 
> Hans
> 
> 
>> ---
>>   drivers/gpu/drm/drm_gem_vram_helper.c | 56 +--------------------------
>>   drivers/gpu/drm/vboxvideo/vbox_mode.c | 10 +++--
>>   include/drm/drm_gem_vram_helper.h     |  3 --
>>   3 files changed, 8 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 07447abb4134..0e3cdc40379c 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -97,8 +97,8 @@ static const struct drm_gem_object_funcs
>> drm_gem_vram_object_funcs;
>>    * hardware's draing engine.
>>    *
>>    * To access a buffer object's memory from the DRM driver, call
>> - * drm_gem_vram_kmap(). It (optionally) maps the buffer into kernel
>> address
>> - * space and returns the memory address. Use drm_gem_vram_kunmap() to
>> + * drm_gem_vram_vmap(). It maps the buffer into kernel address
>> + * space and returns the memory address. Use drm_gem_vram_vunmap() to
>>    * release the mapping.
>>    */
>>   @@ -436,39 +436,6 @@ static void *drm_gem_vram_kmap_locked(struct
>> drm_gem_vram_object *gbo,
>>       return kmap->virtual;
>>   }
>>   -/**
>> - * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address
>> space
>> - * @gbo:    the GEM VRAM object
>> - * @map:    establish a mapping if necessary
>> - * @is_iomem:    returns true if the mapped memory is I/O memory, or
>> false \
>> -    otherwise; can be NULL
>> - *
>> - * This function maps the buffer object into the kernel's address space
>> - * or returns the current mapping. If the parameter map is false, the
>> - * function only queries the current mapping, but does not establish a
>> - * new one.
>> - *
>> - * Returns:
>> - * The buffers virtual address if mapped, or
>> - * NULL if not mapped, or
>> - * an ERR_PTR()-encoded error code otherwise.
>> - */
>> -void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
>> -            bool *is_iomem)
>> -{
>> -    int ret;
>> -    void *virtual;
>> -
>> -    ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
>> -    if (ret)
>> -        return ERR_PTR(ret);
>> -    virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
>> -    ttm_bo_unreserve(&gbo->bo);
>> -
>> -    return virtual;
>> -}
>> -EXPORT_SYMBOL(drm_gem_vram_kmap);
>> -
>>   static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
>>   {
>>       if (WARN_ON_ONCE(!gbo->kmap_use_count))
>> @@ -484,22 +451,6 @@ static void drm_gem_vram_kunmap_locked(struct
>> drm_gem_vram_object *gbo)
>>        */
>>   }
>>   -/**
>> - * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
>> - * @gbo:    the GEM VRAM object
>> - */
>> -void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
>> -{
>> -    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);
>> -    ttm_bo_unreserve(&gbo->bo);
>> -}
>> -EXPORT_SYMBOL(drm_gem_vram_kunmap);
>> -
>>   /**
>>    * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel
>> address
>>    *                       space
>> @@ -511,9 +462,6 @@ EXPORT_SYMBOL(drm_gem_vram_kunmap);
>>    * 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.
>> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c
>> b/drivers/gpu/drm/vboxvideo/vbox_mode.c
>> index d9a5af62af89..4fcc0a542b8a 100644
>> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
>> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
>> @@ -397,11 +397,13 @@ static void vbox_cursor_atomic_update(struct
>> drm_plane *plane,
>>         vbox_crtc->cursor_enabled = true;
>>   -    /* pinning is done in prepare/cleanup framebuffer */
>> -    src = drm_gem_vram_kmap(gbo, true, NULL);
>> +    src = drm_gem_vram_vmap(gbo);
>>       if (IS_ERR(src)) {
>> +        /*
>> +         * BUG: we should have pinned the BO in prepare_fb().
>> +         */
>>           mutex_unlock(&vbox->hw_mutex);
>> -        DRM_WARN("Could not kmap cursor bo, skipping update\n");
>> +        DRM_WARN("Could not map cursor bo, skipping update\n");
>>           return;
>>       }
>>   @@ -414,7 +416,7 @@ static void vbox_cursor_atomic_update(struct
>> drm_plane *plane,
>>       data_size = width * height * 4 + mask_size;
>>         copy_cursor_image(src, vbox->cursor_data, width, height,
>> mask_size);
>> -    drm_gem_vram_kunmap(gbo);
>> +    drm_gem_vram_vunmap(gbo, src);
>>         flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
>>           VBOX_MOUSE_POINTER_ALPHA;
>> diff --git a/include/drm/drm_gem_vram_helper.h
>> b/include/drm/drm_gem_vram_helper.h
>> index 035332f3723f..b34f1da89cc7 100644
>> --- a/include/drm/drm_gem_vram_helper.h
>> +++ b/include/drm/drm_gem_vram_helper.h
>> @@ -101,9 +101,6 @@ u64 drm_gem_vram_mmap_offset(struct
>> drm_gem_vram_object *gbo);
>>   s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
>>   int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long
>> pl_flag);
>>   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);
>>  
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


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

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

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